Page MenuHomePhabricator

ASAN: Support detect_invalid_pointer_pairs=1 with detect_stack_use_after_return=1
ClosedPublic

Authored by marxin on Oct 16 2020, 6:40 AM.

Details

Summary

Do not crash when AsanThread::GetStackVariableShadowStart does not find
a variable for a pointer on a shadow stack.

The patch fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97414

Diff Detail

Event Timeline

marxin requested review of this revision.Oct 16 2020, 6:40 AM
marxin created this revision.
kcc added inline comments.Oct 16 2020, 11:31 AM
compiler-rt/lib/asan/asan_thread.cpp
369–371

when does this happen? When a variable is too large to fit in the fake stack?

compiler-rt/test/asan/TestCases/invalid-pointer-pairs-subtract-success.cpp
3–4

Can we test both modes?

27

If the behavior depends on the size of the stack variable, I'd prefer to extend the test to have both cases.

marxin marked 3 inline comments as done.Oct 19 2020, 1:01 AM
marxin added inline comments.
compiler-rt/lib/asan/asan_thread.cpp
369–371

The function is only called from IsInvalidPointerPair which finds whether a pointer is a pointer to a stack/global/heap variable.
So it's fine to return 0 when the function is called for a non-stack variable and fake stacks are enabled.

marxin updated this revision to Diff 298949.Oct 19 2020, 1:01 AM
marxin marked an inline comment as done.
kcc added inline comments.Oct 19 2020, 10:49 AM
compiler-rt/test/asan/TestCases/invalid-pointer-pairs-subtract-success.cpp
27

any thoughts on this comment?

marxin marked an inline comment as done.Oct 20 2020, 12:41 AM
marxin added inline comments.
compiler-rt/test/asan/TestCases/invalid-pointer-pairs-subtract-success.cpp
27

It does not depend on any size of a stack variable. I thought you mean to run the test-case both with shadow stack and without shadow stack.
That's why I added the another // RUN line.

kcc accepted this revision.Oct 20 2020, 9:54 AM

LGTM, thanks!

This revision is now accepted and ready to land.Oct 20 2020, 9:54 AM
This revision was automatically updated to reflect the committed changes.
marxin marked an inline comment as done.
Herald added a project: Restricted Project. · View Herald TranscriptOct 20 2020, 10:29 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript
MaskRay added inline comments.
compiler-rt/lib/asan/asan_thread.cpp
369–371