-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Add support for using foreign type units in .debug_names. #87740
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
3f99b41
d122c3a
91c3093
7cdf24a
843e0be
04363ba
46cb76b
e0fd069
720f654
6f786cc
49eb373
9a1d8cf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1727,45 +1727,52 @@ lldb::ModuleSP SymbolFileDWARF::GetExternalModule(ConstString name) { | |
return pos->second; | ||
} | ||
|
||
DWARFDIE | ||
SymbolFileDWARF::GetDIE(const DIERef &die_ref) { | ||
// This method can be called without going through the symbol vendor so we | ||
// need to lock the module. | ||
std::lock_guard<std::recursive_mutex> guard(GetModuleMutex()); | ||
|
||
SymbolFileDWARF *symbol_file = nullptr; | ||
|
||
SymbolFileDWARF *SymbolFileDWARF::GetDIERefSymbolFile(const DIERef &die_ref) { | ||
// Anytime we get a "lldb::user_id_t" from an lldb_private::SymbolFile API we | ||
// must make sure we use the correct DWARF file when resolving things. On | ||
// MacOSX, when using SymbolFileDWARFDebugMap, we will use multiple | ||
// SymbolFileDWARF classes, one for each .o file. We can often end up with | ||
// references to other DWARF objects and we must be ready to receive a | ||
// "lldb::user_id_t" that specifies a DIE from another SymbolFileDWARF | ||
// instance. | ||
|
||
std::optional<uint32_t> file_index = die_ref.file_index(); | ||
if (file_index) { | ||
if (SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile()) { | ||
symbol_file = debug_map->GetSymbolFileByOSOIndex(*file_index); // OSO case | ||
if (symbol_file) | ||
return symbol_file->DebugInfo().GetDIE(die_ref.section(), | ||
die_ref.die_offset()); | ||
return DWARFDIE(); | ||
} | ||
|
||
// If the file index matches, then we have the right SymbolFileDWARF already. | ||
// This will work for both .dwo file and DWARF in .o files for mac. Also if | ||
// both the file indexes are invalid, then we have a match. | ||
if (GetFileIndex() == file_index) | ||
return this; | ||
|
||
if (file_index) { | ||
// We have a SymbolFileDWARFDebugMap, so let it find the right file | ||
\ if (SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile()) | ||
return debug_map->GetSymbolFileByOSOIndex(*file_index); | ||
|
||
// Handle the .dwp file case correctly | ||
if (*file_index == DIERef::k_file_index_mask) | ||
symbol_file = GetDwpSymbolFile().get(); // DWP case | ||
else | ||
symbol_file = this->DebugInfo() | ||
.GetUnitAtIndex(*die_ref.file_index()) | ||
->GetDwoSymbolFile(); // DWO case | ||
} else if (die_ref.die_offset() == DW_INVALID_OFFSET) { | ||
return DWARFDIE(); | ||
return GetDwpSymbolFile().get(); // DWP case | ||
|
||
// Handle the .dwo file case correctly | ||
return DebugInfo().GetUnitAtIndex(*die_ref.file_index()) | ||
->GetDwoSymbolFile(); // DWO case | ||
} | ||
return this; | ||
} | ||
|
||
if (symbol_file) | ||
return symbol_file->GetDIE(die_ref); | ||
DWARFDIE | ||
SymbolFileDWARF::GetDIE(const DIERef &die_ref) { | ||
if (die_ref.die_offset() == DW_INVALID_OFFSET) | ||
return DWARFDIE(); | ||
|
||
return DebugInfo().GetDIE(die_ref.section(), die_ref.die_offset()); | ||
// This method can be called without going through the symbol vendor so we | ||
// need to lock the module. | ||
std::lock_guard<std::recursive_mutex> guard(GetModuleMutex()); | ||
SymbolFileDWARF *symbol_file = GetDIERefSymbolFile(die_ref); | ||
if (symbol_file) | ||
Comment on lines
+1771
to
+1772
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar feedback others have given elsewhere, roll the variable declaration into the |
||
return symbol_file->DebugInfo().GetDIE(die_ref.section(), | ||
die_ref.die_offset()); | ||
return DWARFDIE(); | ||
} | ||
|
||
/// Return the DW_AT_(GNU_)dwo_id. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -241,6 +241,15 @@ class SymbolFileDWARF : public SymbolFileCommon { | |
return m_external_type_modules; | ||
} | ||
|
||
/// Given a DIERef, find the correct SymbolFileDWARF. | ||
/// | ||
/// A DIERef contains a file index that can uniquely identify a N_OSO file for | ||
/// DWARF in .o files on mac, or a .dwo or .dwp file index for split DWARF. | ||
/// Calling this function will find the correct symbol file to use so that | ||
/// further lookups can be done on the correct symbol file so that the DIE | ||
/// offset makes sense in the DIERef. | ||
virtual SymbolFileDWARF *GetDIERefSymbolFile(const DIERef &die_ref); | ||
|
||
virtual DWARFDIE GetDIE(const DIERef &die_ref); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now GetDIE is virtual and handles SymbolFile resolution, this one doesn't need to be. |
||
|
||
DWARFDIE GetDIE(lldb::user_id_t uid); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only uses the file_index component of the DIERef, how about we just pass that (and call the function something like
GetSymbolFileByIndex
)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It only uses the file index component, but that component might not be set in the
DIERef
object and if it isn't, it returns the current SymbolFileDWARF object. We can make a function like:The file index only makes sense if it comes from a DIERef object as it understands the value and knows the magic value for the .dwp file, so the above API is much less clear as who else knows what a file index is and would actaully know to ask for the symbol file using such an index? Seems less clear to me than the existing API. Let me know if feel we should still change this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the file index needs to necessarily come from a DIERef. I can imagine other scenarios where it might be interesting to ask for a symbol file, but I think we can deal with it when the time comes.