This revision was automatically updated to reflect the committed changes.
Closed by commit rC321231: [ubsan] Diagnose noreturn functions which return 
(authored by vedantk, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D40698

Files:
  docs/UndefinedBehaviorSanitizer.rst
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprCXX.cpp
  lib/CodeGen/CodeGenFunction.h
  test/CodeGen/ubsan-noreturn.c
  test/CodeGenCXX/ubsan-unreachable.cpp

Index: lib/CodeGen/CGBuiltin.cpp
===================================================================
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -1432,14 +1432,7 @@
   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"));
Index: lib/CodeGen/CodeGenFunction.h
===================================================================
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -3288,11 +3288,15 @@
   /// 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 @@
                             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);
Index: lib/CodeGen/CGExpr.cpp
===================================================================
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -3076,6 +3076,17 @@
   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 @@
     Callee.setFunctionPointer(CalleePtr);
   }
 
-  return EmitCall(FnInfo, Callee, ReturnValue, Args);
+  return EmitCall(FnInfo, Callee, ReturnValue, Args, nullptr, E->getExprLoc());
 }
 
 LValue CodeGenFunction::
Index: lib/CodeGen/CGCall.cpp
===================================================================
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -2758,6 +2758,12 @@
 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 @@
                                  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 @@
       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
Index: lib/CodeGen/CGExprCXX.cpp
===================================================================
--- lib/CodeGen/CGExprCXX.cpp
+++ lib/CodeGen/CGExprCXX.cpp
@@ -89,7 +89,8 @@
       *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 @@
   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
Index: docs/UndefinedBehaviorSanitizer.rst
===================================================================
--- docs/UndefinedBehaviorSanitizer.rst
+++ docs/UndefinedBehaviorSanitizer.rst
@@ -124,8 +124,8 @@
   -  ``-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,
Index: test/CodeGen/ubsan-noreturn.c
===================================================================
--- test/CodeGen/ubsan-noreturn.c
+++ 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
+}
Index: test/CodeGenCXX/ubsan-unreachable.cpp
===================================================================
--- test/CodeGenCXX/ubsan-unreachable.cpp
+++ 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to