This is an archive of the discontinued LLVM Phabricator instance.

[NFC][sanitizer] Remove calls to __asan_get_current_fake_stack
ClosedPublic

Authored by kda on Jun 11 2021, 2:06 PM.

Details

Summary

Unnecessary with -fsanitize-address-use-after-return=never.

for issue: https://github.com/google/sanitizers/issues/1394

Diff Detail

Event Timeline

kda requested review of this revision.Jun 11 2021, 2:06 PM
kda created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJun 11 2021, 2:06 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript

I'd prefer we just update __asan_get_current_fake_stack()

kda added a comment.Jun 14 2021, 8:27 AM

I'd prefer we just update __asan_get_current_fake_stack()

Yeah, but all of these would prefer to assume that there is no fake stack.
With =never, there is no fake stack.

vitalybuka added inline comments.Jun 14 2021, 2:15 PM
compiler-rt/test/asan/TestCases/Posix/stack-overflow.cpp
71–72

And you can revert all "// RUN:" lines

compiler-rt/test/asan/TestCases/contiguous_container.cpp
72

why this does not work for fake stake?
do this tests fail with fake stack without __asan_get_current_fake_stack() check?

vitalybuka added inline comments.Jun 14 2021, 2:17 PM
compiler-rt/test/asan/TestCases/Posix/stack-overflow.cpp
78
87
112
vitalybuka added inline comments.Jun 14 2021, 2:18 PM
compiler-rt/test/asan/TestCases/Posix/stack-overflow.cpp
87

probably __builtin_frame_address is fine here as well

112

and here

kda updated this revision to Diff 352298.Jun 15 2021, 5:00 PM
  • remove UAR=never flag from contiguous_container.cpp.
compiler-rt/test/asan/TestCases/Posix/stack-overflow.cpp
71–72

I tried all these and it does not work.
__builtin_frame_address needs a parameter, I provided '0'.
recursive_func(-1): does not compile (cannot convert 'int' to 'char*').

Also, I think that passing -1 to recursive_func, is like passing a max INT, and consequently will always be greater than any value returned by __builtin_frame_address.
The above assert proves nothing.

compiler-rt/test/asan/TestCases/contiguous_container.cpp
72

I don't know why. I thought this looked useless and redundant, as the fake stack is off by default.
The test passes without the __asan_get_current_fake_stack check.

use __builtin_frame_address

vitalybuka retitled this revision from [sanitizer] Replace calls to __asan_get_current_fake_stack with -fsanitize-address-use-after-return=never in tests. (NFC) to [NFC][sanitizer] Remove calls to __asan_get_current_fake_stack.Jun 15 2021, 6:47 PM
vitalybuka edited the summary of this revision. (Show Details)

I've updated the summary, usually shorter title of the git commit looks nicer.

compiler-rt/test/asan/TestCases/Posix/stack-overflow.cpp
71–72

I've updated the patch. Works this way.

compiler-rt/test/asan/TestCases/contiguous_container.cpp
72

I see
these are special cases when Asan bailout by poisoning entire stack (including callers) to make sure exception handler is executed on poisoned stack.
So for regular stack rezones it will remove red-zones around x[].
But we don't need to clear FakeStack, exception handler will just use new FakeFrame, so we can keep redzones in the current FakeStack around x[]

vitalybuka accepted this revision.Jun 15 2021, 6:49 PM
This revision is now accepted and ready to land.Jun 15 2021, 6:49 PM
This revision was landed with ongoing or failed builds.Jun 15 2021, 6:52 PM
This revision was automatically updated to reflect the committed changes.