`.debug_names` should index the DIE that has a parent chain

[dcci]

#include <iostream>
namespace google {
class LogMessageVoidify {
 public:
  LogMessageVoidify() { }
  // This has to be an operator with a precedence lower than << but
  // higher than ?:
  void operator&(std::ostream&) { }
};
#define LOG(level) \
    !(level) ? (void) 0 : google::LogMessageVoidify() & std::cerr
#define INFO 1
#define DEBUG 0
}

int main() {
  LOG(INFO) << "This is an informational message." << std::endl;
  return 0;
}

Compile with

clang++ -std=c++17 -pthread -glldb -gdwarf-5 -fno-debug-types-section -gpubnames patatino.cpp -fuse-ld=lld -o pat

llvm-dwarfdump shows two duplicate entries for _ZN6google17LogMessageVoidifyanERSo, one with parent namespace, the other without.
In the debug_names mapping the entry that gets indexed is the one without parent namespace. We believe the entry with parent namespace should be indexed/should have precedence, if possible, so that lldb can walk back the parent chain and perform faster lookups.

(note: we've been seeing on large projects lldb taking 3 minutes to print an expression using the expression evaluator due to this lack of filtering).

Thoughts on this? @JDevlieghere / @adrian-prantl / @dwblaikie

[llvmbot]

@llvm/issue-subscribers-debuginfo

Author: Davide Italiano (dcci)

``` #include <iostream> namespace google { class LogMessageVoidify { public: LogMessageVoidify() { } // This has to be an operator with a precedence lower than << but // higher than ?: void operator&(std::ostream&) { } }; #define LOG(level) \ !(level) ? (void) 0 : google::LogMessageVoidify() & std::cerr #define INFO 1 #define DEBUG 0 }

int main() {
LOG(INFO) << "This is an informational message." << std::endl;
return 0;
}

Compile with

clang++ -std=c++17 -pthread -glldb -gdwarf-5 -fno-debug-types-section -gpubnames patatino.cpp -fuse-ld=lld -o pat

`llvm-dwarfdump` shows two duplicate entries, one with parent namespace, the other without.
In the debug_names mapping the entry that gets indexed is the one without parent namespace. We believe the entry with parent namespace should be indexed/should have precedence, if possible, so that lldb can walk back the parent chain and perform faster lookups.

(note: we've been seeing on large projects lldb taking 3 minutes to print an expression using the expression evaluator due to this lack of filtering).

Thoughts on this? @<!-- -->JDevlieghere / @<!-- -->adrian-prantl / @<!-- -->dwblaikie 
</details>

[dwblaikie]

Didn't actually specify what name... but looking at the output ( https://godbolt.org/z/s6v5KdEnj )
it seems this is the one you're interested in?

   Name 16 {
      Hash: 0xFAB3B506
      String: 0x000000a2 "LogMessageVoidify"
      Entry @ 0x30f {
        Abbrev: 0x8
        Tag: DW_TAG_class_type
        DW_IDX_die_offset: 0x00000050
        DW_IDX_parent: Entry @ 0x370
      }
      Entry @ 0x318 {
        Abbrev: 0x2
        Tag: DW_TAG_subprogram
        DW_IDX_die_offset: 0x00000646
        DW_IDX_parent: <parent not indexed>
      }
    }

See if we can reduce this a bit...

[dcci]

@dwblaikie sorry, this is the name I had in mind:

_ZN6google17LogMessageVoidifyanERSo

[dwblaikie]

Bringing it down to ( https://godbolt.org/z/35KjWGe63 )

namespace google {
struct LogMessageVoidify {
  LogMessageVoidify() { }
};
}

int main() {
  google::LogMessageVoidify();
}

The two entries are for the type and the ctor definition (ctor's name is the same as the type). Because we generate the definition DIE at the global scope, as if the user wrote this:

namespace google {
struct LogMessageVoidify {
  LogMessageVoidify();
};
}

google::LogMessageVoidify() { }

int main() {
  google::LogMessageVoidify();
}

Which they could've - and we should consider what the index should do there. Since the user can't find the entity by name in the global scope (without a qualifier) it's probably misleading to index it there.

It should probably be indexed based on the scope it's actually declared in/can be found via name lookup in.

You'd observe the same problem even without member functions, and without the synthetic DWARF spec location, probably:

namespace ns {
void f1();
}
ns::f1() {
}

Ah, nope in this case ( https://godbolt.org/z/87x4PdfbM ) we do put the definition DIE in the namespace (not matching the source, but it's shorter/simpler) - GCC does /always/ put it out of line, even if the user puts the definition inside the namespace ( https://godbolt.org/z/5n9a8s1aT - but GCC doesn't support .debug_names yet, so we don't know which DIE it would index - the def or the decl, but the spec says the def... )

The spec says:

For the purposes of determining whether a debugging information entry has a particular attribute (such as DW_AT_name), if debugging information entry A has a DW_AT_specification or DW_AT_abstract_origin attribute pointing to another debugging information entry B, any attributes of B are considered to be part of A.

But it doesn't talk about the parent scope of the DIE. The spec would read as though it would be the DIE that is the parent of the DIE being indexed - but I think that doesn't handle out of line definitions.

Spec should probably say that the parent is the parent of the DIE specified by an entity's DW_AT_specification or DW_AT_abstract_origin (or both, etc) DIE.

[dwblaikie]

@dwblaikie sorry, this is the name I had in mind:

_ZN6google17LogMessageVoidifyanERSo

That's only got one result though, right?

   Name 3 {
      Hash: 0x7395A510
      String: 0x000000b4 "_ZN6google17LogMessageVoidifyanERSo"
      Entry @ 0x2b9 {
        Abbrev: 0x2
        Tag: DW_TAG_subprogram
        DW_IDX_die_offset: 0x00000662
        DW_IDX_parent: <parent not indexed>
      }
    }

[clayborg]

But it doesn't talk about the parent scope of the DIE. The spec would read as though it would be the DIE that is the parent of the DIE being indexed - but I think that doesn't handle out of line definitions.

Spec should probably say that the parent is the parent of the DIE specified by an entity's DW_AT_specification or DW_AT_abstract_origin (or both, etc) DIE.

I agree. If each .debug_names entry has a correct parent chain by knowing it should index any DW_AT_specification or DW_AT_abstract_origin for the actual declaration context, then it will fix this performance issue.