This is an archive of the discontinued LLVM Phabricator instance.

[scudo][standalone] Enable death tests on Fuchsia
ClosedPublic

Authored by cryptoad on Jan 9 2021, 8:40 AM.

Details

Summary

zxtest doesn't have EXPECT_DEATH and the Scudo unit-tests were
defining it as a no-op.

This enables death tests on Fuchsia by using ASSERT_DEATH instead.
I used a lambda to wrap the expressions as this appears to not be
working the same way as EXPECT_DEATH.

Additionnally, a death test using alarm was failing with the change,
as it's currently not implemented in Fuchsia, so move that test within
a !SCUDO_FUCHSIA block.

Diff Detail

Event Timeline

cryptoad created this revision.Jan 9 2021, 8:40 AM
cryptoad requested review of this revision.Jan 9 2021, 8:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 9 2021, 8:40 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript
mcgrathr added inline comments.Jan 11 2021, 11:16 AM
compiler-rt/lib/scudo/standalone/tests/scudo_unit_test.h
21

The second argument to zxtest's ASSERT_DEATH is not comparable to the gtest EXPECT_DEATH argument.
In zxtest, it's just a message and printf args like in other ASSERT_* macros.
In gtest, it's a matcher for the error output of the crashing process.
In zxtest, the the test is run in a separate thread and not a separate process, so you need to make sure your death tests' assumptions don't include that fouling the process-wide state is OK. There is no facility for matching any particular kind of crash, it just checks that the thread got a fatal exception of some kind.

cryptoad added inline comments.Jan 11 2021, 11:31 AM
compiler-rt/lib/scudo/standalone/tests/scudo_unit_test.h
21

Thank you for pointing this out, I didn't realize it.
Currently most of the matchers are "", with a few being actual regexps, which could lead to confusion when they are dumped out. I could replace Y with "" since I don't think we need additional output information for those.
Regarding the state, for now everything is passing, but I'll go and check them one by one to make sure they don't mess with a process-wide state.

cryptoad updated this revision to Diff 318597.Jan 22 2021, 11:47 AM

Went through the death tests to ensure they were not messing with the
global state.

Ran run fuchsia-pkg://fuchsia.com/libc-unittests-pkg#meta/libc-unittests-pkg.cmx
multiple times on Fuchsia with the patch applied, it all passes.

mcgrathr accepted this revision.Jan 22 2021, 12:41 PM
mcgrathr added a subscriber: michaelrj.

lgtm.
btw, I think fx test --count will do that for you.

This revision is now accepted and ready to land.Jan 22 2021, 12:42 PM
This revision was landed with ongoing or failed builds.Jan 25 2021, 9:20 AM
This revision was automatically updated to reflect the committed changes.