[llvm] 8666463 - [instcombine] Exploit UB implied by nofree attributes

[Philip Reames via llvm-commits]

Author: Philip Reames
Date: 2021-02-18T08:34:22-08:00
New Revision: 86664638898e9c3756ad17d612de1873fead6813

URL: https://github.com/llvm/llvm-project/commit/86664638898e9c3756ad17d612de1873fead6813
DIFF: https://github.com/llvm/llvm-project/commit/86664638898e9c3756ad17d612de1873fead6813.diff

LOG: [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

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 0f2f570615bd..cc16e03c01c6 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -2799,6 +2799,20 @@ Instruction *InstCombinerImpl::visitFree(CallInst &FI) {
   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:

diff  --git a/llvm/test/Transforms/InstCombine/malloc-free-delete.ll b/llvm/test/Transforms/InstCombine/malloc-free-delete.ll
index fad542893c91..c2f35cfe0ad6 100644
--- a/llvm/test/Transforms/InstCombine/malloc-free-delete.ll
+++ b/llvm/test/Transforms/InstCombine/malloc-free-delete.ll
@@ -351,7 +351,7 @@ define void @test10()  {
 
 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 @@ if.end:                                           ; preds = %entry, %if.then
   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
+}