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