r351924 - [ubsan] Check the correct size when sanitizing array new.
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 22 19:37:29 PST 2019
Author: rsmith
Date: Tue Jan 22 19:37:29 2019
New Revision: 351924
URL: http://llvm.org/viewvc/llvm-project?rev=351924&view=rev
Log:
[ubsan] Check the correct size when sanitizing array new.
We previously forgot to multiply the element size by the array bound.
Modified:
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CGExprCXX.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp
Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=351924&r1=351923&r2=351924&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Tue Jan 22 19:37:29 2019
@@ -652,7 +652,8 @@ bool CodeGenFunction::sanitizePerformTyp
void CodeGenFunction::EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc,
llvm::Value *Ptr, QualType Ty,
CharUnits Alignment,
- SanitizerSet SkippedChecks) {
+ SanitizerSet SkippedChecks,
+ llvm::Value *ArraySize) {
if (!sanitizePerformTypeCheck())
return;
@@ -710,21 +711,27 @@ void CodeGenFunction::EmitTypeCheck(Type
if (SanOpts.has(SanitizerKind::ObjectSize) &&
!SkippedChecks.has(SanitizerKind::ObjectSize) &&
!Ty->isIncompleteType()) {
- uint64_t Size = getContext().getTypeSizeInChars(Ty).getQuantity();
-
- // The glvalue must refer to a large enough storage region.
- // FIXME: If Address Sanitizer is enabled, insert dynamic instrumentation
- // to check this.
- // FIXME: Get object address space
- llvm::Type *Tys[2] = { IntPtrTy, Int8PtrTy };
- llvm::Value *F = CGM.getIntrinsic(llvm::Intrinsic::objectsize, Tys);
- llvm::Value *Min = Builder.getFalse();
- llvm::Value *NullIsUnknown = Builder.getFalse();
- llvm::Value *CastAddr = Builder.CreateBitCast(Ptr, Int8PtrTy);
- llvm::Value *LargeEnough = Builder.CreateICmpUGE(
- Builder.CreateCall(F, {CastAddr, Min, NullIsUnknown}),
- llvm::ConstantInt::get(IntPtrTy, Size));
- Checks.push_back(std::make_pair(LargeEnough, SanitizerKind::ObjectSize));
+ uint64_t TySize = getContext().getTypeSizeInChars(Ty).getQuantity();
+ llvm::Value *Size = llvm::ConstantInt::get(IntPtrTy, TySize);
+ if (ArraySize)
+ Size = Builder.CreateMul(Size, ArraySize);
+
+ // Degenerate case: new X[0] does not need an objectsize check.
+ llvm::Constant *ConstantSize = dyn_cast<llvm::Constant>(Size);
+ if (!ConstantSize || !ConstantSize->isNullValue()) {
+ // The glvalue must refer to a large enough storage region.
+ // FIXME: If Address Sanitizer is enabled, insert dynamic instrumentation
+ // to check this.
+ // FIXME: Get object address space
+ llvm::Type *Tys[2] = { IntPtrTy, Int8PtrTy };
+ llvm::Value *F = CGM.getIntrinsic(llvm::Intrinsic::objectsize, Tys);
+ llvm::Value *Min = Builder.getFalse();
+ llvm::Value *NullIsUnknown = Builder.getFalse();
+ llvm::Value *CastAddr = Builder.CreateBitCast(Ptr, Int8PtrTy);
+ llvm::Value *LargeEnough = Builder.CreateICmpUGE(
+ Builder.CreateCall(F, {CastAddr, Min, NullIsUnknown}), Size);
+ Checks.push_back(std::make_pair(LargeEnough, SanitizerKind::ObjectSize));
+ }
}
uint64_t AlignVal = 0;
Modified: cfe/trunk/lib/CodeGen/CGExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprCXX.cpp?rev=351924&r1=351923&r2=351924&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGExprCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp Tue Jan 22 19:37:29 2019
@@ -1714,10 +1714,16 @@ llvm::Value *CodeGenFunction::EmitCXXNew
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.
+ // array it was checked only once and not at each constructor call. We may
+ // have already checked that the pointer is non-null.
+ // FIXME: If we have an array cookie and a potentially-throwing allocator,
+ // we'll null check the wrong pointer here.
+ SanitizerSet SkippedChecks;
+ SkippedChecks.set(SanitizerKind::Null, nullCheck);
EmitTypeCheck(CodeGenFunction::TCK_ConstructorCall,
- E->getAllocatedTypeSourceInfo()->getTypeLoc().getBeginLoc(),
- result.getPointer(), allocType);
+ E->getAllocatedTypeSourceInfo()->getTypeLoc().getBeginLoc(),
+ result.getPointer(), allocType, result.getAlignment(),
+ SkippedChecks, numElements);
EmitNewInitializer(*this, E, allocType, elementTy, result, numElements,
allocSizeWithoutCookie);
Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=351924&r1=351923&r2=351924&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Tue Jan 22 19:37:29 2019
@@ -2620,10 +2620,12 @@ public:
bool sanitizePerformTypeCheck() const;
/// Emit a check that \p V is the address of storage of the
- /// appropriate size and alignment for an object of type \p Type.
+ /// appropriate size and alignment for an object of type \p Type
+ /// (or if ArraySize is provided, for an array of that bound).
void EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc, llvm::Value *V,
QualType Type, CharUnits Alignment = CharUnits::Zero(),
- SanitizerSet SkippedChecks = SanitizerSet());
+ SanitizerSet SkippedChecks = SanitizerSet(),
+ llvm::Value *ArraySize = nullptr);
/// Emit a check that \p Base points into an array object, which
/// we can access at index \p Index. \p Accessed should be \c false if we
Modified: cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp?rev=351924&r1=351923&r2=351924&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp Tue Jan 22 19:37:29 2019
@@ -533,6 +533,7 @@ namespace NothrowNew {
// CHECK: [[nonnull]]:
// CHECK: llvm.objectsize
+ // CHECK: icmp uge i64 {{.*}}, 123456,
// CHECK: br i1
//
// CHECK: call {{.*}}__ubsan_handle_type_mismatch
@@ -550,6 +551,7 @@ namespace NothrowNew {
// CHECK: [[nonnull]]:
// CHECK: llvm.objectsize
+ // CHECK: icmp uge i64 {{.*}}, 123456,
// CHECK: br i1
//
// CHECK: call {{.*}}__ubsan_handle_type_mismatch
@@ -561,6 +563,47 @@ namespace NothrowNew {
// CHECK: ret
return new (nothrow{}) X[123456];
}
+
+ // CHECK-LABEL: define{{.*}}throwing_new
+ void *throwing_new(int size) {
+ // CHECK: icmp ne i8*{{.*}}, null
+ // CHECK: %[[size:.*]] = mul
+ // CHECK: llvm.objectsize
+ // CHECK: icmp uge i64 {{.*}}, %[[size]],
+ // CHECK: %[[ok:.*]] = and
+ // CHECK: br i1 %[[ok]], label %[[good:.*]], label %[[bad:[^,]*]]
+ //
+ // CHECK: [[bad]]:
+ // CHECK: call {{.*}}__ubsan_handle_type_mismatch
+ //
+ // CHECK: [[good]]:
+ // CHECK-NOT: {{ }}br{{ }}
+ // CHECK: ret
+ return new char[size];
+ }
+
+ // CHECK-LABEL: define{{.*}}nothrow_new_zero_size
+ void *nothrow_new_zero_size() {
+ // CHECK: %[[nonnull:.*]] = icmp ne i8*{{.*}}, null
+ // CHECK-NOT: llvm.objectsize
+ // CHECK: br i1 %[[nonnull]], label %[[good:.*]], label %[[bad:[^,]*]]
+ //
+ // CHECK: [[bad]]:
+ // CHECK: call {{.*}}__ubsan_handle_type_mismatch
+ //
+ // CHECK: [[good]]:
+ // CHECK-NOT: {{ }}br{{ }}
+ // CHECK: ret
+ return new char[0];
+ }
+
+ // CHECK-LABEL: define{{.*}}throwing_new_zero_size
+ void *throwing_new_zero_size() {
+ // Nothing to check here.
+ // CHECK-NOT: __ubsan_handle_type_mismatch
+ return new (nothrow{}) char[0];
+ // CHECK: ret
+ }
}
struct ThisAlign {
More information about the cfe-commits
mailing list