Page MenuHomePhabricator

[hwasan,asan] Intercept vfork.
ClosedPublic

Authored by eugenis on Feb 15 2019, 4:31 PM.

Diff Detail

Event Timeline

eugenis created this revision.Feb 15 2019, 4:31 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptFeb 15 2019, 4:31 PM
Herald added subscribers: Restricted Project, jdoerfert, kristof.beyls and 5 others. · View Herald Transcript
eugenis updated this revision to Diff 187110.Feb 15 2019, 4:35 PM

formatting

pcc accepted this revision.Feb 20 2019, 1:43 PM

LGTM

compiler-rt/test/asan/TestCases/Linux/vfork.cc
4 ↗(On Diff #187110)

Doesn't this need to be spelled REQUIRES: aarch64-target-arch, android? (Also, does it really require Android?)

This revision is now accepted and ready to land.Feb 20 2019, 1:43 PM
vitalybuka accepted this revision.Feb 20 2019, 1:56 PM
vitalybuka added inline comments.
compiler-rt/lib/asan/asan_rtl.cc
602 ↗(On Diff #187110)

redundant CHECK here and below

eugenis marked 2 inline comments as done.Feb 20 2019, 5:49 PM
eugenis added inline comments.
compiler-rt/lib/asan/asan_rtl.cc
602 ↗(On Diff #187110)

why? GetCurrentThread may return 0 is called very early or very late.
There is not much we can do in that case, but the check at least replaces UB with a nice crash.

compiler-rt/test/asan/TestCases/Linux/vfork.cc
4 ↗(On Diff #187110)

I've dumped available features, and that's the only relevant one I found.
Perhaps this could be fixed, but in a separate commit.

eugenis updated this revision to Diff 187705.Feb 20 2019, 5:50 PM

update gn files

pcc added inline comments.Feb 20 2019, 5:59 PM
compiler-rt/test/asan/TestCases/Linux/vfork.cc
4 ↗(On Diff #187110)

That's fine. I noticed that compiler-rt/test/shadowcallstack/overflow-aarch64.c was already using aarch64-target-arch, maybe that's never being run? If so that would probably need to be fixed as well.

This revision was automatically updated to reflect the committed changes.
eugenis marked an inline comment as done.Feb 21 2019, 4:10 PM
eugenis added inline comments.
compiler-rt/test/asan/TestCases/Linux/vfork.cc
4 ↗(On Diff #187110)

I've looked, and this aarch64-android-target-arch is only present in asan and scudo, and it affects exactly 0 tests one way or the other.

I'll remove it in a separate change.