Closed Bug 853494 Opened 7 years ago Closed 2 years ago
both --disable-optimize and --enable-debug should imply --disable-icf
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.
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
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).
Summary: --disable-optimize does not imply --disable-icf → both --disable-optimize and --enable-debug should imply --disable-icf
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!
Move --enable-debug and --disable-optimize above MOZ_COMPILER_OPTS and set MOZ_DISABLE_ICF=1
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 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: nobody → bjacob
Target Milestone: --- → mozilla23
Backed out because this apparently breaks everything! https://hg.mozilla.org/integration/mozilla-inbound/rev/fe3fc1746cae
This likely just needs a clobber.
"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).
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.
Ah... OK :-( Thanks for trying / for the information. Will look.
Benoit you should reland this.
By all means, I wouldn't deprive you of that pleasure. Also, I'm off most tomorrow and on Tuesday.
Benoit, did you ever look into what caused the bustage?
No, not carefully.
glandium: could you please triage this bug?
This was essentially made moot by bug 904979.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.