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 c86b9ed640b33d1b80b122ee8701edbc1bcb9f64 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 | 20 +++++++++++++++++
3 files changed, 57 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..5221b42
--- /dev/null
+++ b/test/ubsan/TestCases/Pointer/index-overflow.cpp
@@ -0,0 +1,20 @@
+// 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.0
From 4c9fe11457422aa88142173af04df2ebfcf2c5c5 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 | 95 +++++++++++++++++++++++++++++++++++++-
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, 207 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 08ee37b..9d122fa 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,91 @@ 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) {
+ SanitizerScope SanScope(this);
+ llvm::GEPOperator *GEP = dyn_cast<llvm::GEPOperator>(GEPExpr);
+ assert(GEP || isa<llvm::Constant>(GEPExpr));
+ if (!SanOpts->PointerOverflow || !GEP)
+ return;
+
+ assert(GEP->isInBounds());
+
+ const llvm::DataLayout &DL = CGM.getDataLayout();
+ llvm::Type *IntPtrTy = DL.getIntPtrType(GEP->getPointerOperandType());
+
+ llvm::Function *SAddIntrinsic =
+ CGM.getIntrinsic(llvm::Intrinsic::sadd_with_overflow, IntPtrTy);
+ llvm::Function *SMulIntrinsic =
+ CGM.getIntrinsic(llvm::Intrinsic::smul_with_overflow, IntPtrTy);
+
+ llvm::Value *TotalOffset = NULL;
+ llvm::Value *Valid = Builder.getTrue();
+
+ // Emit check operations for each GEP operand, building up total signed offset
+ llvm::gep_type_iterator GTI = llvm::gep_type_begin(GEP);
+ for (llvm::GetElementPtrInst::op_iterator OI = std::next(GEP->op_begin()),
+ OE = GEP->op_end();
+ OI != OE; ++OI) {
+ llvm::Value *Index = *OI;
+ llvm::Value *LocalOffset;
+ if (llvm::StructType *STy = dyn_cast<llvm::StructType>(*GTI++)) {
+ unsigned FieldNo = cast<llvm::ConstantInt>(Index)->getZExtValue();
+ LocalOffset = llvm::ConstantInt::get(
+ IntPtrTy, DL.getStructLayout(STy)->getElementOffset(FieldNo));
+ } else {
+ // Otherwise emit a multiply to compute to offset
+ llvm::Constant *ElementSize =
+ llvm::ConstantInt::get(IntPtrTy, DL.getTypeAllocSize(*GTI));
+ llvm::Value *IndexS = Builder.CreateIntCast(Index, IntPtrTy, true);
+ llvm::Value *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
+ llvm::Value *TotalAndOverflow =
+ Builder.CreateCall2(SAddIntrinsic, TotalOffset, LocalOffset);
+ TotalOffset = Builder.CreateExtractValue(TotalAndOverflow, 0);
+ Valid = Builder.CreateAnd(
+ Valid,
+ Builder.CreateNot(Builder.CreateExtractValue(TotalAndOverflow, 1)));
+ }
+
+ // Once computed the (signed) offset as specified by all
+ // the index operands, add it to the unsigned base pointer.
+ llvm::Value *Positive = Builder.CreateICmpSGE(
+ TotalOffset, llvm::ConstantInt::getNullValue(IntPtrTy));
+ llvm::Value *PtrInt =
+ Builder.CreatePtrToInt(GEP->getPointerOperand(), IntPtrTy);
+
+ // Compute result, with wrapping semantics
+ llvm::Value *Result = Builder.CreateAdd(PtrInt, TotalOffset);
+ 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,
@@ -2343,6 +2429,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];"
@@ -2380,15 +2467,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 483966f..85eb694 100644
--- a/lib/CodeGen/CGExprCXX.cpp
+++ b/lib/CodeGen/CGExprCXX.cpp
@@ -935,6 +935,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.
@@ -1556,6 +1557,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
@@ -1604,6 +1606,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 8d8fd39..ae1ccca 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 80a7548..e0509b7 100644
--- a/lib/CodeGen/CodeGenFunction.h
+++ b/lib/CodeGen/CodeGenFunction.h
@@ -1802,6 +1802,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.0
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
