Fixes to avoid Valgrind false positives with gcc-4.9.x -O2 builds
RESOLVED
FIXED
in mozilla38
Status
()
People
(Reporter: jseward, Assigned: jseward)
Tracking
Firefox Tracking Flags
(Not tracked)
Details
Attachments
(4 attachments)
2.65 KB,
text/plain
|
Details | |
865 bytes,
patch
|
roc
:
review+
|
Details | Diff | Splinter Review |
4.54 KB,
text/plain
|
Details | |
2.08 KB,
patch
|
dbaron
:
review+
|
Details | Diff | Splinter Review |
gcc-4.9.x on x86_64 (at least, maybe other targets) performs apparently new optimisations at -O2 that confuse Valgrind/Memcheck's uninitialised value tracking instrumentation and result in false error reports. As far as I can tell, this only happens in two places in the tree. On examination of the machine code, in both cases the nature of the transformation makes it difficult and/or a big performance hit to fix Valgrind, so a preferable approach is to provide minimal dummy initialisations.
(Assignee) | ||
Comment 1•4 years ago
|
||
In the first of the two problems, in layout/base/nsDisplayList.cpp, gcc-4.9 appears to transform if (e1 && e2) ... into if (e2 && e1) ... in the case where it can show that e1 is |false| whenever |e2| is undefined. This causes Memcheck to complain because it has the (deeply wired in) assumption that every conditional branch is important and therefore must be based on defined values.
(Assignee) | ||
Comment 2•4 years ago
|
||
Created attachment 8550092 [details] Valgrind error report for the problem in comment 1
(Assignee) | ||
Comment 3•4 years ago
|
||
Created attachment 8550094 [details] [diff] [review] Proposed fix for the problem in comment 1
Attachment #8550094 -
Flags: review?(roc)
(Assignee) | ||
Comment 4•4 years ago
|
||
In the second of the two problems, in layout/style/nsStyleContext.cpp, gcc-4.9 produces code which is pretty gnarly to say the least, but afaics depends on the observation that undefined-value < 0 is defined in the case where the comparison is unsigned, since the result will always be |false|.
(Assignee) | ||
Comment 5•4 years ago
|
||
Created attachment 8550100 [details] Valgrind error reports for the problem in comment 4
(Assignee) | ||
Comment 6•4 years ago
|
||
Created attachment 8550112 [details] [diff] [review] Proposed fix for the problem in comment 4
Attachment #8550112 -
Flags: review?(dbaron)
Comment 7•4 years ago
|
||
Comment on attachment 8550112 [details] [diff] [review] Proposed fix for the problem in comment 4 Ugh. I guess so. But put a space after all the ","s.
Attachment #8550112 -
Flags: review?(dbaron) → review+
Attachment #8550094 -
Flags: review?(roc) → review+
(Assignee) | ||
Comment 8•4 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/59dec5ab3a40 https://hg.mozilla.org/integration/mozilla-inbound/rev/170aee1be0dd
Comment 9•4 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/59dec5ab3a40 https://hg.mozilla.org/mozilla-central/rev/170aee1be0dd
Assignee: nobody → jseward
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in
before you can comment on or make changes to this bug.
Description
•