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