[PATCH] D72550: [SCCIterator] Fix another potential use-after-free

[Keno Fischer via Phabricator via llvm-commits]

loladiro created this revision.
loladiro added reviewers: dexonsmith, vchuravy, lebedev.ri.
Herald added subscribers: llvm-commits, hiraditya, eraman.
Herald added a project: LLVM.

This is a follow up to https://reviews.llvm.org/D72529. Since this
code is a bit old, I decided to see if there were any similar issues
hiding and believe I have found one. The Inliner transform deletes
CallGraphNodes, but doesn't remove the deleted node from the scc
iterator. At first glance, one might think this is fine, because
we don't access the CallGraphNodes through that pointer and it
simply serves as an indication whether or not we have visited
a given CallGraphNode. However, there is a subtle problem:
The pointer of the CallGraphNode we have freed may be re-used
by a later allocation of a new CallGraphNode. This could now
present a problem, because the scc iterator will associate the
old state with the new CallGraphNode. Fix that by adding the
ability to inform the scc iterator of node deletion.

I suspect this situation never actually causes a problem in
practice because the sequence of events (node deleted -> node
reallocated with the same pointer -> node accessed in CFG
walk) is quite long and may not actually lead to a crash or
even incorrect code (I believe the symptom would be a missing
node in the SCC). I'd like to get it fixed anyway. As a bonus,
I believe we can remove an outstanding todo from the Inliner.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72550

Files:
  llvm/include/llvm/ADT/SCCIterator.h
  llvm/include/llvm/Analysis/CallGraphSCCPass.h
  llvm/lib/Analysis/CallGraphSCCPass.cpp
  llvm/lib/Transforms/IPO/Inliner.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D72550.237469.patch
Type: text/x-patch
Size: 5412 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200111/3f97f4b9/attachment.bin>