[PATCH] D96349: [instcombine] Exploit UB implied by nofree attributes

[Philip Reames via Phabricator via llvm-commits]

reames created this revision.
reames added reviewers: jdoerfert, apilipenko.
Herald added subscribers: dantrushin, hiraditya, mcrosier.
Herald added a reviewer: bollu.
reames requested review of this revision.
Herald added a project: LLVM.

This patch simply implements the documented UB of the current nofree attributes as specified.  It doesn't try to be fancy about inference (yet), it just implements the cases already specified and inferred.

Note: When this lands, it may expose miscompiles.  If so, please revert and provide a test case.  It's likely the bug is in the existing inference code and without a relatively complete test case, it will be hard to debug.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96349

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
@@ -351,7 +351,7 @@
 
 define void @test11() {
 ; CHECK-LABEL: @test11(
-; CHECK-NEXT:    [[CALL:%.*]] = call dereferenceable(8) i8* @_Znwm(i64 8) #7
+; CHECK-NEXT:    [[CALL:%.*]] = call dereferenceable(8) i8* @_Znwm(i64 8) #8
 ; CHECK-NEXT:    call void @_ZdlPv(i8* nonnull [[CALL]])
 ; CHECK-NEXT:    ret void
 ;
@@ -390,3 +390,32 @@
   ret void
 }
 
+; Freeing a no-free pointer -> full UB
+define void @test13(i8* nofree %foo) {
+; CHECK-LABEL: @test13(
+; CHECK-NEXT:    store i1 true, i1* undef, align 1
+; CHECK-NEXT:    ret void
+;
+  call void @free(i8* %foo)
+  ret void
+}
+
+; Freeing a no-free pointer -> full UB
+define void @test14(i8* %foo) nofree {
+; CHECK-LABEL: @test14(
+; CHECK-NEXT:    store i1 true, i1* undef, align 1
+; CHECK-NEXT:    ret void
+;
+  call void @free(i8* %foo)
+  ret void
+}
+
+; free call marked no-free -> full UB
+define void @test15(i8* %foo) {
+; CHECK-LABEL: @test15(
+; CHECK-NEXT:    store i1 true, i1* undef, align 1
+; CHECK-NEXT:    ret void
+;
+  call void @free(i8* %foo) nofree
+  ret void
+}
Index: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
===================================================================
--- llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -2799,6 +2799,20 @@
   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:
+  // 1) freeing a pointer which is explicitly nofree
+  // 2) calling free from a call site marked nofree
+  // 3) calling free in a function scope marked nofree
+  if (auto *A = dyn_cast<Argument>(Op->stripPointerCasts()))
+    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);
+      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:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D96349.322414.patch
Type: text/x-patch
Size: 2559 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210209/8c6a9ee1/attachment.bin>