[Lldb-commits] [lldb] Improve type and namespace lookup using parent chain (PR #108907)

[Pavel Labath via lldb-commits]

================
@@ -374,25 +377,40 @@ void DebugNamesDWARFIndex::GetFullyQualifiedType(
   m_fallback.GetFullyQualifiedType(context, callback);
 }
 
+bool DebugNamesDWARFIndex::SameAsEntryATName(
+    llvm::StringRef query_name, const DebugNames::Entry &entry) const {
+  auto maybe_dieoffset = entry.getDIEUnitOffset();
+  if (!maybe_dieoffset)
+    return false;
+
+  // [Optimization] instead of parsing the entry from dwo file, we simply
+  // check if the query_name can point to an entry of the same DIE offset.
+  // This greatly reduced number of dwo file parsed and thus improved the
+  // performance.
+  for (const DebugNames::Entry &query_entry :
+       entry.getNameIndex()->equal_range(query_name)) {
+    auto query_dieoffset = query_entry.getDIEUnitOffset();
+    if (!query_dieoffset)
+      continue;
+
+    if (*query_dieoffset == *maybe_dieoffset) {
+      return true;
+    } else if (*query_dieoffset > *maybe_dieoffset) {
+      // The pool entries of the same name are sequentially cluttered together
+      // so if the query name from `query_name` is after the target entry, this
+      // is definitely not the correct one, we can stop searching.
----------------
labath wrote:

I think you're reading too much into the specification. That's definitely not how I would interpret those sections.

The whole of page 141 is devoted to describing the physical layout of the debug_names section, and that's the context in which I'd read that sentence. It just says that entries in the entry pool should follow one after the other -- i.e., with no intervening padding. It doesn't say anything about the debug info entries (in the debug_info) section which are referred to by the debug_names entries. In fact, at this point in the spec, it hasn't even been established how the index entries refer to DIEs -- that happens six pages later (somewhere around your second quote).

However, the second quote basically restates the same thing. There can be no gaps between entries for the same name (which makes sense, as you couldn't parse them otherwise -- you reach the next one by passing over the first one), but there *can* be gaps between different names (and that also makes sense -- you reach the first entry by following the pointer from the name table, so it does not have to be glued to the previous list).

The non-normative text after it confirms this theory:
```
For example, a producer/consumer combination may find it useful to maintain alignment.
```

The padding is there to (optionally) maintain alignment of the entries *in the debug_names section.* You can't maintain alignment by sorting entries.

In addition, this sorting of entries by die offset is very arbitrary and a remnant (introduced by me) from the apple tables. Since the DIE offset is relative to the CU start, it means that entries from different CUs would be interleaved just because they have interleaving relative offsets.

Bottom line: I'm certain that if the DWARF spec wanted to guarantee this sorting, it would use a much more obvious language, and I suspect that it would choose a different sorting key (e.g. group all entries from the same CU and *then* sort by offset).

I think this part of the patch should be split off to a separate PR.

https://github.com/llvm/llvm-project/pull/108907