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

 
 

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

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

Submitted by:  Yavor Doganov <yavor>
Submitted on:  Wed 09 Jul 2014 01:48:17 PM UTC  
 
Category: Gui/AppKitSeverity: 3 - Normal
Item Group: BugStatus: Fixed
Privacy: PublicAssigned to: Fred Kiefer <FredKiefer>
Open/Closed: Closed

(Jump to the original submission 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>
Project 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>
Project MemberIn charge of this item.
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>
Project 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>
Project MemberIn charge of this item.
Sun 20 Jul 2014 03:30:47 PM UTC, comment #9:

Here's the valgrind output:

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>
Project MemberIn charge of this item.
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>
Project MemberIn charge of this item.
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:

(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:

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>
Project MemberIn charge of this item.
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):

GNUstep GUI 0.24.0, no themes installed.

Yavor Doganov <yavor>

 

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
  • -unavailable- added by rmottola (Posted a comment)
  • -unavailable- added by CaS (Posted a comment)
  • -unavailable- added by FredKiefer (Posted a comment)
  • -unavailable- added by yavor (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 8 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Thu 23 Oct 2014 07:14:18 AM UTCrmottolaStatusReady For Test=>Fixed
      Open/ClosedIn Test=>Closed
    Wed 23 Jul 2014 07:57:27 AM UTCFredKieferStatusIn Progress=>Ready For Test
      Open/ClosedOpen=>In Test
    Mon 21 Jul 2014 08:04:46 AM UTCFredKieferStatusNone=>In Progress
      Assigned toNone=>FredKiefer
    Mon 14 Jul 2014 09:52:41 PM UTCyavorAttached File-=>Added XIB.txt.gz, #31715
    Mon 14 Jul 2014 06:16:58 PM UTCyavorAttached File-=>Added nsbox.patch, #31714

    Back to the top


    Powered by Savane 3.1-cleanup1