commit bee875322d1621964d5b0368e5f48701bde0f839
author Philip Reames <listmail@philipreames.com> Thu Feb 18 08:33:12 2021 -0800
committer Copybara-Service <copybara-worker@google.com> Fri Feb 19 03:32:50 2021 -0800
tree ef8df182d46c2e68f4a0259bc890ed578fc3cde3
parent e882a63db80100617166b716ed154504e7c70926

[instcombine] Exploit UB implied by nofree attributes

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.

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

GitOrigin-RevId: 86664638898e9c3756ad17d612de1873fead6813

lib/Transforms/InstCombine/InstructionCombining.cpp

diff --git a/lib/Transforms/InstCombine/InstructionCombining.cpp b/lib/Transforms/InstCombine/InstructionCombining.cpp
index 0f2f570..cc16e03 100644
--- a/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/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:

test/Transforms/InstCombine/malloc-free-delete.ll

diff --git a/test/Transforms/InstCombine/malloc-free-delete.ll b/test/Transforms/InstCombine/malloc-free-delete.ll
index fad5428..c2f35cf 100644
--- a/test/Transforms/InstCombine/malloc-free-delete.ll
+++ b/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
+}