1039633 - gtest doesn't set the LLVM symbolizer in ASan runs

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Andrew McCreight [:mccr8]]

Bug 1039166 hits an ASan failure, but the symbolizer is clearly not being set, so somebody needs to get the environment variable ASAN_SYMBOLIZER_PATH set up correctly.  I'm not sure where the environment variable setup happens for this test, which is run during make check currently.

Comment 1

[(not currently active) Ted Mielczarek]

http://mxr.mozilla.org/mozilla-central/source/testing/gtest/rungtests.py is the script that runs gtests.

Comment 2

[Ed Morley [:emorley]]

Would you mind taking a look at this? Bug 1039166 is quite frequent at the moment.

Comment 3

[Andrew McCreight [:mccr8]]

Confusingly, mach gtest does not appear to use rungtests.py, but instead sets up the environment in its own way:

http://mxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/mach_commands.py#643

At least, I think that's what is happening.  rungtests only seems to be invoked when you do make check, not make gtest.

Comment 4

[(not currently active) Ted Mielczarek]

Yeah, that's terrible and we've discussed that on other bugs. Runs on TBPL are running via `make check`, so fixing rungtests.py is fine for that. (We should make the mach command call into rungtests.py which would make everything use the same code.)

Comment 5

[Andrew McCreight [:mccr8]]

Yeah, right now I'm just hacking up |make gtest| to be the same thing as |make check| in that directory, so I can test this easily using mach.

Comment 6

[Andrew McCreight [:mccr8]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #4)
> (We should make the mach command call into rungtests.py which would make
> everything use the same code.)

I filed bug 1050878 for that.

Comment 7

[Andrew McCreight [:mccr8]]

Attachment: make mach gtest use rungtests.py DO NOT LAND

In case anybody else runs into this, here's the hacky patch I wrote to make |mach gtest| use rungtests.py

Comment 8

[Andrew McCreight [:mccr8]]

Attachment: Always try to set the ASan symbolizer in gtest runs.

try: https://tbpl.mozilla.org/?tree=Try&rev=fe1823cf3515

Comment 9

[Andrew McCreight [:mccr8]]

Comment on attachment 8470185 [details] [diff] [review]
Always try to set the ASan symbolizer in gtest runs.

I locally confirmed that this seems to make it work.

Interestingly, it was hard to test because on my machine it is automatically setting up the symbolizer path.  It looks like there's some configure-level stuff that tries to find it, so I wonder if there's some fix that could be applied there so we don't have to hack this up in every single test harness instead.
  http://mxr.mozilla.org/mozilla-central/search?string=LLVM_SYMBOLIZER&filter=[Ll]LVM_SYMBOLIZER

Comment 10

[(not currently active) Ted Mielczarek]

Comment on attachment 8470185 [details] [diff] [review]
Always try to set the ASan symbolizer in gtest runs.

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

r=me to stop the bleeding, but I'd prefer if you fixed the loading mozinfo thing so you could make failure to find the symbolizer an error.

::: testing/gtest/rungtests.py
@@ +87,5 @@
>              else:
>                  env[pathvar] = self.xre_path
> +
> +        # ASan specific environment stuff
> +        # mozinfo is not set up properly to detect if ASan is enabled, so just always set these.

Why don't you just fix this script to properly load mozinfo? It's not hard:
http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/runtests.py#429

Comment 11

[Andrew McCreight [:mccr8]]

Attachment: part 2 - Only try to set ASan symbolizer in ASan runs. (Not landed)

Also, set up mozinfo properly in gtest.

Comment 12

[Andrew McCreight [:mccr8]]

Comment on attachment 8473365 [details] [diff] [review]
part 2 - Only try to set ASan symbolizer in ASan runs. (Not landed)

> Why don't you just fix this script to properly load mozinfo? 

Because I don't know what I'm doing!  Thanks, though, it was easy to fix once you pointed out where the code was.  I just copied it over to gtest.  Now we get an error in ASan builds if the symbolizer isn't found.  I'll land this folded into the previous patch.

Comment 14

[Andrew McCreight [:mccr8]]

I'll just land part 1 for now.

https://hg.mozilla.org/integration/mozilla-inbound/rev/fd08e61066de

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/fd08e61066de

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/03beb594ead1
https://hg.mozilla.org/releases/mozilla-beta/rev/92aead6bd5fb

Comment 17

[(not currently active) Ted Mielczarek]

Comment on attachment 8473365 [details] [diff] [review]
part 2 - Only try to set ASan symbolizer in ASan runs. (Not landed)

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

Thanks! Sorry for the review lag.

Comment 18

[Andrew McCreight [:mccr8]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/824144dd7cdf

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to Andrew McCreight [:mccr8] from comment #18)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/824144dd7cdf

Backed this out in https://hg.mozilla.org/integration/mozilla-inbound/rev/3ea1bc86f28c under suspicion of causing OSX opt build errors: 
https://tbpl.mozilla.org/php/getParsedLog.php?id=46971291&tree=Mozilla-Inbound

Comment 20

[Andrew McCreight [:mccr8]]

We've branched, so I filed bug 1064377 for the followup mozinfo work for gtest.

Comment 21

[Andrew McCreight [:mccr8]]

Comment on attachment 8473365 [details] [diff] [review]
part 2 - Only try to set ASan symbolizer in ASan runs. (Not landed)

The checkin- flag seems to have disappeared, so I guess I'll just mark in the patch name that this isn't checked in.