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

[Davide Italiano via lldb-commits]

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