Hi rsmith,

This change makes CodeGenFunction::EmitCheck() take several
conditions that needs to be checked (all of them need to be true),
together with sanitizer kinds these checks are for. This would allow
to split one call into UBSan runtime into several calls in case
different sanitizer kinds would have different recoverability
settings.

Tests should be fixed accordingly, I'm working on it.

http://reviews.llvm.org/D6219

Files:
  lib/CodeGen/CGBuiltin.cpp
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h
Index: lib/CodeGen/CGBuiltin.cpp
===================================================================
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -475,9 +475,10 @@
   case Builtin::BI__builtin_unreachable: {
     if (SanOpts.has(SanitizerKind::Unreachable)) {
       SanitizerScope SanScope(this);
-      EmitCheck(Builder.getFalse(), "builtin_unreachable",
-                EmitCheckSourceLocation(E->getExprLoc()), None,
-                SanitizerKind::Unreachable);
+      EmitCheck(std::make_pair(static_cast<llvm::Value *>(Builder.getFalse()),
+                               SanitizerKind::Unreachable),
+                "builtin_unreachable", EmitCheckSourceLocation(E->getExprLoc()),
+                None);
     } else
       Builder.CreateUnreachable();
 
Index: lib/CodeGen/CGCall.cpp
===================================================================
--- lib/CodeGen/CGCall.cpp
+++ lib/CodeGen/CGCall.cpp
@@ -2300,8 +2300,8 @@
             EmitCheckSourceLocation(EndLoc),
             EmitCheckSourceLocation(RetNNAttr->getLocation()),
         };
-        EmitCheck(Cond, "nonnull_return", StaticData, None,
-                  SanitizerKind::ReturnsNonnullAttribute);
+        EmitCheck(std::make_pair(Cond, SanitizerKind::ReturnsNonnullAttribute),
+                  "nonnull_return", StaticData, None);
       }
     }
     Ret = Builder.CreateRet(RV);
@@ -2636,8 +2636,8 @@
       CGF.EmitCheckSourceLocation(NNAttr->getLocation()),
       llvm::ConstantInt::get(CGF.Int32Ty, ArgNo + 1),
   };
-  CGF.EmitCheck(Cond, "nonnull_arg", StaticData, None,
-                SanitizerKind::NonnullAttribute);
+  CGF.EmitCheck(std::make_pair(Cond, SanitizerKind::NonnullAttribute),
+                "nonnull_arg", StaticData, None);
 }
 
 void CodeGenFunction::EmitCallArgs(CallArgList &Args,
Index: lib/CodeGen/CGExpr.cpp
===================================================================
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -476,30 +476,28 @@
   if (Address->getType()->getPointerAddressSpace())
     return;
 
-  SmallVector<SanitizerKind, 3> Kinds;
   SanitizerScope SanScope(this);
 
-  llvm::Value *Cond = nullptr;
+  SmallVector<std::pair<llvm::Value *, SanitizerKind>, 3> Checks;
   llvm::BasicBlock *Done = nullptr;
 
   bool AllowNullPointers = TCK == TCK_DowncastPointer || TCK == TCK_Upcast ||
                            TCK == TCK_UpcastToVirtualBase;
   if ((SanOpts.has(SanitizerKind::Null) || AllowNullPointers) &&
       !SkipNullCheck) {
     // The glvalue must not be an empty glvalue.
-    Cond = Builder.CreateICmpNE(
+    llvm::Value *IsNonNull = Builder.CreateICmpNE(
         Address, llvm::Constant::getNullValue(Address->getType()));
 
     if (AllowNullPointers) {
       // When performing pointer casts, it's OK if the value is null.
       // Skip the remaining checks in that case.
       Done = createBasicBlock("null");
       llvm::BasicBlock *Rest = createBasicBlock("not.null");
-      Builder.CreateCondBr(Cond, Rest, Done);
+      Builder.CreateCondBr(IsNonNull, Rest, Done);
       EmitBlock(Rest);
-      Cond = nullptr;
     } else {
-      Kinds.push_back(SanitizerKind::Null);
+      Checks.push_back(std::make_pair(IsNonNull, SanitizerKind::Null));
     }
   }
 
@@ -517,8 +515,7 @@
     llvm::Value *LargeEnough =
         Builder.CreateICmpUGE(Builder.CreateCall2(F, CastAddr, Min),
                               llvm::ConstantInt::get(IntPtrTy, Size));
-    Cond = Cond ? Builder.CreateAnd(Cond, LargeEnough) : LargeEnough;
-    Kinds.push_back(SanitizerKind::ObjectSize);
+    Checks.push_back(std::make_pair(LargeEnough, SanitizerKind::ObjectSize));
   }
 
   uint64_t AlignVal = 0;
@@ -535,19 +532,18 @@
                             llvm::ConstantInt::get(IntPtrTy, AlignVal - 1));
       llvm::Value *Aligned =
         Builder.CreateICmpEQ(Align, llvm::ConstantInt::get(IntPtrTy, 0));
-      Cond = Cond ? Builder.CreateAnd(Cond, Aligned) : Aligned;
-      Kinds.push_back(SanitizerKind::Alignment);
+      Checks.push_back(std::make_pair(Aligned, SanitizerKind::Alignment));
     }
   }
 
-  if (Cond) {
+  if (Checks.size() > 0) {
     llvm::Constant *StaticData[] = {
       EmitCheckSourceLocation(Loc),
       EmitCheckTypeDescriptor(Ty),
       llvm::ConstantInt::get(SizeTy, AlignVal),
       llvm::ConstantInt::get(Int8Ty, TCK)
     };
-    EmitCheck(Cond, "type_mismatch", StaticData, Address, Kinds);
+    EmitCheck(Checks, "type_mismatch", StaticData, Address);
   }
 
   // If possible, check that the vptr indicates that there is a subobject of
@@ -605,15 +601,16 @@
       // hard work of checking whether the vptr is for an object of the right
       // type. This will either fill in the cache and return, or produce a
       // diagnostic.
+      llvm::Value *EqualHash = Builder.CreateICmpEQ(CacheVal, Hash);
       llvm::Constant *StaticData[] = {
         EmitCheckSourceLocation(Loc),
         EmitCheckTypeDescriptor(Ty),
         CGM.GetAddrOfRTTIDescriptor(Ty.getUnqualifiedType()),
         llvm::ConstantInt::get(Int8Ty, TCK)
       };
       llvm::Value *DynamicData[] = { Address, Hash };
-      EmitCheck(Builder.CreateICmpEQ(CacheVal, Hash), "dynamic_type_cache_miss",
-                StaticData, DynamicData, SanitizerKind::Vptr);
+      EmitCheck(std::make_pair(EqualHash, SanitizerKind::Vptr),
+                "dynamic_type_cache_miss", StaticData, DynamicData);
     }
   }
 
@@ -701,8 +698,8 @@
   };
   llvm::Value *Check = Accessed ? Builder.CreateICmpULT(IndexVal, BoundVal)
                                 : Builder.CreateICmpULE(IndexVal, BoundVal);
-  EmitCheck(Check, "out_of_bounds", StaticData, Index,
-            SanitizerKind::ArrayBounds);
+  EmitCheck(std::make_pair(Check, SanitizerKind::ArrayBounds), "out_of_bounds",
+            StaticData, Index);
 }
 
 
@@ -1181,8 +1178,9 @@
         EmitCheckSourceLocation(Loc),
         EmitCheckTypeDescriptor(Ty)
       };
-      EmitCheck(Check, "load_invalid_value", StaticArgs, EmitCheckValue(Load),
-                NeedsEnumCheck ? SanitizerKind::Enum : SanitizerKind::Bool);
+      SanitizerKind Kind = NeedsEnumCheck ? SanitizerKind::Enum : SanitizerKind::Bool;
+      EmitCheck(std::make_pair(Check, Kind), "load_invalid_value", StaticArgs,
+                EmitCheckValue(Load));
     }
   } else if (CGM.getCodeGenOpts().OptimizationLevel > 0)
     if (llvm::MDNode *RangeInfo = getRangeForLoadFromType(Ty))
@@ -2221,32 +2219,33 @@
   }
 }
 
-void CodeGenFunction::EmitCheck(llvm::Value *Checked, StringRef CheckName,
-                                ArrayRef<llvm::Constant *> StaticArgs,
-                                ArrayRef<llvm::Value *> DynamicArgs,
-                                ArrayRef<SanitizerKind> Kinds) {
+void CodeGenFunction::EmitCheck(
+    ArrayRef<std::pair<llvm::Value *, SanitizerKind>> Checked,
+    StringRef CheckName, ArrayRef<llvm::Constant *> StaticArgs,
+    ArrayRef<llvm::Value *> DynamicArgs) {
   assert(IsSanitizerScope);
-  assert(Kinds.size() > 0);
-  CheckRecoverableKind RecoverKind = getRecoverableKind(Kinds[0]);
-  for (int i = 1, n = Kinds.size(); i < n; ++i)
-    assert(RecoverKind == getRecoverableKind(Kinds[i]) &&
+  assert(Checked.size() > 0);
+  llvm::Value *Cond = Checked[0].first;
+  CheckRecoverableKind RecoverKind = getRecoverableKind(Checked[0].second);
+  assert(SanOpts.has(Checked[0].second));
+  for (int i = 1, n = Checked.size(); i < n; ++i) {
+    Cond = Builder.CreateAnd(Cond, Checked[i].first);
+    assert(RecoverKind == getRecoverableKind(Checked[i].second) &&
            "All recoverable kinds in a single check must be same!");
-#ifndef NDEBUG
-  for (auto Kind : Kinds)
-    assert(SanOpts.has(Kind));
-#endif
+    assert(SanOpts.has(Checked[i].second));
+  }
 
   if (CGM.getCodeGenOpts().SanitizeUndefinedTrapOnError) {
     assert (RecoverKind != CheckRecoverableKind::AlwaysRecoverable &&
             "Runtime call required for AlwaysRecoverable kind!");
-    return EmitTrapCheck(Checked);
+    return EmitTrapCheck(Cond);
   }
 
   llvm::BasicBlock *Cont = createBasicBlock("cont");
 
   llvm::BasicBlock *Handler = createBasicBlock("handler." + CheckName);
 
-  llvm::Instruction *Branch = Builder.CreateCondBr(Checked, Cont, Handler);
+  llvm::Instruction *Branch = Builder.CreateCondBr(Cond, Cont, Handler);
 
   // Give hint that we very much don't expect to execute the handler
   // Value chosen to match UR_NONTAKEN_WEIGHT, see BranchProbabilityInfo.cpp
@@ -3321,8 +3320,8 @@
         EmitCheckSourceLocation(E->getLocStart()),
         EmitCheckTypeDescriptor(CalleeType)
       };
-      EmitCheck(CalleeRTTIMatch, "function_type_mismatch", StaticData, Callee,
-                SanitizerKind::Function);
+      EmitCheck(std::make_pair(CalleeRTTIMatch, SanitizerKind::Function),
+                "function_type_mismatch", StaticData, Callee);
 
       Builder.CreateBr(Cont);
       EmitBlock(Cont);
Index: lib/CodeGen/CGExprScalar.cpp
===================================================================
--- lib/CodeGen/CGExprScalar.cpp
+++ lib/CodeGen/CGExprScalar.cpp
@@ -85,8 +85,8 @@
     return CGF.EmitCheckedLValue(E, TCK);
   }
 
-  void EmitBinOpCheck(Value *Check, const BinOpInfo &Info,
-                      ArrayRef<SanitizerKind> Kinds);
+  void EmitBinOpCheck(ArrayRef<std::pair<Value *, SanitizerKind>> Checks,
+                      const BinOpInfo &Info);
 
   Value *EmitLoadOfLValue(LValue LV, SourceLocation Loc) {
     return CGF.EmitLoadOfLValue(LV, Loc).getScalarVal();
@@ -726,8 +726,8 @@
     CGF.EmitCheckTypeDescriptor(OrigSrcType),
     CGF.EmitCheckTypeDescriptor(DstType)
   };
-  CGF.EmitCheck(Check, "float_cast_overflow", StaticArgs, OrigSrc,
-                SanitizerKind::FloatCastOverflow);
+  CGF.EmitCheck(std::make_pair(Check, SanitizerKind::FloatCastOverflow),
+                "float_cast_overflow", StaticArgs, OrigSrc);
 }
 
 /// EmitScalarConversion - Emit a conversion from the specified type to the
@@ -885,9 +885,10 @@
 
 /// \brief Emit a sanitization check for the given "binary" operation (which
 /// might actually be a unary increment which has been lowered to a binary
-/// operation). The check passes if \p Check, which is an \c i1, is \c true.
-void ScalarExprEmitter::EmitBinOpCheck(Value *Check, const BinOpInfo &Info,
-                                       ArrayRef<SanitizerKind> Kinds) {
+/// operation). The check passes if all values in \p Checks (which are \c i1),
+/// are \c true.
+void ScalarExprEmitter::EmitBinOpCheck(
+    ArrayRef<std::pair<Value *, SanitizerKind>> Checks, const BinOpInfo &Info) {
   assert(CGF.IsSanitizerScope);
   StringRef CheckName;
   SmallVector<llvm::Constant *, 4> StaticData;
@@ -930,7 +931,7 @@
     DynamicData.push_back(Info.RHS);
   }
 
-  CGF.EmitCheck(Check, CheckName, StaticData, DynamicData, Kinds);
+  CGF.EmitCheck(Checks, CheckName, StaticData, DynamicData);
 }
 
 //===----------------------------------------------------------------------===//
@@ -2179,12 +2180,11 @@
 
 void ScalarExprEmitter::EmitUndefinedBehaviorIntegerDivAndRemCheck(
     const BinOpInfo &Ops, llvm::Value *Zero, bool isDiv) {
-  llvm::Value *Cond = nullptr;
-  SmallVector<SanitizerKind, 2> Kinds;
+  SmallVector<std::pair<llvm::Value *, SanitizerKind>, 2> Checks;
 
   if (CGF.SanOpts.has(SanitizerKind::IntegerDivideByZero)) {
-    Cond = Builder.CreateICmpNE(Ops.RHS, Zero);
-    Kinds.push_back(SanitizerKind::IntegerDivideByZero);
+    Checks.push_back(std::make_pair(Builder.CreateICmpNE(Ops.RHS, Zero),
+                                    SanitizerKind::IntegerDivideByZero));
   }
 
   if (CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow) &&
@@ -2197,13 +2197,13 @@
 
     llvm::Value *LHSCmp = Builder.CreateICmpNE(Ops.LHS, IntMin);
     llvm::Value *RHSCmp = Builder.CreateICmpNE(Ops.RHS, NegOne);
-    llvm::Value *Overflow = Builder.CreateOr(LHSCmp, RHSCmp, "or");
-    Cond = Cond ? Builder.CreateAnd(Cond, Overflow, "and") : Overflow;
-    Kinds.push_back(SanitizerKind::SignedIntegerOverflow);
+    llvm::Value *NotOverflow = Builder.CreateOr(LHSCmp, RHSCmp, "or");
+    Checks.push_back(
+        std::make_pair(NotOverflow, SanitizerKind::SignedIntegerOverflow));
   }
 
-  if (Cond)
-    EmitBinOpCheck(Cond, Ops, Kinds);
+  if (Checks.size() > 0)
+    EmitBinOpCheck(Checks, Ops);
 }
 
 Value *ScalarExprEmitter::EmitDiv(const BinOpInfo &Ops) {
@@ -2217,8 +2217,9 @@
     } else if (CGF.SanOpts.has(SanitizerKind::FloatDivideByZero) &&
                Ops.Ty->isRealFloatingType()) {
       llvm::Value *Zero = llvm::Constant::getNullValue(ConvertType(Ops.Ty));
-      EmitBinOpCheck(Builder.CreateFCmpUNE(Ops.RHS, Zero), Ops,
-                     SanitizerKind::FloatDivideByZero);
+      llvm::Value *NonZero = Builder.CreateFCmpUNE(Ops.RHS, Zero);
+      EmitBinOpCheck(std::make_pair(NonZero, SanitizerKind::FloatDivideByZero),
+                     Ops);
     }
   }
 
@@ -2303,9 +2304,10 @@
     // runtime. Otherwise, this is a -ftrapv check, so just emit a trap.
     if (!isSigned || CGF.SanOpts.has(SanitizerKind::SignedIntegerOverflow)) {
       CodeGenFunction::SanitizerScope SanScope(&CGF);
-      EmitBinOpCheck(Builder.CreateNot(overflow), Ops,
-                     isSigned ? SanitizerKind::SignedIntegerOverflow
-                              : SanitizerKind::UnsignedIntegerOverflow);
+      llvm::Value *NotOverflow = Builder.CreateNot(overflow);
+      SanitizerKind Kind = isSigned ? SanitizerKind::SignedIntegerOverflow
+                              : SanitizerKind::UnsignedIntegerOverflow;
+      EmitBinOpCheck(std::make_pair(NotOverflow, Kind), Ops);
     } else
       CGF.EmitTrapCheck(Builder.CreateNot(overflow));
     return result;
@@ -2695,7 +2697,7 @@
       Valid = P;
     }
 
-    EmitBinOpCheck(Valid, Ops, SanitizerKind::Shift);
+    EmitBinOpCheck(std::make_pair(Valid, SanitizerKind::Shift), Ops);
   }
   // OpenCL 6.3j: shift values are effectively % word size of LHS.
   if (CGF.getLangOpts().OpenCL)
@@ -2714,9 +2716,9 @@
   if (CGF.SanOpts.has(SanitizerKind::Shift) && !CGF.getLangOpts().OpenCL &&
       isa<llvm::IntegerType>(Ops.LHS->getType())) {
     CodeGenFunction::SanitizerScope SanScope(&CGF);
-    EmitBinOpCheck(
-        Builder.CreateICmpULE(RHS, GetWidthMinusOneValue(Ops.LHS, RHS)), Ops,
-        SanitizerKind::Shift);
+    llvm::Value *Valid =
+        Builder.CreateICmpULE(RHS, GetWidthMinusOneValue(Ops.LHS, RHS));
+    EmitBinOpCheck(std::make_pair(Valid, SanitizerKind::Shift), Ops);
   }
 
   // OpenCL 6.3j: shift values are effectively % word size of LHS.
Index: lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -899,9 +899,10 @@
       !FD->getReturnType()->isVoidType() && Builder.GetInsertBlock()) {
     if (SanOpts.has(SanitizerKind::Return)) {
       SanitizerScope SanScope(this);
-      EmitCheck(Builder.getFalse(), "missing_return",
-                EmitCheckSourceLocation(FD->getLocation()), None,
-                SanitizerKind::Return);
+      llvm::Value *IsFalse = Builder.getFalse();
+      EmitCheck(std::make_pair(IsFalse, SanitizerKind::Return),
+                "missing_return", EmitCheckSourceLocation(FD->getLocation()),
+                None);
     } else if (CGM.getCodeGenOpts().OptimizationLevel == 0)
       Builder.CreateCall(CGM.getIntrinsic(llvm::Intrinsic::trap));
     Builder.CreateUnreachable();
@@ -1560,9 +1561,9 @@
               EmitCheckSourceLocation(size->getLocStart()),
               EmitCheckTypeDescriptor(size->getType())
             };
-            EmitCheck(Builder.CreateICmpSGT(Size, Zero),
-                      "vla_bound_not_positive", StaticArgs, Size,
-                      SanitizerKind::VLABound);
+            EmitCheck(std::make_pair(Builder.CreateICmpSGT(Size, Zero),
+                                     SanitizerKind::VLABound),
+                      "vla_bound_not_positive", StaticArgs, Size);
           }
 
           // Always zexting here would be wrong if it weren't
Index: lib/CodeGen/CodeGenFunction.h
===================================================================
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2652,10 +2652,9 @@
   /// \brief Create a basic block that will call a handler function in a
   /// sanitizer runtime with the provided arguments, and create a conditional
   /// branch to it.
-  void EmitCheck(llvm::Value *Checked, StringRef CheckName,
-                 ArrayRef<llvm::Constant *> StaticArgs,
-                 ArrayRef<llvm::Value *> DynamicArgs,
-                 ArrayRef<SanitizerKind> Kinds);
+  void EmitCheck(ArrayRef<std::pair<llvm::Value *, SanitizerKind>> Checked,
+                 StringRef CheckName, ArrayRef<llvm::Constant *> StaticArgs,
+                 ArrayRef<llvm::Value *> DynamicArgs);
 
   /// \brief Create a basic block that will call the trap intrinsic, and emit a
   /// conditional branch to it, for the -ftrapv checks.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to