1289098 - Fixes to avoid Valgrind false positives with gcc-5.4 -O2 builds

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Julian Seward [:jseward]]

This is similar to bug 1122375, which does the same for gcc-4.9.X
builds, except that 5.4 is a bit more aggressive in optimising and so
there are a couple more places that need to be patched.  I had hoped
to do this for gcc-6.0 builds, but at least as of my last testing a
couple of weeks ago, we can't currently build a working Fx with
gcc-6.0 at -O or above.

Comment 1

[Julian Seward [:jseward]]

To summarise, for all of these, the problem is that recent gccs
(and, AFAIK, clangs) will do this transformation

  A && B  -->  B && A    if A is false whenever B is undefined

It's a bit mind-bending, but if you stare at it long enough, you
can see it is correct.  The difficulty is that Valgrind/Memcheck
assumes that all conditional branches have an effect on the outcome
of the computation, and so it complains about branching on B when it
is undefined.

As gcc/clang gradually become more aggressive in optimisation, and in
particular, as they analyse ever larger sections of code, they will
presumably be able to find ever more places where the "if A is false .."
condition holds, and the transformation is desirable.  I assume that's
why we are gradually seeing more of these occurrences.

Comment 2

[Julian Seward [:jseward]]

Attachment: bug1289098-1.diff

Initialisations for nsStyleTextReset::CalcDifference and 
nsStyleContext::CalcStyleDifference.

Comment 3

[Julian Seward [:jseward]]

Comment on attachment 8775918 [details] [diff] [review]
bug1289098-1.diff

This is an ultra-trivial patch that gives two additional initialisations
needed to keep Valgrind/Memcheck quiet on gcc-5.3 -O2 generated code.
Currently these are the only changes of this kind needed in Gecko for
the gcc-4.9 to 5.3 transition, that I am aware of.

Comment 4

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 8775918 [details] [diff] [review]
bug1289098-1.diff

Not happy about it, but r=dbaron.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6041c4a602ea
Fixes to avoid Valgrind false positives with gcc-5.4 -O2 builds.  r=dbaron.

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6041c4a602ea