[PATCH] D59140: [CGP] Fix UB when removing trivial PHINode

[Eugene Leviant via Phabricator via llvm-commits]

evgeny777 created this revision.
evgeny777 added reviewers: efriedma, haicheng, javed.absar, skatkov, qcolombet.

When one of GEP operands is a trivial PHI, it's possible that we do RAUW leaving asserting value handle in the map. 
This causes assertion in build with enabled assertions and UB in `splitLargeGEPOffsets` when assertions are disabled.


https://reviews.llvm.org/D59140

Files:
  lib/CodeGen/CodeGenPrepare.cpp
  test/CodeGen/AArch64/cgp-trivial-phi-node.ll


Index: test/CodeGen/AArch64/cgp-trivial-phi-node.ll
===================================================================
--- test/CodeGen/AArch64/cgp-trivial-phi-node.ll
+++ test/CodeGen/AArch64/cgp-trivial-phi-node.ll
@@ -0,0 +1,35 @@
+; Checks that we don't assert in build with enabled assertions when GEP is
+; bound to trivial PHI node. 
+; REQUIRES: asserts
+; RUN: opt -mtriple=aarch64-linux-gnu -codegenprepare -S -o - | FileCheck %s
+
+; CHECK:      define void @crash([65536 x i32]** %s, i32 %n) {
+; CHECK-NEXT: entry:
+; CHECK-NEXT:   %struct = load [65536 x i32]*, [65536 x i32]** %s
+; CHECK-NEXT:   %gep0 = getelementptr [65536 x i32], [65536 x i32]* %struct, i64 0, i32 20000
+; CHECK-NEXT:   store i32 %n, i32* %gep0
+; CHECK-NEXT:   ret void
+; CHECK-NEXT: }
+
+define void @crash([65536 x i32]** %s, i32 %n) {
+entry:
+  %struct = load [65536 x i32]*, [65536 x i32]** %s
+  %cmp = icmp slt i32 0, %n
+  br i1 %cmp, label %baz, label %bar
+baz:
+  br label %bar
+
+foo:
+  %gep0 = getelementptr [65536 x i32], [65536 x i32]* %phi2, i64 0, i32 20000
+  br label %st
+
+st:
+  store i32 %n, i32* %gep0
+  br label %out
+
+bar:
+  %phi2 = phi [65536 x i32]* [ %struct, %baz ], [ %struct, %entry ]
+  br label %foo
+out:
+  ret void
+}
Index: lib/CodeGen/CodeGenPrepare.cpp
===================================================================
--- lib/CodeGen/CodeGenPrepare.cpp
+++ lib/CodeGen/CodeGenPrepare.cpp
@@ -6841,6 +6841,7 @@
     // to introduce PHI nodes too late to be cleaned up.  If we detect such a
     // trivial PHI, go ahead and zap it here.
     if (Value *V = SimplifyInstruction(P, {*DL, TLInfo})) {
+      LargeOffsetGEPMap.erase(P);
       P->replaceAllUsesWith(V);
       P->eraseFromParent();
       ++NumPHIsElim;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D59140.189875.patch
Type: text/x-patch
Size: 1753 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190308/af669450/attachment.bin>