From 3df3793010cdb7099da1e39ea561da360dca22f4 Mon Sep 17 00:00:00 2001 From: Vedant Kumar Date: Fri, 8 Dec 2017 01:51:47 +0000 Subject: [PATCH] [ubsan] Use pass_object_size info in bounds checks Teach UBSan's bounds check to opportunistically use pass_object_size information to check array accesses. rdar://33272922 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@320128 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/CGExpr.cpp | 54 +++++++++++++++++++++ lib/CodeGen/CodeGenFunction.h | 5 ++ test/CodeGen/ubsan-pass-object-size.c | 69 +++++++++++++++++++++++++++ 3 files changed, 128 insertions(+) create mode 100644 test/CodeGen/ubsan-pass-object-size.c diff --git a/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp index 88116f7d81..64af2096c9 100644 --- a/lib/CodeGen/CGExpr.cpp +++ b/lib/CodeGen/CGExpr.cpp @@ -814,6 +814,53 @@ static bool isFlexibleArrayMemberExpr(const Expr *E) { return false; } +llvm::Value *CodeGenFunction::LoadPassedObjectSize(const Expr *E, + QualType EltTy) { + ASTContext &C = getContext(); + uint64_t EltSize = C.getTypeSizeInChars(EltTy).getQuantity(); + if (!EltSize) + return nullptr; + + auto *ArrayDeclRef = dyn_cast(E->IgnoreParenImpCasts()); + if (!ArrayDeclRef) + return nullptr; + + auto *ParamDecl = dyn_cast(ArrayDeclRef->getDecl()); + if (!ParamDecl) + return nullptr; + + // Arrays don't have pass_object_size attributes, but if they have a constant + // size modifier it's the array size (C99 6.5.7.2p1). + if (auto *DecayedArrayTy = dyn_cast(ParamDecl->getType())) + if (auto *ArrayTy = + dyn_cast(DecayedArrayTy->getOriginalType())) + return llvm::ConstantInt::get(SizeTy, + ArrayTy->getSize().getLimitedValue()); + + auto *POSAttr = ParamDecl->getAttr(); + if (!POSAttr) + return nullptr; + + // Don't load the size if it's a lower bound. + int POSType = POSAttr->getType(); + if (POSType != 0 && POSType != 1) + return nullptr; + + // Find the implicit size parameter. + auto PassedSizeIt = SizeArguments.find(ParamDecl); + if (PassedSizeIt == SizeArguments.end()) + return nullptr; + + const ImplicitParamDecl *PassedSizeDecl = PassedSizeIt->second; + assert(LocalDeclMap.count(PassedSizeDecl) && "Passed size not loadable"); + Address AddrOfSize = LocalDeclMap.find(PassedSizeDecl)->second; + llvm::Value *SizeInBytes = EmitLoadOfScalar(AddrOfSize, /*Volatile=*/false, + C.getSizeType(), E->getExprLoc()); + llvm::Value *SizeOfElement = + llvm::ConstantInt::get(SizeInBytes->getType(), EltSize); + return Builder.CreateUDiv(SizeInBytes, SizeOfElement); +} + /// If Base is known to point to the start of an array, return the length of /// that array. Return 0 if the length cannot be determined. static llvm::Value *getArrayIndexingBound( @@ -835,9 +882,16 @@ static llvm::Value *getArrayIndexingBound( return CGF.Builder.getInt(CAT->getSize()); else if (const auto *VAT = dyn_cast(AT)) return CGF.getVLASize(VAT).first; + // Ignore pass_object_size here. It's not applicable on decayed pointers. } } + QualType EltTy{Base->getType()->getPointeeOrArrayElementType(), 0}; + if (llvm::Value *POS = CGF.LoadPassedObjectSize(Base, EltTy)) { + IndexedType = Base->getType(); + return POS; + } + return nullptr; } diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h index 1c8c2a5e36..1a66533f23 100644 --- a/lib/CodeGen/CodeGenFunction.h +++ b/lib/CodeGen/CodeGenFunction.h @@ -3927,6 +3927,11 @@ public: LValueBaseInfo *BaseInfo = nullptr, TBAAAccessInfo *TBAAInfo = nullptr); + /// If \p E references a parameter with pass_object_size info or a constant + /// array size modifier, emit the object size divided by the size of \p EltTy. + /// Otherwise return null. + llvm::Value *LoadPassedObjectSize(const Expr *E, QualType EltTy); + void EmitSanitizerStatReport(llvm::SanitizerStatKind SSK); private: diff --git a/test/CodeGen/ubsan-pass-object-size.c b/test/CodeGen/ubsan-pass-object-size.c new file mode 100644 index 0000000000..67970d2918 --- /dev/null +++ b/test/CodeGen/ubsan-pass-object-size.c @@ -0,0 +1,69 @@ +// RUN: %clang_cc1 %s -emit-llvm -w -triple x86_64-apple-darwin10 -fsanitize=array-bounds -o - | FileCheck %s + +// CHECK-LABEL: define i32 @foo( +int foo(int *const p __attribute__((pass_object_size(0))), int n) { + int x = (p)[n]; + + // CHECK: [[SIZE_ALLOCA:%.*]] = alloca i64, align 8 + // CHECK: store i64 %{{.*}}, i64* [[SIZE_ALLOCA]], align 8 + // CHECK: [[LOAD_SIZE:%.*]] = load i64, i64* [[SIZE_ALLOCA]], align 8, !nosanitize + // CHECK-NEXT: [[SCALED_SIZE:%.*]] = udiv i64 [[LOAD_SIZE]], 4, !nosanitize + // CHECK-NEXT: [[SEXT_N:%.*]] = sext i32 %{{.*}} to i64, !nosanitize + // CHECK-NEXT: [[ICMP:%.*]] = icmp ult i64 [[SEXT_N]], [[SCALED_SIZE]], !nosanitize + // CHECK-NEXT: br i1 [[ICMP]], {{.*}} !nosanitize + // CHECK: __ubsan_handle_out_of_bounds + + { + int **p = &p; // Shadow the parameter. The pass_object_size info is lost. + // CHECK-NOT: __ubsan_handle_out_of_bounds + x = *p[n]; + } + + // CHECK: ret i32 + return x; +} + +typedef struct {} ZeroSizedType; + +// CHECK-LABEL: define void @bar( +ZeroSizedType bar(ZeroSizedType *const p __attribute__((pass_object_size(0))), int n) { + // CHECK-NOT: __ubsan_handle_out_of_bounds + // CHECK: ret void + return p[n]; +} + +// CHECK-LABEL: define i32 @baz( +int baz(int *const p __attribute__((pass_object_size(1))), int n) { + // CHECK: __ubsan_handle_out_of_bounds + // CHECK: ret i32 + return p[n]; +} + +// CHECK-LABEL: define i32 @mat( +int mat(int *const p __attribute__((pass_object_size(2))), int n) { + // CHECK-NOT: __ubsan_handle_out_of_bounds + // CHECK: ret i32 + return p[n]; +} + +// CHECK-LABEL: define i32 @pat( +int pat(int *const p __attribute__((pass_object_size(3))), int n) { + // CHECK-NOT: __ubsan_handle_out_of_bounds + // CHECK: ret i32 + return p[n]; +} + +// CHECK-LABEL: define i32 @cat( +int cat(int p[static 10], int n) { + // CHECK: icmp ult i64 {{.*}}, 10, !nosanitize + // CHECK: __ubsan_handle_out_of_bounds + // CHECK: ret i32 + return p[n]; +} + +// CHECK-LABEL: define i32 @bat( +int bat(int n, int p[n]) { + // CHECK-NOT: __ubsan_handle_out_of_bounds + // CHECK: ret i32 + return p[n]; +} -- 2.40.0