r352690 - [Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls

Eric Christopher via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 31 00:01:01 PST 2019


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/bc4f2b5a/attachment-0001.html>


More information about the cfe-commits mailing list