[PATCH] D100779: free(nullptr) does not violate the nofree specification

[Philip Reames via Phabricator via llvm-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3b1474cab26b: free(nullptr) does not violate the nofree specification (authored by reames).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100779/new/

https://reviews.llvm.org/D100779

Files:
  llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
  llvm/test/Transforms/InstCombine/malloc-free-delete.ll


Index: llvm/test/Transforms/InstCombine/malloc-free-delete.ll
===================================================================
--- llvm/test/Transforms/InstCombine/malloc-free-delete.ll
+++ llvm/test/Transforms/InstCombine/malloc-free-delete.ll
@@ -391,32 +391,45 @@
   ret void
 }
 
-; Freeing a no-free pointer -> full UB
+; Freeing a no-free pointer -> %foo must be null
 define void @test13(i8* nofree %foo) {
 ; CHECK-LABEL: @test13(
-; CHECK-NEXT:    store i1 true, i1* undef, align 1
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp eq i8* [[FOO:%.*]], null
+; CHECK-NEXT:    call void @llvm.assume(i1 [[TMP1]])
 ; CHECK-NEXT:    ret void
 ;
   call void @free(i8* %foo)
   ret void
 }
 
-; Freeing a no-free pointer -> full UB
+; Freeing a no-free pointer -> %foo must be null
 define void @test14(i8* %foo) nofree {
 ; CHECK-LABEL: @test14(
-; CHECK-NEXT:    store i1 true, i1* undef, align 1
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp eq i8* [[FOO:%.*]], null
+; CHECK-NEXT:    call void @llvm.assume(i1 [[TMP1]])
 ; CHECK-NEXT:    ret void
 ;
   call void @free(i8* %foo)
   ret void
 }
 
-; free call marked no-free -> full UB
+; free call marked no-free ->  %foo must be null
 define void @test15(i8* %foo) {
 ; CHECK-LABEL: @test15(
-; CHECK-NEXT:    store i1 true, i1* undef, align 1
+; CHECK-NEXT:    [[TMP1:%.*]] = icmp eq i8* [[FOO:%.*]], null
+; CHECK-NEXT:    call void @llvm.assume(i1 [[TMP1]])
 ; CHECK-NEXT:    ret void
 ;
   call void @free(i8* %foo) nofree
   ret void
 }
+
+; 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:    ret void
+;
+  call void @free(i8* %foo)
+  ret void
+}
Index: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
===================================================================
--- llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -2799,8 +2799,10 @@
   if (isa<ConstantPointerNull>(Op))
     return eraseInstFromFunction(FI);
 
-  // If we free a pointer we've been explicitly told won't be freed, this
-  // would be full UB and thus we can conclude this is unreachable. Cases:
+  
+  // 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)
@@ -2810,8 +2812,9 @@
     if (A->hasAttribute(Attribute::NoFree) ||
         FI.hasFnAttr(Attribute::NoFree) ||
         FI.getFunction()->hasFnAttribute(Attribute::NoFree)) {
-      // Leave a marker since we can't modify the CFG here.
-      CreateNonTerminatorUnreachable(&FI);
+      Value *Null = ConstantPointerNull::get(cast<PointerType>(A->getType()));
+      Value *Cond = Builder.CreateICmpEQ(A, Null);
+      Builder.CreateAssumption(Cond);
       return eraseInstFromFunction(FI);
     }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D100779.338890.patch
Type: text/x-patch
Size: 3103 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210420/215992e3/attachment.bin>