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

Nicolai Hähnle via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 3 06:21:53 PDT 2016

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);
  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.


More information about the llvm-commits mailing list