bugGNU TeXmacs - Bugs: bug #49655, safely-kill-window with qt may...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #49655: safely-kill-window with qt may causes corruption and crash when fast_alloc is used

Submitter:  Philippe Joyez <pjoyez>
Submitted:  Sun 20 Nov 2016 01:21:24 PM UTC
   
 
Category:  Qt port Priority:  7 - High
Item Group:  Error Status:  Fixed
Privacy:  Public Assigned to:  mgubi
Originator Name:  Open/Closed:  Closed
Release:  None Release: 
Fixed Release:  None Fixed Release: 
Keywords: 

Jump to the original submission

Thu 04 Mar 2021 09:14:03 PM UTC, comment #23: 

Seems to be fine now.

Please re-open if someone again experiences a problem.

Joris van der Hoeven <vdhoeven>
Group administrator
Mon 13 Apr 2020 10:37:57 PM UTC, comment #22: 

Fixed in 12772

Massimiliano Gubinelli <mgubi>
Group administrator
Mon 13 Apr 2020 07:42:35 PM UTC, comment #21: 

Good. It seems fine to me. I'm committing and close the bug.

Thanks!

Massimiliano Gubinelli <mgubi>
Group administrator
Mon 13 Apr 2020 07:28:36 PM UTC, comment #20: 

Hey Max, thanks to you I learned about using templates today! Good way to take profit of confinement :)

The attached patch prevents the bug, but I let you check it and commit it because I'm really a beginner in this stuff.

Stay safe.

(file #48831)

Philippe Joyez <pjoyez>
Group Member
Mon 13 Apr 2020 02:10:54 PM UTC, comment #19: 

Philippe, sorry for bother again. This solution is ok but not perfect because that code has to be #ifdef-fed only when QTTEXMACS is used, otherwise will give a problem.

Can you try a different solution still? Take back the declaration in qt_widget.hpp , just after the declaration of qt_widget_rep but before the qt_widget class. And write it as follows:

template<> void tm_delete<qt_widget_rep>(qt_widget_rep *);

as an explicit template specialization. See e.g.

https://www.geeksforgeeks.org/template-specialization-c/
 
It should work and would not require ifdef. Possibly you will have also to modify the definition in qt_widget.cpp accordingly.

Thanks!

Massimiliano Gubinelli <mgubi>
Group administrator
Mon 13 Apr 2020 01:50:15 PM UTC, comment #18: 

Yes, you are right, it works and it is much more consistent.
Sorry for the noise!

Philippe Joyez <pjoyez>
Group Member
Mon 13 Apr 2020 12:33:10 PM UTC, comment #17: 

Is ok, but I do not understand why do not work. Can you try to move the declaration

 void tm_delete (qt_widget_rep* ptr);

out of the class and in fast_alloc.htpp where similar declarations are: like

 void tm_delete (widget_rep* ptr);

? Thanks. Just to try.

Max

Massimiliano Gubinelli <mgubi>
Group administrator
Mon 13 Apr 2020 12:20:45 PM UTC, comment #16: 

Nope, 12769 still did not do it. It took my chance to further adjust it in Rev 12770. Check if that's ok for you.

Philippe Joyez <pjoyez>
Group Member
Sun 12 Apr 2020 10:53:17 PM UTC, comment #15: 

Is my change that does not make sense. There is a generic version of tm_delete defined in fast_alloc.hpp and it will be used by the ABSTRACT_NULL macro, because I defined the specialised version after. So, as you correctly remark, the specialised version has to be declared before ABSTRACT_NULL(qt_widget). Rev 12769 contains a change which should solve the problem without introducing inline functions. Can you try it? Thanks.

Massimiliano Gubinelli <mgubi>
Group administrator
Sun 12 Apr 2020 10:37:46 PM UTC, comment #14: 

I apologize Max, I realize I did not specify where I added the code and that makes a difference:


@@ -200,6 +205,15 @@ public:
 */
 class qt_widget {
 public:
+#include "fast_alloc.hpp"
+  void tm_delete (qt_widget_rep* ptr) {
+    void *mem= ptr->derived_this ();
+    ptr -> ~qt_widget_rep ();
+    fast_delete (mem);
+  }
+
   ABSTRACT_NULL(qt_widget); // Automagically declared constructor, methods, etc.

   inline bool operator == (qt_widget w) { return rep == w.rep; }


The commits you made do not solve the issue while this does. I ended up putting the declaration there after previous attempts that would not compile. Dont ask me for the full rationale; you may even find this incorrect. Of course it's also cleaner if the code is not in the header.

Philippe Joyez <pjoyez>
Group Member
Sun 12 Apr 2020 12:34:16 PM UTC, comment #13: 

Philippe, should be fixed in rev 12766. Can you check and close the bug if is ok? Thanks.

Massimiliano Gubinelli <mgubi>
Group administrator
Sun 12 Apr 2020 09:34:43 AM UTC, comment #12: 

Great. I will try a cleaner solution: getting rid of qt_widget altogether. It should work the same. I let you know when I've committed it.

Massimiliano Gubinelli <mgubi>
Group administrator
Sun 12 Apr 2020 09:09:14 AM UTC, comment #11: 

Hi Max,
Right, after inserting


#include "fast_alloc.hpp"
  inline void tm_delete (qt_widget_rep* ptr) {
    void *mem= ptr->derived_this ();
    ptr -> ~qt_widget_rep ();
    fast_delete (mem);
  }


in qt_widget.hpp

the problem does not occur anymore! Thanks a lot!!
 
I'll let you commit that if you want (I'm not sure I did it correctly, or at an appropriate place).

Philippe Joyez <pjoyez>
Group Member
Sat 11 Apr 2020 01:56:07 PM UTC, comment #10: 

I was able to make the test code work but i cannot trigger the problem on Mac. Maybe this problem is similar to others we have with tm_delete. You could try to add a function like this


void
tm_delete (widget_rep* ptr) {
  void *mem= ptr->derived_this ();
  ptr -> ~widget_rep ();
  fast_delete (mem);
}

but for qt_widget_rep instead of widget_rep. Add also the corresponding declaration in fast_alloc.hpp before the generic definition. The source of the problem seems the destruction of the qt_widget object in QTMWidget

Massimiliano Gubinelli <mgubi>
Group administrator
Sat 11 Apr 2020 01:29:28 PM UTC, comment #9: 

The test case does not work anymore for me. TeXmacs complains of the following:

/Users/mgubi/t/svn-src/TeXmacs/progs/server/server-base.scm:40:3: While evaluating arguments to remote-equ in expression (cond (# #) (# #) ...):
/Users/mgubi/t/svn-src/TeXmacs/progs/server/server-base.scm:40:3: Wrong number of arguments to #<procedure remote-equ (envelope file latex)>


How can i reproduce the problem? I might have an idea to fix it.


Massimiliano Gubinelli <mgubi>
Group administrator
Sat 11 Apr 2020 01:13:04 PM UTC, comment #8: 

This bug is still present in rev. 12761, compiled with qt5 (and probably with qt4 too; I did not check) in linux. The crash still occurs when a scheme script calls safely-kill-window.

When compiled with --enable-debug I get this error messages printed just before the crash:

> fast_delete 0x55555cd94078 size mismatch at 0x55555cd94060 18446744073709551615:18446744073709551615 :93825099312864:93825099312864
> fast_delete buffer overflow ffff0000
> free(): invalid pointer


When run in gdb, the bt indeed shows the crash occurs when deleting a qt widget (see attached backtrace file)

Interestingly, if I configure with --enable-fastalloc="no", the crash no longer occurs!
 
Looks to me there could be a subtle bug in the fast_alloc code or its interaction with qt (may depend on qt versions - see comments #5 and #7)

---
changing the title
was : recent changes in Qt plugin cause "free() invalid pointer" crash in ubuntu 16.04

now : safely-kill-window with qt may causes corruption and crash when fast_alloc is used

(file #48813)

Philippe Joyez <pjoyez>
Group Member
Wed 07 Aug 2019 05:38:52 PM UTC, comment #7: 

Reopening because rev 11157 did not fix this, at least not in all distributions  (unlike I though earlier).

Interestingly the AppImage I make (https://build.opensuse.org/package/binaries/home:slowphil:texmacs-devel/texmacs/AppImage) is immune from the bug (and it uses Qt 4.8.7).

Philippe Joyez <pjoyez>
Group Member
Tue 21 Aug 2018 10:55:50 PM UTC, comment #6: 

This seems to have been fixed by rev 11157. Closing.

Philippe Joyez <pjoyez>
Group Member
Tue 06 Feb 2018 08:49:21 AM UTC, comment #5: 

I'm trying to reproduce this bug but on my mac (with current svn and qt 4.8.7) it seems I cannot trigger the crash. Can somebody confirm that the crash is still present?

Massimiliano Gubinelli <mgubi>
Group administrator
Fri 03 Nov 2017 10:20:43 PM UTC, comment #4: 

the previous "fix" is not working in ubuntu 17.10; the crash still occurs...

Philippe Joyez <pjoyez>
Group Member
Fri 03 Nov 2017 11:15:14 AM UTC, comment #3: 

Until now I was avoiding the bug for myself by reverting most changes Max had made after 10544...

However my reverting patch came into conflict with r 10874 and I then realized accidentally that with the current svn (r10881) if I configure with --enable-debug the bug is not occurring anymore in ubuntu 16.04 (previously I could get a bt, see initial report) !

Hence, the bug seems to be somehow related to the optimizations performed by the compiler: with -O0 or -Og (default is -O3), the bug does not occur... Weird.

Philippe Joyez <pjoyez>
Group Member
Fri 03 Nov 2017 10:56:13 AM UTC, comment #2: 

here are two file to reproduce the issue (still occurring and also in ubuntu 17.10)


1- place the content of "place-in-my-init-texmacs.scm" in your my-init-texmacs.scm.

2- Start texmacs.On the treminal it should display "starting server".

3-  run the python which will connect to texmacs. This will open a new texmacs window with a third toolbar with buttons "done" and "cancel".

4- Click on any of these buttons => crash when calling safely-kill-window in equ-edit-end

This could probably be simplified further...

(file #42323, file #42324)

Philippe Joyez <pjoyez>
Group Member
Thu 22 Jun 2017 09:14:19 PM UTC, comment #1: 

Massimiliano?

Joris van der Hoeven <vdhoeven>
Group administrator
Sun 20 Nov 2016 01:21:24 PM UTC, original submission:  

I have recently set up a new machine with ubuntu 16.04 and found that one of my scheme scripts that automatically open and close an extra window (and which works perfectly in ubuntu 14.04, debian, and windows) would systematically crash when closing the window, throwing a  "free() invalid pointer" and  SIGABRT.

One possible difference between the setups is that in 14.04 it's Qt4.8.5 while in 16.04 it's Qt4.8.7.

Following this idea I found that going back to svn versions 10544 or earlier, the problem does not occur anymore in the newer os, and indeed the changes made in 10545 regarded the qt plugin.

I attach the gdb output.

Philippe Joyez <pjoyez>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #48831:  another_fix_for_49655.patch added by pjoyez (1KiB - text/x-patch)
file #48813:  backtrace added by pjoyez (3KiB - application/octet-stream)
file #42323:  place-in-my-init-texmacs.scm added by pjoyez (3KiB - text/x-scheme)
file #42324:  simple_texmacs_connect.py added by pjoyez (3KiB - text/x-python)
file #39029:  gdb_output added by pjoyez (25KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mgubi (Updated the item)
  • -email is unavailable- added by vdhoeven (Posted a comment)
  • -email is unavailable- added by pjoyez (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 20 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-03-04 vdhoeven Open/ClosedOpen Closed
    2020-04-13 mgubi StatusReady For Test Fixed
    2020-04-13 pjoyez Attached File- Added another_fix_for_49655.patch, #48831
    2020-04-12 mgubi StatusConfirmed Ready For Test
    2020-04-11 pjoyez Summaryrecent changes in Qt plugin cause &quot;free() invalid pointer&quot; crash in ubuntu 16.04 safely-kill-window with qt may causes corruption and crash when fast_alloc is used
    2020-04-11 pjoyez Attached File- Added backtrace, #48813
    2019-10-19 vdhoeven Priority5 - Normal 7 - High
        StatusNone Confirmed
    2019-08-07 pjoyez StatusFixed None
        Open/ClosedClosed Open
    2018-08-21 pjoyez StatusNone Fixed
        Open/ClosedOpen Closed
    2018-02-06 mgubi Originator NameJoyez
    2018-02-06 mgubi Priority9 - Immediate 5 - Normal
        Originator Name Joyez
    2018-02-05 mgubi Priority5 - Normal 9 - Immediate
    2017-11-03 pjoyez Attached File- Added place-in-my-init-texmacs.scm, #42323
        Attached File- Added simple_texmacs_connect.py, #42324
    2017-06-22 vdhoeven Assigned toNone mgubi
    2016-11-20 pjoyez Attached File- Added gdb_output, #39029

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code