[PATCH] D37933: Prevent InstCombine from miscompiling `operator delete`

[Davide Italiano via Phabricator via llvm-commits]

davide created this revision.

It's not legal inventing calls to operator delete.


https://reviews.llvm.org/D37933

Files:
  include/llvm/Analysis/MemoryBuiltins.h
  lib/Analysis/MemoryBuiltins.cpp
  lib/Transforms/InstCombine/InstructionCombining.cpp
  test/Transforms/InstCombine/pr34581.ll


Index: test/Transforms/InstCombine/pr34581.ll
===================================================================
--- /dev/null
+++ test/Transforms/InstCombine/pr34581.ll
@@ -0,0 +1,32 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; Instcombine is not free of doing anything hazy like hoisting calls to operator
+; delete. This is PR34581.
+
+; RUN: opt -instcombine %s -S | FileCheck %s
+
+define void @patatino(i8* %c) #0 {
+; CHECK-LABEL: @patatino(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[ISNULL:%.*]] = icmp eq i8* [[C:%.*]], null
+; CHECK-NEXT:    br i1 [[ISNULL]], label [[DELETE_END:%.*]], label [[DELETE_NOTNULL:%.*]]
+; CHECK:       delete.notnull:
+; CHECK-NEXT:    call void @_ZdlPv(i8* [[C]])
+; CHECK-NEXT:    br label [[DELETE_END]]
+; CHECK:       delete.end:
+; CHECK-NEXT:    ret void
+;
+entry:
+  %isnull = icmp eq i8* %c, null
+  br i1 %isnull, label %delete.end, label %delete.notnull
+
+delete.notnull:
+  call void @_ZdlPv(i8* %c)
+  br label %delete.end
+
+delete.end:
+  ret void
+}
+
+declare void @_ZdlPv(i8*)
+
+attributes #0 = { minsize }
Index: lib/Transforms/InstCombine/InstructionCombining.cpp
===================================================================
--- lib/Transforms/InstCombine/InstructionCombining.cpp
+++ lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -2247,7 +2247,9 @@
   // if (foo) free(foo);
   // into
   // free(foo);
-  if (MinimizeSize)
+  // Note we can't really do it in the case of `operator delete` as this one
+  // can have visible side effects.
+  if (MinimizeSize && !hasSideEffectsFreeCall(&FI, TLI))
     if (Instruction *I = tryToMoveFreeBeforeNullTest(FI))
       return I;
 
Index: lib/Analysis/MemoryBuiltins.cpp
===================================================================
--- lib/Analysis/MemoryBuiltins.cpp
+++ lib/Analysis/MemoryBuiltins.cpp
@@ -348,6 +348,26 @@
   return isCallocLikeFn(I, TLI) ? cast<CallInst>(I) : nullptr;
 }
 
+/// hasSideEffectsFreeCall - Returns true if the call has side effects.
+bool llvm::hasSideEffectsFreeCall(const CallInst *CI,
+                                  const TargetLibraryInfo &TLI) {
+  Function *Callee = CI->getCalledFunction();
+  if (Callee == nullptr)
+    return false;
+
+  StringRef FnName = Callee->getName();
+  LibFunc TLIFn;
+  if (!TLI.getLibFunc(FnName, TLIFn) || !TLI.has(TLIFn))
+    return false;
+
+  return TLIFn == LibFunc_ZdlPv ||                   // operator delete(void*)
+         TLIFn == LibFunc_ZdaPv ||                   // operator delete[](void*)
+         TLIFn == LibFunc_msvc_delete_ptr32 ||       // operator delete(void*)
+         TLIFn == LibFunc_msvc_delete_ptr64 ||       // operator delete(void*)
+         TLIFn == LibFunc_msvc_delete_array_ptr32 || // operator delete[](void*)
+         TLIFn == LibFunc_msvc_delete_array_ptr64;   // operator delete[](void*)
+}
+
 /// isFreeCall - Returns non-null if the value is a call to the builtin free()
 const CallInst *llvm::isFreeCall(const Value *I, const TargetLibraryInfo *TLI) {
   const CallInst *CI = dyn_cast<CallInst>(I);
Index: include/llvm/Analysis/MemoryBuiltins.h
===================================================================
--- include/llvm/Analysis/MemoryBuiltins.h
+++ include/llvm/Analysis/MemoryBuiltins.h
@@ -137,6 +137,9 @@
 //  free Call Utility Functions.
 //
 
+/// hasSideEffectsFreeCall - Returns true if the call has side effects.
+bool hasSideEffectsFreeCall(const CallInst *CI, const TargetLibraryInfo &TLI);
+
 /// isFreeCall - Returns non-null if the value is a call to the builtin free()
 const CallInst *isFreeCall(const Value *I, const TargetLibraryInfo *TLI);
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D37933.115501.patch
Type: text/x-patch
Size: 3660 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170915/2af0eab0/attachment-0001.bin>