Skip to content

Commit 2f63718

Browse files
authored
[lldb] Don't clear a Module's UnwindTable when adding a SymbolFile (#86603)
Fixing a crash in lldb when `symbols.auto-download` setting is enabled. When doing a backtrace, this feature has lldb search for a SymbolFile for stack frames when we are backtracing, and add them either synchoronously or asynchronously, depending on the specific setting used. Module::SetSymbolFileFileSpec clears the Module's UnwindTable, once we find a new SymbolFile. We may be adding a source of unwind information that we did not have when lldb was working only with the executable binary. What happens in practice is that we're using a reference to the Module's UnwindTable, and then the other thread getting the SymbolFile clears it and now the first thread is referring to freed memory and we can crash. When built with address sanitizer, it crashes much more reliably. Given that unwind information used for exception handling -- eh_frame, compact unwind -- is present in executable binaries, the only thing we're likely to *add* would be DWARF's `debug_frame` if that was also available. The actual value of re-creating the UnwindTable when we have added a SymbolFile is not large. I also tried fixing this by changing the Module to have a shared_ptr to the UnwindTable, so we could have two different UnwindTable's in use simultaneously for a brief period. This would be fine TODAY, but it introduces a very subtle bug that someone will have a heck of a time figuring out in the future. In the end, I believe the safest approach is to sacrifice the possible marginal gain of reconstructing the UnwindTable once a SymbolFile has been added, to sidestep this whole problem area. Also, in `Module::GetUnwindTable()`, call `DownloadSymbolFileAsync` before we create the UnwindTable for the first time, in case the symbol file is fetched synchronously, we will have it for that possible marginal gain.
1 parent f50f0ca commit 2f63718

File tree

1 file changed

+1
-6
lines changed

1 file changed

+1
-6
lines changed

lldb/source/Core/Module.cpp

+1-6
Original file line numberDiff line numberDiff line change
@@ -1239,9 +1239,9 @@ void Module::SectionFileAddressesChanged() {
12391239

12401240
UnwindTable &Module::GetUnwindTable() {
12411241
if (!m_unwind_table) {
1242-
m_unwind_table.emplace(*this);
12431242
if (!m_symfile_spec)
12441243
SymbolLocator::DownloadSymbolFileAsync(GetUUID());
1244+
m_unwind_table.emplace(*this);
12451245
}
12461246
return *m_unwind_table;
12471247
}
@@ -1359,15 +1359,10 @@ void Module::SetSymbolFileFileSpec(const FileSpec &file) {
13591359
// one
13601360
obj_file->ClearSymtab();
13611361

1362-
// Clear the unwind table too, as that may also be affected by the
1363-
// symbol file information.
1364-
m_unwind_table.reset();
1365-
13661362
// The symbol file might be a directory bundle ("/tmp/a.out.dSYM")
13671363
// instead of a full path to the symbol file within the bundle
13681364
// ("/tmp/a.out.dSYM/Contents/Resources/DWARF/a.out"). So we need to
13691365
// check this
1370-
13711366
if (FileSystem::Instance().IsDirectory(file)) {
13721367
std::string new_path(file.GetPath());
13731368
std::string old_path(obj_file->GetFileSpec().GetPath());

0 commit comments

Comments
 (0)