[ubsan] Diagnose noreturn functions which return

Diagnose 'unreachable' UB when a noreturn function returns.

  1. Insert a check at the end of functions marked noreturn.

  2. A decl may be marked noreturn in the caller TU, but not marked in
     the TU where it's defined. To diagnose this scenario, strip away the
     noreturn attribute on the callee and insert check after calls to it.

Testing: check-clang, check-ubsan, check-ubsan-minimal, D40700

rdar://33660464

Differential Revision: https://reviews.llvm.org/D40698

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@321231 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/docs/UndefinedBehaviorSanitizer.rst b/docs/UndefinedBehaviorSanitizer.rst
index 0a08a41e2d9b2e7badd5a9619d103fcd5e2e62f7..e9f85c24dde0ec56b4630d6a0e8df1d3495390bf 100644 (file)
--- a/docs/UndefinedBehaviorSanitizer.rst
+++ b/docs/UndefinedBehaviorSanitizer.rst
@@ -124,8 +124,8 @@ Available checks are:
   -  ``-fsanitize=signed-integer-overflow``: Signed integer overflow,
      including all the checks added by ``-ftrapv``, and checking for
      overflow in signed division (``INT_MIN / -1``).
-  -  ``-fsanitize=unreachable``: If control flow reaches
-     ``__builtin_unreachable``.
+  -  ``-fsanitize=unreachable``: If control flow reaches an unreachable
+     program point.
   -  ``-fsanitize=unsigned-integer-overflow``: Unsigned integer
      overflows. Note that unlike signed integer overflow, unsigned integer
      is not undefined behavior. However, while it has well-defined semantics,

diff --git a/lib/CodeGen/CGBuiltin.cpp b/lib/CodeGen/CGBuiltin.cpp
index 01cc0187fb57edfe05d5ee64b0d7cc89ff7e5f96..d67f2fede389f4ec10e3c052cd1d0704c506b1d5 100644 (file)
--- a/lib/CodeGen/CGBuiltin.cpp
+++ b/lib/CodeGen/CGBuiltin.cpp
@@ -1432,14 +1432,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(const FunctionDecl *FD,
   case Builtin::BI__debugbreak:
     return RValue::get(EmitTrapCall(Intrinsic::debugtrap));
   case Builtin::BI__builtin_unreachable: {
-    if (SanOpts.has(SanitizerKind::Unreachable)) {
-      SanitizerScope SanScope(this);
-      EmitCheck(std::make_pair(static_cast<llvm::Value *>(Builder.getFalse()),
-                               SanitizerKind::Unreachable),
-                SanitizerHandler::BuiltinUnreachable,
-                EmitCheckSourceLocation(E->getExprLoc()), None);
-    } else
-      Builder.CreateUnreachable();
+    EmitUnreachable(E->getExprLoc());
 
     // We do need to preserve an insertion point.
     EmitBlock(createBasicBlock("unreachable.cont"));

diff --git a/lib/CodeGen/CGCall.cpp b/lib/CodeGen/CGCall.cpp
index eea074e0307e4e1c92094fb7f6b56eea208261b0..38d7344572d3d32e1641f4dc42d8484baa2452c2 100644 (file)
--- a/lib/CodeGen/CGCall.cpp
+++ b/lib/CodeGen/CGCall.cpp
@@ -2758,6 +2758,12 @@ static llvm::StoreInst *findDominatingStoreToReturnValue(CodeGenFunction &CGF) {
 void CodeGenFunction::EmitFunctionEpilog(const CGFunctionInfo &FI,
                                          bool EmitRetDbgLoc,
                                          SourceLocation EndLoc) {
+  if (FI.isNoReturn()) {
+    // Noreturn functions don't return.
+    EmitUnreachable(EndLoc);
+    return;
+  }
+
   if (CurCodeDecl && CurCodeDecl->hasAttr<NakedAttr>()) {
     // Naked functions don't have epilogues.
     Builder.CreateUnreachable();
@@ -3718,7 +3724,8 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
                                  const CGCallee &Callee,
                                  ReturnValueSlot ReturnValue,
                                  const CallArgList &CallArgs,
-                                 llvm::Instruction **callOrInvoke) {
+                                 llvm::Instruction **callOrInvoke,
+                                 SourceLocation Loc) {
   // FIXME: We no longer need the types from CallArgs; lift up and simplify.
 
   assert(Callee.isOrdinary());
@@ -4241,7 +4248,15 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
       EmitLifetimeEnd(llvm::ConstantInt::get(Int64Ty, UnusedReturnSize),
                       SRetPtr.getPointer());
 
-    Builder.CreateUnreachable();
+    // Strip away the noreturn attribute to better diagnose unreachable UB.
+    if (SanOpts.has(SanitizerKind::Unreachable)) {
+      if (auto *F = CS.getCalledFunction())
+        F->removeFnAttr(llvm::Attribute::NoReturn);
+      CS.removeAttribute(llvm::AttributeList::FunctionIndex,
+                         llvm::Attribute::NoReturn);
+    }
+
+    EmitUnreachable(Loc);
     Builder.ClearInsertionPoint();
 
     // FIXME: For now, emit a dummy basic block because expr emitters in

diff --git a/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp
index 98740e8f9aabe5e9b0e2d1c52e82cf8161f84ac7..b61b273977dcd3e7240b0e0f5d15fe8684cbfd1d 100644 (file)
--- a/lib/CodeGen/CGExpr.cpp
+++ b/lib/CodeGen/CGExpr.cpp
@@ -3076,6 +3076,17 @@ void CodeGenFunction::EmitCfiCheckFail() {
   CGM.addUsedGlobal(F);
 }
 
+void CodeGenFunction::EmitUnreachable(SourceLocation Loc) {
+  if (SanOpts.has(SanitizerKind::Unreachable)) {
+    SanitizerScope SanScope(this);
+    EmitCheck(std::make_pair(static_cast<llvm::Value *>(Builder.getFalse()),
+                             SanitizerKind::Unreachable),
+              SanitizerHandler::BuiltinUnreachable,
+              EmitCheckSourceLocation(Loc), None);
+  }
+  Builder.CreateUnreachable();
+}
+
 void CodeGenFunction::EmitTrapCheck(llvm::Value *Checked) {
   llvm::BasicBlock *Cont = createBasicBlock("cont");
 
@@ -4616,7 +4627,7 @@ RValue CodeGenFunction::EmitCall(QualType CalleeType, const CGCallee &OrigCallee
     Callee.setFunctionPointer(CalleePtr);
   }
 
-  return EmitCall(FnInfo, Callee, ReturnValue, Args);
+  return EmitCall(FnInfo, Callee, ReturnValue, Args, nullptr, E->getExprLoc());
 }
 
 LValue CodeGenFunction::

diff --git a/lib/CodeGen/CGExprCXX.cpp b/lib/CodeGen/CGExprCXX.cpp
index 9e301bd0574ae41a7a5662f52cec0c8233d5ff73..0749b0ac46a786299474d6f357d7d0903b43f19c 100644 (file)
--- a/lib/CodeGen/CGExprCXX.cpp
+++ b/lib/CodeGen/CGExprCXX.cpp
@@ -89,7 +89,8 @@ RValue CodeGenFunction::EmitCXXMemberOrOperatorCall(
       *this, MD, This, ImplicitParam, ImplicitParamTy, CE, Args, RtlArgs);
   auto &FnInfo = CGM.getTypes().arrangeCXXMethodCall(
       Args, FPT, CallInfo.ReqArgs, CallInfo.PrefixSize);
-  return EmitCall(FnInfo, Callee, ReturnValue, Args);
+  return EmitCall(FnInfo, Callee, ReturnValue, Args, nullptr,
+                  CE ? CE->getExprLoc() : SourceLocation());
 }
 
 RValue CodeGenFunction::EmitCXXDestructorCall(
@@ -446,7 +447,7 @@ CodeGenFunction::EmitCXXMemberPointerCallExpr(const CXXMemberCallExpr *E,
   EmitCallArgs(Args, FPT, E->arguments());
   return EmitCall(CGM.getTypes().arrangeCXXMethodCall(Args, FPT, required,
                                                       /*PrefixSize=*/0),
-                  Callee, ReturnValue, Args);
+                  Callee, ReturnValue, Args, nullptr, E->getExprLoc());
 }
 
 RValue

diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h
index ab5bbc03db9552887c700517e06f7e1d85d593a3..c6ee97142aa03aa0f1c9b48be242fe69eec12c3e 100644 (file)
--- a/lib/CodeGen/CodeGenFunction.h
+++ b/lib/CodeGen/CodeGenFunction.h
@@ -3288,11 +3288,15 @@ public:
   /// LLVM arguments and the types they were derived from.
   RValue EmitCall(const CGFunctionInfo &CallInfo, const CGCallee &Callee,
                   ReturnValueSlot ReturnValue, const CallArgList &Args,
-                  llvm::Instruction **callOrInvoke = nullptr);
-
+                  llvm::Instruction **callOrInvoke, SourceLocation Loc);
+  RValue EmitCall(const CGFunctionInfo &CallInfo, const CGCallee &Callee,
+                  ReturnValueSlot ReturnValue, const CallArgList &Args,
+                  llvm::Instruction **callOrInvoke = nullptr) {
+    return EmitCall(CallInfo, Callee, ReturnValue, Args, callOrInvoke,
+                    SourceLocation());
+  }
   RValue EmitCall(QualType FnType, const CGCallee &Callee, const CallExpr *E,
-                  ReturnValueSlot ReturnValue,
-                  llvm::Value *Chain = nullptr);
+                  ReturnValueSlot ReturnValue, llvm::Value *Chain = nullptr);
   RValue EmitCallExpr(const CallExpr *E,
                       ReturnValueSlot ReturnValue = ReturnValueSlot());
   RValue EmitSimpleCallExpr(const CallExpr *E, ReturnValueSlot ReturnValue);
@@ -3747,6 +3751,10 @@ public:
                             llvm::ConstantInt *TypeId, llvm::Value *Ptr,
                             ArrayRef<llvm::Constant *> StaticArgs);
 
+  /// Emit a reached-unreachable diagnostic if \p Loc is valid and runtime
+  /// checking is enabled. Otherwise, just emit an unreachable instruction.
+  void EmitUnreachable(SourceLocation Loc);
+
   /// \brief Create a basic block that will call the trap intrinsic, and emit a
   /// conditional branch to it, for the -ftrapv checks.
   void EmitTrapCheck(llvm::Value *Checked);

diff --git a/test/CodeGen/ubsan-noreturn.c b/test/CodeGen/ubsan-noreturn.c
new file mode 100644 (file)
index 0000000..7de24e0
--- /dev/null
+++ b/test/CodeGen/ubsan-noreturn.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 %s -emit-llvm -fsanitize=unreachable -o - | FileCheck %s
+
+// CHECK-LABEL: @f(
+void __attribute__((noreturn)) f() {
+  // CHECK: __ubsan_handle_builtin_unreachable
+  // CHECK: unreachable
+}

diff --git a/test/CodeGenCXX/ubsan-unreachable.cpp b/test/CodeGenCXX/ubsan-unreachable.cpp
new file mode 100644 (file)
index 0000000..32a7804
--- /dev/null
+++ b/test/CodeGenCXX/ubsan-unreachable.cpp
@@ -0,0 +1,81 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -o - %s -fsanitize=unreachable | FileCheck %s
+
+extern void __attribute__((noreturn)) abort();
+
+// CHECK-LABEL: define void @_Z14calls_noreturnv
+void calls_noreturn() {
+  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-LABEL: define linkonce_odr void @_ZN1A5call1Ev
+  void call1() {
+    // CHECK-NOT: 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{{.*}}#
+    abort();
+  }
+
+  // CHECK-LABEL: define linkonce_odr void @_ZN1A5call2Ev
+  void call2() {
+    // CHECK-NOT: call void @_ZN1A16does_not_return1Ev{{.*}}#
+    does_not_return1();
+
+    // CHECK: __ubsan_handle_builtin_unreachable
+    // CHECK: unreachable
+  }
+
+  // Test calls through pointers to non-static member functions.
+  typedef void __attribute__((noreturn)) (A::*MemFn)();
+
+  // CHECK-LABEL: define linkonce_odr void @_ZN1A5call3Ev
+  void call3() {
+    MemFn MF = &A::does_not_return2;
+    (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(){{.*}}#
+    abort();
+
+    // CHECK: call void @__ubsan_handle_builtin_unreachable
+    // CHECK: unreachable
+
+    // CHECK: call void @__ubsan_handle_builtin_unreachable
+    // CHECK: unreachable
+  }
+};
+
+// CHECK: define linkonce_odr void @_ZN1A16does_not_return1Ev() [[DOES_NOT_RETURN_ATTR]]
+
+void force_irgen() {
+  A a;
+  a.call1();
+  a.call2();
+  a.call3();
+}
+
+// CHECK-NOT: [[ABORT_ATTR]] = {{[^}]+}}noreturn
+// CHECK-NOT: [[DOES_NOT_RETURN_ATTR]] = {{[^}]+}}noreturn