In bug 889078, support for stripping the cppunittest binaries was added. However, the binaries are stripped regardless of the --disable-install-strip setting which is supposed to prevent stripping of packaged binaries (which should include test binaries). Stripped binaries cannot be used to symbolize ASan traces, so it would be good to get unstripped tests when this particular option is set. Especially when the cppunittests are moved into a test job, this would break ASan symbolizing. Patch coming soon.
Does ASan need debug info, or just unstripped symbols? In the latter case, then you don't want disable-install-strip, you want to set STRIP_FLAGS.
(In reply to Mike Hommey [:glandium] from comment #1) > Does ASan need debug info, or just unstripped symbols? In the latter case, > then you don't want disable-install-strip, you want to set STRIP_FLAGS. I think it just needs symbols. What kind of strip flags should I try for that purpose? I will try it locally then and if that works, I'll adjust our configurations. Even if that works though, we still need a patch here because according to the code, STRIP_FLAGS is also not used when stripping the tests.
Okay, I've tried out --strip-debug but with that, we miss the file and line information which is critical. However, --only-keep-debug seems to do what we want. Currently doing a try run with the changes.
I don't think you want --only-keep-debug. If you need file/line info then --strip-debug isn't going to work, you're going to have to stick with unstripped, or fix everything to use Breakpad symbols.
Summary: Don't strip cppunittests when --disable-install-strip is set → Honor --disable-install-strip and STRIP_FLAGS when packaging compiled tests
Created attachment 811038 [details] [diff] [review] strip-flags.patch Okay, I've written a patch now that just ensures that both the install-strip option and the STRIP_FLAGS variable are honored when packaging the tests. I did not change any of the default options for ASan builds. In another bug, I might consider building ASan with -gline-tables-only which makes stripping unnecessary, while still reducing the file size dramatically.
Assignee: nobody → choller
Status: NEW → ASSIGNED
Comment on attachment 811038 [details] [diff] [review] strip-flags.patch Review of attachment 811038 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/testsuite-targets.mk @@ +500,5 @@ > + STRIP_FLAGS=--strip-unneeded > +endif > +else > + STRIP_FLAGS= > +endif I don't think I'd bother with this block. Doesn't seem worth fiddling the STRIP_FLAGS if they're not already set. I'd probably just do Instead of the above, you could just replace this with ifdef OBJCOPY ifndef PKG_SKIP_STRIP STRIP_CPP_TESTS := 1 endif endif Then replace the ifdef OBJCOPY below with ifdef STRIP_CPP_TESTS.
Created attachment 811089 [details] [diff] [review] strip-flags.patch Patch v2 with comments addressed.
Attachment #811089 - Flags: review?(ted) → review+
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
You need to log in before you can comment on or make changes to this bug.