[cfe-commits] r160855 - in /cfe/trunk: lib/Sema/SemaLookup.cpp test/SemaCXX/pr13394-crash-on-invalid.cpp

[Benjamin Kramer]

On 27.07.2012, at 20:00, Eli Friedman <eli.friedman at gmail.com> wrote:

> On Fri, Jul 27, 2012 at 3:21 AM, Benjamin Kramer
> <benny.kra at googlemail.com> wrote:
>> Author: d0k
>> Date: Fri Jul 27 05:21:08 2012
>> New Revision: 160855
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=160855&view=rev
>> Log:
>> Fix PR13394: Erasing from a vector changes the end of the vector, so make sure we always have the right end.
>> 
>> Added:
>>    cfe/trunk/test/SemaCXX/pr13394-crash-on-invalid.cpp
>> Modified:
>>    cfe/trunk/lib/Sema/SemaLookup.cpp
>> 
>> Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=160855&r1=160854&r2=160855&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaLookup.cpp Fri Jul 27 05:21:08 2012
>> @@ -3895,13 +3895,13 @@
>>       // If a validator callback object was given, drop the correction
>>       // unless it passes validation.
>>       bool Viable = false;
>> -      for (TypoResultList::iterator RI = I->second.begin(), RIEnd = I->second.end();
>> -           RI != RIEnd; /* Increment in loop. */) {
>> +      for (TypoResultList::iterator RI = I->second.begin();
>> +           RI != I->second.end(); /* Increment in loop. */) {
>>         TypoResultList::iterator Prev = RI;
>>         ++RI;
>>         if (Prev->isResolved()) {
>>           if (!isCandidateViable(CCC, *Prev))
>> -            I->second.erase(Prev);
>> +            RI = I->second.erase(Prev);
>>           else
>>             Viable = true;
>>         }
> 
> I only looked briefly before, but I think there's another iterator
> invalidation issue nearby.

The others are erasing from a StringMap, this one was a SmallVector. It's still nasty code but the iterator invalidation characteristics of StringMap::erase (just zeroing out the bucket) should be safe for this use case.

- Ben