This is an archive of the discontinued LLVM Phabricator instance.

[asan][test] Several Posix/unpoison-alternate-stack.cpp fixes
ClosedPublic

Authored by ro on Sep 29 2020, 10:10 AM.

Details

Summary

Posix/unpoison-alternate-stack.cpp currently FAILs on Solaris/i386 and the investigation revealed several generic problems:

  • clang warns compiling the testcase:
/vol/llvm/src/llvm-project/local/compiler-rt/test/asan/TestCases/Posix/unpoison-alternate-stack.cpp:83:7: warning: nested designators are a C99 extension [-Wc99-designator]
      .sa_sigaction = signalHandler,
      ^~~~~~~~~~~~~
/vol/llvm/src/llvm-project/local/compiler-rt/test/asan/TestCases/Posix/unpoison-alternate-stack.cpp:84:7: warning: ISO C++ requires field designators to be specified in declaration order; field '_funcptr' will be initialized after field 'sa_flags' [-Wreorder-init-list]
      .sa_flags = SA_SIGINFO | SA_NODEFER | SA_ONSTACK,
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/vol/llvm/src/llvm-project/local/compiler-rt/test/asan/TestCases/Posix/unpoison-alternate-stack.cpp:83:8: note: previous initialization for field '_funcptr' is here
      .sa_sigaction = signalHandler,
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/sys/signal.h:71:30: note: expanded from macro 'sa_sigaction'
#define sa_sigaction    _funcptr._sigaction
                                ^
/vol/llvm/src/llvm-project/local/compiler-rt/test/asan/TestCases/Posix/unpoison-alternate-stack.cpp:149:7: warning: ISO C++ requires field designators to be specified in declaration order; field 'ss_flags' will be initialized after field 'ss_size' [-Wreorder-init-list]
      .ss_size = AltStackSize,
      ^~~~~~~~~~~~~~~~~~~~~~~
/vol/llvm/src/llvm-project/local/compiler-rt/test/asan/TestCases/Posix/unpoison-alternate-stack.cpp:148:19: note: previous initialization for field 'ss_flags' is here
      .ss_flags = 0,
                  ^
This can all easily be avoided by initializing each field separately.
  • The test SEGVs:
Thread 4 received signal SIGSEGV, Segmentation fault.
[Switching to Thread 2 (LWP 2)]
0x08157f0f in __asan_memcpy (to=0xfe5d2e30, from=0x808cda0 <__const._ZN12_GLOBAL__N_123setSignalAlternateStackEPv.Action>, size=32) at /vol/llvm/src/llvm-project/dist/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:21
21	void *__asan_memcpy(void *to, const void *from, uptr size) {
(gdb) where
#0  0x08157f0f in __asan_memcpy (to=0xfe5d2e30, from=0x808cda0 <__const._ZN12_GLOBAL__N_123setSignalAlternateStackEPv.Action>, size=32) at /vol/llvm/src/llvm-project/dist/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:21
#1  0x081b080e in (anonymous namespace)::setSignalAlternateStack(void*) () at /vol/llvm/src/llvm-project/dist/compiler-rt/test/asan/TestCases/Posix/unpoison-alternate-stack.cpp:86
#2  0x081b02bc in (anonymous namespace)::threadFun(void*) () at /vol/llvm/src/llvm-project/dist/compiler-rt/test/asan/TestCases/Posix/unpoison-alternate-stack.cpp:107
#3  0x0816dd2a in __asan::AsanThread::ThreadStart (this=0xfe3f3000, os_id=2, signal_thread_is_registered=0xfeffd724) at /vol/llvm/src/llvm-project/dist/compiler-rt/lib/asan/asan_thread.cpp:262
#4  0x081400d0 in asan_thread_start (arg=0xfeffd720) at /vol/llvm/src/llvm-project/dist/compiler-rt/lib/asan/asan_interceptors.cpp:205
#5  0xfe281d3b in _thrp_setup () from /lib/libc.so.1
#6  0xfe282090 in ?? () from /lib/libc.so.1
in the sequence determing the GOT address at the `calll` insn:
	subl	$5424, %esp                     # imm = 0x1530
	calll	.L1$pb
.L1$pb:
The problem is that the default Solaris/i386 stack size is only 4 kB, while `__asan_memcpy` tries to allocate either 5436 (32-bit) or 10688 bytes (64-bit) on the stack.  This patch avoids this by requiring at least 16 kB stack size.
  • When I later investigated PlatformUnpoisonStacks calling GetThreadStackAndTls, I was reminded that GetTls is a dummy on Solaris. I do have a (not yet fully portable) implemtation, however that's moot for this testcase.
  • Even without -fsanitize=address or in a minimal testcase derived from this one, I get an assertion failure:
Assertion failed: !isOnSignalStack(), file /vol/llvm/src/llvm-project/dist/compiler-rt/test/asan/TestCases/Posix/unpoison-alternate-stack.cpp, line 117
The fundamental problem with this testcase is that `longjmp` from a signal handler is highly unportable; XPG7 strongly warns against it and it is thus unspecified which stack is used when `longjmp`ing from a signal handler running on an alternative stack.

So I'm XFAILing this testcases on Solaris.

Tested on amd64-pc-solaris2.11 and x86_64-pc-linux-gnu.

Diff Detail

Event Timeline

ro created this revision.Sep 29 2020, 10:10 AM
Herald added subscribers: Restricted Project, fedor.sergeev. · View Herald TranscriptSep 29 2020, 10:10 AM
ro requested review of this revision.Sep 29 2020, 10:10 AM
vitalybuka accepted this revision.Sep 30 2020, 4:01 AM

Please do not include entire description into git commit.
A couple of sentences should be enough.

compiler-rt/test/asan/TestCases/Posix/unpoison-alternate-stack.cpp
89–91
156–159

same

This revision is now accepted and ready to land.Sep 30 2020, 4:01 AM
ro marked an inline comment as done.Sep 30 2020, 7:03 AM
ro added inline comments.
compiler-rt/test/asan/TestCases/Posix/unpoison-alternate-stack.cpp
89–91

This doesn't compile on Linux/x86_64:

/vol/llvm/src/llvm-project/local/compiler-rt/test/asan/TestCases/Posix/unpoison-alternate-stack.cpp:89:3: error: must use 'struct' tag to refer to type 'sigaction' in this scope
  sigaction Action = {};
  ^
  struct

Will go for

struct sigaction Action = {};

instead.

This revision was automatically updated to reflect the committed changes.
ro marked an inline comment as done.