bugGNUstep - Bugs: bug #42717, Crash in -[NSBox(Private)...

Group
 
 

bug #42717: Crash in -[NSBox(Private) calcSizesAllowingNegative:]

Submitter:  Yavor Doganov <yavor>
Submitted:  Wed 09 Jul 2014 01:48:17 PM UTC
   
 
Category:  Gui/AppKit Severity:  3 - Normal
Item Group:  Bug Status:  Fixed
Privacy:  Public Assigned to:  FredKiefer
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 23 Oct 2014 05:59:27 AM UTC, comment #15: 

Yes, this bug has been fixed.

Yavor Doganov <yavor>
Wed 22 Oct 2014 10:37:27 PM UTC, comment #14: 

Yavor, can we consider this solved?

Riccardo Mottola <rmottola>
Group Member
Wed 23 Jul 2014 07:57:27 AM UTC, comment #13: 

You are correct, calling this a compiler bug was incorrect. It is just changed behaviour in a corner case.
The method in this case was supposed to return an NSSize value, that is two CGFloats and I think even on x86 this should be possible in registers, which are easlily set to zero.

I still think it wasn't a great idea of the gcc people to change the optimisation in this case, but GNUstep code should take care to handle it correctly anayway, as we want to support Sparc and other platforms as well.


What needs to be done now is to test as much of GNUstep code on x86 wiht gcc 4.9 as possible to avoid a massive issue when this compiler gets into more widespread use.

Fred Kiefer <FredKiefer>
Group Member
Mon 21 Jul 2014 08:49:35 AM UTC, comment #12: 


> both being caused by a compiler that crashes when calling
> a method on nil that returns a structure type


My understanding is that the behavior when calling a method expected to return a struct on a nil receiver is (and has always been) undefined.
Certainly I know that it has always crashed on sparc machines.

So I think it's a bit harsh to refer to a change in the undefined behavior as a new compiler bug .... I expect any code calling a method which returns a struct ought to check that the receiver is non-null fist.

Richard Frith-Macdonald <CaS>
Group Member
Mon 21 Jul 2014 08:43:45 AM UTC, comment #11: 

No crash if I apply your changes in r38003.

If it is a compiler bug, most probably it is present in earlier versions as this bug was reported last year when 4.9 was not released yet.  However, it may be that GCC is taking advantage of the "undefined behaviour" scenario and is optimizing in a way that leads to a crash.  Very often code that works with -O0 but fails with -O2 reveals a bug in the code, not the compiler.  In any case, if it is a compiler bug, it should be haunted down and fixed rather than the code adapted to cope with it.  IMHO.  It might be difficult to write a self-contained example exposing the bug, though.

Yavor Doganov <yavor>
Mon 21 Jul 2014 08:04:46 AM UTC, comment #10: 

I now think that you did run into two independent issue, both being caused by a compiler that crashes when calling a method on nil that returns a structure type. It looks like gcc 4.9 has a new bug there and we need to prepare the whole GNUstep code for this.

I tried to fix the two specific issues in the gui code, but if the above is true we are going to expect more problems. Pleas ebe patient with us and we may be able to squash all of them one by one.

Fred Kiefer <FredKiefer>
Group Member
Sun 20 Jul 2014 03:30:47 PM UTC, comment #9: 

Here's the valgrind output:

$ valgrind Cenon
==14425== Memcheck, a memory error detector
==14425== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==14425== Using Valgrind-3.9.0 and LibVEX; rerun with -h for copyright info
==14425== Command: Cenon
==14425==
==14425== Invalid read of size 4
==14425==    at 0x4AD5058: objc_msg_lookup (sendmsg.c:448)
==14425==    by 0x41211B8: _i_NSBox_Private_calcSizesAllowingNegative_ (in /usr/lib/libgnustep-gui.so.0.24.0)
==14425==  Address 0xffffff98 is not stack'd, malloc'd or (recently) free'd
==14425==
==14425==
==14425== Process terminating with default action of signal 11 (SIGSEGV)
==14425==  Access not within mapped region at address 0xFFFFFF98
==14425==    at 0x4AD5058: objc_msg_lookup (sendmsg.c:448)
==14425==    by 0x41211B8: _i_NSBox_Private_calcSizesAllowingNegative_ (in /usr/lib/libgnustep-gui.so.0.24.0)
==14425==  If you believe this happened as a result of a stack
==14425==  overflow in your program's main thread (unlikely but
==14425==  possible), you can try to increase the size of the
==14425==  main thread stack using the --main-stacksize= flag.
==14425==  The main thread stack size used in this run was 8388608.
==14425==
==14425== HEAP SUMMARY:
==14425==     in use at exit: 17,125,664 bytes in 378,360 blocks
==14425==   total heap usage: 647,770 allocs, 269,410 frees, 39,616,506 bytes allocated
==14425==
==14425== LEAK SUMMARY:
==14425==    definitely lost: 34,376 bytes in 691 blocks
==14425==    indirectly lost: 50,125 bytes in 3,300 blocks
==14425==      possibly lost: 14,724,851 bytes in 340,448 blocks
==14425==    still reachable: 2,316,312 bytes in 33,921 blocks
==14425==         suppressed: 0 bytes in 0 blocks
==14425== Rerun with --leak-check=full to see details of leaked memory
==14425==
==14425== For counts of detected and suppressed errors, rerun with: -v
==14425== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 1 from 1)
Нарушение на разделянето(segfault)


Yavor Doganov <yavor>
Sat 19 Jul 2014 03:47:24 PM UTC, comment #8: 

Again, please try to run this code with valgrind. Se my comment on bug #42782.

Fred Kiefer <FredKiefer>
Group Member
Mon 14 Jul 2014 09:52:42 PM UTC, comment #7: 

I was sure I mentioned it was the Main.xib file, but apparently missed that.  Sorry.  I tried --GNU-Debug=XIB as soon as I found out the crash happens when loading the XIB file, but was completely lost in the data it spits.  Attached is the compressed output with pristine (unpatched) GNUstep GUI 0.24.0.

As for Valgrind, it used to be completely unusable for GNUstep IIRC.  Haven't touched it for ages, so if any special setup is needed I'd appreciate if you give me some directions.

The Cenon version is 4.0.2; I see I missed mentioning that too.

(file #31715)

Yavor Doganov <yavor>
Mon 14 Jul 2014 09:27:13 PM UTC, comment #6: 

Thank you for finding the commit that caused this problem. If you look at the files included in that commit you will find that it also includes a lot of other changes that actually result in the XIB file being loaded completely not just parts of it. If I remember correctly this was the time when I added the reading of the flattend properties, that is all the out of line changes in the XIB file.
It could well be that we handle one of these extra properties incorrectly.
Now it would be very helpful to know which of the 12 XIB files included in Cenon is causing the issue. To find that out it should be sufficient to start Cenon with the parameter --GNU-Debug=dflt
that should output the file it is currently loading.
With that information I could have a look at the XIB file to see if there is anything special.

You should also use --GNU-Debug=XIB to get more details about the XIB loading.

I really think that you are on to something and most likely it is in the XIB loading code, not just in the NSBox code. Although the code there in initWithCoder: could be improved a lot. It uses the calcSizesAllowingNegative: method way too often.

One other thing that you could try is to run Cenon with valgrind the memcheck setup. If you need help with that, I can provide it.


Fred Kiefer <FredKiefer>
Group Member
Mon 14 Jul 2014 06:16:58 PM UTC, comment #5: 

Moving the NSTitleCell decoding at the beginning as in the attached patch makes Cenon load with 0.24.0.  -calcSizesAllowingNegative: accesses _cell so at first glance it looks like the correct change.  However, if I apply the patch against trunk I get a different crash:

Program received signal SIGSEGV, Segmentation fault.
0xb753c05b in objc_msg_lookup (receiver=0x548b5a59,
    op=0xb7f392b8 <_OBJC_SELECTOR_TABLE+568>)
    at /build/gcc-4.9-C54cEj/gcc-4.9-4.9.0/src/libobjc/sendmsg.c:448
448        /build/gcc-4.9-C54cEj/gcc-4.9-4.9.0/src/libobjc/sendmsg.c: Няма такъв файл или директория.
(gdb) bt
#0  0xb753c05b in objc_msg_lookup (receiver=0x548b5a59,
    op=0xb7f392b8 <_OBJC_SELECTOR_TABLE+568>)
    at /build/gcc-4.9-C54cEj/gcc-4.9-4.9.0/src/libobjc/sendmsg.c:448
#1  0xb7c12567 in -[NSMenu(GNUstepPrivate) _setGeometry] (
    self=0xb7d1c173 <-[NSMenu(NibCompatibility) _setMain:]+675>,
    _cmd=0x8a722a0) at NSMenu.m:505
#2  0x08a722a0 in ?? ()
Backtrace stopped: previous frame inner to this frame (corrupt stack?)


(file #31714)

Yavor Doganov <yavor>
Mon 14 Jul 2014 01:11:04 PM UTC, comment #4: 

As I ran out of ideas I made a binary search.  The "bad" commit is r34050.  It's not easy for me to spot what's wrong, though...

Yavor Doganov <yavor>
Thu 10 Jul 2014 04:26:20 PM UTC, comment #3: 

When built with -O1, -calcSizesAllowingNegative: doesn't crash when called from -setBorderType: (aType there is 3 as expected) but from -setTitlePosition:


Breakpoint 1, -[NSBox setTitlePosition:] (self=0x921f6b0,
    _cmd=0xb7edb618 <_OBJC_SELECTOR_TABLE+600>, aPosition=NSAtTop)
    at NSBox.m:192
192        {
(gdb) n
193          if (_title_position != aPosition)
(gdb)
195              _title_position = aPosition;
(gdb)
196              [_content_view setFrame: [self calcSizesAllowingNegative: NO]];
(gdb)

Program received signal SIGSEGV, Segmentation fault.
0x0921f6c9 in ?? ()
(gdb) bt
#0  0x0921f6c9 in ?? ()
#1  0x42500000 in ?? ()
#2  0x00180071 in ?? ()
#3  0xb7edab8b in _OBJC_METH_VAR_TYPE_5 ()
   from /usr/lib/libgnustep-gui.so.0.24
#4  0x00120056 in ?? ()
#5  0xb7edab95 in _OBJC_METH_VAR_TYPE_4 ()
   from /usr/lib/libgnustep-gui.so.0.24
#6  0x000c0080 in ?? ()
#7  0xb7edab95 in _OBJC_METH_VAR_TYPE_4 ()
   from /usr/lib/libgnustep-gui.so.0.24
#8  0x00190069 in ?? ()
#9  0xb7edab95 in _OBJC_METH_VAR_TYPE_4 ()
   from /usr/lib/libgnustep-gui.so.0.24
#10 0x000f0036 in ?? ()
#11 0xb7edab6c in _OBJC_METH_VAR_TYPE_15 ()
   from /usr/lib/libgnustep-gui.so.0.24
#12 0x000d0080 in ?? ()
#13 0xb7edab6c in _OBJC_METH_VAR_TYPE_15 ()
   from /usr/lib/libgnustep-gui.so.0.24
#14 0x000e0080 in ?? ()
#15 0xb7edab20 in _OBJC_METH_VAR_TYPE_17 ()
   from /usr/lib/libgnustep-gui.so.0.24
#16 0x00090001 in ?? ()
#17 0xb7edab6c in _OBJC_METH_VAR_TYPE_15 ()
   from /usr/lib/libgnustep-gui.so.0.24
#18 0x000a0001 in ?? ()
#19 0xb7eda9a7 in _OBJC_METH_VAR_TYPE_85 ()
   from /usr/lib/libgnustep-gui.so.0.24
#20 0x00130003 in ?? ()
#21 0xb7eda991 in _OBJC_METH_VAR_TYPE_87 ()
   from /usr/lib/libgnustep-gui.so.0.24
#22 0x000f0080 in ?? ()
#23 0xb7edaafb in _OBJC_METH_VAR_TYPE_21 ()
   from /usr/lib/libgnustep-gui.so.0.24
#24 0x00100080 in ?? ()
#25 0xb7eda940 in ?? () from /usr/lib/libgnustep-gui.so.0.24
Backtrace stopped: previous frame inner to this frame (corrupt stack?)

Corrupt stack again.

Yavor Doganov <yavor>
Thu 10 Jul 2014 09:42:35 AM UTC, comment #2: 

Yep, I noticed that dubious aFlag too.  It looks like it is exactly the same problem that was reported in 2013.

If I put a break on -[NSBox setBorderType:], aType is garbage (10-digit number) instead of the expected value of 3.  Going further back to -[GSXibKeyedUnarchiver decodeObjectForXib:forClassName:withID:], "o" is optimized on i386, while it is not on amd64.  So I tried with gnustep-gui rebuilt with no optimization (-O0) and the problem doesn't happen -- Cenon starts and works.  Hope that this gives you some clue. 

It seems that -allocObjectForClassName: doesn't return a valid object and then it gets messy in -initWithCoder:.  But I may be wrong here.
I'd appreciate any directions how to proceed further.

Yavor Doganov <yavor>
Wed 09 Jul 2014 09:31:55 PM UTC, comment #1: 

Looks very similar to the problem that was reported by German Arias on the GNUstep mailing list in January 2013.
At that time I could not reproduce it on my 64 bit machine.

There is one bit in your stack dump that looks suspicious: aFlag=88 'X'
The flag should be a boolean and we only use YES or NO for it. This means something destroyed your stack and we need to find out what this was.

Fred Kiefer <FredKiefer>
Group Member
Wed 09 Jul 2014 01:48:17 PM UTC, original submission:  

Cenon crashes on startup on a 32-bit system (cannot reproduce on amd64/x86_64):


Program received signal SIGSEGV, Segmentation fault.
objc_msg_lookup (receiver=0x85ebc50, op=0xffffff98)
    at /build/gcc-4.9-C54cEj/gcc-4.9-4.9.0/src/libobjc/sendmsg.c:448
448        /build/gcc-4.9-C54cEj/gcc-4.9-4.9.0/src/libobjc/sendmsg.c: Няма такъв файл или директория.
(gdb) bt full
#0  objc_msg_lookup (receiver=0x85ebc50, op=0xffffff98)
    at /build/gcc-4.9-C54cEj/gcc-4.9-4.9.0/src/libobjc/sendmsg.c:448
        result = <optimized out>
#1  0xb7b741b9 in -[NSBox(Private) calcSizesAllowingNegative:] (
    self=0xbfffe940, _cmd=0x921f4a8, aFlag=88 'X') at NSBox.m:757
        titleSize = <optimized out>
        c = <optimized out>
        topMargin = <optimized out>
        borderSize = <optimized out>
        topOffset = <optimized out>
        theme = <optimized out>
        r = {origin = {x = 0, y = 0}, size = {width = 0, height = 0}}
#2  0x00000000 in ?? ()
No symbol table info available.
(gdb) frame 1
#1  0xb7b741b9 in -[NSBox(Private) calcSizesAllowingNegative:] (
    self=0xbfffe940, _cmd=0x921f2b0, aFlag=88 'X') at NSBox.m:757
757        NSBox.m: Няма такъв файл или директория.
(gdb) po [GSTheme theme]
<GSTheme: 0x85ebc50>
(gdb) p _border_type
$8 = 143517512


GNUstep GUI 0.24.0, no themes installed.

Yavor Doganov <yavor>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #31715:  XIB.txt.gz added by yavor (103KiB - application/x-gzip)
file #31714:  nsbox.patch added by yavor (941B - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rmottola (Posted a comment)
  • -email is unavailable- added by CaS (Posted a comment)
  • -email is unavailable- added by FredKiefer (Posted a comment)
  • -email is unavailable- added by yavor (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.

    Only logged-in users can vote.

     

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2014-10-23 rmottola StatusReady For Test Fixed
        Open/ClosedIn Test Closed
    2014-07-23 FredKiefer StatusIn Progress Ready For Test
        Open/ClosedOpen In Test
    2014-07-21 FredKiefer StatusNone In Progress
        Assigned toNone FredKiefer
    2014-07-14 yavor Attached File- Added XIB.txt.gz, #31715
    2014-07-14 yavor Attached File- Added nsbox.patch, #31714

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code