[PATCH] D95815: (Corrrectly) support hoisting of loads following calls to noalias functions

[Philip Reames via Phabricator via llvm-commits]

reames updated this revision to Diff 333187.
reames retitled this revision from "Restrict hoisting of potentially freed memory " to "(Corrrectly) support hoisting of loads following calls to noalias functions".
reames edited the summary of this revision.
reames added a comment.
Herald added a subscriber: dexonsmith.

Rebase over the infrastructure added for deref-at-point semantics.  This is effectively unconditionally using deref-at-point, but since if we don't, we have a miscompile actively happening, I'm not too worried about that.


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

https://reviews.llvm.org/D95815

Files:
  llvm/include/llvm/IR/Value.h
  llvm/lib/Analysis/Loads.cpp
  llvm/lib/IR/Value.cpp
  llvm/test/Transforms/LoopVectorize/X86/load-deref-pred.ll


Index: llvm/test/Transforms/LoopVectorize/X86/load-deref-pred.ll
===================================================================
--- llvm/test/Transforms/LoopVectorize/X86/load-deref-pred.ll
+++ llvm/test/Transforms/LoopVectorize/X86/load-deref-pred.ll
@@ -2214,7 +2214,7 @@
 declare align 8 dereferenceable_or_null(8) i8* @my_alloc(i32) allocsize(0)
 declare align 8 dereferenceable_or_null(8) i8* @my_array_alloc(i32, i32) allocsize(0, 1)
 
-define i32 @test_allocsize(i64 %len, i1* %test_base) {
+define i32 @test_allocsize(i64 %len, i1* %test_base) nofree nosync {
 ; CHECK-LABEL: @test_allocsize(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[ALLOCATION:%.*]] = call nonnull i8* @my_alloc(i32 16384)
@@ -2382,7 +2382,7 @@
 }
 
 
-define i32 @test_allocsize_array(i64 %len, i1* %test_base) {
+define i32 @test_allocsize_array(i64 %len, i1* %test_base) nofree nosync {
 ; CHECK-LABEL: @test_allocsize_array(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[ALLOCATION:%.*]] = call nonnull i8* @my_array_alloc(i32 4096, i32 4)
Index: llvm/lib/IR/Value.cpp
===================================================================
--- llvm/lib/IR/Value.cpp
+++ llvm/lib/IR/Value.cpp
@@ -785,6 +785,11 @@
   return false;
 }
 
+bool Value::canBeFreed() const {
+  // NOTE: Wrapper exists only to simplify patch rebase.  Will be inlined once
+  // landed.
+  return ::canBeFreed(this);
+}
 
 uint64_t Value::getPointerDereferenceableBytes(const DataLayout &DL,
                                                bool &CanBeNull,
@@ -793,7 +798,7 @@
 
   uint64_t DerefBytes = 0;
   CanBeNull = false;
-  CanBeFreed = UseDerefAtPointSemantics && canBeFreed(this);
+  CanBeFreed = UseDerefAtPointSemantics && canBeFreed();
   if (const Argument *A = dyn_cast<Argument>(this)) {
     DerefBytes = A->getDereferenceableBytes();
     if (DerefBytes == 0) {
Index: llvm/lib/Analysis/Loads.cpp
===================================================================
--- llvm/lib/Analysis/Loads.cpp
+++ llvm/lib/Analysis/Loads.cpp
@@ -162,19 +162,11 @@
     Opts.RoundToAlign = false;
     Opts.NullIsUnknownSize = true;
     uint64_t ObjSize;
-    // TODO: Plumb through TLI so that malloc routines and such working.
-    if (getObjectSize(V, ObjSize, DL, nullptr, Opts)) {
+    if (getObjectSize(V, ObjSize, DL, TLI, Opts)) {
       APInt KnownDerefBytes(Size.getBitWidth(), ObjSize);
       if (KnownDerefBytes.getBoolValue() && KnownDerefBytes.uge(Size) &&
           isKnownNonZero(V, DL, 0, nullptr, CtxI, DT) &&
-          // TODO: We're currently inconsistent about whether deref(N) is a
-          // global fact or a point in time fact.  Once D61652 eventually
-          // lands, this check will be restricted to the point in time
-          // variant. For that variant, we need to prove that object hasn't
-          // been conditionally freed before ontext instruction - if it has, we
-          // might be hoisting over the inverse conditional and creating a
-          // dynamic use after free. 
-          !PointerMayBeCapturedBefore(V, true, true, CtxI, DT, true)) {
+          !V->canBeFreed()) {
         // As we recursed through GEPs to get here, we've incrementally
         // checked that each step advanced by a multiple of the alignment. If
         // our base is properly aligned, then the original offset accessed
Index: llvm/include/llvm/IR/Value.h
===================================================================
--- llvm/include/llvm/IR/Value.h
+++ llvm/include/llvm/IR/Value.h
@@ -738,6 +738,12 @@
         static_cast<const Value *>(this)->stripInBoundsOffsets(Func));
   }
 
+  /// Return true if the memory object referred to by V can by freed in the
+  /// scope for which the SSA value defining the allocation is statically
+  /// defined.  E.g.  deallocation after the static scope of a value does not
+  /// count, but a deallocation before that does.
+  bool canBeFreed() const;
+
   /// Returns the number of bytes known to be dereferenceable for the
   /// pointer value.
   ///


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D95815.333187.patch
Type: text/x-patch
Size: 4003 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210325/5b1d464d/attachment.bin>