[all-commits] [llvm/llvm-project] 9258f3: [lldb] Fix a crash when using .dwp files and make ...

[Greg Clayton via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 9258f3e692493a69e0f4755bb129a5391ef10b50
      https://github.com/llvm/llvm-project/commit/9258f3e692493a69e0f4755bb129a5391ef10b50
  Author: Greg Clayton <gclayton at fb.com>
  Date:   2024-02-01 (Thu, 01 Feb 2024)

  Changed paths:
    M lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
    M lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
    M lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
    M lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
    M lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
    M lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
    M lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
    M lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
    M lldb/test/Shell/SymbolFile/DWARF/x86/dwp-separate-debug-file.cpp

  Log Message:
  -----------
  [lldb] Fix a crash when using .dwp files and make type lookup reliable with the index cache (#79544)

When using split DWARF with .dwp files we had an issue where sometimes
the DWO file within the .dwp file would be parsed _before_ the skeleton
compile unit. The DWO file expects to be able to always be able to get a
link back to the skeleton compile unit. Prior to this fix, the only time
the skeleton compile unit backlink would get set, was if the unit
headers for the main executable have been parsed _and_ if the unit DIE
was parsed in that DWARFUnit. This patch ensures that we can always get
the skeleton compile unit for a DWO file by adding a function:

```
DWARFCompileUnit *DWARFUnit::GetSkeletonUnit();
```

Prior to this fix DWARFUnit had some unsafe accessors that were used to
store two different things:

```
  void *DWARFUnit::GetUserData() const;
  void DWARFUnit::SetUserData(void *d);
```

This was used by SymbolFileDWARF to cache the `lldb_private::CompileUnit
*` for a SymbolFileDWARF and was also used to store the `DWARFUnit *`
for SymbolFileDWARFDwo. This patch clears up this unsafe usage by adding
two separate accessors and ivars for this:
```
lldb_private::CompileUnit *DWARFUnit::GetLLDBCompUnit() const { return m_lldb_cu; }
void DWARFUnit::SetLLDBCompUnit(lldb_private::CompileUnit *cu) { m_lldb_cu = cu; }
DWARFCompileUnit *DWARFUnit::GetSkeletonUnit();
void DWARFUnit::SetSkeletonUnit(DWARFUnit *skeleton_unit);
```
This will stop anyone from calling `void *DWARFUnit::GetUserData()
const;` and casting the value to an incorrect value.

A crash could occur in `SymbolFileDWARF::GetCompUnitForDWARFCompUnit()`
when the `non_dwo_cu`, which is a backlink to the skeleton compile unit,
was not set and was NULL. There is an assert() in the code, and then the
code just will kill the program if the assert isn't enabled because the
code looked like:
```
  if (dwarf_cu.IsDWOUnit()) {
    DWARFCompileUnit *non_dwo_cu =
        static_cast<DWARFCompileUnit *>(dwarf_cu.GetUserData());
    assert(non_dwo_cu);
    return non_dwo_cu->GetSymbolFileDWARF().GetCompUnitForDWARFCompUnit(
        *non_dwo_cu);
  }
```
This is now fixed by calling the `DWARFUnit::GetSkeletonUnit()` which
will correctly always get the skeleton compile uint for a DWO file
regardless of if the skeleton unit headers have been parse or if the
skeleton unit DIE wasn't parsed yet.

To implement the ability to get the skeleton compile units, I added code
the DWARFDebugInfo.cpp/.h that make a map of DWO ID -> skeleton
DWARFUnit * that gets filled in for DWARF5 when the unit headers are
parsed. The `DWARFUnit::GetSkeletonUnit()` will end up parsing the unit
headers of the main executable to fill in this map if it already hasn't
been done. For DWARF4 and earlier we maintain a separate map that gets
filled in only for any DWARF4 compile units that have a DW_AT_dwo_id or
DW_AT_gnu_dwo_id attributes. This is more expensive, so this is done
lazily and in a thread safe manor. This allows us to be as efficient as
possible when using DWARF5 and also be backward compatible with DWARF4 +
split DWARF.

There was also an issue that stopped type lookups from succeeding in
`DWARFDIE SymbolFileDWARF::GetDIE(const DIERef &die_ref)` where it
directly was accessing the `m_dwp_symfile` ivar without calling the
accessor function that could end up needing to locate and load the .dwp
file. This was fixed by calling the
`SymbolFileDWARF::GetDwpSymbolFile()` accessor to ensure we always get a
valid value back if we can find the .dwp file. Prior to this fix it was
down which APIs were called and if any APIs were called that loaded the
.dwp file, it worked fine, but it might not if no APIs were called that
did cause it to get loaded.

When we have valid debug info indexes and when the lldb index cache was
enabled, this would cause this issue to show up more often.

I modified an existing test case to test that all of this works
correctly and doesn't crash.