[PATCH] [ELF] Use llvm ADT's instead of std.

Duncan P. N. Exon Smith dexonsmith at apple.com
Wed Feb 25 13:16:47 PST 2015


> On 2015 Feb 25, at 13:06, Sanjoy Das <sanjoy at playingwithpointers.com> wrote:
> 
>> These properties are not guaranteed by DenseMap. So, if you add a new item to a map while you are iterating over elements of the map, it may crash. It actually crashes only when the hash table is resized to add a new element for LHS, so it could produce a nasty flaky bug. For example the bug in the LayoutPass was there for more than 1 year until I fixed that.
> 
> A slight tangent:  do you guys think it will be worth the effort to
> turn such errors to deterministically fail an assert in debug mode?
> For instance, I can imagine having an "epoch" counter in the DenseMap
> and DenseMapIterator, with modifications to the DenseMap bumping the
> counter and iterator accesses asserting "ParentDenseMap->epoch ==
> this->epoch" on access.

This is an interesting idea.  Maybe propose something to llvmdev.
Note that `DenseMap::erase()` has partial guarantees about iterator
validity (i.e., other iterators aren't invalidated).

>> 
>> Even
>> 
>> m[x] = m[y]
>> 
>> is not guaranteed to be safe for a DenseMap m, because a reference returned by m[y] can be invalidated by m[x]. This particular one is the bug in LayoutPass.cpp that Reid mentioned above (fix is r213969).
>> 
>> I checked the code quickly, and it looks safe to me, but please review your patch again with the above thing in your mind.
>> 
>> 
>> http://reviews.llvm.org/D7885
>> 
>> EMAIL PREFERENCES
>>  http://reviews.llvm.org/settings/panel/emailpreferences/
>> 
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits





More information about the llvm-commits mailing list