This is an archive of the discontinued LLVM Phabricator instance.

[compiler-rt] implement sigaltstack interception
ClosedPublic

Authored by sugak on Jan 31 2020, 3:48 PM.

Details

Summary

An implementation for sigaltstack to make its side effect be visible to MSAN.

ninja check-msan

Diff Detail

Event Timeline

sugak created this revision.Jan 31 2020, 3:48 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJan 31 2020, 3:48 PM
Herald added subscribers: llvm-commits, Restricted Project, dberris. · View Herald Transcript
sugak added a comment.Jan 31 2020, 3:52 PM

I manually tested this on CentOS and would need guidance about the use of SI_POSIX, as well as how to make a unittest for this (if needed).

I manually tested this on CentOS and would need guidance about the use of SI_POSIX, as well as how to make a unittest for this (if needed).

SI_POSIX seems alright. Please add a simple test under compiler-rt/test/msan - just call the function and check that accessing memory does not trigger msan.

compiler-rt/lib/sanitizer_common/sanitizer_common_syscalls.inc
2901

POST_READ is unnecessary here - it is mainly for the case when the read range is not known until the syscall returns.

sugak updated this revision to Diff 242138.Feb 3 2020, 11:22 AM
sugak edited the summary of this revision. (Show Details)

Thank you @eugenis! I've updated the change with you feedback and added a test.

sugak marked an inline comment as done.Feb 3 2020, 11:28 AM
eugenis accepted this revision.Feb 3 2020, 3:07 PM

LGTM
Thank you!

This revision is now accepted and ready to land.Feb 3 2020, 3:07 PM
sugak added a comment.Feb 3 2020, 4:12 PM

@eugenis, thank you for the review! Would you please commit this for me?

This revision was automatically updated to reflect the committed changes.
mgorny added a subscriber: mgorny.Feb 4 2020, 5:51 AM

This seems to have broken NetBSD:

FAILED: projects/compiler-rt/lib/tsan/CMakeFiles/clang_rt.tsan-x86_64.dir/rtl/tsan_interceptors_posix.cpp.o 
/home/motus/netbsd8/netbsd8/wrappers/clang++  -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Iprojects/compiler-rt/lib/tsan -I/home/motus/netbsd8/netbsd8/llvm-project/llvm/projects/compiler-rt/lib/tsan -Iinclude -I/home/motus/netbsd8/netbsd8/llvm-project/llvm/include -I/home/motus/netbsd8/netbsd8/llvm-project/llvm/projects/compiler-rt/lib/tsan/.. -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections -Wall -std=c++14 -Wno-unused-parameter -O3 -DNDEBUG    -m64 -fno-lto -fPIC -fno-builtin -fno-exceptions -fomit-frame-pointer -funwind-tables -fno-stack-protector -fno-sanitize=safe-stack -fvisibility=hidden -fno-lto -O3 -gline-tables-only -Wno-gnu -Wno-variadic-macros -Wno-c99-extensions -Wno-non-virtual-dtor -fPIE -fno-rtti -msse3 -Wframe-larger-than=530 -Wglobal-constructors -std=c++14 -MD -MT projects/compiler-rt/lib/tsan/CMakeFiles/clang_rt.tsan-x86_64.dir/rtl/tsan_interceptors_posix.cpp.o -MF projects/compiler-rt/lib/tsan/CMakeFiles/clang_rt.tsan-x86_64.dir/rtl/tsan_interceptors_posix.cpp.o.d -o projects/compiler-rt/lib/tsan/CMakeFiles/clang_rt.tsan-x86_64.dir/rtl/tsan_interceptors_posix.cpp.o -c /home/motus/netbsd8/netbsd8/llvm-project/llvm/projects/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
In file included from /home/motus/netbsd8/netbsd8/llvm-project/llvm/projects/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:2337:
/home/motus/netbsd8/netbsd8/llvm-project/llvm/projects/compiler-rt/lib/tsan/../sanitizer_common/sanitizer_common_interceptors.inc:9739:44: error: use of undeclared identifier 'struct_stack_t_sz'; did you mean 'struct_stat_sz'?
    COMMON_INTERCEPTOR_READ_RANGE(ctx, ss, struct_stack_t_sz);
                                           ^~~~~~~~~~~~~~~~~
                                           struct_stat_sz
/home/motus/netbsd8/netbsd8/llvm-project/llvm/projects/compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:2223:71: note: expanded from macro 'COMMON_INTERCEPTOR_READ_RANGE'
                    ((TsanInterceptorContext *) ctx)->pc, (uptr) ptr, size, \
                                                                      ^
/home/motus/netbsd8/netbsd8/llvm-project/llvm/projects/compiler-rt/lib/tsan/../sanitizer_common/sanitizer_platform_limits_netbsd.h:35:17: note: 'struct_stat_sz' declared here
extern unsigned struct_stat_sz;
                ^

http://lab.llvm.org:8014/builders/netbsd-amd64/builds/968/steps/ninja%20build%20local/logs/stdio

For fixing NetBSD, remember to go for #define sigaltstack __sigaltstack14 as the symbol is renamed.

sugak added a comment.Feb 4 2020, 10:00 AM

Sorry for breaking NetBSD builds! An attempt to fix it in D73976.

Another problem with this interceptor - stack_t on linux has internal padding that is not required to be initialized.

See this bug: https://bugs.chromium.org/p/chromium/issues/detail?id=1050279

sugak added a comment.Feb 10 2020, 3:34 PM

@eugenis: would you want me to send a patch for it or are you looking at this?

I'm not looking into it. Could you?

sugak added a comment.Feb 10 2020, 4:33 PM

Yes, I'll fix this!