Skip to content

Commit

Permalink
Revert "[InstCombine] Check inbounds in load/store of gep null transf…
Browse files Browse the repository at this point in the history
…orm (PR48577)"

This reverts commit 899faa5.

Upon further consideration, this does not fix the right issue.
Doing this fold for non-inbounds GEPs is legal, because the
resulting pointer is still based-on null, which has no associated
address range, and as such and access to it is UB.

https://bugs.llvm.org/show_bug.cgi?id=48577#c3
  • Loading branch information
nikic committed Dec 24, 2020
1 parent e075123 commit ef2f843
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -908,16 +908,15 @@ static bool canSimplifyNullStoreOrGEP(StoreInst &SI) {

auto *Ptr = SI.getPointerOperand();
if (GetElementPtrInst *GEPI = dyn_cast<GetElementPtrInst>(Ptr))
if (GEPI->isInBounds())
Ptr = GEPI->getOperand(0);
Ptr = GEPI->getOperand(0);
return (isa<ConstantPointerNull>(Ptr) &&
!NullPointerIsDefined(SI.getFunction(), SI.getPointerAddressSpace()));
}

static bool canSimplifyNullLoadOrGEP(LoadInst &LI, Value *Op) {
if (GetElementPtrInst *GEPI = dyn_cast<GetElementPtrInst>(Op)) {
const Value *GEPI0 = GEPI->getOperand(0);
if (isa<ConstantPointerNull>(GEPI0) && GEPI->isInBounds() &&
if (isa<ConstantPointerNull>(GEPI0) &&
!NullPointerIsDefined(LI.getFunction(), GEPI->getPointerAddressSpace()))
return true;
}
Expand Down
5 changes: 2 additions & 3 deletions llvm/test/Transforms/InstCombine/load.ll
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,8 @@ define i32 @load_gep_null_inbounds(i64 %X) {

define i32 @load_gep_null_not_inbounds(i64 %X) {
; CHECK-LABEL: @load_gep_null_not_inbounds(
; CHECK-NEXT: [[V:%.*]] = getelementptr i32, i32* null, i64 [[X:%.*]]
; CHECK-NEXT: [[R:%.*]] = load i32, i32* [[V]], align 4
; CHECK-NEXT: ret i32 [[R]]
; CHECK-NEXT: store i32 undef, i32* null, align 536870912
; CHECK-NEXT: ret i32 undef
;
%V = getelementptr i32, i32* null, i64 %X
%R = load i32, i32* %V
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/Transforms/InstCombine/store.ll
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ define void @store_at_gep_off_null_inbounds(i64 %offset) {
define void @store_at_gep_off_null_not_inbounds(i64 %offset) {
; CHECK-LABEL: @store_at_gep_off_null_not_inbounds(
; CHECK-NEXT: [[PTR:%.*]] = getelementptr i32, i32* null, i64 [[OFFSET:%.*]]
; CHECK-NEXT: store i32 24, i32* [[PTR]], align 4
; CHECK-NEXT: store i32 undef, i32* [[PTR]], align 4
; CHECK-NEXT: ret void
;
%ptr = getelementptr i32, i32 *null, i64 %offset
Expand Down

0 comments on commit ef2f843

Please sign in to comment.