[PATCH] D111112: [SCCPSolver] Fix use-after-free in markArgInFuncSpecialization.

[duk via Phabricator via llvm-commits]

duck-37 added a comment.

In D111112#3041838 <https://reviews.llvm.org/D111112#3041838>, @SjoerdMeijer wrote:

> Thanks for looking into this!
>
> I think I may have been bitten by this before (or something similar), but it's easy to forget the details here.... So perhaps you can help me a bit by expanding a on this:
>
>> In SCCPSolver::markArgInFuncSpecialization, the ValueState map may be reallocated *after* the initial ValueLatticeElement reference is grabbed, but *before* its use in copy initialization.
>
> I know the `operator[]` is kind of considered harmful for maps, but in this case that seemed fine and doesn't seem the cause of the problem, is that right? So perhaps you can expand on the reallocation (of the map?) that I don't quite get.

Sure! The issue here isn't `operator[]` itself, it's the order in which the operations here are done. The `DenseMap` class differs from `std::unordered_map` in a few ways; the important one here is that references can be invalidated if you insert a new element, due to the fact that everything is in one block of memory <https://llvm.org/docs/ProgrammersManual.html#llvm-adt-densemap-h>.
What happens here is that the code takes a reference to `ValueState[I]`, then tries to index into `ValueState[J]`. Since `J` doesn't exist in the `ValueState` map, the DenseMap may be resized to accommodate it. After this, the reference to `ValueState[I]` points to invalid or garbage memory. 
The copy assignment operator for `ValueState[J]` is then called with the now-invalid reference to `ValueState[I]`, causing all sorts of fun things to happen.

The fix here is just to make sure that `ValueState[J]` exists in the map *before* indexing into `ValueState[I]` so that no references get invalidated.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111112/new/

https://reviews.llvm.org/D111112