Closed Bug 1251576 Opened 8 years ago Closed 8 years ago

GCC6 - TB crashes due to removed null pointer checks for "this"

Categories

(Thunderbird :: General, defect)

defect
Not set
normal

Tracking

(thunderbird52?)

RESOLVED DUPLICATE of bug 1278795
Tracking Status
thunderbird52 ? ---

People

(Reporter: stransky, Unassigned)

References

()

Details

Attachments

(1 file)

GCC6 Optimizations remove null pointer checks for this which causes crashes so this code (and similar) no longer works:

http://mxr.mozilla.org/comm-central/source/db/mork/src/morkRowObject.cpp#331

Firefox already fixed such parts.

To avoid crashes it's possible to build with "-fno-delete-null-pointer-checks" flag which disables such optimization. see https://gcc.gnu.org/gcc-6/porting_to.html for details.
I am not sure where to ask. But now that
this GCC6 issue is filed and blocks C-C TB, too.

Is GCC 5.3 ever going to be used by mozilla compilation farm? (Tryserver, say?)
I am asking because when I began using 5.3.x on Debian GNU/Linux locally on my PC, I needed to apply a patch about incomplete array
to compile files under media/libvpx.
I obtained a patch from Ubuntu bugzilla. 

Should I submit a bugzilla like this bugzilla entry?

Or is Mozilla going to skip to GCC6?
(In that case also, though, the patch about incomplete array needs to be applied if I understand correctly.).

TIA
"Firefox already fixed such parts." Do you have any examples of this? Seeing what FF did would help us to also fix our issues.
(In reply to Kent James (:rkent) from comment #2)
> "Firefox already fixed such parts." Do you have any examples of this? Seeing
> what FF did would help us to also fix our issues.

Se Bug 1167145 for reference. The problem is when you call method on null object. You can use those two build option to find affected code:

-Wnonnull-compare - checks build-time
-fsanitize=null - checks run-time
Attached file null this warning list
There's a list of warnings produced by -Wnonnull-compare parameter. "this" should not be compared to NULL because it's supposed to be always valid.
Thunderbird became unusable in openSUSE Tumbleweed, after last night it appears to have done some move toward GCC6. I hope a fix can be provided urgently so that functionality may be restored! I'm noticing this has been known for months... it's hard to think of why nothing was done for so long, considering this should have been suspected.
(In reply to sonichedgehog_hyperblast00@yahoo.com from comment #5)
> Thunderbird became unusable in openSUSE Tumbleweed, after last night it
> appears to have done some move toward GCC6.

That's being worked around in https://bugzilla.suse.com/show_bug.cgi?id=986162 - but this upstream bug here should be fixed to not need the workarounds in the future.
I understand. I thought the bug is related to a problem in main Thunderbird, which affects it whenever compiled with gcc6. I am waiting for a fix on any end, depending where the real fault lies.
I'd like to remove if(this) but I need to understand the ramifications.

Why does the crash occur? That is, is the existing if(this) doing some positive function, such that removing it will cause other users to start crashing?
All I know is that, Thunderbird needs to be compiled with the flag -fno-delete-null-pointer-checks on gcc6, otherwise it will crash. Whether that's a fault in the mainstream Thunderbird code or in how openSUSE compiles it, the developers know best.
aceman: I remember some if(this) stuff cropping up in the mega warnings-as-errors cleanup a few months ago. Does that mean this is now fixed?  We should probably take a look before this ESR.
Flags: needinfo?(acelists)
This seems to be the same as bug 1278795. I commented there in comment 14.

So can anybody reproduce the problem with current trunk? Last comment here is June 2016.

I still have not heard if the crash is caused by:
1. the fact the checks are there (and gcc6 forcibly crashes the app);
2. gcc6 optimizes away the checks and we hit some corrupt object and crash (so that the checks actually are needed).

Can anybody do tests confirming either of those claims? I do not have gcc6 yet. But as we have removed the checks from the tree, the checks are not there even on gcc5. And we do not see the crashes. So that would point to case 1. being the problem. Which is then solved as the checks were removed since.
Flags: needinfo?(acelists)
(In reply to :aceman from comment #11)
> This seems to be the same as bug 1278795. I commented there in comment 14.
> 
> So can anybody reproduce the problem with current trunk? Last comment here
> is June 2016.

dupe to bug 1278795?  (which has more detail)
Flags: needinfo?(vseerror)
(In reply to Wayne Mery (:wsmwk, NI for questions) from comment #12)
> (In reply to :aceman from comment #11)
> > This seems to be the same as bug 1278795. I commented there in comment 14.
> > 
> > So can anybody reproduce the problem with current trunk? Last comment here
> > is June 2016.
> 
> dupe to bug 1278795?  (which has more detail)

apologies, I can't remember which of you, Martin or aceman, I intended to ask
Flags: needinfo?(vseerror)
Flags: needinfo?(stransky)
Flags: needinfo?(acelists)
Nobody responded to comment 11 so I can't say more.
Flags: needinfo?(acelists)
Yes, it's dupe of Bug 1278795.
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: needinfo?(stransky)
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: