Hi all! Attached are updated copies of the patches, previous ones no longer apply cleanly to ToT. Also cleaned up the clang patch a bit.
Enjoy, feedback/review requested :). ~Will On Tue, Oct 28, 2014 at 7:42 PM, Will Dietz <[email protected]> wrote: > Hi all, > > Attached are updated patches for adding -fsanitize=pointer-overflow. > > Now with quite a bit more thorough testing of various constructs :). > > On my blog I wrote a post detailing a few bugs found with this tool[1][2]. > > At least some developers care: > > * LLVM accepted a patch to ASTVector to fix this behavior[3] > (disclosure: I committed this one, but no one objected O:)) > * ffmpeg fixed reported issue[4] > > I haven't reported other issues yet, probably will do that soon :). > > Anyway, please review and let me know any thoughts you have about this > checker :). > > Enjoy! > > ~Will > > [1] http://wdtz.org/catching-pointer-overflow-bugs.html > [2] http://wdtz.org/undefined-behavior-in-binutils-causes-segfault.html > [3] http://llvm.org/viewvc/llvm-project?view=revision&revision=216385 > [4] https://trac.ffmpeg.org/ticket/3152 > > ~Will > > On Mon, Nov 18, 2013 at 11:13 PM, Will Dietz <[email protected]> wrote: >> Attached are updated patches, please take a look :). >> >> For now not checking struct indexing as doing so caught no additional bugs >> on my test programs and doing so requires a fair bit of plumbing to get >> the SourceLocation down into the struct indexing helpers. >> >> That said, this has been useful in catching bugs in LLVM and >> elsewhere, as previously reported. >> >> Thanks! >> >> ~Will >> >> On Mon, Oct 28, 2013 at 7:56 PM, Will Dietz <[email protected]> wrote: >>> Glad there's some interest. >>> >>> I have no test coverage of anything other than the Driver component, >>> that will be included. >>> I also need to do some plumbing work to support adding checks to >>> struct indexing. >>> >>> I've tried this on: >>> * LLVM/Clang >>> * ImageMagick >>> * binutils >>> * curl >>> * ffmpeg (w/FATE samples) >>> * openldap >>> * openssh >>> * pcre >>> * postgresql >>> * sqlite >>> >>> And the programs seem to build and at least pass their own non-trivial >>> test-suites. >>> >>> So far detected bugs in: >>> * binutils (what inspired this sanitizer) >>> * clang (reported earlier today) >>> * curl (unreported) >>> * pcre (unreported) >>> * ffmpeg (unreported) >>> >>> With a single bug location per software so far :). >>> >>> I also expect this to work particularly well with fuzz testing. >>> >>> ~Will >>> >>> >>> On Mon, Oct 28, 2013 at 5:44 PM, Richard Smith <[email protected]> >>> wrote: >>>> Seems like a nice idea to me. (Your test coverage is pretty weak, though.) >>>> Have you tried this much on large codebases? Does this find many bugs? (I >>>> can imagine it would be effective when combined with fuzz testing...) >>>> >>>> >>>> On Mon, Oct 28, 2013 at 3:39 PM, Will Dietz <[email protected]> wrote: >>>>> >>>>> Hi all, >>>>> >>>>> Recently I thought it would be useful to have a sanitizer for >>>>> detecting overflows in pointer expressions. Such overflows are >>>>> undefined behavior and are pretty much always bugs. While it's true >>>>> that if such an overflowed pointer is dereferenced a tool such as ASan >>>>> will catch the error, detection of these bugs when the occur helps fix >>>>> them without requiring an input that triggers a crash. >>>>> >>>>> Two examples of this in the wild: >>>>> >>>>> * binutils undefined behavior bug that leads to segfault when built >>>>> with clang[1] >>>>> * ASTVector bug I just submitted patch for, discovered using this >>>>> sanitizer[2] >>>>> >>>>> Attached are patches for clang and compiler-rt that implement this >>>>> sanitizer and seem to work well in my testing so far. >>>>> >>>>> There is some work to do yet: >>>>> >>>>> * Adding lit tests to clang/compiler-rt >>>>> * Finalizing what constructs are useful/worth checking (iterative >>>>> process, I imagine) >>>>> * More testing/benchmarking >>>>> >>>>> Before tackling the above, I was hoping to get some early feedback: >>>>> >>>>> * Is this something the community is interested in/would find useful? >>>>> * Code review (the current implementation should be complete in terms >>>>> of the checking code itself) >>>>> >>>>> Thank you for your time, here's to finding even more bugs! :) >>>>> >>>>> ~Will >>>>> >>>>> [1] http://wdtz.org/undefined-behavior-in-binutils-causes-segfault.html >>>>> [2] >>>>> http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20131028/091878.html >>>>> >>>>> _______________________________________________ >>>>> cfe-commits mailing list >>>>> [email protected] >>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>>>> >>>>
From 146e68b827d0581cb1fbb5000ae6ce9d5feeb65c Mon Sep 17 00:00:00 2001 From: Will Dietz <[email protected]> Date: Tue, 28 Oct 2014 21:45:44 +0000 Subject: [PATCH] Add handler for printing pointer overflow diagnostics. --- lib/ubsan/ubsan_handlers.cc | 30 +++++++++++++++++++++++++ lib/ubsan/ubsan_handlers.h | 7 ++++++ test/ubsan/TestCases/Pointer/index-overflow.cpp | 18 +++++++++++++++ 3 files changed, 55 insertions(+) create mode 100644 test/ubsan/TestCases/Pointer/index-overflow.cpp diff --git a/lib/ubsan/ubsan_handlers.cc b/lib/ubsan/ubsan_handlers.cc index a0ecff9..04b10ef 100644 --- a/lib/ubsan/ubsan_handlers.cc +++ b/lib/ubsan/ubsan_handlers.cc @@ -410,3 +410,33 @@ void __ubsan::__ubsan_handle_nonnull_arg_abort(NonNullArgData *Data) { handleNonNullArg(Data, Opts); Die(); } + +static void handlePointerOverflowImpl(PointerOverflowData *Data, + ValueHandle Base, + ValueHandle Result, + ReportOptions Opts) { + SourceLocation Loc = Data->Loc.acquire(); + if (ignoreReport(Loc, Opts)) + return; + + ScopedReport R(Opts, Loc); + + Diag(Loc, DL_Error, + "pointer index expression with base %0 overflowed to %1") + << (void *)Base << (void*)Result; +} + +void __ubsan::__ubsan_handle_pointer_overflow(PointerOverflowData *Data, + ValueHandle Base, + ValueHandle Result) { + GET_REPORT_OPTIONS(false); + handlePointerOverflowImpl(Data, Base, Result, Opts); +} + +void __ubsan::__ubsan_handle_pointer_overflow_abort(PointerOverflowData *Data, + ValueHandle Base, + ValueHandle Result) { + GET_REPORT_OPTIONS(true); + handlePointerOverflowImpl(Data, Base, Result, Opts); + Die(); +} diff --git a/lib/ubsan/ubsan_handlers.h b/lib/ubsan/ubsan_handlers.h index 87149f2..ddd4d03 100644 --- a/lib/ubsan/ubsan_handlers.h +++ b/lib/ubsan/ubsan_handlers.h @@ -140,6 +140,13 @@ struct NonNullArgData { /// \brief Handle passing null pointer to function with nonnull attribute. RECOVERABLE(nonnull_arg, NonNullArgData *Data) +struct PointerOverflowData { + SourceLocation Loc; +}; + +RECOVERABLE(pointer_overflow, PointerOverflowData *Data, ValueHandle Base, + ValueHandle Result) + } #endif // UBSAN_HANDLERS_H diff --git a/test/ubsan/TestCases/Pointer/index-overflow.cpp b/test/ubsan/TestCases/Pointer/index-overflow.cpp new file mode 100644 index 0000000..0a764ee --- /dev/null +++ b/test/ubsan/TestCases/Pointer/index-overflow.cpp @@ -0,0 +1,18 @@ +// RUN: %clangxx -fsanitize=pointer-overflow %s -o %t +// RUN: %t 0 2>&1 | FileCheck %s --check-prefix=SAFE +// RUN: %t 1 2>&1 | FileCheck %s --check-prefix=ERR +// RUN: %t -1 2>&1 | FileCheck %s --check-prefix=SAFE + + +#include <stdint.h> +#include <stdio.h> +#include <stdlib.h> + +int main(int argc, char *argv[]) { + // SAFE-NOT: runtime error + // ERR: runtime error: pointer index expression with base {{.*}} overflowed to + uintptr_t p = (uintptr_t)argv[0] + atoi(argv[1]); + printf("%p\n", argv[0] - p); + + return 0; +} -- 2.1.3
From ebe3179fd93bb19f2d136faaff94c3eb6199ad99 Mon Sep 17 00:00:00 2001 From: Will Dietz <[email protected]> Date: Tue, 28 Oct 2014 20:59:28 +0000 Subject: [PATCH] Add 'pointer-overflow' sanitizer for undefined overflow in GEP's. Handles things like: char *p = NULL; --p; --- include/clang/Basic/Sanitizers.def | 12 +++-- lib/CodeGen/CGExpr.cpp | 102 ++++++++++++++++++++++++++++++++++++- lib/CodeGen/CGExprAgg.cpp | 7 +++ lib/CodeGen/CGExprCXX.cpp | 3 ++ lib/CodeGen/CGExprScalar.cpp | 21 ++++++-- lib/CodeGen/CodeGenFunction.h | 3 ++ test/CodeGen/pointer-overflow.c | 74 +++++++++++++++++++++++++++ test/Driver/fsanitize.c | 8 +-- 8 files changed, 214 insertions(+), 16 deletions(-) create mode 100644 test/CodeGen/pointer-overflow.c diff --git a/include/clang/Basic/Sanitizers.def b/include/clang/Basic/Sanitizers.def index adb9b04..a89aefc 100644 --- a/include/clang/Basic/Sanitizers.def +++ b/include/clang/Basic/Sanitizers.def @@ -62,6 +62,7 @@ SANITIZER("integer-divide-by-zero", IntegerDivideByZero) SANITIZER("nonnull-attribute", NonnullAttribute) SANITIZER("null", Null) SANITIZER("object-size", ObjectSize) +SANITIZER("pointer-overflow", PointerOverflow) SANITIZER("return", Return) SANITIZER("returns-nonnull-attribute", ReturnsNonnullAttribute) SANITIZER("shift", Shift) @@ -81,9 +82,9 @@ SANITIZER("dataflow", DataFlow) SANITIZER_GROUP("undefined", Undefined, Alignment | Bool | ArrayBounds | Enum | FloatCastOverflow | FloatDivideByZero | Function | IntegerDivideByZero | - NonnullAttribute | Null | ObjectSize | Return | - ReturnsNonnullAttribute | Shift | SignedIntegerOverflow | - Unreachable | VLABound | Vptr) + NonnullAttribute | Null | ObjectSize | PointerOverflow | + Return | ReturnsNonnullAttribute | Shift | + SignedIntegerOverflow | Unreachable | VLABound | Vptr) // -fsanitize=undefined-trap includes // all sanitizers included by -fsanitize=undefined, except those that require @@ -92,8 +93,9 @@ SANITIZER_GROUP("undefined", Undefined, SANITIZER_GROUP("undefined-trap", UndefinedTrap, Alignment | Bool | ArrayBounds | Enum | FloatCastOverflow | FloatDivideByZero | IntegerDivideByZero | NonnullAttribute | - Null | ObjectSize | Return | ReturnsNonnullAttribute | - Shift | SignedIntegerOverflow | Unreachable | VLABound) + Null | ObjectSize | PointerOverflow | Return | + ReturnsNonnullAttribute | Shift | SignedIntegerOverflow | + Unreachable | VLABound) SANITIZER_GROUP("integer", Integer, SignedIntegerOverflow | UnsignedIntegerOverflow | Shift | diff --git a/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp index 3fae6b8..dd99155 100644 --- a/lib/CodeGen/CGExpr.cpp +++ b/lib/CodeGen/CGExpr.cpp @@ -26,6 +26,7 @@ #include "llvm/ADT/Hashing.h" #include "llvm/ADT/StringExtras.h" #include "llvm/IR/DataLayout.h" +#include "llvm/IR/GetElementPtrTypeIterator.h" #include "llvm/IR/Intrinsics.h" #include "llvm/IR/LLVMContext.h" #include "llvm/IR/MDBuilder.h" @@ -696,6 +697,98 @@ void CodeGenFunction::EmitBoundsCheck(const Expr *E, const Expr *Base, EmitCheck(Check, "out_of_bounds", StaticData, Index, CRK_Recoverable); } +void CodeGenFunction::EmitInBoundsGEPOverflowCheck(llvm::Value *GEPExpr, + SourceLocation Loc) { + // If the pointer overflow sanitizer isn't enabled, do nothing. + if (!SanOpts.PointerOverflow) + return; + + // If the GEP has already been reduced to a constant, leave it be. + if (isa<llvm::Constant>(GEPExpr)) + return; + + auto *GEP = cast<llvm::GEPOperator>(GEPExpr); + assert(GEP->isInBounds()); + + SanitizerScope SanScope(this); + const auto &DL = CGM.getDataLayout(); + auto *IntPtrTy = DL.getIntPtrType(GEP->getPointerOperandType()); + + // Grab references to the signed add/mult overflow intrinsics for intptr_t: + auto *SAddIntrinsic = + CGM.getIntrinsic(llvm::Intrinsic::sadd_with_overflow, IntPtrTy); + auto *SMulIntrinsic = + CGM.getIntrinsic(llvm::Intrinsic::smul_with_overflow, IntPtrTy); + + // TotalOffset - Sum of offsets for GEP operands visited so far. + llvm::Value *TotalOffset = NULL; + // Valid - True iff final result did not overflow. + llvm::Value *Valid = Builder.getTrue(); + + // Emit check operations for each GEP operand, building up total signed offset + for (auto GTI = llvm::gep_type_begin(GEP), GTE = llvm::gep_type_end(GEP); + GTI != GTE; ++GTI) { + auto *Index = GTI.getOperand(); + llvm::Value *LocalOffset; + // Compute the local offset contributed by this indexing step: + if (auto *STy = dyn_cast<llvm::StructType>(*GTI)) { + // For struct indexing, local offset is position of specified field. + unsigned FieldNo = cast<llvm::ConstantInt>(Index)->getZExtValue(); + LocalOffset = llvm::ConstantInt::get( + IntPtrTy, DL.getStructLayout(STy)->getElementOffset(FieldNo)); + } else { + // Otherwise this is array-like indexing and requires a checked multiply. + auto *ElementSize = llvm::ConstantInt::get( + IntPtrTy, DL.getTypeAllocSize(GTI.getIndexedType())); + auto *IndexS = Builder.CreateIntCast(Index, IntPtrTy, true); + auto *ResultAndOverflow = + Builder.CreateCall2(SMulIntrinsic, ElementSize, IndexS); + LocalOffset = Builder.CreateExtractValue(ResultAndOverflow, 0); + Valid = Builder.CreateAnd( + Valid, + Builder.CreateNot(Builder.CreateExtractValue(ResultAndOverflow, 1))); + } + + // If this is first offset, use it as total offset. + if (!TotalOffset) { + TotalOffset = LocalOffset; + continue; + } + + // Otherwise add LocalOffset to running total in TotalOffset + auto *TotalAndOverflow = + Builder.CreateCall2(SAddIntrinsic, TotalOffset, LocalOffset); + TotalOffset = Builder.CreateExtractValue(TotalAndOverflow, 0); + Valid = Builder.CreateAnd( + Valid, + Builder.CreateNot(Builder.CreateExtractValue(TotalAndOverflow, 1))); + } + + // Now that we've computed the (signed) offset as specified + // by all the GEP operands, add it to the unsigned base pointer. + auto *Positive = Builder.CreateICmpSGE( + TotalOffset, llvm::ConstantInt::getNullValue(IntPtrTy)); + auto *PtrInt = Builder.CreatePtrToInt(GEP->getPointerOperand(), IntPtrTy); + + // Compute result, with wrapping semantics + auto *Result = Builder.CreateAdd(PtrInt, TotalOffset); + + // Validity depends on whether the TotalOffset was positive or negative + llvm::Value *PosValid = Builder.CreateICmpUGE(Result, PtrInt); + llvm::Value *NegValid = Builder.CreateICmpULT(Result, PtrInt); + Valid = Builder.CreateAnd(Valid, + Builder.CreateSelect(Positive, PosValid, NegValid)); + + llvm::Constant *StaticArgs[] = { + EmitCheckSourceLocation(Loc) + }; + llvm::Value *DynamicArgs[] = { + EmitCheckValue(GEP->getPointerOperand()), + EmitCheckValue(GEP) + }; + EmitCheck(Valid, "pointer_overflow", StaticArgs, DynamicArgs, + CRK_Recoverable); +} CodeGenFunction::ComplexPairTy CodeGenFunction:: EmitComplexPrePostIncDec(const UnaryOperator *E, LValue LV, @@ -2342,6 +2435,7 @@ LValue CodeGenFunction::EmitArraySubscriptExpr(const ArraySubscriptExpr *E, } else { Idx = Builder.CreateNSWMul(Idx, numElements); Address = Builder.CreateInBoundsGEP(Address, Idx, "arrayidx"); + EmitInBoundsGEPOverflowCheck(Address, E->getExprLoc()); } } else if (const ObjCObjectType *OIT = E->getType()->getAs<ObjCObjectType>()){ // Indexing over an interface, as in "NSString *P; P[4];" @@ -2379,15 +2473,19 @@ LValue CodeGenFunction::EmitArraySubscriptExpr(const ArraySubscriptExpr *E, if (getLangOpts().isSignedOverflowDefined()) Address = Builder.CreateGEP(ArrayPtr, Args, "arrayidx"); - else + else { Address = Builder.CreateInBoundsGEP(ArrayPtr, Args, "arrayidx"); + EmitInBoundsGEPOverflowCheck(Address, E->getExprLoc()); + } } else { // The base must be a pointer, which is not an aggregate. Emit it. llvm::Value *Base = EmitScalarExpr(E->getBase()); if (getLangOpts().isSignedOverflowDefined()) Address = Builder.CreateGEP(Base, Idx, "arrayidx"); - else + else { Address = Builder.CreateInBoundsGEP(Base, Idx, "arrayidx"); + EmitInBoundsGEPOverflowCheck(Address, E->getExprLoc()); + } } QualType T = E->getBase()->getType()->getPointeeType(); diff --git a/lib/CodeGen/CGExprAgg.cpp b/lib/CodeGen/CGExprAgg.cpp index 4cf94c0..1abda1f 100644 --- a/lib/CodeGen/CGExprAgg.cpp +++ b/lib/CodeGen/CGExprAgg.cpp @@ -343,6 +343,7 @@ AggExprEmitter::VisitCXXStdInitializerListExpr(CXXStdInitializerListExpr *E) { llvm::Value *IdxStart[] = { Zero, Zero }; llvm::Value *ArrayStart = Builder.CreateInBoundsGEP(ArrayPtr, IdxStart, "arraystart"); + CGF.EmitInBoundsGEPOverflowCheck(ArrayStart, E->getExprLoc()); CGF.EmitStoreThroughLValue(RValue::get(ArrayStart), Start); ++Field; @@ -360,6 +361,7 @@ AggExprEmitter::VisitCXXStdInitializerListExpr(CXXStdInitializerListExpr *E) { llvm::Value *IdxEnd[] = { Zero, Size }; llvm::Value *ArrayEnd = Builder.CreateInBoundsGEP(ArrayPtr, IdxEnd, "arrayend"); + CGF.EmitInBoundsGEPOverflowCheck(ArrayEnd, E->getExprLoc()); CGF.EmitStoreThroughLValue(RValue::get(ArrayEnd), EndOrLength); } else if (Ctx.hasSameType(Field->getType(), Ctx.getSizeType())) { // Length. @@ -407,6 +409,7 @@ void AggExprEmitter::EmitArrayInit(llvm::Value *DestPtr, llvm::ArrayType *AType, llvm::Value *indices[] = { zero, zero }; llvm::Value *begin = Builder.CreateInBoundsGEP(DestPtr, indices, "arrayinit.begin"); + CGF.EmitInBoundsGEPOverflowCheck(begin, E->getExprLoc()); // Exception safety requires us to destroy all the // already-constructed members if an initializer throws. @@ -446,6 +449,7 @@ void AggExprEmitter::EmitArrayInit(llvm::Value *DestPtr, llvm::ArrayType *AType, // Advance to the next element. if (i > 0) { element = Builder.CreateInBoundsGEP(element, one, "arrayinit.element"); + CGF.EmitInBoundsGEPOverflowCheck(element, E->getExprLoc()); // Tell the cleanup that it needs to destroy up to this // element. TODO: some of these stores can be trivially @@ -474,6 +478,7 @@ void AggExprEmitter::EmitArrayInit(llvm::Value *DestPtr, llvm::ArrayType *AType, // Advance to the start of the rest of the array. if (NumInitElements) { element = Builder.CreateInBoundsGEP(element, one, "arrayinit.start"); + CGF.EmitInBoundsGEPOverflowCheck(element, E->getExprLoc()); if (endOfInit) Builder.CreateStore(element, endOfInit); } @@ -481,6 +486,7 @@ void AggExprEmitter::EmitArrayInit(llvm::Value *DestPtr, llvm::ArrayType *AType, llvm::Value *end = Builder.CreateInBoundsGEP(begin, llvm::ConstantInt::get(CGF.SizeTy, NumArrayElements), "arrayinit.end"); + CGF.EmitInBoundsGEPOverflowCheck(end, E->getExprLoc()); llvm::BasicBlock *entryBB = Builder.GetInsertBlock(); llvm::BasicBlock *bodyBB = CGF.createBasicBlock("arrayinit.body"); @@ -501,6 +507,7 @@ void AggExprEmitter::EmitArrayInit(llvm::Value *DestPtr, llvm::ArrayType *AType, // Move on to the next element. llvm::Value *nextElement = Builder.CreateInBoundsGEP(currentElement, one, "arrayinit.next"); + CGF.EmitInBoundsGEPOverflowCheck(nextElement, E->getExprLoc()); // Tell the EH cleanup that we finished with the last element. if (endOfInit) Builder.CreateStore(nextElement, endOfInit); diff --git a/lib/CodeGen/CGExprCXX.cpp b/lib/CodeGen/CGExprCXX.cpp index 3d1283f..f132eee 100644 --- a/lib/CodeGen/CGExprCXX.cpp +++ b/lib/CodeGen/CGExprCXX.cpp @@ -956,6 +956,7 @@ CodeGenFunction::EmitNewArrayInitializer(const CXXNewExpr *E, // Find the end of the array, hoisted out of the loop. llvm::Value *EndPtr = Builder.CreateInBoundsGEP(BeginPtr, NumElements, "array.end"); + EmitInBoundsGEPOverflowCheck(EndPtr, E->getExprLoc()); // If the number of elements isn't constant, we have to now check if there is // anything left to initialize. @@ -1563,6 +1564,7 @@ static void EmitArrayDelete(CodeGenFunction &CGF, llvm::Value *arrayEnd = CGF.Builder.CreateInBoundsGEP(deletedPtr, numElements, "delete.end"); + CGF.EmitInBoundsGEPOverflowCheck(arrayEnd, E->getExprLoc()); // Note that it is legal to allocate a zero-length array, and we // can never fold the check away because the length should always @@ -1611,6 +1613,7 @@ void CodeGenFunction::EmitCXXDeleteExpr(const CXXDeleteExpr *E) { } Ptr = Builder.CreateInBoundsGEP(Ptr, GEP, "del.first"); + EmitInBoundsGEPOverflowCheck(Ptr, E->getExprLoc()); } assert(ConvertTypeForMem(DeleteTy) == diff --git a/lib/CodeGen/CGExprScalar.cpp b/lib/CodeGen/CGExprScalar.cpp index 02ecdbd..577859e 100644 --- a/lib/CodeGen/CGExprScalar.cpp +++ b/lib/CodeGen/CGExprScalar.cpp @@ -1694,8 +1694,10 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV, if (!isInc) numElts = Builder.CreateNSWNeg(numElts, "vla.negsize"); if (CGF.getLangOpts().isSignedOverflowDefined()) value = Builder.CreateGEP(value, numElts, "vla.inc"); - else + else { value = Builder.CreateInBoundsGEP(value, numElts, "vla.inc"); + CGF.EmitInBoundsGEPOverflowCheck(value, E->getExprLoc()); + } // Arithmetic on function pointers (!) is just +-1. } else if (type->isFunctionType()) { @@ -1704,8 +1706,10 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV, value = CGF.EmitCastToVoidPtr(value); if (CGF.getLangOpts().isSignedOverflowDefined()) value = Builder.CreateGEP(value, amt, "incdec.funcptr"); - else + else { value = Builder.CreateInBoundsGEP(value, amt, "incdec.funcptr"); + CGF.EmitInBoundsGEPOverflowCheck(value, E->getExprLoc()); + } value = Builder.CreateBitCast(value, input->getType()); // For everything else, we can just do a simple increment. @@ -1713,8 +1717,10 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV, llvm::Value *amt = Builder.getInt32(amount); if (CGF.getLangOpts().isSignedOverflowDefined()) value = Builder.CreateGEP(value, amt, "incdec.ptr"); - else + else { value = Builder.CreateInBoundsGEP(value, amt, "incdec.ptr"); + CGF.EmitInBoundsGEPOverflowCheck(value, E->getExprLoc()); + } } // Vector increment/decrement. @@ -1778,8 +1784,10 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV, if (CGF.getLangOpts().isSignedOverflowDefined()) value = Builder.CreateGEP(value, sizeValue, "incdec.objptr"); - else + else { value = Builder.CreateInBoundsGEP(value, sizeValue, "incdec.objptr"); + CGF.EmitInBoundsGEPOverflowCheck(value, E->getExprLoc()); + } value = Builder.CreateBitCast(value, input->getType()); } @@ -2418,6 +2426,7 @@ static Value *emitPointerArithmetic(CodeGenFunction &CGF, } else { index = CGF.Builder.CreateNSWMul(index, numElements, "vla.index"); pointer = CGF.Builder.CreateInBoundsGEP(pointer, index, "add.ptr"); + CGF.EmitInBoundsGEPOverflowCheck(pointer, expr->getExprLoc()); } return pointer; } @@ -2434,7 +2443,9 @@ static Value *emitPointerArithmetic(CodeGenFunction &CGF, if (CGF.getLangOpts().isSignedOverflowDefined()) return CGF.Builder.CreateGEP(pointer, index, "add.ptr"); - return CGF.Builder.CreateInBoundsGEP(pointer, index, "add.ptr"); + Value *GEP = CGF.Builder.CreateInBoundsGEP(pointer, index, "add.ptr"); + CGF.EmitInBoundsGEPOverflowCheck(GEP, expr->getExprLoc()); + return GEP; } // Construct an fmuladd intrinsic to represent a fused mul-add of MulOp and diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h index a683e88..7f5ad14 100644 --- a/lib/CodeGen/CodeGenFunction.h +++ b/lib/CodeGen/CodeGenFunction.h @@ -1805,6 +1805,9 @@ public: void EmitBoundsCheck(const Expr *E, const Expr *Base, llvm::Value *Index, QualType IndexType, bool Accessed); + /// \brief Emit overflow checks for inbounds GEP's if enabled. + void EmitInBoundsGEPOverflowCheck(llvm::Value *GEP, SourceLocation Loc); + llvm::Value *EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV, bool isInc, bool isPre); ComplexPairTy EmitComplexPrePostIncDec(const UnaryOperator *E, LValue LV, diff --git a/test/CodeGen/pointer-overflow.c b/test/CodeGen/pointer-overflow.c new file mode 100644 index 0000000..897ffd9 --- /dev/null +++ b/test/CodeGen/pointer-overflow.c @@ -0,0 +1,74 @@ +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsanitize=pointer-overflow %s -emit-llvm -o - -O0 | FileCheck %s + +// CHECK-LABEL: @inc( +char *inc(char *p) { + // CHECK: ubsan_handle_pointer_overflow + return p + 1; +} + +// CHECK-LABEL: @dec( +char *dec(char *p) { + // CHECK: ubsan_handle_pointer_overflow + return p - 1; +} + +// CHECK-LABEL: @predec( +char *predec(char *p) { + // CHECK: ubsan_handle_pointer_overflow + return --p; +} + +// CHECK-LABEL: @preinc( +char *preinc(char *p) { + // CHECK: ubsan_handle_pointer_overflow + return ++p; +} + +// CHECK-LABEL: @array( +char *array(char p[10][10], int a, int b) { + // CHECK: ubsan_handle_pointer_overflow + return &p[a][b]; +} + +// CHECK-LABEL: @array_vla( +int array_vla(int n, int a) { + char p[n]; + // CHECK: ubsan_handle_pointer_overflow + char *q = &p[0]; + return n; +} + +// CHECK-LABEL: @array_vla2 +int array_vla2(int n, int a) { + char p[n][n]; + // CHECK: ubsan_handle_pointer_overflow + char (*q)[n] = &p[a]; + + return n; +} + +// CHECK-LABEL: @vla_inc( +int vla_inc(int n) { + char p[n]; + // CHECK: ubsan_handle_pointer_overflow + p[0] = 5; + char (*q)[n] = &p; + // CHECK: ubsan_handle_pointer_overflow + ++q; +} + +typedef void (*FP)(void); + +// CHECK-LABEL: @fp_inc( +FP fp_inc(FP fp, int a) { + // CHECK: ubsan_handle_pointer_overflow + return ++fp; +} + +// CHECK-LABEL: @noop_index( +char *noop_index(char *p) { + // This will be optimized out, + // but would be nice to avoid emitting it. + // CHECK: ubsan_handle_pointer_overflow + return &p[0]; +} diff --git a/test/Driver/fsanitize.c b/test/Driver/fsanitize.c index 8fec80d..13b9361 100644 --- a/test/Driver/fsanitize.c +++ b/test/Driver/fsanitize.c @@ -1,13 +1,13 @@ // RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined-trap -fsanitize-undefined-trap-on-error %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-TRAP // RUN: %clang -target x86_64-linux-gnu -fsanitize-undefined-trap-on-error -fsanitize=undefined-trap %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-TRAP -// CHECK-UNDEFINED-TRAP: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|shift|unreachable|return|vla-bound|alignment|null|object-size|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute),?){16}"}} +// CHECK-UNDEFINED-TRAP: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|shift|unreachable|return|vla-bound|alignment|null|object-size|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute|pointer-overflow),?){17}"}} // CHECK-UNDEFINED-TRAP: "-fsanitize-undefined-trap-on-error" // RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED -// CHECK-UNDEFINED: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|function|shift|unreachable|return|vla-bound|alignment|null|vptr|object-size|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute),?){18}"}} +// CHECK-UNDEFINED: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|function|shift|unreachable|return|vla-bound|alignment|null|vptr|object-size|pointer-overflow|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute|pointer-overflow),?){19}"}} // RUN: %clang -target x86_64-apple-darwin10 -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-DARWIN -// CHECK-UNDEFINED-DARWIN: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|shift|unreachable|return|vla-bound|alignment|null|vptr|object-size|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute),?){17}"}} +// CHECK-UNDEFINED-DARWIN: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|shift|unreachable|return|vla-bound|alignment|null|vptr|object-size|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute|pointer-overflow),?){18}"}} // RUN: %clang -target x86_64-linux-gnu -fsanitize=integer %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-INTEGER // CHECK-INTEGER: "-fsanitize={{((signed-integer-overflow|unsigned-integer-overflow|integer-divide-by-zero|shift),?){4}"}} @@ -15,7 +15,7 @@ // RUN: %clang -fsanitize=bounds -### -fsyntax-only %s 2>&1 | FileCheck %s --check-prefix=CHECK-BOUNDS // CHECK-BOUNDS: "-fsanitize={{((array-bounds|local-bounds),?){2}"}} -// RUN: %clang -target x86_64-linux-gnu -fsanitize=thread,undefined -fno-sanitize=thread -fno-sanitize=float-cast-overflow,vptr,bool,enum %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-PARTIAL-UNDEFINED +// RUN: %clang -target x86_64-linux-gnu -fsanitize=thread,undefined -fno-sanitize=thread -fno-sanitize=float-cast-overflow,vptr,bool,enum,pointer-overflow %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-PARTIAL-UNDEFINED // CHECK-PARTIAL-UNDEFINED: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|function|shift|unreachable|return|vla-bound|alignment|null|object-size|array-bounds|returns-nonnull-attribute|nonnull-attribute),?){14}"}} // RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined -fsanitize-undefined-trap-on-error %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-TRAP-ON-ERROR-UNDEF -- 2.1.3
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
