RPCS3/llvm-mirror
1
0
Fork 0
mirror of https://github.com/RPCS3/llvm-mirror.git synced 2024-11-23 03:02:36 +01:00
Code Issues Projects Releases Wiki Activity

Revert "[instcombine] Exploit UB implied by nofree attributes"

Browse Source 
This change effectively reverts 86664638, but since there have been some changes on top and I wanted to leave the tests in, it's not a mechanical revert.

Why revert this now?  Two main reasons:
1) There are continuing discussion around what the semantics of nofree.  I am getting increasing uncomfortable with the seeming possibility we might redefine nofree in a way incompatible with these changes.
2) There was a reported miscompile triggered by this change (https://github.com/emscripten-core/emscripten/issues/9443).  At first, I was making good progress on tracking down the issues exposed and those issues appeared to be unrelated latent bugs.  Now that we've found at least one bug in the original change, and the investigation has stalled, I'm no longer comfortable leaving this in tree.  In retrospect, I probably should have reverted this earlier and investigated the issues once the triggering change was out of tree.
This commit is contained in:
Philip Reames 2021-04-22 10:47:41 -07:00
parent ae70485f74
commit 8361e53fbe
2 changed files with 12 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
lib/Transforms/InstCombine/InstructionCombining.cpp
View File

@ -2799,25 +2799,6 @@ Instruction *InstCombinerImpl::visitFree(CallInst &FI) {
if (isa<ConstantPointerNull>(Op))
return eraseInstFromFunction(FI);
// If we free a non-null pointer we've been explicitly told won't be freed,
// this would be full UB and thus we can conclude that the argument must
// be null at the point of the free. Cases:
// 1) freeing a pointer which is explicitly nofree
// 2) calling free from a call site marked nofree (TODO: can generalize
// for non-arguments)
// 3) calling free in a function scope marked nofree (when we can prove
// the allocation existed before the start of the function scope)
if (auto *A = dyn_cast<Argument>(Op->stripPointerCasts()))
if (A->hasAttribute(Attribute::NoFree) ||
FI.hasFnAttr(Attribute::NoFree) ||
FI.getFunction()->hasFnAttribute(Attribute::NoFree)) {
Value *Null = ConstantPointerNull::get(cast<PointerType>(A->getType()));
Value *Cond = Builder.CreateICmpEQ(A, Null);
Builder.CreateAssumption(Cond);
return eraseInstFromFunction(FI);
}
// If we optimize for code size, try to move the call to free before the null
// test so that simplify cfg can remove the empty block and dead code
// elimination the branch. I.e., helps to turn something like:

23
test/Transforms/InstCombine/malloc-free-delete.ll
View File

@ -391,43 +391,44 @@ if.end: ; preds = %entry, %if.then
ret void
}
; Freeing a no-free pointer -> %foo must be null
; The next four tests cover the semantics of the nofree attributes. These
; are thought to be legal transforms, but an implementation thereof has
; been reverted once due to difficult to isolate fallout.
; TODO: Freeing a no-free pointer -> %foo must be null
define void @test13(i8* nofree %foo) {
; CHECK-LABEL: @test13(
; CHECK-NEXT: [[TMP1:%.*]] = icmp eq i8* [[FOO:%.*]], null
; CHECK-NEXT: call void @llvm.assume(i1 [[TMP1]])
; CHECK-NEXT: call void @free(i8* [[FOO:%.*]])
; CHECK-NEXT: ret void
;
call void @free(i8* %foo)
ret void
}
; Freeing a no-free pointer -> %foo must be null
; TODO: Freeing a no-free pointer -> %foo must be null
define void @test14(i8* %foo) nofree {
; CHECK-LABEL: @test14(
; CHECK-NEXT: [[TMP1:%.*]] = icmp eq i8* [[FOO:%.*]], null
; CHECK-NEXT: call void @llvm.assume(i1 [[TMP1]])
; CHECK-NEXT: call void @free(i8* [[FOO:%.*]])
; CHECK-NEXT: ret void
;
call void @free(i8* %foo)
ret void
}
; free call marked no-free -> %foo must be null
; TODO: free call marked no-free -> %foo must be null
define void @test15(i8* %foo) {
; CHECK-LABEL: @test15(
; CHECK-NEXT: [[TMP1:%.*]] = icmp eq i8* [[FOO:%.*]], null
; CHECK-NEXT: call void @llvm.assume(i1 [[TMP1]])
; CHECK-NEXT: call void @free(i8* [[FOO:%.*]]) #[[ATTR6:[0-9]+]]
; CHECK-NEXT: ret void
;
call void @free(i8* %foo) nofree
ret void
}
; freeing a nonnull nofree pointer -> full UB
; TODO: freeing a nonnull nofree pointer -> full UB
define void @test16(i8* nonnull nofree %foo) {
; CHECK-LABEL: @test16(
; CHECK-NEXT: call void @llvm.assume(i1 false)
; CHECK-NEXT: call void @free(i8* [[FOO:%.*]])
; CHECK-NEXT: ret void
;
call void @free(i8* %foo)