[Lldb-commits] [PATCH] D51578: Contiguous .debug_info+.debug_types for D32167

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil marked 2 inline comments as done.
jankratochvil added a comment.

There is a `_dwarf` -> `_dwarf_type_units` regression for: `-f TestCppTypeLookup.test_namespace_only_dwarf_type_units`

  ./bin/clang -o 1 ../llvm-git/tools/lldb/packages/Python/lldbsuite/test/lang/cpp/type_lookup/main.cpp -g -fdebug-types-section;./bin/lldb -o 'settings set target.experimental.inject-local-vars false' -o 'target create "./1"' -o 'b 66' -o r -o 'p *((in_contains_type *)&i)'

Expected:

  (lldb) p *((in_contains_type *)&i)
  error: use of undeclared identifier 'in_contains_type'
  error: expected expression

Actual:

  (lldb) p *((in_contains_type *)&i)
  (in_contains_type) $0 = (aaa = 123)

I will check it more.



================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h:65
   typedef std::vector<DWARFUnitSP> CompileUnitColl;
+  typedef std::unordered_map<uint64_t, uint32_t> TypeSignatureMap;
 
----------------
clayborg wrote:
> Use llvm::DenseMap here?
Yes, I agree that is a good idea - changed it (BTW this is your part of the patch).


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:797
+DWARFCompileUnit *DWARFUnit::GetAsCompileUnit() {
+  if (GetUnitDIEOnly().Tag() == DW_TAG_compile_unit)
+    return static_cast<DWARFCompileUnit *>(this);
----------------
clayborg wrote:
> Should this just be "if (GetUnitDIEOnly().Tag() != DW_TAG_type_unit)"? Partial units and many others can be top level DIEs.
When you ask then I disagree. Currently `GetAsCompileUnit()` is used only for checking `DW_TAG_partial_unit::DW_AT_name` (from D11003) which is never present in files by DWZ tool. DWZ tool never puts DIEs referencing any code into `DW_TAG_partial_unit` so D11003 should not be needed; although DWZ intentionally sometimes uses `DW_TAG_partial_unit::DW_AT_stmt_list` but I haven't found when/how it could be useful.
//> many others//
Top level DIEs can be according to DWARF-5 7.5 just `DW_TAG_compile_unit`, `DW_TAG_partial_unit` or `DW_TAG_type_unit`.
So personally I would keep `GetAsCompileUnit()` to do what its name exactly says.



Repository:
  rLLDB LLDB

https://reviews.llvm.org/D51578