[PATCH] D25601: DAGCombiner: fix use-after-free when merging consecutive stores

[Nicolai Hähnle via llvm-commits]

nhaehnle added a comment.

This doesn't reduce the number of merges. The patch has 0 changes in any of the test/CodeGen tests across all backends, and I don't have any out-of-tree tests with changes either. There is no real functional change.

Even before this patch, the code satisfied: The store nodes that are deleted for the merge are exactly a prefix of the StoreNodes vector. The patch (a) truncates the StoreNodes vector to that prefix and (b) passes the vector out so that we can safely decide whether ST was deleted without implicitly relying on the behaviour of the SDNode recycler.

Look, I'll do you one better and do a llvm-lit test/CodeGen run with the following change in a debug build:

  if (any_of(StoreNodes,
             [ST](const MemOpLink &Link) { return Link.MemNode == ST; })) {
    assert(ST->getOpcode() == ISD::DELETED_NODE);
    break;
  }
  
  assert(ST->getOpcode() != ISD::DELETED_NODE);

Of course the assertions re-introduce the problematic reliance on the recycler behaviour, but the point is that neither assertion ever triggers on any of the in-tree CodeGen tests.


https://reviews.llvm.org/D25601