[Lldb-commits] [lldb] b6b3fcd - [lldb] Don't iterate over a std::set<Type*> in SymbolFileDWARF::GetTypes to make it deterministic

[Raphael “Teemperor” Isemann via lldb-commits]

I was just grepping for unordered data structures (e.g. ’std::set<‘) that use pointers/pointer-like objects (e.g. CompilerType with its operator<) and then reading the related code. Not sure if there is a good way to detect this stuff automatically. I guess we could have had a Clang plugin that creates a warning when code iterates over an unordered data structure that has a pointer-like type as a key (probably would cause a bunch of false-positives but if someone ran this on his own machine from time to time that would be enough I think).

> On Mar 2, 2020, at 3:07 PM, Davide Italiano <ditaliano at apple.com> wrote:
> 
> This is really good. How did you find it, Raphael?
> I generally use LLVM_REVERSE_ITERATOR but given this wasn’t a container in llvm, I’m curious to hear.
> 
> —
> Davide
> 
>> On Mar 2, 2020, at 15:04, Raphael Isemann via lldb-commits <lldb-commits at lists.llvm.org> wrote:
>> 
>> 
>> Author: Raphael Isemann
>> Date: 2020-03-02T15:03:45-08:00
>> New Revision: b6b3fcdcb8cdfb887e26d27bee03b997d2d65888
>> 
>> URL: https://github.com/llvm/llvm-project/commit/b6b3fcdcb8cdfb887e26d27bee03b997d2d65888
>> DIFF: https://github.com/llvm/llvm-project/commit/b6b3fcdcb8cdfb887e26d27bee03b997d2d65888.diff
>> 
>> LOG: [lldb] Don't iterate over a std::set<Type*> in SymbolFileDWARF::GetTypes to make it deterministic
>> 
>> Summary:
>> Currently `SymbolFileDWARF::TypeSet` is a typedef to a `std::set<Type *>`.
>> In `SymbolFileDWARF::GetTypes` we iterate over a TypeSet variable when finding
>> types so that logic is non-deterministic as it depends on the actual pointer address values.
>> 
>> This patch changes the `TypeSet` to a `llvm::UniqueVector` which always iterates in
>> the order in which we inserted the types into the list.
>> 
>> Reviewers: JDevlieghere, aprantl
>> 
>> Reviewed By: JDevlieghere
>> 
>> Subscribers: mgrang, abidh, lldb-commits
>> 
>> Tags: #lldb
>> 
>> Differential Revision: https://reviews.llvm.org/D75481
>> 
>> Added: 
>> 
>> 
>> Modified: 
>>   lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
>>   lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
>> 
>> Removed: 
>> 
>> 
>> 
>> ################################################################################
>> diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
>> index c89ccb5bf960..c27b5c4c3495 100644
>> --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
>> +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
>> @@ -324,10 +324,8 @@ void SymbolFileDWARF::GetTypes(const DWARFDIE &die, dw_offset_t min_die_offset,
>>      if (add_type) {
>>        const bool assert_not_being_parsed = true;
>>        Type *type = ResolveTypeUID(die, assert_not_being_parsed);
>> -        if (type) {
>> -          if (type_set.find(type) == type_set.end())
>> -            type_set.insert(type);
>> -        }
>> +        if (type)
>> +          type_set.insert(type);
>>      }
>>    }
>> 
>> 
>> diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
>> index a3928c8c3dd4..479235c0d86f 100644
>> --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
>> +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
>> @@ -12,11 +12,11 @@
>> #include <list>
>> #include <map>
>> #include <mutex>
>> -#include <set>
>> #include <unordered_map>
>> #include <vector>
>> 
>> #include "llvm/ADT/DenseMap.h"
>> +#include "llvm/ADT/SetVector.h"
>> #include "llvm/Support/Threading.h"
>> 
>> #include "lldb/Core/UniqueCStringMap.h"
>> @@ -439,7 +439,7 @@ class SymbolFileDWARF : public lldb_private::SymbolFile,
>> 
>>  bool FixupAddress(lldb_private::Address &addr);
>> 
>> -  typedef std::set<lldb_private::Type *> TypeSet;
>> +  typedef llvm::SetVector<lldb_private::Type *> TypeSet;
>> 
>>  void GetTypes(const DWARFDIE &die, dw_offset_t min_die_offset,
>>                dw_offset_t max_die_offset, uint32_t type_mask,
>> 
>> 
>> 
>> _______________________________________________
>> lldb-commits mailing list
>> lldb-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>