[llvm] r293522 - SDAG: Update ChainNodesMatched during UpdateChains if a node is replaced

[Hans Wennborg via llvm-commits]

Merged to 4.0 in r293650.

On Mon, Jan 30, 2017 at 10:29 AM, Justin Bogner via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: bogner
> Date: Mon Jan 30 12:29:46 2017
> New Revision: 293522
>
> URL: http://llvm.org/viewvc/llvm-project?rev=293522&view=rev
> Log:
> 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
>
> Added:
>     llvm/trunk/test/CodeGen/SystemZ/pr31710.ll
> Modified:
>     llvm/trunk/include/llvm/CodeGen/SelectionDAGISel.h
>     llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
>
> Modified: llvm/trunk/include/llvm/CodeGen/SelectionDAGISel.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/SelectionDAGISel.h?rev=293522&r1=293521&r2=293522&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/SelectionDAGISel.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/SelectionDAGISel.h Mon Jan 30 12:29:46 2017
> @@ -307,7 +307,7 @@ private:
>    std::vector<unsigned> OpcodeOffset;
>
>    void UpdateChains(SDNode *NodeToMatch, SDValue InputChain,
> -                    const SmallVectorImpl<SDNode *> &ChainNodesMatched,
> +                    SmallVectorImpl<SDNode *> &ChainNodesMatched,
>                      bool isMorphNodeTo);
>  };
>
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp?rev=293522&r1=293521&r2=293522&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp Mon Jan 30 12:29:46 2017
> @@ -2308,7 +2308,7 @@ GetVBR(uint64_t Val, const unsigned char
>  /// 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
> @@ -2320,6 +2320,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");
>
> @@ -2332,6 +2337,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.
>
> Added: llvm/trunk/test/CodeGen/SystemZ/pr31710.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/SystemZ/pr31710.ll?rev=293522&view=auto
> ==============================================================================
> --- llvm/trunk/test/CodeGen/SystemZ/pr31710.ll (added)
> +++ llvm/trunk/test/CodeGen/SystemZ/pr31710.ll Mon Jan 30 12:29:46 2017
> @@ -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}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits