From a22c72ca8ff55a70e38ea0d40929a48324e5b8a4 Mon Sep 17 00:00:00 2001 From: Eric Liu Date: Thu, 31 Jan 2019 14:20:02 +0000 Subject: [PATCH] Revert "[Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls" This reverts commit r352690. This causes clang to crash. Sent reproducer to the author in the orginal commit. llvm-svn: 352755 --- clang/lib/CodeGen/CGCall.cpp | 13 ------- clang/lib/CodeGen/CodeGenFunction.h | 4 +-- clang/test/CodeGen/ubsan-asan-noreturn.c | 21 ----------- clang/test/CodeGenCXX/ubsan-unreachable.cpp | 40 ++++++++++----------- 4 files changed, 21 insertions(+), 57 deletions(-) delete mode 100644 clang/test/CodeGen/ubsan-asan-noreturn.c diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp index 655c75c01f6a..893a6f2f902d 100644 --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -4398,23 +4398,10 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo, // 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); diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 575a500c29d9..a989a0e46cad 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -4084,8 +4084,8 @@ public: /// passing to a runtime sanitizer handler. llvm::Constant *EmitCheckSourceLocation(SourceLocation Loc); - /// 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 + /// Create a basic block that will call a handler function in a + /// sanitizer runtime with the provided arguments, and create a conditional /// branch to it. void EmitCheck(ArrayRef> Checked, SanitizerHandler Check, ArrayRef StaticArgs, diff --git a/clang/test/CodeGen/ubsan-asan-noreturn.c b/clang/test/CodeGen/ubsan-asan-noreturn.c deleted file mode 100644 index cd7b8425796f..000000000000 --- a/clang/test/CodeGen/ubsan-asan-noreturn.c +++ /dev/null @@ -1,21 +0,0 @@ -// 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.*}} } diff --git a/clang/test/CodeGenCXX/ubsan-unreachable.cpp b/clang/test/CodeGenCXX/ubsan-unreachable.cpp index 70a487a17738..32a78048cfd6 100644 --- a/clang/test/CodeGenCXX/ubsan-unreachable.cpp +++ b/clang/test/CodeGenCXX/ubsan-unreachable.cpp @@ -1,37 +1,39 @@ // RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -o - %s -fsanitize=unreachable | FileCheck %s -void abort() __attribute__((noreturn)); +extern void __attribute__((noreturn)) abort(); -// 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() [[EXTERN_FN_ATTR:#[0-9]+]] + // CHECK: declare void @_Z5abortv{{.*}} [[ABORT_ATTR:#[0-9]+]] // CHECK-LABEL: define linkonce_odr void @_ZN1A5call1Ev void call1() { - // CHECK: call void @_ZN1A16does_not_return2Ev({{.*}}){{[^#]*}} + // CHECK-NOT: call void @_ZN1A16does_not_return2Ev{{.*}}# does_not_return2(); // CHECK: __ubsan_handle_builtin_unreachable // CHECK: unreachable } - // Test static members. Checks are below after `struct A` scope ends. - static void does_not_return1() __attribute__((noreturn)) { + // Test static members. + static void __attribute__((noreturn)) does_not_return1() { + // CHECK-NOT: call void @_Z5abortv{{.*}}# abort(); } // CHECK-LABEL: define linkonce_odr void @_ZN1A5call2Ev void call2() { - // CHECK: call void @_ZN1A16does_not_return1Ev(){{[^#]*}} + // CHECK-NOT: call void @_ZN1A16does_not_return1Ev{{.*}}# does_not_return1(); // CHECK: __ubsan_handle_builtin_unreachable @@ -39,23 +41,23 @@ struct A { } // Test calls through pointers to non-static member functions. - typedef void (A::*MemFn)() __attribute__((noreturn)); + typedef void __attribute__((noreturn)) (A::*MemFn)(); // 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: [[USER_FN_ATTR:#[0-9]+]] - void does_not_return2() __attribute__((noreturn)) { - // CHECK: call void @_Z5abortv(){{[^#]*}} + // CHECK-SAME: [[DOES_NOT_RETURN_ATTR:#[0-9]+]] + void __attribute__((noreturn)) does_not_return2() { + // CHECK-NOT: call void @_Z5abortv(){{.*}}# abort(); // CHECK: call void @__ubsan_handle_builtin_unreachable @@ -66,9 +68,7 @@ struct A { } }; -// CHECK-LABEL: define linkonce_odr void @_ZN1A16does_not_return1Ev() -// CHECK-SAME: [[USER_FN_ATTR]] -// CHECK: call void @_Z5abortv(){{[^#]*}} +// CHECK: define linkonce_odr void @_ZN1A16does_not_return1Ev() [[DOES_NOT_RETURN_ATTR]] void force_irgen() { A a; @@ -77,7 +77,5 @@ void force_irgen() { a.call3(); } -// `noreturn` should be removed from functions and call sites -// CHECK-LABEL: attributes -// CHECK-NOT: [[USER_FN_ATTR]] = { {{.*noreturn.*}} } -// CHECK-NOT: [[EXTERN_FN_ATTR]] = { {{.*noreturn.*}} } +// CHECK-NOT: [[ABORT_ATTR]] = {{[^}]+}}noreturn +// CHECK-NOT: [[DOES_NOT_RETURN_ATTR]] = {{[^}]+}}noreturn