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

RESOLVED DUPLICATE of bug 1278795

Status

Thunderbird
General
RESOLVED DUPLICATE of bug 1278795
2 years ago
a year ago

People

(Reporter: stransky, Unassigned)

Tracking

(Blocks: 1 bug)

Trunk

Thunderbird Tracking Flags

(thunderbird52?)

Details

(URL)

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
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.

Updated

2 years ago
Blocks: 1054354

Comment 1

2 years ago
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

Comment 2

2 years ago
"Firefox already fixed such parts." Do you have any examples of this? Seeing what FF did would help us to also fix our issues.
(Reporter)

Comment 3

2 years ago
(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
(Reporter)

Comment 4

2 years ago
Created attachment 8725203 [details]
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.

Comment 6

2 years ago
(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.

Comment 8

2 years ago
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.

Comment 10

a year ago
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)

Comment 11

a year ago
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)

Updated

a year ago
tracking-thunderbird52: --- → ?
(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)

Comment 14

a year ago
Nobody responded to comment 11 so I can't say more.
Flags: needinfo?(acelists)
(Reporter)

Comment 15

a year ago
Yes, it's dupe of Bug 1278795.
Status: NEW → RESOLVED
Last Resolved: a year ago
Flags: needinfo?(stransky)
Resolution: --- → DUPLICATE
Duplicate of bug: 1278795
You need to log in before you can comment on or make changes to this bug.