[llvm] r250692 - [GlobalsAA] Fix a really horrible iterator invalidation bug

[Sanjoy Das via llvm-commits]

James Molloy wrote:
> Me neither. I was debugging with Release+Asserts, if that helps.
>
>> On 19 Oct 2015, at 10:34, Sanjoy Das<sanjoy at playingwithpointers.com>  wrote:
>>
>> Did this bug trip manage to trip one of the EpochTracker related
>> asserts?  EpochTracker is supposed to eagerly catch issues like these
>> without actually needing the DenseMap resize.

I just read the code -- the bug was clobbering only a reference, not an 
iterator (which is what you said in the commit message); so the 
EpochTracker checks did not fire.

```
     FunctionInfo &FI = FunctionInfos[SCC[0]->getFunction()];
```

-- Sanjoy


>>
>> -- Sanjoy
>>
>> On Mon, Oct 19, 2015 at 1:55 AM, James Molloy via llvm-commits
>> <llvm-commits at lists.llvm.org>  wrote:
>>> Author: jamesm
>>> Date: Mon Oct 19 03:54:59 2015
>>> New Revision: 250692
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=250692&view=rev
>>> Log:
>>> [GlobalsAA] Fix a really horrible iterator invalidation bug
>>>
>>> We were keeping a reference to an object in a DenseMap then mutating it. At the end of the function we were attempting to clone that reference into other keys in the DenseMap, but DenseMap may well decide to resize its hashtable which would invalidate the reference!
>>>
>>> It took an extremely complex testcase to catch this - many thanks to Zhendong Su for catching it in PR25225.
>>>
>>> This fixes PR25225.
>>>
>>> Modified:
>>>     llvm/trunk/lib/Analysis/GlobalsModRef.cpp
>>>
>>> Modified: llvm/trunk/lib/Analysis/GlobalsModRef.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/GlobalsModRef.cpp?rev=250692&r1=250691&r2=250692&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/Analysis/GlobalsModRef.cpp (original)
>>> +++ llvm/trunk/lib/Analysis/GlobalsModRef.cpp Mon Oct 19 03:54:59 2015
>>> @@ -587,8 +587,11 @@ void GlobalsAAResult::AnalyzeCallGraph(C
>>>
>>>      // Finally, now that we know the full effect on this SCC, clone the
>>>      // information to each function in the SCC.
>>> +    // FI is a reference into FunctionInfos, so copy it now so that it doesn't
>>> +    // get invalidated if DenseMap decides to re-hash.
>>> +    FunctionInfo CachedFI = FI;
>>>      for (unsigned i = 1, e = SCC.size(); i != e; ++i)
>>> -      FunctionInfos[SCC[i]->getFunction()] = FI;
>>> +      FunctionInfos[SCC[i]->getFunction()] = CachedFI;
>>>    }
>>> }
>>>
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>>
>> --
>> Sanjoy Das
>> http://playingwithpointers.com
>>
>
>
> ________________________________
>
> -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
>