This revision was automatically updated to reflect the committed changes.
Closed by commit rL338199: [UBSan] Strengthen pointer checks in 'new'
expressions (authored by sepavloff, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D49589
Files:
cfe/trunk/lib/CodeGen/CGClass.cpp
cfe/trunk/lib/CodeGen/CGExprCXX.cpp
cfe/trunk/lib/CodeGen/CGValue.h
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/test/CodeGenCXX/ubsan-new-checks.cpp
Index: cfe/trunk/test/CodeGenCXX/ubsan-new-checks.cpp
===================================================================
--- cfe/trunk/test/CodeGenCXX/ubsan-new-checks.cpp
+++ cfe/trunk/test/CodeGenCXX/ubsan-new-checks.cpp
@@ -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);
+}
Index: cfe/trunk/lib/CodeGen/CGExprCXX.cpp
===================================================================
--- cfe/trunk/lib/CodeGen/CGExprCXX.cpp
+++ cfe/trunk/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()) {
Index: cfe/trunk/lib/CodeGen/CGValue.h
===================================================================
--- cfe/trunk/lib/CodeGen/CGValue.h
+++ cfe/trunk/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,17 +533,19 @@
AV.ZeroedFlag = isZeroed;
AV.AliasedFlag = isAliased;
AV.OverlapFlag = mayOverlap;
+ AV.SanitizerCheckedFlag = isChecked;
return AV;
}
static AggValueSlot forLValue(const LValue &LV,
IsDestructed_t isDestructed,
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();
Index: cfe/trunk/lib/CodeGen/CGClass.cpp
===================================================================
--- cfe/trunk/lib/CodeGen/CGClass.cpp
+++ cfe/trunk/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);
Index: cfe/trunk/lib/CodeGen/CodeGenFunction.h
===================================================================
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h
+++ cfe/trunk/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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
