[PATCH] D45889: [MemCpyOpt] Do not iterate beyond beginning of basic block

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope created this revision.

This patch avoids asserts like this:

  "Assertion `!NodePtr->isKnownSentinel()' failed."

when iterating past basic block beginning in
MemCpyOptPass::processStore.

I've only seen this happening with some fuzz testing, so it is
probably not that common, but annoying when running fuzz tests.

The failing test case actually involves some dead code, so if
we always remove dead code before memcpyopt in regular pass
pipelines, then the likelyhood of the assert is even smaller.


Repository:
  rL LLVM

https://reviews.llvm.org/D45889

Files:
  lib/Transforms/Scalar/MemCpyOptimizer.cpp
  test/Transforms/MemCpyOpt/process_store.ll


Index: test/Transforms/MemCpyOpt/process_store.ll
===================================================================
--- /dev/null
+++ test/Transforms/MemCpyOpt/process_store.ll
@@ -0,0 +1,39 @@
+; RUN: opt < %s -memcpyopt -disable-output
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+ at b = common dso_local local_unnamed_addr global i32 0, align 4
+ at a = common dso_local local_unnamed_addr global i32 0, align 4
+
+declare dso_local i32 @f1()
+
+; Do not crash due to store first in BB.
+define dso_local void @f2() {
+for.end:
+  %0 = load i32, i32* @b, align 4
+  ret void
+
+for.body:
+  store i32 %1, i32* @a, align 4
+  %call = call i32 @f1()
+  %cmp = icmp sge i32 %call, 0
+  %1 = load i32, i32* @b, align 4
+  br label %for.body
+}
+
+; Do not crash due to call not before store in BB.
+define dso_local void @f3() {
+for.end:
+  %0 = load i32, i32* @b, align 4
+  ret void
+
+for.body:
+  %t = add i32 %t2, 1
+  store i32 %1, i32* @a, align 4
+  %call = call i32 @f1()
+  %cmp = icmp sge i32 %call, 0
+  %1 = load i32, i32* @b, align 4
+  %t2 = xor i32 %t, 5
+  br label %for.body
+}
Index: lib/Transforms/Scalar/MemCpyOptimizer.cpp
===================================================================
--- lib/Transforms/Scalar/MemCpyOptimizer.cpp
+++ lib/Transforms/Scalar/MemCpyOptimizer.cpp
@@ -710,19 +710,31 @@
         bool CpyDestIsLocal = isa<AllocaInst>(CpyDest);
         AliasAnalysis &AA = LookupAliasAnalysis();
         MemoryLocation StoreLoc = MemoryLocation::get(SI);
-        for (BasicBlock::iterator I = --SI->getIterator(), E = C->getIterator();
-             I != E; --I) {
-          if (isModOrRefSet(AA.getModRefInfo(&*I, StoreLoc))) {
-            C = nullptr;
-            break;
-          }
-          // The store to dest may never happen if an exception can be thrown
-          // between the load and the store.
-          if (I->mayThrow() && !CpyDestIsLocal) {
-            C = nullptr;
-            break;
+        BasicBlock::iterator BBBegin = SI->getParent()->begin();
+        if (SI->getIterator() == BBBegin)
+          // SI is first in BB, so we can't search backwards to find C unless we
+          // examine the predecessors. Simply bail out.
+          C = nullptr;
+        else
+          for (BasicBlock::iterator I = --SI->getIterator(),
+                                    E = C->getIterator();
+               I != E; --I) {
+            if (I == BBBegin) {
+              // We reached the beginning of the BB without finding C.
+              C = nullptr;
+              break;
+            }
+            if (isModOrRefSet(AA.getModRefInfo(&*I, StoreLoc))) {
+              C = nullptr;
+              break;
+            }
+            // The store to dest may never happen if an exception can be thrown
+            // between the load and the store.
+            if (I->mayThrow() && !CpyDestIsLocal) {
+              C = nullptr;
+              break;
+            }
           }
-        }
       }
 
       if (C) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D45889.143337.patch
Type: text/x-patch
Size: 3053 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180420/d8f9c8b6/attachment.bin>