Page MenuHomePhabricator

[compiler-rt][sanitizers] Fix GetPcSpBp determination of SP on 32-bit Solaris/x86
ClosedPublic

Authored by ro on Jul 13 2020, 3:04 AM.

Details

Summary

A dozen 32-bit AddressSanitizer testcases FAIL on the latest beta of Solaris 11.4/x86, e.g.
AddressSanitizer-i386-sunos :: TestCases/null_deref.cpp produces

AddressSanitizer:DEADLYSIGNAL
=================================================================
==29274==ERROR: AddressSanitizer: stack-overflow on address 0x00000028 (pc 0x08135efd bp 0xfeffdfd8 sp 0x00000000 T0)
    #0 0x8135efd in NullDeref(int*) /vol/llvm/src/llvm-project/dist/compiler-rt/test/asan/TestCases/null_deref.cpp:15:10
    #1 0x8135ea6 in main /vol/llvm/src/llvm-project/dist/compiler-rt/test/asan/TestCases/null_deref.cpp:21:3
    #2 0x8084b85 in _start (null_deref.cpp.tmp+0x8084b85)

 SUMMARY: AddressSanitizer: stack-overflow /vol/llvm/src/llvm-project/dist/compiler-rt/test/asan/TestCases/null_deref.cpp:15:10 in NullDeref(int*)
==29274==ABORTING

instead of the expected

AddressSanitizer:DEADLYSIGNAL
=================================================================
==29276==ERROR: AddressSanitizer: SEGV on unknown address 0x00000028 (pc 0x08135f1f bp 0xfeffdf48 sp 0xfeffdf40 T0)
==29276==The signal is caused by a WRITE memory access.
==29276==Hint: address points to the zero page.
    #0 0x8135f1f in NullDeref(int*) /vol/llvm/src/llvm-project/local/compiler-rt/test/asan/TestCases/null_deref.cpp:15:10
    #1 0x8135efa in main /vol/llvm/src/llvm-project/local/compiler-rt/test/asan/TestCases/null_deref.cpp:21:3
    #2 0x8084be5 in _start (null_deref.cpp.tmp+0x8084be5)

AddressSanitizer can not provide additional info.
 SUMMARY: AddressSanitizer: SEGV /vol/llvm/src/llvm-project/local/compiler-rt/test/asan/TestCases/null_deref.cpp:15:10 in NullDeref(int*)
==29276==ABORTING

I managed to trace this to a change in <sys/regset.h>: previously the header would
primarily define the short register indices (like UESP). While they are required by the
i386 psABI, they are only required in <ucontext.h> and could previously leak into
unsuspecting user code, polluting the namespace and requiring elaborate workarounds
like that in llvm/include/llvm/Support/Solaris/sys/regset.h. The change fixed that by restricting
the definition of the short forms appropriately, at the same time defining all REG_ prefixed
forms for compatiblity with other systems. This exposed a bug in compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp, however:
Previously, the index for the user stack pointer would be hardcoded if REG_ESP
wasn't defined. Now with that definition present, it turned out that REG_ESP was the wrong index to use: the previous value 17 (and REG_SP) corresponds to REG_UESP
instead.

With that change, the failures are all gone.

Tested on amd-pc-solaris2.11. Ok for master?

Diff Detail

Event Timeline

ro created this revision.Jul 13 2020, 3:04 AM
Herald added subscribers: Restricted Project, fedor.sergeev, dberris. · View Herald TranscriptJul 13 2020, 3:04 AM
vitalybuka accepted this revision.Jul 13 2020, 6:37 PM
This revision is now accepted and ready to land.Jul 13 2020, 6:37 PM
ro added a comment.Jul 14 2020, 2:39 AM

Thanks. However, I'm uncertain what to do about the pre-merge check lint warning to re-indent the code. The current style (with whitespace after the #) is pervasive in this file, and changing it in only one place seems to make things worse to me. What should I do here?

In D83664#2149678, @ro wrote:

Thanks. However, I'm uncertain what to do about the pre-merge check lint warning to re-indent the code. The current style (with whitespace after the #) is pervasive in this file, and changing it in only one place seems to make things worse to me. What should I do here?

Please ignore for this patch.

This revision was automatically updated to reflect the committed changes.