Closed
Bug 1039633
Opened 7 years ago
Closed 7 years ago
gtest doesn't set the LLVM symbolizer in ASan runs
Categories
(Testing :: General, defect)
Testing
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla34
People
(Reporter: mccr8, Assigned: mccr8)
References
(Blocks 1 open bug)
Details
Attachments
(3 files)
1.56 KB,
patch
|
Details | Diff | Splinter Review | |
1.55 KB,
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
3.19 KB,
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
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•7 years ago
|
||
http://mxr.mozilla.org/mozilla-central/source/testing/gtest/rungtests.py is the script that runs gtests.
Comment 2•7 years ago
|
||
Would you mind taking a look at this? Bug 1039166 is quite frequent at the moment.
Flags: needinfo?(continuation)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → continuation
Flags: needinfo?(continuation)
Assignee | ||
Comment 3•7 years ago
|
||
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•7 years ago
|
||
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.)
Assignee | ||
Comment 5•7 years ago
|
||
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.
Assignee | ||
Comment 6•7 years ago
|
||
(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.
Assignee | ||
Comment 7•7 years ago
|
||
In case anybody else runs into this, here's the hacky patch I wrote to make |mach gtest| use rungtests.py
Assignee | ||
Comment 8•7 years ago
|
||
try: https://tbpl.mozilla.org/?tree=Try&rev=fe1823cf3515
Assignee | ||
Comment 9•7 years ago
|
||
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 10•7 years ago
|
||
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+
Assignee | ||
Comment 11•7 years ago
|
||
Also, set up mozinfo properly in gtest.
Assignee | ||
Comment 12•7 years ago
|
||
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)
Assignee | ||
Comment 14•7 years ago
|
||
I'll just land part 1 for now. https://hg.mozilla.org/integration/mozilla-inbound/rev/fd08e61066de
Keywords: leave-open
Comment 16•7 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/03beb594ead1 https://hg.mozilla.org/releases/mozilla-beta/rev/92aead6bd5fb
Comment 17•7 years ago
|
||
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+
Assignee | ||
Comment 18•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/824144dd7cdf
Keywords: leave-open
(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
Assignee | ||
Comment 20•7 years ago
|
||
We've branched, so I filed bug 1064377 for the followup mozinfo work for gtest.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Assignee | ||
Comment 21•7 years ago
|
||
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.
Description
•