[Lldb-commits] [lldb] Fix GetDIE is outside of its CU error from .debug_names (PR #157574)

[via lldb-commits]

================
@@ -131,8 +131,12 @@ DebugNamesDWARFIndex::GetNonSkeletonUnit(const DebugNames::Entry &entry) const {
     unit_offset = entry.getLocalTUOffset();
   if (unit_offset) {
     if (DWARFUnit *cu = m_debug_info.GetUnitAtOffset(DIERef::Section::DebugInfo,
-                                                     *unit_offset))
-      return &cu->GetNonSkeletonUnit();
+                                                     *unit_offset)) {
+      DWARFUnit &ret = cu->GetNonSkeletonUnit();
----------------
jeffreytan81 wrote:

I agree with this in general. Actually, that's the original approach I suggested while discussing with Greg. 

Unfortunately, there are many existing other callers/code paths of `DWARFUnit::GetNonSkeletonUnit` are expecting the default behavior to fallback to return skeleton unit if failing to find dwo files. Changing `DWARFUnit::GetNonSkeletonUnit`'s semantics requiring auditing all other callers/code paths to ensure the behaviors are expected which is a much bigger task than thought. Yesterday, I tried to change all callers of `DWARFUnit::GetNonSkeletonUnit` to use new API/semantics, it is failing several tests. Some tests are related with apple debug names, -gmodules flag, PCH modules containing CU with only dwo_id without dwo_name (resulting in dwo error) which I am not feeling comfortable/justified to fix. 

Overall, I feel fixing this known code path is safer (not failing any tests) with better scope to reason about.

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