Merging r293522:

------------------------------------------------------------------------
r293522 | bogner | 2017-01-30 10:29:46 -0800 (Mon, 30 Jan 2017) | 8 lines

SDAG: Update ChainNodesMatched during UpdateChains if a node is replaced

Previously, we would hit UB (or the ISD::DELETED_NODE assert) if we
happened to replace a node during UpdateChains, because it would be
left in the list we were iterating over. This nulls out the pointer
when that happens so that we can avoid the issue.

Fixes llvm.org/PR31710
------------------------------------------------------------------------

git-svn-id: https://llvm.org/svn/llvm-project/llvm/branches/release_40@293650 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/CodeGen/SelectionDAGISel.h b/include/llvm/CodeGen/SelectionDAGISel.h
index 7f4549d3058f480316cb52d9b6a5d125a8ff193c..61d7ec4ecf5b01551738beaf2aafb21ce0f29a73 100644 (file)
--- a/include/llvm/CodeGen/SelectionDAGISel.h
+++ b/include/llvm/CodeGen/SelectionDAGISel.h
@@ -305,7 +305,7 @@ private:
   std::vector<unsigned> OpcodeOffset;
 
   void UpdateChains(SDNode *NodeToMatch, SDValue InputChain,
-                    const SmallVectorImpl<SDNode *> &ChainNodesMatched,
+                    SmallVectorImpl<SDNode *> &ChainNodesMatched,
                     bool isMorphNodeTo);
 };
 

diff --git a/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index 6d717b44eb72b56faee8505c6cc6fd0bfbc3279c..004fa703c192f70ef4092a1255d9c90399c3b4c8 100644 (file)
--- a/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -2248,7 +2248,7 @@ GetVBR(uint64_t Val, const unsigned char *MatcherTable, unsigned &Idx) {
 /// to use the new results.
 void SelectionDAGISel::UpdateChains(
     SDNode *NodeToMatch, SDValue InputChain,
-    const SmallVectorImpl<SDNode *> &ChainNodesMatched, bool isMorphNodeTo) {
+    SmallVectorImpl<SDNode *> &ChainNodesMatched, bool isMorphNodeTo) {
   SmallVector<SDNode*, 4> NowDeadNodes;
 
   // Now that all the normal results are replaced, we replace the chain and
@@ -2260,6 +2260,11 @@ void SelectionDAGISel::UpdateChains(
     // Replace all the chain results with the final chain we ended up with.
     for (unsigned i = 0, e = ChainNodesMatched.size(); i != e; ++i) {
       SDNode *ChainNode = ChainNodesMatched[i];
+      // If ChainNode is null, it's because we replaced it on a previous
+      // iteration and we cleared it out of the map. Just skip it.
+      if (!ChainNode)
+        continue;
+
       assert(ChainNode->getOpcode() != ISD::DELETED_NODE &&
              "Deleted node left in chain");
 
@@ -2272,6 +2277,11 @@ void SelectionDAGISel::UpdateChains(
       if (ChainVal.getValueType() == MVT::Glue)
         ChainVal = ChainVal.getValue(ChainVal->getNumValues()-2);
       assert(ChainVal.getValueType() == MVT::Other && "Not a chain?");
+      SelectionDAG::DAGNodeDeletedListener NDL(
+          *CurDAG, [&](SDNode *N, SDNode *E) {
+            std::replace(ChainNodesMatched.begin(), ChainNodesMatched.end(), N,
+                         static_cast<SDNode *>(nullptr));
+          });
       CurDAG->ReplaceAllUsesOfValueWith(ChainVal, InputChain);
 
       // If the node became dead and we haven't already seen it, delete it.

diff --git a/test/CodeGen/SystemZ/pr31710.ll b/test/CodeGen/SystemZ/pr31710.ll
new file mode 100644 (file)
index 0000000..1123827
--- /dev/null
+++ b/test/CodeGen/SystemZ/pr31710.ll
@@ -0,0 +1,39 @@
+; RUN: llc < %s -mtriple=s390x-redhat-linux | FileCheck %s
+;
+; Triggers a path in SelectionDAG's UpdateChains where a node is
+; deleted but we try to read it later (pr31710), invoking UB in
+; release mode or hitting an assert if they're enabled.
+
+; CHECK: btldata:
+define void @btldata(i64* %u0, i32** %p0, i32** %p1, i32** %p3, i32** %p5, i32** %p7) {
+entry:
+  %x0 = load i32*, i32** %p0, align 8, !tbaa !0
+  store i64 0, i64* %u0, align 8, !tbaa !4
+  %x1 = load i32*, i32** %p1, align 8, !tbaa !0
+  %x2 = load i32, i32* %x1, align 4, !tbaa !6
+  %x2ext = sext i32 %x2 to i64
+  store i32 %x2, i32* %x1, align 4, !tbaa !6
+  %x3 = load i32*, i32** %p3, align 8, !tbaa !0
+  %ptr = getelementptr inbounds i32, i32* %x3, i64 %x2ext
+  %x4 = load i32, i32* %ptr, align 4, !tbaa !6
+  %x4inc = add nsw i32 %x4, 1
+  store i32 %x4inc, i32* %ptr, align 4, !tbaa !6
+  store i64 undef, i64* %u0, align 8, !tbaa !4
+  %x5 = load i32*, i32** %p5, align 8, !tbaa !0
+  %x6 = load i32, i32* %x5, align 4, !tbaa !6
+  store i32 %x6, i32* %x5, align 4, !tbaa !6
+  %x7 = load i32*, i32** %p7, align 8, !tbaa !0
+  %x8 = load i32, i32* %x7, align 4, !tbaa !6
+  %x8inc = add nsw i32 %x8, 1
+  store i32 %x8inc, i32* %x7, align 4, !tbaa !6
+  ret void
+}
+
+!0 = !{!1, !1, i64 0}
+!1 = !{!"any pointer", !2, i64 0}
+!2 = !{!"omnipotent char", !3, i64 0}
+!3 = !{!"Simple C/C++ TBAA"}
+!4 = !{!5, !5, i64 0}
+!5 = !{!"long", !2, i64 0}
+!6 = !{!7, !7, i64 0}
+!7 = !{!"int", !2, i64 0}