r352690 - [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls
Eric Liu via cfe-commits
cfe-commits at lists.llvm.org
Thu Jan 31 06:11:05 PST 2019
I managed to get a reproducer (attached) from absl:
```
clang++ -std=c++17 -fsanitize=address,unreachable throw_delegate.pic.ii
```
You could regenerate the preprocessed code:
```
git clone https://github.com/abseil/abseil-cpp.git
cd abseil-cpp/absl
bazel build --compilation_mode=fastbuild --save_temps
--compile_one_dependency base/internal/throw_delegate.cc
```
I'll revert the commit to unblock our integration process. Let us know if
you need more information.
- Eric
On Thu, Jan 31, 2019 at 9:01 AM Eric Christopher <echristo at gmail.com> wrote:
> Looks like this broke optimized asan builds via an assert in SCCP. I'll
> see what I can do about a testcase (or Eric will), however, would you mind
> reverting in the meantime?
>
> Thanks!
>
> -eric
>
> On Wed, Jan 30, 2019 at 4:41 PM Julian Lettner via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> Author: yln
>> Date: Wed Jan 30 15:42:13 2019
>> New Revision: 352690
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=352690&view=rev
>> Log:
>> [Sanitizers] UBSan unreachable incompatible with ASan in the presence of
>> `noreturn` calls
>>
>> Summary:
>> UBSan wants to detect when unreachable code is actually reached, so it
>> adds instrumentation before every unreachable instruction. However, the
>> optimizer will remove code after calls to functions marked with
>> noreturn. To avoid this UBSan removes noreturn from both the call
>> instruction as well as from the function itself. Unfortunately, ASan
>> relies on this annotation to unpoison the stack by inserting calls to
>> _asan_handle_no_return before noreturn functions. This is important for
>> functions that do not return but access the the stack memory, e.g.,
>> unwinder functions *like* longjmp (longjmp itself is actually
>> "double-proofed" via its interceptor). The result is that when ASan and
>> UBSan are combined, the noreturn attributes are missing and ASan cannot
>> unpoison the stack, so it has false positives when stack unwinding is
>> used.
>>
>> Changes:
>> Clang-CodeGen now directly insert calls to `__asan_handle_no_return`
>> when a call to a noreturn function is encountered and both
>> UBsan-unreachable and ASan are enabled. This allows UBSan to continue
>> removing the noreturn attribute from functions without any changes to
>> the ASan pass.
>>
>> Previously generated code:
>> ```
>> call void @longjmp
>> call void @__asan_handle_no_return
>> call void @__ubsan_handle_builtin_unreachable
>> ```
>>
>> Generated code (for now):
>> ```
>> call void @__asan_handle_no_return
>> call void @longjmp
>> call void @__asan_handle_no_return
>> call void @__ubsan_handle_builtin_unreachable
>> ```
>>
>> rdar://problem/40723397
>>
>> Reviewers: delcypher, eugenis, vsk
>>
>> Differential Revision: https://reviews.llvm.org/D57278
>>
>> Added:
>> cfe/trunk/test/CodeGen/ubsan-asan-noreturn.c
>> Modified:
>> cfe/trunk/lib/CodeGen/CGCall.cpp
>> cfe/trunk/lib/CodeGen/CodeGenFunction.h
>> cfe/trunk/test/CodeGenCXX/ubsan-unreachable.cpp
>>
>> Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=352690&r1=352689&r2=352690&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CGCall.cpp Wed Jan 30 15:42:13 2019
>> @@ -4398,10 +4398,23 @@ RValue CodeGenFunction::EmitCall(const C
>>
>> // Strip away the noreturn attribute to better diagnose unreachable
>> UB.
>> if (SanOpts.has(SanitizerKind::Unreachable)) {
>> + // Also remove from function since CI->hasFnAttr(..) also checks
>> attributes
>> + // of the called function.
>> if (auto *F = CI->getCalledFunction())
>> F->removeFnAttr(llvm::Attribute::NoReturn);
>> CI->removeAttribute(llvm::AttributeList::FunctionIndex,
>> llvm::Attribute::NoReturn);
>> +
>> + // Avoid incompatibility with ASan which relies on the `noreturn`
>> + // attribute to insert handler calls.
>> + if (SanOpts.has(SanitizerKind::Address)) {
>> + SanitizerScope SanScope(this);
>> + Builder.SetInsertPoint(CI);
>> + auto *FnType = llvm::FunctionType::get(CGM.VoidTy,
>> /*isVarArg=*/false);
>> + auto *Fn = CGM.CreateRuntimeFunction(FnType,
>> "__asan_handle_no_return");
>> + EmitNounwindRuntimeCall(Fn);
>> + Builder.SetInsertPoint(CI->getParent());
>> + }
>> }
>>
>> EmitUnreachable(Loc);
>>
>> Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=352690&r1=352689&r2=352690&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
>> +++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Wed Jan 30 15:42:13 2019
>> @@ -4084,8 +4084,8 @@ public:
>> /// passing to a runtime sanitizer handler.
>> llvm::Constant *EmitCheckSourceLocation(SourceLocation Loc);
>>
>> - /// Create a basic block that will call a handler function in a
>> - /// sanitizer runtime with the provided arguments, and create a
>> conditional
>> + /// Create a basic block that will either trap or call a handler
>> function in
>> + /// the UBSan runtime with the provided arguments, and create a
>> conditional
>> /// branch to it.
>> void EmitCheck(ArrayRef<std::pair<llvm::Value *, SanitizerMask>>
>> Checked,
>> SanitizerHandler Check, ArrayRef<llvm::Constant *>
>> StaticArgs,
>>
>> Added: cfe/trunk/test/CodeGen/ubsan-asan-noreturn.c
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ubsan-asan-noreturn.c?rev=352690&view=auto
>>
>> ==============================================================================
>> --- cfe/trunk/test/CodeGen/ubsan-asan-noreturn.c (added)
>> +++ cfe/trunk/test/CodeGen/ubsan-asan-noreturn.c Wed Jan 30 15:42:13 2019
>> @@ -0,0 +1,21 @@
>> +// Ensure compatiblity of UBSan unreachable with ASan in the presence of
>> +// noreturn functions.
>> +// RUN: %clang_cc1 -fsanitize=unreachable,address -triple x86_64-linux
>> -emit-llvm -o - %s | FileCheck %s
>> +
>> +void my_longjmp(void) __attribute__((noreturn));
>> +
>> +// CHECK-LABEL: define void @calls_noreturn()
>> +void calls_noreturn() {
>> + my_longjmp();
>> + // CHECK: @__asan_handle_no_return{{.*}} !nosanitize
>> + // CHECK-NEXT: @my_longjmp(){{[^#]*}}
>> + // CHECK: @__asan_handle_no_return()
>> + // CHECK-NEXT: @__ubsan_handle_builtin_unreachable{{.*}} !nosanitize
>> + // CHECK-NEXT: unreachable
>> +}
>> +
>> +// CHECK: declare void @my_longjmp() [[FN_ATTR:#[0-9]+]]
>> +// CHECK: declare void @__asan_handle_no_return()
>> +
>> +// CHECK-LABEL: attributes
>> +// CHECK-NOT: [[FN_ATTR]] = { {{.*noreturn.*}} }
>>
>> Modified: cfe/trunk/test/CodeGenCXX/ubsan-unreachable.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/ubsan-unreachable.cpp?rev=352690&r1=352689&r2=352690&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/CodeGenCXX/ubsan-unreachable.cpp (original)
>> +++ cfe/trunk/test/CodeGenCXX/ubsan-unreachable.cpp Wed Jan 30 15:42:13
>> 2019
>> @@ -1,39 +1,37 @@
>> // RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -o - %s
>> -fsanitize=unreachable | FileCheck %s
>>
>> -extern void __attribute__((noreturn)) abort();
>> +void abort() __attribute__((noreturn));
>>
>> -// CHECK-LABEL: define void @_Z14calls_noreturnv
>> +// CHECK-LABEL: define void @_Z14calls_noreturnv()
>> void calls_noreturn() {
>> + // Check absence ([^#]*) of call site attributes (including noreturn)
>> + // CHECK: call void @_Z5abortv(){{[^#]*}}
>> abort();
>>
>> - // Check that there are no attributes on the call site.
>> - // CHECK-NOT: call void @_Z5abortv{{.*}}#
>> -
>> // CHECK: __ubsan_handle_builtin_unreachable
>> // CHECK: unreachable
>> }
>>
>> struct A {
>> - // CHECK: declare void @_Z5abortv{{.*}} [[ABORT_ATTR:#[0-9]+]]
>> + // CHECK: declare void @_Z5abortv() [[EXTERN_FN_ATTR:#[0-9]+]]
>>
>> // CHECK-LABEL: define linkonce_odr void @_ZN1A5call1Ev
>> void call1() {
>> - // CHECK-NOT: call void @_ZN1A16does_not_return2Ev{{.*}}#
>> + // CHECK: call void @_ZN1A16does_not_return2Ev({{.*}}){{[^#]*}}
>> does_not_return2();
>>
>> // CHECK: __ubsan_handle_builtin_unreachable
>> // CHECK: unreachable
>> }
>>
>> - // Test static members.
>> - static void __attribute__((noreturn)) does_not_return1() {
>> - // CHECK-NOT: call void @_Z5abortv{{.*}}#
>> + // Test static members. Checks are below after `struct A` scope ends.
>> + static void does_not_return1() __attribute__((noreturn)) {
>> abort();
>> }
>>
>> // CHECK-LABEL: define linkonce_odr void @_ZN1A5call2Ev
>> void call2() {
>> - // CHECK-NOT: call void @_ZN1A16does_not_return1Ev{{.*}}#
>> + // CHECK: call void @_ZN1A16does_not_return1Ev(){{[^#]*}}
>> does_not_return1();
>>
>> // CHECK: __ubsan_handle_builtin_unreachable
>> @@ -41,23 +39,23 @@ struct A {
>> }
>>
>> // Test calls through pointers to non-static member functions.
>> - typedef void __attribute__((noreturn)) (A::*MemFn)();
>> + typedef void (A::*MemFn)() __attribute__((noreturn));
>>
>> // CHECK-LABEL: define linkonce_odr void @_ZN1A5call3Ev
>> void call3() {
>> MemFn MF = &A::does_not_return2;
>> + // CHECK: call void %{{[0-9]+\(.*}}){{[^#]*}}
>> (this->*MF)();
>>
>> - // CHECK-NOT: call void %{{.*}}#
>> // CHECK: __ubsan_handle_builtin_unreachable
>> // CHECK: unreachable
>> }
>>
>> // Test regular members.
>> // CHECK-LABEL: define linkonce_odr void
>> @_ZN1A16does_not_return2Ev({{.*}})
>> - // CHECK-SAME: [[DOES_NOT_RETURN_ATTR:#[0-9]+]]
>> - void __attribute__((noreturn)) does_not_return2() {
>> - // CHECK-NOT: call void @_Z5abortv(){{.*}}#
>> + // CHECK-SAME: [[USER_FN_ATTR:#[0-9]+]]
>> + void does_not_return2() __attribute__((noreturn)) {
>> + // CHECK: call void @_Z5abortv(){{[^#]*}}
>> abort();
>>
>> // CHECK: call void @__ubsan_handle_builtin_unreachable
>> @@ -68,7 +66,9 @@ struct A {
>> }
>> };
>>
>> -// CHECK: define linkonce_odr void @_ZN1A16does_not_return1Ev()
>> [[DOES_NOT_RETURN_ATTR]]
>> +// CHECK-LABEL: define linkonce_odr void @_ZN1A16does_not_return1Ev()
>> +// CHECK-SAME: [[USER_FN_ATTR]]
>> +// CHECK: call void @_Z5abortv(){{[^#]*}}
>>
>> void force_irgen() {
>> A a;
>> @@ -77,5 +77,7 @@ void force_irgen() {
>> a.call3();
>> }
>>
>> -// CHECK-NOT: [[ABORT_ATTR]] = {{[^}]+}}noreturn
>> -// CHECK-NOT: [[DOES_NOT_RETURN_ATTR]] = {{[^}]+}}noreturn
>> +// `noreturn` should be removed from functions and call sites
>> +// CHECK-LABEL: attributes
>> +// CHECK-NOT: [[USER_FN_ATTR]] = { {{.*noreturn.*}} }
>> +// CHECK-NOT: [[EXTERN_FN_ATTR]] = { {{.*noreturn.*}} }
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190131/79392472/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: throw_delegate.pic.ii
Type: application/octet-stream
Size: 617447 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190131/79392472/attachment-0001.obj>
More information about the cfe-commits
mailing list