<br><br><div class="gmail_quote">On Thu, Oct 18, 2012 at 11:59 PM, Robert Muth <span dir="ltr"><<a href="mailto:robertm@google.com" target="_blank">robertm@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">On Thu, Oct 18, 2012 at 4:48 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
> On Thu, Oct 18, 2012 at 1:45 PM, Douglas Gregor <<a href="mailto:dgregor@apple.com">dgregor@apple.com</a>> wrote:<br>
>><br>
>> On Oct 18, 2012, at 9:57 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
>><br>
>>> Author: dblaikie<br>
>>> Date: Thu Oct 18 11:57:32 2012<br>
>>> New Revision: 166188<br>
>>><br>
>>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=166188&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=166188&view=rev</a><br>
>>> Log:<br>
>>> PR14021: Copy lookup results to ensure safe iteration.<br>
>>><br>
>>> Within the body of the loop the underlying map may be modified via<br>
>>><br>
>>>  Sema::AddOverloadCandidate<br>
>>>    -> Sema::CompareReferenceRelationship<br>
>>>    -> Sema::RequireCompleteType<br>
>>><br>
>>> to avoid the use of invalid iterators the sequence is copied first.<br>
>><br>
>> Did you audit other uses of LookupConstructors to ensure that this is the only ticking time bomb in this area?<br>
><br>
> [+Robert Muth]<br>
><br>
> I've not performed any such audit, no. With the hack debug check<br>
> inserted we could just run the whole test suite to see if anything<br>
> pops up.<br>
<br>
</div>I did not  perform any such audit either. I feel those are only of limited use<br>
and the time would be better spend addressing the root of the issue.<br>
If there is interest I could  try add some diagnostic code to the DenseMap<br>
which would only be enabled in Debug mode.<br>
We kicked around some ideas over lunch and here was one suggestion<br>
that could work:<br>
add an atomic timestamp to each DenseMap.<br>
increment the time step whenever iterators are invalidated (e.g. insertions),<br>
Each iterator also holds a copy of the timestamp form when it was created<br>
so we can compare it against the parent container timestamp when\ever<br>
the iterator is used.<br>
I have not thought this through all this much, so it may not work in the end.<br>
Any feedback would be welcome.<br></blockquote></div><br><br>I remember using such a trick. It covers the requiring of invalidating iterators quite well. Also, since it requires embedding a pointer to the original container within the iterator it also makes it possible whenever comparing two iterators whether they originated from the same container (comparing iterators from different containers does not make sense).<br>
<br>However it seriously increases the footprint of the iterator (triples its size) and makes it impossible to mix Debug and Release libraries (because the types end up being different).<br><br>-- Matthieu<br>