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@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@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits