[llvm-dev] Should analyses be able to hold AssertingVH to IR? (related to PR28400)
Sanjoy Das via llvm-dev
llvm-dev at lists.llvm.org
Mon Jul 11 19:51:51 PDT 2016
Sean Silva wrote:
> While building test-suite with the new PM, I ran into problems with
> AssertingVH being triggered which is obvious in retrospect:
> Both cases I ran into revolve around LVI which holds AssertingVH.
> Essentially, what happens is this:
> 1. LVI holds an AssertingVH pointing at a BasicBlock
> 2. Some other pass ends up deleting that BB (e.g. SimplifyCFG)
> 3. BOOM
> Notice that this ends up happening even though SimplifyCFG ultimately
> invalidates LVI. But that only happens after the AssertingVH has been
> We would avoid this issue in principle by just switching those
> AssertingVH to observing pointers, but then we would have dangling
> pointers. I'm pretty sure that any analysis that keeps pointers to IR is
> in fact ending up with dangling pointers like this.
> I've noticed that AssumptionCache using CallbackVH that allow it to
> update its data structures correctly in this case.
> Just using regular pointers will end up with dangling pointers in this
> scenario. This isn't a problem in practice since the analysis will
> hopefully be invalidated and stop holding the dangling pointers, but it
> just seems weird. Thankfully, ASan can generally catch problems if we do
> try to access through any of these dangling pointers.
But asan won't catch problems (insofar I understand how it works) if
the free'ed BasicBlock is used as a key in a DenseMap or something --
if another BasicBlock gets allocated to the same location we'll end up
returning bad cached results.
I think the Right Solution(TM) here is to create a new wrapper around
(Small)DenseMap that in debug mode uses a CallbackVH in a way that:
- The map is "poisoned" if a key is deleted.
- A poisoned map can only be cleared; inserting, removing or looking
up elements in a poisoned map fails a assertions.
If needed, we can allow erasing deleted keys from poisoned maps, but
then we won't catch bugs like:
M[k] = v;
k1 = new Key(); // allocated to the same location as k1
M.erase(k1); // "successful" deletion.
However, this quite a bit of work, and I'm not sure if the payoff will
be worth it.
> Thoughts? For the moment I have put in a workaround (r274457) that makes
> jump-threading invalidate LVI.
> -- Sean Silva
More information about the llvm-dev