[ubsan] Add a check for pointer overflow UB
Check pointer arithmetic for overflow.
For some more background on this check, see:
https://wdtz.org/catching-pointer-overflow-bugs.html
https://reviews.llvm.org/D20322
Patch by Will Dietz and John Regehr!
This version of the patch is different from the original in a few ways:
- It introduces the EmitCheckedInBoundsGEP utility which inserts
checks when the pointer overflow check is enabled.
- It does some constant-folding to reduce instrumentation overhead.
- It does not check some GEPs in CGExprCXX. I'm not sure that
inserting checks here, or in CGClass, would catch many bugs.
Possible future directions for this check:
- Introduce CGF.EmitCheckedStructGEP, to detect overflows when
accessing structures.
Testing: Apart from the added lit test, I ran check-llvm and check-clang
with a stage2, ubsan-instrumented clang. Will and John have also done
extensive testing on numerous open source projects.
Differential Revision: https://reviews.llvm.org/D33305
llvm-svn: 304459
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 6808c69..84ce896 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -3002,9 +3002,10 @@
llvm::Value *ptr,
ArrayRef<llvm::Value*> indices,
bool inbounds,
+ SourceLocation loc,
const llvm::Twine &name = "arrayidx") {
if (inbounds) {
- return CGF.Builder.CreateInBoundsGEP(ptr, indices, name);
+ return CGF.EmitCheckedInBoundsGEP(ptr, indices, loc, name);
} else {
return CGF.Builder.CreateGEP(ptr, indices, name);
}
@@ -3035,8 +3036,9 @@
}
static Address emitArraySubscriptGEP(CodeGenFunction &CGF, Address addr,
- ArrayRef<llvm::Value*> indices,
+ ArrayRef<llvm::Value *> indices,
QualType eltType, bool inbounds,
+ SourceLocation loc,
const llvm::Twine &name = "arrayidx") {
// All the indices except that last must be zero.
#ifndef NDEBUG
@@ -3057,7 +3059,7 @@
getArrayElementAlign(addr.getAlignment(), indices.back(), eltSize);
llvm::Value *eltPtr =
- emitArraySubscriptGEP(CGF, addr.getPointer(), indices, inbounds, name);
+ emitArraySubscriptGEP(CGF, addr.getPointer(), indices, inbounds, loc, name);
return Address(eltPtr, eltAlign);
}
@@ -3110,7 +3112,8 @@
Address Addr = EmitExtVectorElementLValue(LV);
QualType EltType = LV.getType()->castAs<VectorType>()->getElementType();
- Addr = emitArraySubscriptGEP(*this, Addr, Idx, EltType, /*inbounds*/ true);
+ Addr = emitArraySubscriptGEP(*this, Addr, Idx, EltType, /*inbounds*/ true,
+ E->getExprLoc());
return MakeAddrLValue(Addr, EltType, LV.getBaseInfo());
}
@@ -3138,7 +3141,8 @@
}
Addr = emitArraySubscriptGEP(*this, Addr, Idx, vla->getElementType(),
- !getLangOpts().isSignedOverflowDefined());
+ !getLangOpts().isSignedOverflowDefined(),
+ E->getExprLoc());
} else if (const ObjCObjectType *OIT = E->getType()->getAs<ObjCObjectType>()){
// Indexing over an interface, as in "NSString *P; P[4];"
@@ -3163,8 +3167,8 @@
// Do the GEP.
CharUnits EltAlign =
getArrayElementAlign(Addr.getAlignment(), Idx, InterfaceSize);
- llvm::Value *EltPtr =
- emitArraySubscriptGEP(*this, Addr.getPointer(), ScaledIdx, false);
+ llvm::Value *EltPtr = emitArraySubscriptGEP(
+ *this, Addr.getPointer(), ScaledIdx, false, E->getExprLoc());
Addr = Address(EltPtr, EltAlign);
// Cast back.
@@ -3189,14 +3193,16 @@
Addr = emitArraySubscriptGEP(*this, ArrayLV.getAddress(),
{CGM.getSize(CharUnits::Zero()), Idx},
E->getType(),
- !getLangOpts().isSignedOverflowDefined());
+ !getLangOpts().isSignedOverflowDefined(),
+ E->getExprLoc());
BaseInfo = ArrayLV.getBaseInfo();
} else {
// The base must be a pointer; emit it with an estimate of its alignment.
Addr = EmitPointerWithAlignment(E->getBase(), &BaseInfo);
auto *Idx = EmitIdxAfterBase(/*Promote*/true);
Addr = emitArraySubscriptGEP(*this, Addr, Idx, E->getType(),
- !getLangOpts().isSignedOverflowDefined());
+ !getLangOpts().isSignedOverflowDefined(),
+ E->getExprLoc());
}
LValue LV = MakeAddrLValue(Addr, E->getType(), BaseInfo);
@@ -3368,7 +3374,8 @@
else
Idx = Builder.CreateNSWMul(Idx, NumElements);
EltPtr = emitArraySubscriptGEP(*this, Base, Idx, VLA->getElementType(),
- !getLangOpts().isSignedOverflowDefined());
+ !getLangOpts().isSignedOverflowDefined(),
+ E->getExprLoc());
} else if (const Expr *Array = isSimpleArrayDecayOperand(E->getBase())) {
// If this is A[i] where A is an array, the frontend will have decayed the
// base to be a ArrayToPointerDecay implicit cast. While correct, it is
@@ -3387,13 +3394,15 @@
// Propagate the alignment from the array itself to the result.
EltPtr = emitArraySubscriptGEP(
*this, ArrayLV.getAddress(), {CGM.getSize(CharUnits::Zero()), Idx},
- ResultExprTy, !getLangOpts().isSignedOverflowDefined());
+ ResultExprTy, !getLangOpts().isSignedOverflowDefined(),
+ E->getExprLoc());
BaseInfo = ArrayLV.getBaseInfo();
} else {
Address Base = emitOMPArraySectionBase(*this, E->getBase(), BaseInfo,
BaseTy, ResultExprTy, IsLowerBound);
EltPtr = emitArraySubscriptGEP(*this, Base, Idx, ResultExprTy,
- !getLangOpts().isSignedOverflowDefined());
+ !getLangOpts().isSignedOverflowDefined(),
+ E->getExprLoc());
}
return MakeAddrLValue(EltPtr, ResultExprTy, BaseInfo);