r338199 - [UBSan] Strengthen pointer checks in 'new' expressions
Serge Pavlov via cfe-commits
cfe-commits at lists.llvm.org
Sat Jul 28 08:33:03 PDT 2018
Author: sepavloff
Date: Sat Jul 28 08:33:03 2018
New Revision: 338199
URL: http://llvm.org/viewvc/llvm-project?rev=338199&view=rev
Log:
[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
Added:
cfe/trunk/test/CodeGenCXX/ubsan-new-checks.cpp
Modified:
cfe/trunk/lib/CodeGen/CGClass.cpp
cfe/trunk/lib/CodeGen/CGExprCXX.cpp
cfe/trunk/lib/CodeGen/CGValue.h
cfe/trunk/lib/CodeGen/CodeGenFunction.h
Modified: cfe/trunk/lib/CodeGen/CGClass.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=338199&r1=338198&r2=338199&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGClass.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGClass.cpp Sat Jul 28 08:33:03 2018
@@ -685,7 +685,10 @@ void CodeGenFunction::EmitInitializerFor
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 @@ void CodeGenFunction::EnterDtorCleanups(
/// 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 @@ void CodeGenFunction::EmitCXXAggrConstru
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 @@ void CodeGenFunction::EmitCXXAggrConstru
EmitCXXConstructorCall(ctor, Ctor_Complete, /*ForVirtualBase=*/false,
/*Delegating=*/false, curAddr, E,
- AggValueSlot::DoesNotOverlap);
+ AggValueSlot::DoesNotOverlap, NewPointerIsChecked);
}
// Go to the next element.
@@ -2002,7 +2008,8 @@ void CodeGenFunction::EmitCXXConstructor
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 @@ void CodeGenFunction::EmitCXXConstructor
/*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 @@ void CodeGenFunction::EmitCXXConstructor
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 @@ void CodeGenFunction::EmitInheritedCXXCo
EmitCXXConstructorCall(D, Ctor_Base, ForVirtualBase, /*Delegating*/false,
This, Args, AggValueSlot::MayOverlap,
- E->getLocation());
+ E->getLocation(), /*NewPointerIsChecked*/true);
}
void CodeGenFunction::EmitInlinedInheritingCXXConstructorCall(
@@ -2277,8 +2283,10 @@ CodeGenFunction::EmitSynthesizedCXXCopyC
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 @@ CodeGenFunction::EmitDelegateCXXConstruc
EmitCXXConstructorCall(Ctor, CtorType, /*ForVirtualBase=*/false,
/*Delegating=*/true, This, DelegateArgs,
- AggValueSlot::MayOverlap, Loc);
+ AggValueSlot::MayOverlap, Loc,
+ /*NewPointerIsChecked=*/true);
}
namespace {
@@ -2346,7 +2355,10 @@ CodeGenFunction::EmitDelegatingCXXConstr
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);
Modified: cfe/trunk/lib/CodeGen/CGExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprCXX.cpp?rev=338199&r1=338198&r2=338199&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGExprCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp Sat Jul 28 08:33:03 2018
@@ -607,7 +607,8 @@ CodeGenFunction::EmitCXXConstructExpr(co
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 @@ CodeGenFunction::EmitCXXConstructExpr(co
// Call the constructor.
EmitCXXConstructorCall(CD, Type, ForVirtualBase, Delegating,
- Dest.getAddress(), E, Dest.mayOverlap());
+ Dest.getAddress(), E, Dest.mayOverlap(),
+ Dest.isSanitizerChecked());
}
}
@@ -954,7 +956,8 @@ static void StoreAnyExprIntoOneUnit(Code
AggValueSlot::IsDestructed,
AggValueSlot::DoesNotNeedGCBarriers,
AggValueSlot::IsNotAliased,
- MayOverlap);
+ MayOverlap, AggValueSlot::IsNotZeroed,
+ AggValueSlot::IsSanitizerChecked);
CGF.EmitAggExpr(Init, Slot);
return;
}
@@ -1024,7 +1027,9 @@ void CodeGenFunction::EmitNewArrayInitia
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 @@ void CodeGenFunction::EmitNewArrayInitia
NumElements,
llvm::ConstantInt::get(NumElements->getType(), InitListElements));
EmitCXXAggrConstructorCall(Ctor, NumElements, CurPtr, CCE,
+ /*NewPointerIsChecked*/true,
CCE->requiresZeroInitialization());
return;
}
@@ -1705,6 +1711,12 @@ llvm::Value *CodeGenFunction::EmitCXXNew
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()) {
Modified: cfe/trunk/lib/CodeGen/CGValue.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGValue.h?rev=338199&r1=338198&r2=338199&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGValue.h (original)
+++ cfe/trunk/lib/CodeGen/CGValue.h Sat Jul 28 08:33:03 2018
@@ -479,12 +479,20 @@ class AggValueSlot {
/// 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 @@ public:
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 @@ public:
AV.ZeroedFlag = isZeroed;
AV.AliasedFlag = isAliased;
AV.OverlapFlag = mayOverlap;
+ AV.SanitizerCheckedFlag = isChecked;
return AV;
}
@@ -532,9 +542,10 @@ public:
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 @@ public:
return Overlap_t(OverlapFlag);
}
+ bool isSanitizerChecked() const {
+ return SanitizerCheckedFlag;
+ }
+
RValue asRValue() const {
if (isIgnored()) {
return RValue::getIgnored();
Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=338199&r1=338198&r2=338199&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Sat Jul 28 08:33:03 2018
@@ -2490,13 +2490,15 @@ public:
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 @@ public:
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;
Added: cfe/trunk/test/CodeGenCXX/ubsan-new-checks.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/ubsan-new-checks.cpp?rev=338199&view=auto
==============================================================================
--- cfe/trunk/test/CodeGenCXX/ubsan-new-checks.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/ubsan-new-checks.cpp Sat Jul 28 08:33:03 2018
@@ -0,0 +1,146 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -std=c++11 -S -emit-llvm -fsanitize=alignment %s -o - | FileCheck %s
+
+struct alignas(32) S1 {
+ int x;
+ S1();
+};
+
+struct alignas(32) S2 {
+ int x;
+};
+
+struct alignas(32) S3 {
+ int x;
+ S3(int *p = new int[4]);
+};
+
+struct S4 : public S3 {
+ S4() : S3() {}
+};
+
+typedef __attribute__((ext_vector_type(2), aligned(32))) float float32x2_t;
+
+struct S5 {
+ float32x2_t x;
+};
+
+void *operator new (unsigned long, void *p) { return p; }
+void *operator new[] (unsigned long, void *p) { return p; }
+
+S1 *func_01() {
+ // CHECK-LABEL: define {{.*}} @_Z7func_01v
+ // CHECK: and i64 %{{.*}}, 31, !nosanitize
+ // CHECK: icmp eq i64 %{{.*}}, 0, !nosanitize
+ // CHECK: call void @_ZN2S1C1Ev(
+ // CHECK-NOT: and i64 %{{.*}}, 31
+ // CHECK: ret %struct.S1*
+ return new S1[20];
+}
+
+S2 *func_02() {
+ // CHECK-LABEL: define {{.*}} @_Z7func_02v
+ // CHECK: and i64 %{{.*}}, 31, !nosanitize
+ // CHECK: icmp eq i64 %{{.*}}, 0, !nosanitize
+ // CHECK: ret %struct.S2*
+ return new S2;
+}
+
+S2 *func_03() {
+ // CHECK-LABEL: define {{.*}} @_Z7func_03v
+ // CHECK: and i64 %{{.*}}, 31, !nosanitize
+ // CHECK: icmp eq i64 %{{.*}}, 0, !nosanitize
+ // CHECK-NOT: and i64 %{{.*}}, 31
+ // CHECK: ret %struct.S2*
+ return new S2[20];
+}
+
+float32x2_t *func_04() {
+ // CHECK-LABEL: define {{.*}} @_Z7func_04v
+ // CHECK: and i64 %{{.*}}, 31, !nosanitize
+ // CHECK: icmp eq i64 %{{.*}}, 0, !nosanitize
+ // CHECK: ret <2 x float>*
+ return new float32x2_t;
+}
+
+float32x2_t *func_05() {
+ // CHECK-LABEL: define {{.*}} @_Z7func_05v
+ // CHECK: and i64 %{{.*}}, 31, !nosanitize
+ // CHECK: icmp eq i64 %{{.*}}, 0, !nosanitize
+ // CHECK-NOT: and i64 %{{.*}}, 31
+ // CHECK: ret <2 x float>*
+ return new float32x2_t[20];
+}
+
+S3 *func_07() {
+ // CHECK-LABEL: define {{.*}} @_Z7func_07v
+ // CHECK: and i64 %{{.*}}, 31, !nosanitize
+ // CHECK: icmp eq i64 %{{.*}}, 0, !nosanitize
+ // CHECK: and i64 %{{.*}}, 3, !nosanitize
+ // CHECK: icmp eq i64 %{{.*}}, 0, !nosanitize
+ // CHECK: ret %struct.S3*
+ return new S3;
+}
+
+S3 *func_08() {
+ // CHECK-LABEL: define {{.*}} @_Z7func_08v
+ // CHECK: and i64 %{{.*}}, 31, !nosanitize
+ // CHECK: icmp eq i64 %{{.*}}, 0, !nosanitize
+ // CHECK: and i64 %{{.*}}, 3, !nosanitize
+ // CHECK: icmp eq i64 %{{.*}}, 0, !nosanitize
+ // CHECK: ret %struct.S3*
+ return new S3[10];
+}
+
+
+S2 *func_10(void *p) {
+ // CHECK-LABEL: define {{.*}} @_Z7func_10Pv
+ // CHECK: and i64 %{{.*}}, 31, !nosanitize
+ // CHECK: icmp eq i64 %{{.*}}, 0, !nosanitize
+ // CHECK: ret %struct.S2*
+ return new(p) S2;
+}
+
+S2 *func_11(void *p) {
+ // CHECK-LABEL: define {{.*}} @_Z7func_11Pv
+ // CHECK: and i64 %{{.*}}, 31, !nosanitize
+ // CHECK: icmp eq i64 %{{.*}}, 0, !nosanitize
+ // CHECK-NOT: and i64 %{{.*}}, 31, !nosanitize
+ // CHECK-NOT: icmp eq i64 %{{.*}}, 0, !nosanitize
+ // CHECK: ret %struct.S2*
+ return new(p) S2[10];
+}
+
+float32x2_t *func_12() {
+ // CHECK-LABEL: define {{.*}} @_Z7func_12v
+ // CHECK: and i64 %{{.*}}, 31, !nosanitize
+ // CHECK: icmp eq i64 %{{.*}}, 0, !nosanitize
+ // CHECK: ret <2 x float>*
+ return new float32x2_t;
+}
+
+float32x2_t *func_13() {
+ // CHECK-LABEL: define {{.*}} @_Z7func_13v
+ // CHECK: and i64 %{{.*}}, 31, !nosanitize
+ // CHECK: icmp eq i64 %{{.*}}, 0, !nosanitize
+ // CHECK-NOT: and i64 %{{.*}}, 31
+ // CHECK: ret <2 x float>*
+ return new float32x2_t[20];
+}
+
+S4 *func_14() {
+ // CHECK-LABEL: define {{.*}} @_Z7func_14v
+ // CHECK: and i64 %{{.*}}, 31, !nosanitize
+ // CHECK: icmp eq i64 %{{.*}}, 0, !nosanitize
+ // CHECK-NOT: and i64 %{{.*}}, 31
+ // CHECK: ret %struct.S4*
+ return new S4;
+}
+
+S5 *func_15(const S5 *ptr) {
+ // CHECK-LABEL: define {{.*}} @_Z7func_15PK2S5
+ // CHECK: and i64 %{{.*}}, 31, !nosanitize
+ // CHECK: icmp eq i64 %{{.*}}, 0, !nosanitize
+ // CHECK-NOT: and i64
+ // CHECK: ret %struct.S5*
+ return new S5(*ptr);
+}
More information about the cfe-commits
mailing list