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 b83d172f45e3a69081a5f6604f368a573d694616 Mon Sep 17 00:00:00 2001
From: Will Dietz <[email protected]>
Date: Sun, 27 Oct 2013 14:53:17 -0500
Subject: [PATCH] Add 'pointer-overflow' sanitizer for undefined overflow in GEP's.

Handles things like: "char *p = NULL; --p;".
---
 include/clang/Basic/Sanitizers.def |    9 ++--
 lib/CodeGen/CGExpr.cpp             |   94 +++++++++++++++++++++++++++++++++++-
 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    |   39 +++++++++++++++
 test/Driver/fsanitize.c            |    8 ++--
 8 files changed, 169 insertions(+), 15 deletions(-)
 create mode 100644 test/CodeGen/pointer-overflow.c

diff --git a/include/clang/Basic/Sanitizers.def b/include/clang/Basic/Sanitizers.def
index c9b31a3..188e62f 100644
--- a/include/clang/Basic/Sanitizers.def
+++ b/include/clang/Basic/Sanitizers.def
@@ -68,6 +68,7 @@ SANITIZER("function", Function)
 SANITIZER("integer-divide-by-zero", IntegerDivideByZero)
 SANITIZER("null", Null)
 SANITIZER("object-size", ObjectSize)
+SANITIZER("pointer-overflow", PointerOverflow)
 SANITIZER("return", Return)
 SANITIZER("shift", Shift)
 SANITIZER("signed-integer-overflow", SignedIntegerOverflow)
@@ -86,8 +87,8 @@ SANITIZER("dataflow", DataFlow)
 SANITIZER_GROUP("undefined", Undefined,
                 Alignment | Bool | ArrayBounds | Enum | FloatCastOverflow |
                 FloatDivideByZero | Function | IntegerDivideByZero | Null |
-                ObjectSize | Return | Shift | SignedIntegerOverflow |
-                Unreachable | VLABound | Vptr)
+                ObjectSize | PointerOverflow | Return | Shift |
+                SignedIntegerOverflow | Unreachable | VLABound | Vptr)
 
 // -fsanitize=undefined-trap (and its alias -fcatch-undefined-behavior) includes
 // all sanitizers included by -fsanitize=undefined, except those that require
@@ -96,8 +97,8 @@ SANITIZER_GROUP("undefined", Undefined,
 SANITIZER_GROUP("undefined-trap", UndefinedTrap,
                 Alignment | Bool | ArrayBounds | Enum | FloatCastOverflow |
                 FloatDivideByZero | IntegerDivideByZero | Null | ObjectSize |
-                Return | Shift | SignedIntegerOverflow | Unreachable |
-                VLABound)
+                PointerOverflow | Return | Shift | SignedIntegerOverflow |
+                Unreachable | VLABound)
 
 SANITIZER_GROUP("integer", Integer,
                 SignedIntegerOverflow | UnsignedIntegerOverflow | Shift |
diff --git a/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp
index be50f4e..ddf0896 100644
--- a/lib/CodeGen/CGExpr.cpp
+++ b/lib/CodeGen/CGExpr.cpp
@@ -28,6 +28,7 @@
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/GetElementPtrTypeIterator.h"
 
 using namespace clang;
 using namespace CodeGen;
@@ -663,6 +664,90 @@ 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) {
+  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 = llvm::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,
@@ -2285,6 +2370,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];"
@@ -2322,15 +2408,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 9d0f3a9..ebb605e 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.
@@ -384,6 +386,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.
@@ -423,6 +426,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
@@ -457,6 +461,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);
     }
 
@@ -464,6 +469,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");
@@ -484,6 +490,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 a748620..9e21f00 100644
--- a/lib/CodeGen/CGExprCXX.cpp
+++ b/lib/CodeGen/CGExprCXX.cpp
@@ -758,6 +758,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());
 
   unsigned initializerElements = 0;
 
@@ -1475,6 +1476,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
@@ -1523,6 +1525,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 f3a5387..90f85c0 100644
--- a/lib/CodeGen/CGExprScalar.cpp
+++ b/lib/CodeGen/CGExprScalar.cpp
@@ -1639,8 +1639,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()) {
@@ -1649,8 +1651,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.
@@ -1658,8 +1662,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.
@@ -1719,8 +1725,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());
   }
 
@@ -2350,6 +2358,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;
   }
@@ -2366,7 +2375,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 db291e3..cda3bad 100644
--- a/lib/CodeGen/CodeGenFunction.h
+++ b/lib/CodeGen/CodeGenFunction.h
@@ -1667,6 +1667,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..ccbf19a
--- /dev/null
+++ b/test/CodeGen/pointer-overflow.c
@@ -0,0 +1,39 @@
+// 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: @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 2d07923..0a5603b 100644
--- a/test/Driver/fsanitize.c
+++ b/test/Driver/fsanitize.c
@@ -1,19 +1,19 @@
 // RUN: %clang -target x86_64-linux-gnu -fcatch-undefined-behavior %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-TRAP
 // 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),?){14}"}}
+// 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|pointer-overflow),?){15}"}}
 // 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),?){16}"}}
+// 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|pointer-overflow),?){17}"}}
 
 // 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),?){15}"}}
+// 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|pointer-overflow),?){16}"}}
 
 // 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}"}}
 
-// RUN: %clang -target x86_64-linux-gnu -fsanitize=thread,undefined -fno-thread-sanitizer -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-thread-sanitizer -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),?){12}"}}
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address-full %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-FULL
-- 
1.7.1

From e49fefd43f0aaf02b2d51e539e06f7290a61a43d Mon Sep 17 00:00:00 2001
From: Will Dietz <[email protected]>
Date: Sun, 27 Oct 2013 20:19:55 -0500
Subject: [PATCH] Add handler for pointer overflow sanitizer.

---
 .../lit_tests/TestCases/Pointer/index-overflow.cpp |   19 +++++++++++++++++++
 lib/ubsan/ubsan_handlers.cc                        |   19 +++++++++++++++++++
 lib/ubsan/ubsan_handlers.h                         |    6 ++++++
 3 files changed, 44 insertions(+), 0 deletions(-)
 create mode 100644 lib/ubsan/lit_tests/TestCases/Pointer/index-overflow.cpp

diff --git a/lib/ubsan/lit_tests/TestCases/Pointer/index-overflow.cpp b/lib/ubsan/lit_tests/TestCases/Pointer/index-overflow.cpp
new file mode 100644
index 0000000..e0b4c8c
--- /dev/null
+++ b/lib/ubsan/lit_tests/TestCases/Pointer/index-overflow.cpp
@@ -0,0 +1,19 @@
+// 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;
+}
+
diff --git a/lib/ubsan/ubsan_handlers.cc b/lib/ubsan/ubsan_handlers.cc
index d556431..fbaa8c2 100644
--- a/lib/ubsan/ubsan_handlers.cc
+++ b/lib/ubsan/ubsan_handlers.cc
@@ -279,3 +279,22 @@ void __ubsan::__ubsan_handle_function_type_mismatch_abort(
   __ubsan_handle_function_type_mismatch(Data, Function);
   Die();
 }
+
+void __ubsan::__ubsan_handle_pointer_overflow(PointerOverflowData *Data,
+                                              ValueHandle Base,
+                                              ValueHandle Result) {
+  SourceLocation Loc = Data->Loc.acquire();
+  if (Loc.isDisabled())
+    return;
+
+  Diag(Loc, DL_Error,
+       "pointer index expression with base %0 overflowed to %1")
+      << (void *)Base << (void*)Result;
+}
+
+void __ubsan::__ubsan_handle_pointer_overflow_abort(PointerOverflowData *Data,
+                                                    ValueHandle Base,
+                                                    ValueHandle Result) {
+  __ubsan_handle_pointer_overflow(Data, Base, Result);
+  Die();
+}
diff --git a/lib/ubsan/ubsan_handlers.h b/lib/ubsan/ubsan_handlers.h
index 14e6f04..d56bfa7 100644
--- a/lib/ubsan/ubsan_handlers.h
+++ b/lib/ubsan/ubsan_handlers.h
@@ -121,6 +121,12 @@ RECOVERABLE(function_type_mismatch,
             FunctionTypeMismatchData *Data,
             ValueHandle Val)
 
+struct PointerOverflowData {
+  SourceLocation Loc;
+};
+
+RECOVERABLE(pointer_overflow, PointerOverflowData *Data, ValueHandle Base,
+            ValueHandle Result)
 }
 
 #endif // UBSAN_HANDLERS_H
-- 
1.7.1

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to