[InstCombine] Remove attributes after hoisting free above null check

If the parameter had been annotated as nonnull because of the null
check, we want to remove the attribute, since it may no longer apply and
could result in miscompiles if left. Similarly, we also want to remove
undef-implying attributes, since they may not apply anymore either.

Fixes PR52110.

Reviewed By: nikic

Differential Revision: https://reviews.llvm.org/D111515

(cherry picked from commit 6404f4b5af39840a2dad27abc3924eb3846ae8a4)
This commit is contained in:
Shoaib Meenai 2021-10-10 14:06:49 -07:00 committed by Tom Stellard
parent 35df3f9863
commit 12aaa8553f
2 changed files with 99 additions and 0 deletions

View File

@ -2843,6 +2843,26 @@ static Instruction *tryToMoveFreeBeforeNullTest(CallInst &FI,
}
assert(FreeInstrBB->size() == 1 &&
"Only the branch instruction should remain");
// Now that we've moved the call to free before the NULL check, we have to
// remove any attributes on its parameter that imply it's non-null, because
// those attributes might have only been valid because of the NULL check, and
// we can get miscompiles if we keep them. This is conservative if non-null is
// also implied by something other than the NULL check, but it's guaranteed to
// be correct, and the conservativeness won't matter in practice, since the
// attributes are irrelevant for the call to free itself and the pointer
// shouldn't be used after the call.
AttributeList Attrs = FI.getAttributes();
Attrs = Attrs.removeParamAttribute(FI.getContext(), 0, Attribute::NonNull);
Attribute Dereferenceable = Attrs.getParamAttr(0, Attribute::Dereferenceable);
if (Dereferenceable.isValid()) {
uint64_t Bytes = Dereferenceable.getDereferenceableBytes();
Attrs = Attrs.removeParamAttribute(FI.getContext(), 0,
Attribute::Dereferenceable);
Attrs = Attrs.addDereferenceableOrNullParamAttr(FI.getContext(), 0, Bytes);
}
FI.setAttributes(Attrs);
return &FI;
}

View File

@ -0,0 +1,79 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
; RUN: opt < %s -instcombine -S | FileCheck %s
declare noalias i8* @calloc(i32, i32) nounwind
declare noalias i8* @malloc(i32)
declare noalias i8* @aligned_alloc(i32, i32)
declare void @free(i8*)
;; Test that nonnull-implying attributes on the parameter are adjusted when the
;; call is moved, since they may no longer be valid and result in miscompiles if
;; kept unchanged.
define void @test_nonnull_free_move(i8* %foo) minsize {
; CHECK-LABEL: @test_nonnull_free_move(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[TOBOOL:%.*]] = icmp eq i8* [[FOO:%.*]], null
; CHECK-NEXT: tail call void @free(i8* [[FOO]])
; CHECK-NEXT: br i1 [[TOBOOL]], label [[IF_END:%.*]], label [[IF_THEN:%.*]]
; CHECK: if.then:
; CHECK-NEXT: br label [[IF_END]]
; CHECK: if.end:
; CHECK-NEXT: ret void
;
entry:
%tobool = icmp eq i8* %foo, null
br i1 %tobool, label %if.end, label %if.then
if.then: ; preds = %entry
tail call void @free(i8* nonnull %foo)
br label %if.end
if.end: ; preds = %entry, %if.then
ret void
}
define void @test_dereferenceable_free_move(i8* %foo) minsize {
; CHECK-LABEL: @test_dereferenceable_free_move(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[TOBOOL:%.*]] = icmp eq i8* [[FOO:%.*]], null
; CHECK-NEXT: tail call void @free(i8* dereferenceable_or_null(4) [[FOO]])
; CHECK-NEXT: br i1 [[TOBOOL]], label [[IF_END:%.*]], label [[IF_THEN:%.*]]
; CHECK: if.then:
; CHECK-NEXT: br label [[IF_END]]
; CHECK: if.end:
; CHECK-NEXT: ret void
;
entry:
%tobool = icmp eq i8* %foo, null
br i1 %tobool, label %if.end, label %if.then
if.then: ; preds = %entry
tail call void @free(i8* dereferenceable(4) %foo)
br label %if.end
if.end: ; preds = %entry, %if.then
ret void
}
define void @test_nonnull_dereferenceable_free_move(i8* %foo) minsize {
; CHECK-LABEL: @test_nonnull_dereferenceable_free_move(
; CHECK-NEXT: entry:
; CHECK-NEXT: [[TOBOOL:%.*]] = icmp eq i8* [[FOO:%.*]], null
; CHECK-NEXT: tail call void @free(i8* dereferenceable_or_null(16) [[FOO]])
; CHECK-NEXT: br i1 [[TOBOOL]], label [[IF_END:%.*]], label [[IF_THEN:%.*]]
; CHECK: if.then:
; CHECK-NEXT: br label [[IF_END]]
; CHECK: if.end:
; CHECK-NEXT: ret void
;
entry:
%tobool = icmp eq i8* %foo, null
br i1 %tobool, label %if.end, label %if.then
if.then: ; preds = %entry
tail call void @free(i8* nonnull dereferenceable(16) %foo)
br label %if.end
if.end: ; preds = %entry, %if.then
ret void
}