<div dir="ltr">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?<div><br></div><div>Thanks!</div><div><br></div><div>-eric<br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jan 30, 2019 at 4:41 PM Julian Lettner via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: yln<br>
Date: Wed Jan 30 15:42:13 2019<br>
New Revision: 352690<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=352690&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=352690&view=rev</a><br>
Log:<br>
[Sanitizers] UBSan unreachable incompatible with ASan in the presence of `noreturn` calls<br>
<br>
Summary:<br>
UBSan wants to detect when unreachable code is actually reached, so it<br>
adds instrumentation before every unreachable instruction. However, the<br>
optimizer will remove code after calls to functions marked with<br>
noreturn. To avoid this UBSan removes noreturn from both the call<br>
instruction as well as from the function itself. Unfortunately, ASan<br>
relies on this annotation to unpoison the stack by inserting calls to<br>
_asan_handle_no_return before noreturn functions. This is important for<br>
functions that do not return but access the the stack memory, e.g.,<br>
unwinder functions *like* longjmp (longjmp itself is actually<br>
"double-proofed" via its interceptor). The result is that when ASan and<br>
UBSan are combined, the noreturn attributes are missing and ASan cannot<br>
unpoison the stack, so it has false positives when stack unwinding is<br>
used.<br>
<br>
Changes:<br>
Clang-CodeGen now directly insert calls to `__asan_handle_no_return`<br>
when a call to a noreturn function is encountered and both<br>
UBsan-unreachable and ASan are enabled. This allows UBSan to continue<br>
removing the noreturn attribute from functions without any changes to<br>
the ASan pass.<br>
<br>
Previously generated code:<br>
```<br>
call void @longjmp<br>
call void @__asan_handle_no_return<br>
call void @__ubsan_handle_builtin_unreachable<br>
```<br>
<br>
Generated code (for now):<br>
```<br>
call void @__asan_handle_no_return<br>
call void @longjmp<br>
call void @__asan_handle_no_return<br>
call void @__ubsan_handle_builtin_unreachable<br>
```<br>
<br>
rdar://problem/40723397<br>
<br>
Reviewers: delcypher, eugenis, vsk<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D57278" rel="noreferrer" target="_blank">https://reviews.llvm.org/D57278</a><br>
<br>
Added:<br>
cfe/trunk/test/CodeGen/ubsan-asan-noreturn.c<br>
Modified:<br>
cfe/trunk/lib/CodeGen/CGCall.cpp<br>
cfe/trunk/lib/CodeGen/CodeGenFunction.h<br>
cfe/trunk/test/CodeGenCXX/ubsan-unreachable.cpp<br>
<br>
Modified: cfe/trunk/lib/CodeGen/CGCall.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=352690&r1=352689&r2=352690&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=352690&r1=352689&r2=352690&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)<br>
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Wed Jan 30 15:42:13 2019<br>
@@ -4398,10 +4398,23 @@ RValue CodeGenFunction::EmitCall(const C<br>
<br>
// Strip away the noreturn attribute to better diagnose unreachable UB.<br>
if (SanOpts.has(SanitizerKind::Unreachable)) {<br>
+ // Also remove from function since CI->hasFnAttr(..) also checks attributes<br>
+ // of the called function.<br>
if (auto *F = CI->getCalledFunction())<br>
F->removeFnAttr(llvm::Attribute::NoReturn);<br>
CI->removeAttribute(llvm::AttributeList::FunctionIndex,<br>
llvm::Attribute::NoReturn);<br>
+<br>
+ // Avoid incompatibility with ASan which relies on the `noreturn`<br>
+ // attribute to insert handler calls.<br>
+ if (SanOpts.has(SanitizerKind::Address)) {<br>
+ SanitizerScope SanScope(this);<br>
+ Builder.SetInsertPoint(CI);<br>
+ auto *FnType = llvm::FunctionType::get(CGM.VoidTy, /*isVarArg=*/false);<br>
+ auto *Fn = CGM.CreateRuntimeFunction(FnType, "__asan_handle_no_return");<br>
+ EmitNounwindRuntimeCall(Fn);<br>
+ Builder.SetInsertPoint(CI->getParent());<br>
+ }<br>
}<br>
<br>
EmitUnreachable(Loc);<br>
<br>
Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=352690&r1=352689&r2=352690&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=352690&r1=352689&r2=352690&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)<br>
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Wed Jan 30 15:42:13 2019<br>
@@ -4084,8 +4084,8 @@ public:<br>
/// passing to a runtime sanitizer handler.<br>
llvm::Constant *EmitCheckSourceLocation(SourceLocation Loc);<br>
<br>
- /// Create a basic block that will call a handler function in a<br>
- /// sanitizer runtime with the provided arguments, and create a conditional<br>
+ /// Create a basic block that will either trap or call a handler function in<br>
+ /// the UBSan runtime with the provided arguments, and create a conditional<br>
/// branch to it.<br>
void EmitCheck(ArrayRef<std::pair<llvm::Value *, SanitizerMask>> Checked,<br>
SanitizerHandler Check, ArrayRef<llvm::Constant *> StaticArgs,<br>
<br>
Added: cfe/trunk/test/CodeGen/ubsan-asan-noreturn.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ubsan-asan-noreturn.c?rev=352690&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ubsan-asan-noreturn.c?rev=352690&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/CodeGen/ubsan-asan-noreturn.c (added)<br>
+++ cfe/trunk/test/CodeGen/ubsan-asan-noreturn.c Wed Jan 30 15:42:13 2019<br>
@@ -0,0 +1,21 @@<br>
+// Ensure compatiblity of UBSan unreachable with ASan in the presence of<br>
+// noreturn functions.<br>
+// RUN: %clang_cc1 -fsanitize=unreachable,address -triple x86_64-linux -emit-llvm -o - %s | FileCheck %s<br>
+<br>
+void my_longjmp(void) __attribute__((noreturn));<br>
+<br>
+// CHECK-LABEL: define void @calls_noreturn()<br>
+void calls_noreturn() {<br>
+ my_longjmp();<br>
+ // CHECK: @__asan_handle_no_return{{.*}} !nosanitize<br>
+ // CHECK-NEXT: @my_longjmp(){{[^#]*}}<br>
+ // CHECK: @__asan_handle_no_return()<br>
+ // CHECK-NEXT: @__ubsan_handle_builtin_unreachable{{.*}} !nosanitize<br>
+ // CHECK-NEXT: unreachable<br>
+}<br>
+<br>
+// CHECK: declare void @my_longjmp() [[FN_ATTR:#[0-9]+]]<br>
+// CHECK: declare void @__asan_handle_no_return()<br>
+<br>
+// CHECK-LABEL: attributes<br>
+// CHECK-NOT: [[FN_ATTR]] = { {{.*noreturn.*}} }<br>
<br>
Modified: cfe/trunk/test/CodeGenCXX/ubsan-unreachable.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/ubsan-unreachable.cpp?rev=352690&r1=352689&r2=352690&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/ubsan-unreachable.cpp?rev=352690&r1=352689&r2=352690&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/CodeGenCXX/ubsan-unreachable.cpp (original)<br>
+++ cfe/trunk/test/CodeGenCXX/ubsan-unreachable.cpp Wed Jan 30 15:42:13 2019<br>
@@ -1,39 +1,37 @@<br>
// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -o - %s -fsanitize=unreachable | FileCheck %s<br>
<br>
-extern void __attribute__((noreturn)) abort();<br>
+void abort() __attribute__((noreturn));<br>
<br>
-// CHECK-LABEL: define void @_Z14calls_noreturnv<br>
+// CHECK-LABEL: define void @_Z14calls_noreturnv()<br>
void calls_noreturn() {<br>
+ // Check absence ([^#]*) of call site attributes (including noreturn)<br>
+ // CHECK: call void @_Z5abortv(){{[^#]*}}<br>
abort();<br>
<br>
- // Check that there are no attributes on the call site.<br>
- // CHECK-NOT: call void @_Z5abortv{{.*}}#<br>
-<br>
// CHECK: __ubsan_handle_builtin_unreachable<br>
// CHECK: unreachable<br>
}<br>
<br>
struct A {<br>
- // CHECK: declare void @_Z5abortv{{.*}} [[ABORT_ATTR:#[0-9]+]]<br>
+ // CHECK: declare void @_Z5abortv() [[EXTERN_FN_ATTR:#[0-9]+]]<br>
<br>
// CHECK-LABEL: define linkonce_odr void @_ZN1A5call1Ev<br>
void call1() {<br>
- // CHECK-NOT: call void @_ZN1A16does_not_return2Ev{{.*}}#<br>
+ // CHECK: call void @_ZN1A16does_not_return2Ev({{.*}}){{[^#]*}}<br>
does_not_return2();<br>
<br>
// CHECK: __ubsan_handle_builtin_unreachable<br>
// CHECK: unreachable<br>
}<br>
<br>
- // Test static members.<br>
- static void __attribute__((noreturn)) does_not_return1() {<br>
- // CHECK-NOT: call void @_Z5abortv{{.*}}#<br>
+ // Test static members. Checks are below after `struct A` scope ends.<br>
+ static void does_not_return1() __attribute__((noreturn)) {<br>
abort();<br>
}<br>
<br>
// CHECK-LABEL: define linkonce_odr void @_ZN1A5call2Ev<br>
void call2() {<br>
- // CHECK-NOT: call void @_ZN1A16does_not_return1Ev{{.*}}#<br>
+ // CHECK: call void @_ZN1A16does_not_return1Ev(){{[^#]*}}<br>
does_not_return1();<br>
<br>
// CHECK: __ubsan_handle_builtin_unreachable<br>
@@ -41,23 +39,23 @@ struct A {<br>
}<br>
<br>
// Test calls through pointers to non-static member functions.<br>
- typedef void __attribute__((noreturn)) (A::*MemFn)();<br>
+ typedef void (A::*MemFn)() __attribute__((noreturn));<br>
<br>
// CHECK-LABEL: define linkonce_odr void @_ZN1A5call3Ev<br>
void call3() {<br>
MemFn MF = &A::does_not_return2;<br>
+ // CHECK: call void %{{[0-9]+\(.*}}){{[^#]*}}<br>
(this->*MF)();<br>
<br>
- // CHECK-NOT: call void %{{.*}}#<br>
// CHECK: __ubsan_handle_builtin_unreachable<br>
// CHECK: unreachable<br>
}<br>
<br>
// Test regular members.<br>
// CHECK-LABEL: define linkonce_odr void @_ZN1A16does_not_return2Ev({{.*}})<br>
- // CHECK-SAME: [[DOES_NOT_RETURN_ATTR:#[0-9]+]]<br>
- void __attribute__((noreturn)) does_not_return2() {<br>
- // CHECK-NOT: call void @_Z5abortv(){{.*}}#<br>
+ // CHECK-SAME: [[USER_FN_ATTR:#[0-9]+]]<br>
+ void does_not_return2() __attribute__((noreturn)) {<br>
+ // CHECK: call void @_Z5abortv(){{[^#]*}}<br>
abort();<br>
<br>
// CHECK: call void @__ubsan_handle_builtin_unreachable<br>
@@ -68,7 +66,9 @@ struct A {<br>
}<br>
};<br>
<br>
-// CHECK: define linkonce_odr void @_ZN1A16does_not_return1Ev() [[DOES_NOT_RETURN_ATTR]]<br>
+// CHECK-LABEL: define linkonce_odr void @_ZN1A16does_not_return1Ev()<br>
+// CHECK-SAME: [[USER_FN_ATTR]]<br>
+// CHECK: call void @_Z5abortv(){{[^#]*}}<br>
<br>
void force_irgen() {<br>
A a;<br>
@@ -77,5 +77,7 @@ void force_irgen() {<br>
a.call3();<br>
}<br>
<br>
-// CHECK-NOT: [[ABORT_ATTR]] = {{[^}]+}}noreturn<br>
-// CHECK-NOT: [[DOES_NOT_RETURN_ATTR]] = {{[^}]+}}noreturn<br>
+// `noreturn` should be removed from functions and call sites<br>
+// CHECK-LABEL: attributes<br>
+// CHECK-NOT: [[USER_FN_ATTR]] = { {{.*noreturn.*}} }<br>
+// CHECK-NOT: [[EXTERN_FN_ATTR]] = { {{.*noreturn.*}} }<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div></div>