[UBSan] Strengthen pointer checks in 'new' expressions
With this change compiler generates alignment checks for wider range
of types. Previously such checks were generated only for the record types
with non-trivial default constructor. So the types like:
struct alignas(32) S2 { int x; };
typedef __attribute__((ext_vector_type(2), aligned(32))) float float32x2_t;
did not get checks when allocated by 'new' expression.
This change also optimizes the checks generated for the arrays created
in 'new' expressions. Previously the check was generated for each
invocation of type constructor. Now the check is generated only once
for entire array.
Differential Revision: https://reviews.llvm.org/D49589
llvm-svn: 338199
diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp
index 0b9311f..c1915e4 100644
--- a/clang/lib/CodeGen/CGClass.cpp
+++ b/clang/lib/CodeGen/CGClass.cpp
@@ -685,7 +685,10 @@
AggValueSlot::IsDestructed,
AggValueSlot::DoesNotNeedGCBarriers,
AggValueSlot::IsNotAliased,
- overlapForFieldInit(Field));
+ overlapForFieldInit(Field),
+ AggValueSlot::IsNotZeroed,
+ // Checks are made by the code that calls constructor.
+ AggValueSlot::IsSanitizerChecked);
EmitAggExpr(Init, Slot);
break;
}
@@ -1869,12 +1872,14 @@
/// zero-initialized before it is constructed
void CodeGenFunction::EmitCXXAggrConstructorCall(
const CXXConstructorDecl *ctor, const ArrayType *arrayType,
- Address arrayBegin, const CXXConstructExpr *E, bool zeroInitialize) {
+ Address arrayBegin, const CXXConstructExpr *E, bool NewPointerIsChecked,
+ bool zeroInitialize) {
QualType elementType;
llvm::Value *numElements =
emitArrayLength(arrayType, elementType, arrayBegin);
- EmitCXXAggrConstructorCall(ctor, numElements, arrayBegin, E, zeroInitialize);
+ EmitCXXAggrConstructorCall(ctor, numElements, arrayBegin, E,
+ NewPointerIsChecked, zeroInitialize);
}
/// EmitCXXAggrConstructorCall - Emit a loop to call a particular
@@ -1890,6 +1895,7 @@
llvm::Value *numElements,
Address arrayBase,
const CXXConstructExpr *E,
+ bool NewPointerIsChecked,
bool zeroInitialize) {
// It's legal for numElements to be zero. This can happen both
// dynamically, because x can be zero in 'new A[x]', and statically,
@@ -1966,7 +1972,7 @@
EmitCXXConstructorCall(ctor, Ctor_Complete, /*ForVirtualBase=*/false,
/*Delegating=*/false, curAddr, E,
- AggValueSlot::DoesNotOverlap);
+ AggValueSlot::DoesNotOverlap, NewPointerIsChecked);
}
// Go to the next element.
@@ -2002,7 +2008,8 @@
bool ForVirtualBase,
bool Delegating, Address This,
const CXXConstructExpr *E,
- AggValueSlot::Overlap_t Overlap) {
+ AggValueSlot::Overlap_t Overlap,
+ bool NewPointerIsChecked) {
CallArgList Args;
// Push the this ptr.
@@ -2031,7 +2038,7 @@
/*ParamsToSkip*/ 0, Order);
EmitCXXConstructorCall(D, Type, ForVirtualBase, Delegating, This, Args,
- Overlap, E->getExprLoc());
+ Overlap, E->getExprLoc(), NewPointerIsChecked);
}
static bool canEmitDelegateCallArgs(CodeGenFunction &CGF,
@@ -2065,14 +2072,13 @@
Address This,
CallArgList &Args,
AggValueSlot::Overlap_t Overlap,
- SourceLocation Loc) {
+ SourceLocation Loc,
+ bool NewPointerIsChecked) {
const CXXRecordDecl *ClassDecl = D->getParent();
- // C++11 [class.mfct.non-static]p2:
- // If a non-static member function of a class X is called for an object that
- // is not of type X, or of a type derived from X, the behavior is undefined.
- EmitTypeCheck(CodeGenFunction::TCK_ConstructorCall, Loc,
- This.getPointer(), getContext().getRecordType(ClassDecl));
+ if (!NewPointerIsChecked)
+ EmitTypeCheck(CodeGenFunction::TCK_ConstructorCall, Loc, This.getPointer(),
+ getContext().getRecordType(ClassDecl), CharUnits::Zero());
if (D->isTrivial() && D->isDefaultConstructor()) {
assert(Args.size() == 1 && "trivial default ctor with args");
@@ -2181,7 +2187,7 @@
EmitCXXConstructorCall(D, Ctor_Base, ForVirtualBase, /*Delegating*/false,
This, Args, AggValueSlot::MayOverlap,
- E->getLocation());
+ E->getLocation(), /*NewPointerIsChecked*/true);
}
void CodeGenFunction::EmitInlinedInheritingCXXConstructorCall(
@@ -2277,8 +2283,10 @@
EmitCallArgs(Args, FPT, drop_begin(E->arguments(), 1), E->getConstructor(),
/*ParamsToSkip*/ 1);
- EmitCXXConstructorCall(D, Ctor_Complete, false, false, This, Args,
- AggValueSlot::MayOverlap, E->getExprLoc());
+ EmitCXXConstructorCall(D, Ctor_Complete, /*ForVirtualBase*/false,
+ /*Delegating*/false, This, Args,
+ AggValueSlot::MayOverlap, E->getExprLoc(),
+ /*NewPointerIsChecked*/false);
}
void
@@ -2314,7 +2322,8 @@
EmitCXXConstructorCall(Ctor, CtorType, /*ForVirtualBase=*/false,
/*Delegating=*/true, This, DelegateArgs,
- AggValueSlot::MayOverlap, Loc);
+ AggValueSlot::MayOverlap, Loc,
+ /*NewPointerIsChecked=*/true);
}
namespace {
@@ -2346,7 +2355,10 @@
AggValueSlot::IsDestructed,
AggValueSlot::DoesNotNeedGCBarriers,
AggValueSlot::IsNotAliased,
- AggValueSlot::MayOverlap);
+ AggValueSlot::MayOverlap,
+ AggValueSlot::IsNotZeroed,
+ // Checks are made by the code that calls constructor.
+ AggValueSlot::IsSanitizerChecked);
EmitAggExpr(Ctor->init_begin()[0]->getInit(), AggSlot);
diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp
index 8955d8a..26cb42c 100644
--- a/clang/lib/CodeGen/CGExprCXX.cpp
+++ b/clang/lib/CodeGen/CGExprCXX.cpp
@@ -607,7 +607,8 @@
if (const ArrayType *arrayType
= getContext().getAsArrayType(E->getType())) {
- EmitCXXAggrConstructorCall(CD, arrayType, Dest.getAddress(), E);
+ EmitCXXAggrConstructorCall(CD, arrayType, Dest.getAddress(), E,
+ Dest.isSanitizerChecked());
} else {
CXXCtorType Type = Ctor_Complete;
bool ForVirtualBase = false;
@@ -634,7 +635,8 @@
// Call the constructor.
EmitCXXConstructorCall(CD, Type, ForVirtualBase, Delegating,
- Dest.getAddress(), E, Dest.mayOverlap());
+ Dest.getAddress(), E, Dest.mayOverlap(),
+ Dest.isSanitizerChecked());
}
}
@@ -954,7 +956,8 @@
AggValueSlot::IsDestructed,
AggValueSlot::DoesNotNeedGCBarriers,
AggValueSlot::IsNotAliased,
- MayOverlap);
+ MayOverlap, AggValueSlot::IsNotZeroed,
+ AggValueSlot::IsSanitizerChecked);
CGF.EmitAggExpr(Init, Slot);
return;
}
@@ -1024,7 +1027,9 @@
AggValueSlot::IsDestructed,
AggValueSlot::DoesNotNeedGCBarriers,
AggValueSlot::IsNotAliased,
- AggValueSlot::DoesNotOverlap);
+ AggValueSlot::DoesNotOverlap,
+ AggValueSlot::IsNotZeroed,
+ AggValueSlot::IsSanitizerChecked);
EmitAggExpr(ILE->getInit(0), Slot);
// Move past these elements.
@@ -1154,6 +1159,7 @@
NumElements,
llvm::ConstantInt::get(NumElements->getType(), InitListElements));
EmitCXXAggrConstructorCall(Ctor, NumElements, CurPtr, CCE,
+ /*NewPointerIsChecked*/true,
CCE->requiresZeroInitialization());
return;
}
@@ -1705,6 +1711,12 @@
result = Address(Builder.CreateLaunderInvariantGroup(result.getPointer()),
result.getAlignment());
+ // Emit sanitizer checks for pointer value now, so that in the case of an
+ // array it was checked only once and not at each constructor call.
+ EmitTypeCheck(CodeGenFunction::TCK_ConstructorCall,
+ E->getAllocatedTypeSourceInfo()->getTypeLoc().getBeginLoc(),
+ result.getPointer(), allocType);
+
EmitNewInitializer(*this, E, allocType, elementTy, result, numElements,
allocSizeWithoutCookie);
if (E->isArray()) {
diff --git a/clang/lib/CodeGen/CGValue.h b/clang/lib/CodeGen/CGValue.h
index 418bda1..60f2059 100644
--- a/clang/lib/CodeGen/CGValue.h
+++ b/clang/lib/CodeGen/CGValue.h
@@ -479,12 +479,20 @@
/// the size of the type.
bool OverlapFlag : 1;
+ /// If is set to true, sanitizer checks are already generated for this address
+ /// or not required. For instance, if this address represents an object
+ /// created in 'new' expression, sanitizer checks for memory is made as a part
+ /// of 'operator new' emission and object constructor should not generate
+ /// them.
+ bool SanitizerCheckedFlag : 1;
+
public:
enum IsAliased_t { IsNotAliased, IsAliased };
enum IsDestructed_t { IsNotDestructed, IsDestructed };
enum IsZeroed_t { IsNotZeroed, IsZeroed };
enum Overlap_t { DoesNotOverlap, MayOverlap };
enum NeedsGCBarriers_t { DoesNotNeedGCBarriers, NeedsGCBarriers };
+ enum IsSanitizerChecked_t { IsNotSanitizerChecked, IsSanitizerChecked };
/// ignored - Returns an aggregate value slot indicating that the
/// aggregate value is being ignored.
@@ -509,7 +517,8 @@
NeedsGCBarriers_t needsGC,
IsAliased_t isAliased,
Overlap_t mayOverlap,
- IsZeroed_t isZeroed = IsNotZeroed) {
+ IsZeroed_t isZeroed = IsNotZeroed,
+ IsSanitizerChecked_t isChecked = IsNotSanitizerChecked) {
AggValueSlot AV;
if (addr.isValid()) {
AV.Addr = addr.getPointer();
@@ -524,6 +533,7 @@
AV.ZeroedFlag = isZeroed;
AV.AliasedFlag = isAliased;
AV.OverlapFlag = mayOverlap;
+ AV.SanitizerCheckedFlag = isChecked;
return AV;
}
@@ -532,9 +542,10 @@
NeedsGCBarriers_t needsGC,
IsAliased_t isAliased,
Overlap_t mayOverlap,
- IsZeroed_t isZeroed = IsNotZeroed) {
+ IsZeroed_t isZeroed = IsNotZeroed,
+ IsSanitizerChecked_t isChecked = IsNotSanitizerChecked) {
return forAddr(LV.getAddress(), LV.getQuals(), isDestructed, needsGC,
- isAliased, mayOverlap, isZeroed);
+ isAliased, mayOverlap, isZeroed, isChecked);
}
IsDestructed_t isExternallyDestructed() const {
@@ -586,6 +597,10 @@
return Overlap_t(OverlapFlag);
}
+ bool isSanitizerChecked() const {
+ return SanitizerCheckedFlag;
+ }
+
RValue asRValue() const {
if (isIgnored()) {
return RValue::getIgnored();
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 79870ed..1f8f336 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -2490,13 +2490,15 @@
void EmitCXXConstructorCall(const CXXConstructorDecl *D, CXXCtorType Type,
bool ForVirtualBase, bool Delegating,
Address This, const CXXConstructExpr *E,
- AggValueSlot::Overlap_t Overlap);
+ AggValueSlot::Overlap_t Overlap,
+ bool NewPointerIsChecked);
void EmitCXXConstructorCall(const CXXConstructorDecl *D, CXXCtorType Type,
bool ForVirtualBase, bool Delegating,
Address This, CallArgList &Args,
AggValueSlot::Overlap_t Overlap,
- SourceLocation Loc);
+ SourceLocation Loc,
+ bool NewPointerIsChecked);
/// Emit assumption load for all bases. Requires to be be called only on
/// most-derived class and not under construction of the object.
@@ -2513,12 +2515,14 @@
const ArrayType *ArrayTy,
Address ArrayPtr,
const CXXConstructExpr *E,
+ bool NewPointerIsChecked,
bool ZeroInitialization = false);
void EmitCXXAggrConstructorCall(const CXXConstructorDecl *D,
llvm::Value *NumElements,
Address ArrayPtr,
const CXXConstructExpr *E,
+ bool NewPointerIsChecked,
bool ZeroInitialization = false);
static Destroyer destroyCXXObject;