[Lldb-commits] [lldb] [llvm] Add support for using foreign type units in .debug_names. (PR #87740)

[Pavel Labath via lldb-commits]

labath wrote:

> BOLT does as I mentioned in: [#87740 (comment)](https://github.com/llvm/llvm-project/pull/87740#issuecomment-2060023287)

I'm sorry, I missed that part. I'm bit of late to the party here.

> 
> It will put all the CUs/Tus into one one module, thus for foreign TUs both DW_IDX_compile_unit and DW_IDX_type_unit is needed.

Right, the CU is necessary to find the DWO file containing the type unit. I forgot about that. That resolves _some_ of my questions. However, I still find the current implementation of `getCUOffset` strange, namely that:
- if the entry contains a `DW_IDX_compile_unit` (perhaps because we have a multi-cu index), it returns that CU, regardless of anything else
- if it does not contain the attribute (and we have a single-CU index), it returns that CU *only* if the index also does not contain a DW_IDX_type_unit (even though the standard says "In a per-CU index, index entries without this attribute implicitly refer to the single CU")

This doesn't seem particularly useful. I think it should either always return CU offset (regardless of whether it's implicit or explicit) or *only* return it if the DW_IDX_type_unit is not present. And of these two, I think the first one is better, because the caller can always explicitly check for the presence of the type attribute, and it avoids the need to introduce another function (which is needed because we now have a caller who wants the CU offset even though the entry refers to a type unit).

>From what I can see, that type unit check was added relatively recently (#72952), and it was to support this piece of code in lldb:
```
+  // Look for a DWARF unit offset (CU offset or local TU offset) as they are
+  // both offsets into the .debug_info section.
+  std::optional<uint64_t> unit_offset = entry.getCUOffset();
+  if (!unit_offset) {
+    unit_offset = entry.getLocalTUOffset();
+    if (!unit_offset)
+      return std::nullopt;
+  }
```
If we just reversed the order of those checks in lldb, then it wouldn't matter that this function return a CU offset even if the index entry does not refer to a DIE in that CU.

Or am I (still) missing something?

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