This is an archive of the discontinued LLVM Phabricator instance.

[hwasan] work around lifetime issue with setjmp.
ClosedPublic

Authored by fmayer on Jan 31 2022, 1:29 PM.

Details

Summary

setjmp can return twice, but PostDominatorTree is unaware of this. as
such, it overestimates postdominance, leaving some cases (see attached
compiler-rt) where memory does not get untagged on return. this causes
false positives later in the program execution.

this is a crude workaround to unblock use-after-scope for now, in the
longer term PostDominatorTree should bemade aware of returns_twice
function, as this may cause problems elsewhere.

Diff Detail

Event Timeline

fmayer created this revision.Jan 31 2022, 1:29 PM
fmayer requested review of this revision.Jan 31 2022, 1:29 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJan 31 2022, 1:29 PM
Herald added subscribers: llvm-commits, Restricted Project. · View Herald Transcript
fmayer updated this revision to Diff 404708.Jan 31 2022, 1:30 PM

missing newline at eof

fmayer updated this revision to Diff 404713.Jan 31 2022, 1:33 PM

even more comments

fmayer updated this revision to Diff 404716.Jan 31 2022, 1:36 PM

cosmetic changes

fmayer updated this revision to Diff 404718.Jan 31 2022, 1:37 PM

more cosmetic changes

This is a great find. @pcc
It seems like it would affect AArch64StackTagging, and some of the sanitizers as well (ASan in particular).
Also I don't see anything in CodeGen/StackColoring.cpp to avoid this case - is it possible for it to cause erroneous stack slot reuse?

llvm/test/Instrumentation/HWAddressSanitizer/use-after-scope-setjmp.ll
27

so the callee of this may longjmp, bypassing the lifetime.end. Effectively, this adds a DT edge from any call site in this function to immediately after the setjmp call.

Please add some comments here explaining this.

eugenis added inline comments.Jan 31 2022, 1:54 PM
llvm/test/Instrumentation/HWAddressSanitizer/use-after-scope-setjmp.ll
27

I meant to say "call graph edge".

fmayer updated this revision to Diff 404742.Jan 31 2022, 2:49 PM
fmayer marked an inline comment as done.

cosmetic changes

fmayer marked an inline comment as done.Jan 31 2022, 2:50 PM

This is a great find. @pcc
It seems like it would affect AArch64StackTagging, and some of the sanitizers as well (ASan in particular).
Also I don't see anything in CodeGen/StackColoring.cpp to avoid this case - is it possible for it to cause erroneous stack slot reuse?

yep, as explained in the commit message i sent this CL to unblock the use-after-scope rollout while we figure out how to deal with the potentially bigger problem.

fmayer updated this revision to Diff 404801.Jan 31 2022, 8:18 PM

stylistic change

Did you forget to include a file?

Oh I get it you are shadowing a class member. Let's not do that, it's confusing.

Oh I get it you are shadowing a class member. Let's not do that, it's confusing.

Fixed.

eugenis accepted this revision.Feb 1 2022, 11:30 AM

LGTM

compiler-rt/test/hwasan/TestCases/use-after-scope-setjmp.cpp
4

I don't see any reason to limit the test to aarch64.

This revision is now accepted and ready to land.Feb 1 2022, 11:30 AM
fmayer marked an inline comment as done.Feb 1 2022, 12:13 PM
fmayer added inline comments.
compiler-rt/test/hwasan/TestCases/use-after-scope-setjmp.cpp
4

I'll leave this for now for consistency with the other hwasan compiler-rt tests. We can bulk remove them after.

This revision was landed with ongoing or failed builds.Feb 1 2022, 12:14 PM
This revision was automatically updated to reflect the committed changes.
fmayer marked an inline comment as done.