Closed
Bug 853494
Opened 9 years ago
Closed 5 years ago
both --disable-optimize and --enable-debug should imply --disable-icf
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
WONTFIX
mozilla23
People
(Reporter: bjacob, Assigned: bjacob)
Details
Attachments
(1 file)
15.83 KB,
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
People use --disable-optimize to get debuggable builds. --disable-icf is very important in this respect because without it, breakpoints are garbled. Please make --disable-optimize imply --disable-icf.
Comment 1•9 years ago
|
||
This is one of the most frustrating aspects of debugging B2G. It's already cost me a lot of time as you find yourself wondering what you're doing in <insert random class here>::<insert random function here> when you haven't even instantiated <said random class> and waste a lot of time only to find out ICF messed something up.
Severity: normal → major
Assignee | ||
Comment 2•9 years ago
|
||
That also means that --disable-icf should be disabled not just by --disable-optimize, but also by something like --enable-debug. The idea is that on B2G, practically nobody is patient enough to use --disable-optimize (people only pass -O0 in specific dirs).
Assignee | ||
Updated•9 years ago
|
Summary: --disable-optimize does not imply --disable-icf → both --disable-optimize and --enable-debug should imply --disable-icf
Assignee | ||
Comment 3•9 years ago
|
||
I tried to fix this, but I'm running into an issue: the configure code that checks for --disable-icf and runs the test to verify that the linker supports the corresponding LDFLAGS, is not coming from configure.in. So I don't see a way to implement "let --disable-optimize and --enable-debug should imply --disable-icf". Halp!
Flags: needinfo?(mh+mozilla)
Comment 4•9 years ago
|
||
Move --enable-debug and --disable-optimize above MOZ_COMPILER_OPTS and set MOZ_DISABLE_ICF=1
Flags: needinfo?(mh+mozilla)
Assignee | ||
Comment 5•9 years ago
|
||
Thanks! This seems to be working here: libxul.so is built with --icf=safe by default in debug builds, and no longer with this patch. However, I am very scared by this patch moving sizable chunks of code by thousands of lines up in configure.in. I have no confidence that this won't break things, e.g. if that code relies on state that changes in between these two distant places.
Attachment #746066 -
Flags: review?(mh+mozilla)
Comment 6•9 years ago
|
||
Comment on attachment 746066 [details] [diff] [review] both --disable-optimize and --enable-debug should imply --disable-icf Review of attachment 746066 [details] [diff] [review]: ----------------------------------------------------------------- ::: configure.in @@ +2622,5 @@ > +dnl = > +dnl = Debugging Options > +dnl = > +dnl ======================================================== > +MOZ_ARG_HEADER(Debugging and Optimizations) You can remove this, it doesn't do anything anyways, and is now inacurate.
Attachment #746066 -
Flags: review?(mh+mozilla) → review+
Assignee | ||
Comment 7•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/11a3ee38afff
Assignee: nobody → bjacob
Target Milestone: --- → mozilla23
Comment 8•9 years ago
|
||
Backed out because this apparently breaks everything! https://hg.mozilla.org/integration/mozilla-inbound/rev/fe3fc1746cae
Assignee | ||
Comment 9•9 years ago
|
||
This likely just needs a clobber.
Comment 10•9 years ago
|
||
"Just needs a clobber" made sense to me, until https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=883e35db4839 came along, built Mac debug with a clobber, and still crashed both the build during leaktests and all the tests. Linux debug also seems to have crashed during startupcache precompilation (which is before packaging, so it didn't get to run tests that also crashed).
Comment 11•9 years ago
|
||
And I retriggered a Mac build on your push after I set it to clobber, back when I thought it was a Mac-only problem, and that one was also exactly as busted as an unclobbered build+tests.
Assignee | ||
Comment 12•9 years ago
|
||
Ah... OK :-( Thanks for trying / for the information. Will look.
Comment 13•9 years ago
|
||
Benoit you should reland this.
Assignee | ||
Comment 14•9 years ago
|
||
By all means, I wouldn't deprive you of that pleasure. Also, I'm off most tomorrow and on Tuesday.
Comment 15•9 years ago
|
||
Benoit, did you ever look into what caused the bustage?
Flags: needinfo?(bjacob)
Comment 18•5 years ago
|
||
This was essentially made moot by bug 904979.
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: needinfo?(mh+mozilla)
Resolution: --- → WONTFIX
Updated•4 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•