Index: lib/CodeGen/CGExpr.cpp =================================================================== --- lib/CodeGen/CGExpr.cpp +++ lib/CodeGen/CGExpr.cpp @@ -3006,7 +3006,8 @@ SourceLocation loc, const llvm::Twine &name = "arrayidx") { if (inbounds) { - return CGF.EmitCheckedInBoundsGEP(ptr, indices, signedIndices, loc, name); + return CGF.EmitCheckedInBoundsGEP(ptr, indices, signedIndices, + /*IsSubtraction=*/false, loc, name); } else { return CGF.Builder.CreateGEP(ptr, indices, name); } Index: lib/CodeGen/CGExprScalar.cpp =================================================================== --- lib/CodeGen/CGExprScalar.cpp +++ lib/CodeGen/CGExprScalar.cpp @@ -1851,7 +1851,6 @@ llvm::Value *input; int amount = (isInc ? 1 : -1); - bool signedIndex = !isInc; if (const AtomicType *atomicTy = type->getAs()) { type = atomicTy->getValueType(); @@ -1941,8 +1940,9 @@ if (CGF.getLangOpts().isSignedOverflowDefined()) value = Builder.CreateGEP(value, numElts, "vla.inc"); else - value = CGF.EmitCheckedInBoundsGEP(value, numElts, signedIndex, - E->getExprLoc(), "vla.inc"); + value = CGF.EmitCheckedInBoundsGEP( + value, numElts, /*SignedIndices=*/false, /*IsSubtraction=*/!isInc, + E->getExprLoc(), "vla.inc"); // Arithmetic on function pointers (!) is just +-1. } else if (type->isFunctionType()) { @@ -1952,7 +1952,8 @@ if (CGF.getLangOpts().isSignedOverflowDefined()) value = Builder.CreateGEP(value, amt, "incdec.funcptr"); else - value = CGF.EmitCheckedInBoundsGEP(value, amt, signedIndex, + value = CGF.EmitCheckedInBoundsGEP(value, amt, /*SignedIndices=*/false, + /*IsSubtraction=*/!isInc, E->getExprLoc(), "incdec.funcptr"); value = Builder.CreateBitCast(value, input->getType()); @@ -1962,7 +1963,8 @@ if (CGF.getLangOpts().isSignedOverflowDefined()) value = Builder.CreateGEP(value, amt, "incdec.ptr"); else - value = CGF.EmitCheckedInBoundsGEP(value, amt, signedIndex, + value = CGF.EmitCheckedInBoundsGEP(value, amt, /*SignedIndices=*/false, + /*IsSubtraction=*/!isInc, E->getExprLoc(), "incdec.ptr"); } @@ -2044,8 +2046,9 @@ if (CGF.getLangOpts().isSignedOverflowDefined()) value = Builder.CreateGEP(value, sizeValue, "incdec.objptr"); else - value = CGF.EmitCheckedInBoundsGEP(value, sizeValue, signedIndex, - E->getExprLoc(), "incdec.objptr"); + value = CGF.EmitCheckedInBoundsGEP( + value, sizeValue, /*SignedIndices=*/false, /*IsSubtraction=*/!isInc, + E->getExprLoc(), "incdec.objptr"); value = Builder.CreateBitCast(value, input->getType()); } @@ -2663,7 +2666,6 @@ } bool isSigned = indexOperand->getType()->isSignedIntegerOrEnumerationType(); - bool mayHaveNegativeGEPIndex = isSigned || isSubtraction; unsigned width = cast(index->getType())->getBitWidth(); auto &DL = CGF.CGM.getDataLayout(); @@ -2715,7 +2717,7 @@ } else { index = CGF.Builder.CreateNSWMul(index, numElements, "vla.index"); pointer = - CGF.EmitCheckedInBoundsGEP(pointer, index, mayHaveNegativeGEPIndex, + CGF.EmitCheckedInBoundsGEP(pointer, index, isSigned, isSubtraction, op.E->getExprLoc(), "add.ptr"); } return pointer; @@ -2733,7 +2735,7 @@ if (CGF.getLangOpts().isSignedOverflowDefined()) return CGF.Builder.CreateGEP(pointer, index, "add.ptr"); - return CGF.EmitCheckedInBoundsGEP(pointer, index, mayHaveNegativeGEPIndex, + return CGF.EmitCheckedInBoundsGEP(pointer, index, isSigned, isSubtraction, op.E->getExprLoc(), "add.ptr"); } @@ -3853,6 +3855,7 @@ Value *CodeGenFunction::EmitCheckedInBoundsGEP(Value *Ptr, ArrayRef IdxList, bool SignedIndices, + bool IsSubtraction, SourceLocation Loc, const Twine &Name) { Value *GEPVal = Builder.CreateInBoundsGEP(Ptr, IdxList, Name); @@ -3958,15 +3961,19 @@ // pointer matches the sign of the total offset. llvm::Value *ValidGEP; auto *NoOffsetOverflow = Builder.CreateNot(OffsetOverflows); - auto *PosOrZeroValid = Builder.CreateICmpUGE(ComputedGEP, IntPtr); if (SignedIndices) { + auto *PosOrZeroValid = Builder.CreateICmpUGE(ComputedGEP, IntPtr); auto *PosOrZeroOffset = Builder.CreateICmpSGE(TotalOffset, Zero); llvm::Value *NegValid = Builder.CreateICmpULT(ComputedGEP, IntPtr); ValidGEP = Builder.CreateAnd( Builder.CreateSelect(PosOrZeroOffset, PosOrZeroValid, NegValid), NoOffsetOverflow); - } else { + } else if (!SignedIndices && !IsSubtraction) { + auto *PosOrZeroValid = Builder.CreateICmpUGE(ComputedGEP, IntPtr); ValidGEP = Builder.CreateAnd(PosOrZeroValid, NoOffsetOverflow); + } else { + auto *NegOrZeroValid = Builder.CreateICmpULE(ComputedGEP, IntPtr); + ValidGEP = Builder.CreateAnd(NegOrZeroValid, NoOffsetOverflow); } llvm::Constant *StaticArgs[] = {EmitCheckSourceLocation(Loc)}; Index: lib/CodeGen/CodeGenFunction.h =================================================================== --- lib/CodeGen/CodeGenFunction.h +++ lib/CodeGen/CodeGenFunction.h @@ -3555,9 +3555,12 @@ /// Same as IRBuilder::CreateInBoundsGEP, but additionally emits a check to /// detect undefined behavior when the pointer overflow sanitizer is enabled. /// \p SignedIndices indicates whether any of the GEP indices are signed. + /// \p IsSubtraction indicates whether the expression used to form the GEP + /// is a subtraction. llvm::Value *EmitCheckedInBoundsGEP(llvm::Value *Ptr, ArrayRef IdxList, bool SignedIndices, + bool IsSubtraction, SourceLocation Loc, const Twine &Name = ""); Index: test/CodeGen/ubsan-pointer-overflow.m =================================================================== --- test/CodeGen/ubsan-pointer-overflow.m +++ test/CodeGen/ubsan-pointer-overflow.m @@ -10,16 +10,20 @@ ++p; // CHECK: ptrtoint i8* {{.*}} to i64, !nosanitize - // CHECK-NEXT: add i64 {{.*}}, -1, !nosanitize - // CHECK: select i1 false{{.*}}, !nosanitize + // CHECK-NEXT: [[COMPGEP:%.*]] = add i64 {{.*}}, -1, !nosanitize + // CHECK: [[NEGVALID:%.*]] = icmp ule i64 [[COMPGEP]], {{.*}}, !nosanitize + // CHECK-NOT: select + // CHECK: br i1 [[NEGVALID]]{{.*}}, !nosanitize // CHECK: call void @__ubsan_handle_pointer_overflow{{.*}} --p; + // CHECK: icmp uge i64 // CHECK-NOT: select // CHECK: call void @__ubsan_handle_pointer_overflow{{.*}} p++; - // CHECK: select + // CHECK: icmp ule i64 + // CHECK-NOT: select // CHECK: call void @__ubsan_handle_pointer_overflow{{.*}} p--; } @@ -64,7 +68,8 @@ // CHECK: [[OFFSET:%.*]] = sub i64 0, {{.*}} // CHECK-NEXT: getelementptr inbounds {{.*}} [[OFFSET]] - // CHECK: select + // CHECK: icmp ule i64 + // CHECK-NOT: select // CHECK: call void @__ubsan_handle_pointer_overflow{{.*}} p - i; }