[ASan] Do not unpoison stack before !nosanitize calls

[yln]

Summary

ASan inserts calls to __asan_handle_noreturn in front of all calls to noreturn
functions. This is needed for functions that move SP without going through ASan
epilogue, in order to maintain the requirement that stack below SP has clean
shadow.
(Thanks to Evgenii for the explanation.)

However, UBSan does not violate the above contract. The additional
instrumentation added by ASan before __ubsan_handle_builtin_unreachable is
therefore unnecessary. This patch changes ASan to not instrument call sites
marked with !nosanitize.

rdar://problem/40723397

[yln]

I just noticed that the previous revision was marked with

This revision was not accepted when it landed; it landed in state Needs Review.

I apologize for that. It was not my intention to land the revision before a formal accept. (Somehow I was under the impression that it was "green" already.)
Please voice it here if there is any cleanup or additional testing you want me to do.

[yln]

Superseded by: https://reviews.llvm.org/D57278

compiler-rt/test/ubsan/TestCases/Misc/unreachable_asan-compatibility.c

 // Ensure compatiblity of UBSan unreachable with ASan in the presence of
 // noreturn functions
 // RUN: %clang -O2 -fsanitize=address,unreachable %s -emit-llvm -S -o - | FileCheck %s
 // REQUIRES: ubsan-asan
 
 void bar(void) __attribute__((noreturn));
 
 void foo() {
   bar();
 }
 // CHECK-LABEL: define void @foo()
 // CHECK:       call void @__asan_handle_no_return
 // CHECK-NEXT:  call void @bar
-// CHECK-NEXT:  call void @__asan_handle_no_return
 // CHECK-NEXT:  call void @__ubsan_handle_builtin_unreachable
 // CHECK-NEXT:  unreachable

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

for (auto &Inst : BB) {
       } else if (isa<MemIntrinsic>(Inst)) {
         // ok, take it.
       } else {
         if (isa<AllocaInst>(Inst)) NumAllocas++;
         CallSite CS(&Inst);
         if (CS) {
           // A call inside BB.
           TempsToInstrument.clear();
-          if (CS.doesNotReturn() || CS.hasFnAttr(Attribute::ExpectNoReturn))
+          bool NoReturn =
+              CS.doesNotReturn() || CS.hasFnAttr(Attribute::ExpectNoReturn);
+          bool Instrument = !CS->getMetadata("nosanitize");
+          if (NoReturn && Instrument)
             NoReturnCalls.push_back(CS.getInstruction());
         }
         if (CallInst *CI = dyn_cast<CallInst>(&Inst))
           maybeMarkSanitizerLibraryCallNoBuiltin(CI, TLI);
         continue;
       }
       ToInstrument.push_back(&Inst);
       NumInsnsPerBB++;
if (ClDebugMin < 0 || ClDebugMax < 0 ||
         instrumentMemIntrinsic(cast<MemIntrinsic>(Inst));
     }
     NumInstrumented++;
   }
 
   FunctionStackPoisoner FSP(F, *this);
   bool ChangedStack = FSP.runOnFunction();
 
-  // We must unpoison the stack before every NoReturn call (throw, _exit, etc).
+  // We must unpoison the stack before most NoReturn call (throw, _exit, etc).
   // See e.g. https://github.com/google/sanitizers/issues/37
   for (auto CI : NoReturnCalls) {
     IRBuilder<> IRB(CI);
     IRB.CreateCall(AsanHandleNoReturnFunc, {});
   }
 
   for (auto Inst : PointerComparisonsOrSubtracts) {
     instrumentPointerComparisonOrSubtraction(Inst);

llvm/test/Instrumentation/AddressSanitizer/instrument-no-return.ll

 ; RUN: opt < %s -asan -S | FileCheck %s
 ; AddressSanitizer must insert __asan_handle_no_return
 ; before every noreturn call or invoke.
 
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
 target triple = "x86_64-unknown-linux-gnu"
 
 declare void @NormalFunc()
 declare void @NoReturnFunc() noreturn
 
-; Instrument calls to noreturn functions (regardless of callsite)
+; Instrument calls to noreturn functions (regardless of call site)
 define i32 @Call1() sanitize_address {
   call void @NoReturnFunc()
   unreachable
 }
 ; CHECK-LABEL:  @Call1
 ; CHECK:        call void @__asan_handle_no_return
 ; CHECK-NEXT:   call void @NoReturnFunc
 

 define i32 @Call3() sanitize_address {
   call void @NormalFunc() expect_noreturn
   ret i32 0
 }
 ; CHECK-LABEL:  @Call3
 ; CHECK:        call void @__asan_handle_no_return
 ; CHECK-NEXT:   call void @NormalFunc
 
+; Never instrument !nosanitize call sites
+define i32 @Call4() sanitize_address {
+  call void @NoReturnFunc() noreturn, !nosanitize !{}
+  ret i32 0
+}
+; CHECK-LABEL:  @Call4
+; CHECK-NOT:    call void @__asan_handle_no_return
+; CHECK:        call void @NoReturnFunc
+
 declare i32 @__gxx_personality_v0(...)
 
 define i64 @Invoke1(i8** %esc) sanitize_address personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
 entry:
   invoke void @NoReturnFunc()
           to label %invoke.cont unwind label %lpad
 
 invoke.cont: