[ubsan] add pointer overflow checking

[regehr]

Summary

This is a patch by Will Dietz that I dusted off and would like to try to get accepted. It teaches UBSan about overflowing pointers. Will wrote a blog post about using it:

https://wdtz.org/catching-pointer-overflow-bugs.html

There is a corresponding compiler-rt patch that I'll submit next.

Not really sure who to add as reviewers for this, suggestions welcome.

[filcab]

For EmitInBoundsGEPOverflowCheck, since it's easier to have clang build the GEP and then add the verifications, it might be better to create an IRBuilder that inserts before the GEP instructions and use that builder so your verification comes before the GEP calculation (and the UB).

lib/CodeGen/CGExpr.cpp

2798

Nit: Since you're maintaining the naming convention that is already there, you could just call this addr too. Otherwise, seems out of place.

2799

Shouldn't you check before you create the pointer?

3150

Shouldn't we be looking at the sanitizer's PointerOverflow?

3254

Since you always used the IntPtrTy s{add,mul}_with_overflow operations, won't this always be true, if Valid is true by this time?

lib/CodeGen/CodeGenFunction.h

1974

No need for \brief anymore.

[filcab]

lib/CodeGen/CGExpr.cpp

3254

Derp, you can add or subtract stuff. Nevermind.

[dtzWill]

This comment was removed by dtzWill.

[dtzWill]

lib/CodeGen/CGExpr.cpp

3150

No, at least I don't think so.

PointerOverflow is checked in EmitInBoundsGEPOverflowCheck-- what's going on here is also what is going on elsewhere-- we want to only call EmitInBoundsGEPOverflowCheck on GEP's that are 'inbounds'. Here that means checking isSignedOverflowDefined().

That said, I think we may be emitting GEP checks twice on this path, will fix if so.

[regehr]

updated patch from Will:

• fix pointer-overflow.c
• only check for overflows in the default address space
• avoid a duplicate overflow check
• address reviewer feedback about variable names
• address reviewer feedback: remove \brief

[regehr]

I should add a note about testing:

• I built SPEC CINT 2006 with pointer overflow checking, it runs normally (at 5% slowdown) and two overflows in 400.perlbench were found

• FFmpeg builds normally and dies during testing with an overflow

• PHP builds normally and a couple of overflows during testing

• I haven't yet bootstrapped LLVM with pointer checking but will do that next, if I don't report back here then you can assume this worked and nothing interesting was found

[filcab]

I still think there's a problem here:
The check is meant to protect against creating invalid pointers via GEP. Which means that this should be done before the GEP instruction, not after. Since the GEP instruction is the one that would trigger UB.
With this patch, you're creating the pointer, and then checking it. The way I see it, you should be using something like (in EmitInBoundsGEPOverflowCheck):

auto Builder = IRBuilder(GEPExpr); // Shadowing CGF.Builder

Maybe there are better constructors of IRBuilder to use, but this one seems appropriate.
I'm not the most well-versed person in building IR, so I'd like to have Richard Smith (or another person knowing a bit more about this) double check.

Also add a CHECK-NOT line for the getelementptr intructions before your CHECK for function calls, please.
The tests are mostly testing that we're "most likely" instrumenting the proper expressions. Please add at least one test which actually checks the instructions added and the math performed.

[tss]

Is this patch missing something? I applied this patch to a recent clang git, but when compiling a test program I get:

test.c:(.text+0xb0): undefined reference to `__ubsan_handle_pointer_overflow'

According to the linked web page, it should print errors such as:

./test.c:7:19: runtime error: pointer index expression with base 0x7fffffffd3cb overflowed to 0xffffffffffffffff

But I don't see any such text in this patch.

[rsmith]

In D20322#600582, @tss wrote:

Is this patch missing something? I applied this patch to a recent clang git, but when compiling a test program I get:

You also need to apply the patch https://reviews.llvm.org/D20323 to compiler-rt.

lib/CodeGen/CGExpr.cpp

2799

I'd suggest:

• Instead of adding an Emit...Check call after creating each InBounds GEP, factor out a function that does both (EmitCheckedInBoundsGEP maybe?) (This would match how we deal with other similar cases such as EmitCheckedLValue.)
• Either pass your just-computed value to the callback rather than the GEP value, or drop the 'inbounds' flag from the GEP instruction when you're going to emit a check for it. (In the former case, it shouldn't matter which order the GEP and check are emitted in; the value of the GEP is not used by the check, so we don't get UB at the IR level.)

As things stand, LLVM is permitted to delete the checks on the basis that the argument to the runtime library call would always be a poison value.

3150

@dtzWill, did you ever get round to checking for repeated GEPs on this path?

[regehr]

I had some deadlines to get through, but now I'll get back to updating this patch, hopefully in a few days!

[filcab]

Just changing the state so it doesn't show up on the reviewers' TODO lists on phab.

[vsk]

Hi @dtzWill, I have some time to work on this, and would be happy to implement the changes suggested by @rsmith. Let me know if it's all right for me to take a stab at this.

[regehr]

In D20322#750381, @vsk wrote:

Hi @dtzWill, I have some time to work on this, and would be happy to implement the changes suggested by @rsmith. Let me know if it's all right for me to take a stab at this.

I"m not @dtzWill but I'm guessing he'll be happy for you to do this (same with me)!

[dtzWill]

In D20322#750438, @regehr wrote:

In D20322#750381, @vsk wrote:

Hi @dtzWill, I have some time to work on this, and would be happy to implement the changes suggested by @rsmith. Let me know if it's all right for me to take a stab at this.

I"m not @dtzWill but I'm guessing he'll be happy for you to do this (same with me)!

Yes, that would be great! Let me know if there are any problems or questions :).

[sberg]

This is apparently superseded by https://reviews.llvm.org/D33305 "[ubsan] Add a check for pointer overflow UB" (and should probably be abandoned or whatever the appropriate Phabricator state?).

include/clang/Basic/Sanitizers.def

 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-base", ShiftBase)

 SANITIZER_GROUP("undefined", Undefined,
                 Alignment | Bool | ArrayBounds | Enum | FloatCastOverflow |
                     FloatDivideByZero | IntegerDivideByZero | NonnullAttribute |
-                    Null | ObjectSize | Return | ReturnsNonnullAttribute |
-                    Shift | SignedIntegerOverflow | Unreachable | VLABound |
-                    Function | Vptr)
+                    Null | ObjectSize | PointerOverflow | Return |
+                    ReturnsNonnullAttribute | Shift | SignedIntegerOverflow |
+                    Unreachable | VLABound | Function | Vptr)
 
 // -fsanitize=undefined-trap is an alias for -fsanitize=undefined.
 SANITIZER_GROUP("undefined-trap", UndefinedTrap, Undefined)

lib/CodeGen/CGExpr.cpp

 #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"

                                           llvm::Value *ptr,
                                           ArrayRef<llvm::Value*> indices,
                                           bool inbounds,
+                                          SourceLocation loc,
                                     const llvm::Twine &name = "arrayidx") {
   if (inbounds) {
-    return CGF.Builder.CreateInBoundsGEP(ptr, indices, name);
+    auto *Addr = CGF.Builder.CreateInBoundsGEP(ptr, indices, name);
+    CGF.EmitInBoundsGEPOverflowCheck(Addr, loc);
+    return Addr;
   } else {
     return CGF.Builder.CreateGEP(ptr, indices, name);
   }

 static Address emitArraySubscriptGEP(CodeGenFunction &CGF, Address addr,
                                      ArrayRef<llvm::Value*> indices,
                                      QualType eltType, bool inbounds,
+                                     SourceLocation loc,
                                      const llvm::Twine &name = "arrayidx") {
   // All the indices except that last must be zero.
 #ifndef NDEBUG

     getArrayElementAlign(addr.getAlignment(), indices.back(), eltSize);
 
   llvm::Value *eltPtr =
-    emitArraySubscriptGEP(CGF, addr.getPointer(), indices, inbounds, name);
+    emitArraySubscriptGEP(CGF, addr.getPointer(), indices, inbounds, loc, name);
   return Address(eltPtr, eltAlign);
 }
 

     Address Addr = EmitExtVectorElementLValue(LV);
 
     QualType EltType = LV.getType()->castAs<VectorType>()->getElementType();
-    Addr = emitArraySubscriptGEP(*this, Addr, Idx, EltType, /*inbounds*/ true);
+    Addr = emitArraySubscriptGEP(*this, Addr, Idx, EltType, /*inbounds*/ true,
+                                 E->getExprLoc());
     return MakeAddrLValue(Addr, EltType, LV.getAlignmentSource());
   }
 

     }
 
     Addr = emitArraySubscriptGEP(*this, Addr, Idx, vla->getElementType(),
-                                 !getLangOpts().isSignedOverflowDefined());
+                                 !getLangOpts().isSignedOverflowDefined(),
+                                 E->getExprLoc());
 
   } else if (const ObjCObjectType *OIT = E->getType()->getAs<ObjCObjectType>()){
     // Indexing over an interface, as in "NSString *P; P[4];"

     CharUnits EltAlign =
       getArrayElementAlign(Addr.getAlignment(), Idx, InterfaceSize);
     llvm::Value *EltPtr =
-      emitArraySubscriptGEP(*this, Addr.getPointer(), ScaledIdx, false);
+      emitArraySubscriptGEP(*this, Addr.getPointer(), ScaledIdx, false,
+                            E->getExprLoc());
     Addr = Address(EltPtr, EltAlign);
 
     // Cast back.

     Addr = emitArraySubscriptGEP(*this, ArrayLV.getAddress(),
                                  {CGM.getSize(CharUnits::Zero()), Idx},
                                  E->getType(),
-                                 !getLangOpts().isSignedOverflowDefined());
+                                 !getLangOpts().isSignedOverflowDefined(),
+                                 E->getExprLoc());
     AlignSource = ArrayLV.getAlignmentSource();
   } else {
     // The base must be a pointer; emit it with an estimate of its alignment.
     Addr = EmitPointerWithAlignment(E->getBase(), &AlignSource);
     Addr = emitArraySubscriptGEP(*this, Addr, Idx, E->getType(),
-                                 !getLangOpts().isSignedOverflowDefined());
+                                 !getLangOpts().isSignedOverflowDefined(),
+                                 E->getExprLoc());
   }
 
   LValue LV = MakeAddrLValue(Addr, E->getType(), AlignSource);

     else
       Idx = Builder.CreateNSWMul(Idx, NumElements);
     EltPtr = emitArraySubscriptGEP(*this, Base, Idx, VLA->getElementType(),
-                                   !getLangOpts().isSignedOverflowDefined());
+                                   !getLangOpts().isSignedOverflowDefined(),
+                                   E->getExprLoc());
+    if (!getLangOpts().isSignedOverflowDefined())
+      EmitInBoundsGEPOverflowCheck(EltPtr.getPointer(), E->getExprLoc());
   } else if (const Expr *Array = isSimpleArrayDecayOperand(E->getBase())) {
     // If this is A[i] where A is an array, the frontend will have decayed the
     // base to be a ArrayToPointerDecay implicit cast.  While correct, it is

     // Propagate the alignment from the array itself to the result.
     EltPtr = emitArraySubscriptGEP(
         *this, ArrayLV.getAddress(), {CGM.getSize(CharUnits::Zero()), Idx},
-        ResultExprTy, !getLangOpts().isSignedOverflowDefined());
+        ResultExprTy, !getLangOpts().isSignedOverflowDefined(),
+        E->getExprLoc());
     AlignSource = ArrayLV.getAlignmentSource();
   } else {
     Address Base = emitOMPArraySectionBase(*this, E->getBase(), AlignSource,
                                            BaseTy, ResultExprTy, IsLowerBound);
     EltPtr = emitArraySubscriptGEP(*this, Base, Idx, ResultExprTy,
-                                   !getLangOpts().isSignedOverflowDefined());
+                                   !getLangOpts().isSignedOverflowDefined(),
+                                   E->getExprLoc());
   }
 
   return MakeAddrLValue(EltPtr, ResultExprTy, AlignSource);
 }
 
+void CodeGenFunction::EmitInBoundsGEPOverflowCheck(llvm::Value *GEPExpr,
+                                                   SourceLocation Loc) {
+  // If the pointer overflow sanitizer isn't enabled, do nothing.
+  if (!SanOpts.has(SanitizerKind::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.CreateCall(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.CreateCall(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(std::make_pair(Valid, SanitizerKind::PointerOverflow),
+            "pointer_overflow", StaticArgs, DynamicArgs);
+}
+
 LValue CodeGenFunction::
 EmitExtVectorElementExpr(const ExtVectorElementExpr *E) {
   // Emit the base vector as an l-value.

lib/CodeGen/CGExprAgg.cpp

   llvm::Value *IdxStart[] = { Zero, Zero };
   llvm::Value *ArrayStart =
       Builder.CreateInBoundsGEP(ArrayPtr.getPointer(), IdxStart, "arraystart");
+  CGF.EmitInBoundsGEPOverflowCheck(ArrayStart, E->getExprLoc());
   CGF.EmitStoreThroughLValue(RValue::get(ArrayStart), Start);
   ++Field;
 

     llvm::Value *IdxEnd[] = { Zero, Size };
     llvm::Value *ArrayEnd =
         Builder.CreateInBoundsGEP(ArrayPtr.getPointer(), IdxEnd, "arrayend");
+    CGF.EmitInBoundsGEPOverflowCheck(ArrayEnd, E->getExprLoc());
     CGF.EmitStoreThroughLValue(RValue::get(ArrayEnd), EndOrLength);
   } else if (Ctx.hasSameType(Field->getType(), Ctx.getSizeType())) {
     // Length.

   llvm::Value *indices[] = { zero, zero };
   llvm::Value *begin =
     Builder.CreateInBoundsGEP(DestPtr.getPointer(), indices, "arrayinit.begin");
+  CGF.EmitInBoundsGEPOverflowCheck(begin, E->getExprLoc());
 
   CharUnits elementSize = CGF.getContext().getTypeSizeInChars(elementType);
   CharUnits elementAlign =

     // 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

     // 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.isValid()) Builder.CreateStore(element, endOfInit);
     }
 

     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");

     // 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.isValid()) Builder.CreateStore(nextElement, endOfInit);

lib/CodeGen/CGExprCXX.cpp

   // Find the end of the array, hoisted out of the loop.
   llvm::Value *EndPtr =
     Builder.CreateInBoundsGEP(BeginPtr.getPointer(), 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.

     llvm::Value *arrayBegin = deletedPtr.getPointer();
     llvm::Value *arrayEnd =
       CGF.Builder.CreateInBoundsGEP(arrayBegin, 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

       GEP.push_back(Zero);
     }
 
-    Ptr = Address(Builder.CreateInBoundsGEP(Ptr.getPointer(), GEP, "del.first"),
-                  Ptr.getAlignment());
+    auto Addr = Builder.CreateInBoundsGEP(Ptr.getPointer(), GEP, "del.first");
+    Ptr = Address(Addr, Ptr.getAlignment());
+    EmitInBoundsGEPOverflowCheck(Addr, E->getExprLoc());
   }
 
   assert(ConvertTypeForMem(DeleteTy) == Ptr.getElementType());

lib/CodeGen/CGExprScalar.cpp

           = CGF.getContext().getAsVariableArrayType(type)) {
       llvm::Value *numElts = CGF.getVLASize(vla).first;
       if (!isInc) numElts = Builder.CreateNSWNeg(numElts, "vla.negsize");
-      if (CGF.getLangOpts().isSignedOverflowDefined())
+      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()) {
       llvm::Value *amt = Builder.getInt32(amount);
 
       value = CGF.EmitCastToVoidPtr(value);
-      if (CGF.getLangOpts().isSignedOverflowDefined())
+      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.
     } else {
       llvm::Value *amt = Builder.getInt32(amount);
-      if (CGF.getLangOpts().isSignedOverflowDefined())
+      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.

     llvm::Value *sizeValue =
       llvm::ConstantInt::get(CGF.SizeTy, size.getQuantity());
 
-    if (CGF.getLangOpts().isSignedOverflowDefined())
+    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());
   }
 

     } else {
       index = CGF.Builder.CreateNSWMul(index, numElements, "vla.index");
       pointer = CGF.Builder.CreateInBoundsGEP(pointer, index, "add.ptr");
+      CGF.EmitInBoundsGEPOverflowCheck(pointer, expr->getExprLoc());
     }
     return pointer;
   }

   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

lib/CodeGen/CodeGenFunction.h

   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,

test/CodeGen/pointer-overflow.c

+// 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];
+}

test/Driver/fsanitize.c

 // RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined -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 -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-base|shift-exponent|unreachable|return|vla-bound|alignment|null|object-size|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute|function),?){18}"}}
-// CHECK-UNDEFINED-TRAP: "-fsanitize-trap=alignment,array-bounds,bool,enum,float-cast-overflow,float-divide-by-zero,function,integer-divide-by-zero,nonnull-attribute,null,object-size,return,returns-nonnull-attribute,shift-base,shift-exponent,signed-integer-overflow,unreachable,vla-bound"
-// CHECK-UNDEFINED-TRAP2: "-fsanitize-trap=alignment,array-bounds,bool,enum,float-cast-overflow,float-divide-by-zero,function,integer-divide-by-zero,nonnull-attribute,null,object-size,return,returns-nonnull-attribute,shift-base,shift-exponent,unreachable,vla-bound"
+// CHECK-UNDEFINED-TRAP: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|shift-base|shift-exponent|unreachable|return|vla-bound|alignment|null|object-size|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute|function|pointer-overflow),?){19}"}}
+// CHECK-UNDEFINED-TRAP: "-fsanitize-trap=alignment,array-bounds,bool,enum,float-cast-overflow,float-divide-by-zero,function,integer-divide-by-zero,nonnull-attribute,null,object-size,pointer-overflow,return,returns-nonnull-attribute,shift-base,shift-exponent,signed-integer-overflow,unreachable,vla-bound"
+// CHECK-UNDEFINED-TRAP2: "-fsanitize-trap=alignment,array-bounds,bool,enum,float-cast-overflow,float-divide-by-zero,function,integer-divide-by-zero,nonnull-attribute,null,object-size,pointer-overflow,return,returns-nonnull-attribute,shift-base,shift-exponent,unreachable,vla-bound"
 
 // 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-base|shift-exponent|unreachable|return|vla-bound|alignment|null|vptr|object-size|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute),?){19}"}}
+// CHECK-UNDEFINED: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|function|shift-base|shift-exponent|unreachable|return|vla-bound|alignment|null|vptr|object-size|pointer-overflow|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute),?){20}"}}
 
 // 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-base|shift-exponent|unreachable|return|vla-bound|alignment|null|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-base|shift-exponent|unreachable|return|vla-bound|alignment|null|object-size|pointer-overflow|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute),?){18}"}}
 
 // RUN: %clang -target i386-unknown-openbsd -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-OPENBSD
-// CHECK-UNDEFINED-OPENBSD: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|shift-base|shift-exponent|unreachable|return|vla-bound|alignment|null|object-size|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute),?){17}"}}
+// CHECK-UNDEFINED-OPENBSD: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|shift-base|shift-exponent|unreachable|return|vla-bound|alignment|null|object-size|pointer-overflow|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute),?){18}"}}
 
 // RUN: %clang -target i386-pc-win32 -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-WIN --check-prefix=CHECK-UNDEFINED-WIN32
 // RUN: %clang -target i386-pc-win32 -fsanitize=undefined -x c++ %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-WIN --check-prefix=CHECK-UNDEFINED-WIN32 --check-prefix=CHECK-UNDEFINED-WIN-CXX
 // RUN: %clang -target x86_64-pc-win32 -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-WIN --check-prefix=CHECK-UNDEFINED-WIN64
 // RUN: %clang -target x86_64-pc-win32 -fsanitize=undefined -x c++ %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-WIN --check-prefix=CHECK-UNDEFINED-WIN64 --check-prefix=CHECK-UNDEFINED-WIN-CXX
-// CHECK-UNDEFINED-WIN: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|shift-base|shift-exponent|unreachable|return|vla-bound|alignment|null|object-size|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute),?){17}"}}
+// CHECK-UNDEFINED-WIN: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|shift-base|shift-exponent|unreachable|return|vla-bound|alignment|null|object-size|pointer-overflow|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute),?){18}"}}
 // CHECK-UNDEFINED-WIN32-SAME: "--dependent-lib={{[^"]*}}ubsan_standalone-i386.lib"
 // CHECK-UNDEFINED-WIN64-SAME: "--dependent-lib={{[^"]*}}ubsan_standalone-x86_64.lib"
 // CHECK-UNDEFINED-WIN-CXX-SAME: "--dependent-lib={{[^"]*}}ubsan_standalone_cxx{{[^"]*}}.lib"

 // CHECK-FNO-SANITIZE-ALL: "-fsanitize=thread"
 
 // 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
-// CHECK-PARTIAL-UNDEFINED: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|function|shift-base|shift-exponent|unreachable|return|vla-bound|alignment|null|object-size|array-bounds|returns-nonnull-attribute|nonnull-attribute),?){15}"}}
+// CHECK-PARTIAL-UNDEFINED: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|function|shift-base|shift-exponent|unreachable|return|vla-bound|alignment|null|object-size|pointer-overflow|array-bounds|returns-nonnull-attribute|nonnull-attribute),?){16}"}}
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=shift -fno-sanitize=shift-base %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-FSANITIZE-SHIFT-PARTIAL
 // CHECK-FSANITIZE-SHIFT-PARTIAL: "-fsanitize=shift-exponent"

 // RUN: %clang -target x86_64-linux-gnu %s -fsanitize=undefined -fno-sanitize-recover=undefined -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-RECOVER-UBSAN
 // RUN: %clang -target x86_64-linux-gnu %s -fsanitize=undefined -fno-sanitize-recover=all -fsanitize-recover=thread -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-RECOVER-UBSAN
 // RUN: %clang -target x86_64-linux-gnu %s -fsanitize=undefined -fsanitize-recover=all -fno-sanitize-recover=undefined -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-RECOVER-UBSAN
-// CHECK-RECOVER-UBSAN: "-fsanitize-recover={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|function|shift-base|shift-exponent|vla-bound|alignment|null|vptr|object-size|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute),?){17}"}}
+// CHECK-RECOVER-UBSAN: "-fsanitize-recover={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|function|shift-base|shift-exponent|vla-bound|alignment|null|vptr|object-size|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute|pointer-overflow),?){18}"}}
 // CHECK-NO-RECOVER-UBSAN-NOT: sanitize-recover
 
 // RUN: %clang -target x86_64-linux-gnu %s -fsanitize=undefined -fno-sanitize-recover=all -fsanitize-recover=object-size,shift-base -### 2>&1 | FileCheck %s --check-prefix=CHECK-PARTIAL-RECOVER