[PATCH] D157267: [NewGVN] Fix an use after free when updating use count

[Vladimir Radosavljevic via Phabricator via llvm-commits]

vladimirradosavljevic added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/NewGVN.cpp:4119
           if (isSSACopy) {
             unsigned &IIUseCount = UseCounts[II];
             if (--IIUseCount == 0)
----------------
kmitropoulou wrote:
> The test case does not crash in my workspace. Anyway, I think the problem is here. If II is not in the UseCounts map, then it just adds it and it returns zero. Will the following change solve the problem?
> 
> if (isSSACopy) {
>     auto It = UseCounts.find(II);
>     if (It != UseCounts.end()) {
>         unsigned &IIUseCount = It->second;
>         if (--IIUseCount == 0)
>              ProbablyDead.insert(II);
>     }
> }
> 
Could you please try to run it with valgrind/sanitizers, because it happens occasionally and this is why it is such a dangerous bug.
Yes, proposed solution fixes the problem. Please note that I'm not familiar with the NewGVN pass, but if II is not in UseCounts map, should we also insert it into ProbablyDead?


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

https://reviews.llvm.org/D157267