917242 - Symbolize ASan traces in automation

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Christian Holler (:decoder)]

Now that we have tests running under ASan visible on TBPL, it's important to see a symbolized trace for each failure (especially for the sheriffs).

There are two possible ways to do this:

1) Pipe the output of every test harness through asan_symbolize.py

or 

2) Run every binary with the ASAN_SYMBOLIZER_PATH environment variable set to the llvm-symbolizer binary that comes with Clang.


The second approach seems easier to implement because some tests like compiled unit tests or jit-tests don't have any existing support for post-processing the logs (like e.g. Mochitests have). However, because most of the tests don't run on the build host, but on test slaves, the llvm-symbolizer binary is not available. I therefore suggest we package the llvm-symbolizer binary.

I have a working patch right now that does the following things (for ASan builds only):

1) Packages llvm-symbolizer together with the rest of firefox. I explicitly decided to put it in this package because people that download the builds will also want to have that binary for easier symbolizing. One more reason is that not all test frameworks are aware of a utilityPath, so it's easier to put it in the binary package.

2) Everything using automation.py.in will automatically symbolize stuff if llvm-symbolizer is found in the xrePath.

3) For jit-tests and compiled tests, I install the llvm-symbolizer binary to $(DIST)/bin and ensure they use it. These two need special treatment because they run after the build, rather than running on the test slaves from our packaged build. When these two are moved to test slaves, they probably can use the same method that e.g. mochitests usw.


Here's an example try run with an orange cppunit test and an orange mochitest, both symbolized: https://tbpl.mozilla.org/?tree=Try&rev=2a1d0a0cc1f0


The patch has one unsolved problem right now: The initial path to llvm-symbolizer is hardcoded for TBPL, rather than being specified in the mozconfig like CC/CXX is. Maybe someone more familiar with the build system can help me implement that last step.

Comment 1

[Christian Holler (:decoder)]

Attachment: asan-stacks-test.patch

Here's my proof-of-concept patch. Ted, can you help me to get rid of the hardcoded paths (typically "$(topsrcdir)/clang/bin/llvm-symbolizer") and replace it by a variable specified in the mozconfig, just like we specify the path the clang there?

If there is something else wrong with the patch, please also let me know.

Thanks!

Comment 2

[Ed Morley [:emorley]]

(TBPL is just a lowly dashboard :-))

Comment 3

[(not currently active) Ted Mielczarek]

Comment on attachment 805934 [details] [diff] [review]
asan-stacks-test.patch

Review of attachment 805934 [details] [diff] [review]:
-----------------------------------------------------------------

So this is mostly okay, but the copying bits are a little weird. I'd do the following:
1) use a MOZ_PATH_PROG(LLVM_SYMBOLIZER, llvm-symbolizer) then AC_SUBST(LLVM_SYMBOLIZER) in configure to find the path and get it into a Makefile variable.
2) Use INSTALL_TARGETS somewhere (maybe build/unix/Makefile.in?) to get the binary installed into dist/bin.
3) Add the binary to package-manifest.in #ifdef ASAN.

Comment 4

[Christian Holler (:decoder)]

Attachment: asan-stacks-test.patch

Alright, here's the new version, that implements what Ted suggested during the previous feedback cycle. I ran this through try and it's green on normal and ASan linux builds. I also tested it with two ASan failures on try, one I introduced in make check, the other on mochitest-1, and both symbolize correctly.

If there's anything left unclear, let me know here or ping me on IRC :) Thanks.

Comment 5

[(not currently active) Ted Mielczarek]

Comment on attachment 807331 [details] [diff] [review]
asan-stacks-test.patch

Review of attachment 807331 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/Makefile.in
@@ +300,5 @@
>  
> +ifdef MOZ_ASAN
> +ifneq ($(LLVM_SYMBOLIZER),)
> +# Use the LLVM symbolizer when running jit-tests under ASan, if available
> +JITTEST_ASAN_ENV=ASAN_SYMBOLIZER_PATH='$(LLVM_SYMBOLIZER)'

You'll want to get this fixed for jit tests running from the test package as well. Ask dminor about this.

Comment 6

[Christian Holler (:decoder)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/67fa64cde217

Comment 7

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/67fa64cde217

Comment 8

[Christian Holler (:decoder)]

It seems like we're doing ASan also on mozilla-aurora now and yesterday we hit an important intermittent which we couldn't symbolize later on. Should we uplift this to aurora so we can see intermittent traces when they happen there?

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Yes, please request approval.

Comment 10

[Christian Holler (:decoder)]

Comment on attachment 807331 [details] [diff] [review]
asan-stacks-test.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Not a regression, ASan tests have been enabled by bug 831491.
User impact if declined: Intermittent ASan regressions shown on TBPL for mozilla-aurora cannot be properly triaged/identified.
Testing completed (on m-c, etc.): Tested in production (m-c) for 2 days now.
Risk to taking this patch (and alternatives if risky): None, this patch is automation/infrastructure only, restricted to ASan, and no code changes are made.
String or IDL/UUID changes made by this patch: None

Comment 11

[(not currently active) Ted Mielczarek]

Considering this is 90% test-only, and only impacts ASAN builds, this should be very low-risk.

Comment 12

[Alex Keybl [:akeybl]]

Comment on attachment 807331 [details] [diff] [review]
asan-stacks-test.patch

No risk to the client, and better for our development. Approving for Aurora.

Comment 13

[Christian Holler (:decoder)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/7eebf2d65429