Closed Bug 1039633 Opened 6 years ago Closed 5 years ago

gtest doesn't set the LLVM symbolizer in ASan runs

Categories

(Testing :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla34

People

(Reporter: mccr8, Assigned: mccr8)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

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.
Blocks: 778688
Would you mind taking a look at this? Bug 1039166 is quite frequent at the moment.
Flags: needinfo?(continuation)
Assignee: nobody → continuation
Flags: needinfo?(continuation)
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.
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.)
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.
Blocks: 1050891
(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.
In case anybody else runs into this, here's the hacky patch I wrote to make |mach gtest| use rungtests.py
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
Attachment #8470185 - Flags: review?(ted)
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
Attachment #8470185 - Flags: review?(ted) → review+
Also, set up mozinfo properly in gtest.
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.
Attachment #8473365 - Flags: review?(ted)
No longer blocks: 1050891
Duplicate of this bug: 1050891
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.
Attachment #8473365 - Flags: review?(ted) → review+
Blocks: 1064377
We've branched, so I filed bug 1064377 for the followup mozinfo work for gtest.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
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.
Attachment #8473365 - Attachment description: part 2 - Only try to set ASan symbolizer in ASan runs. → part 2 - Only try to set ASan symbolizer in ASan runs. (Not landed)
You need to log in before you can comment on or make changes to this bug.