[Lldb-commits] [lldb] 1d6243d - [lldb] Fix symbol table use after free

[Augusto Noronha via lldb-commits]

Author: Augusto Noronha
Date: 2023-01-09T10:27:18-08:00
New Revision: 1d6243db90b09c61d78a14268bb88a73792b63ab

URL: https://github.com/llvm/llvm-project/commit/1d6243db90b09c61d78a14268bb88a73792b63ab
DIFF: https://github.com/llvm/llvm-project/commit/1d6243db90b09c61d78a14268bb88a73792b63ab.diff

LOG: [lldb] Fix symbol table use after free

The symbol file stores a raw pointer to the main object file's symbol
table. This pointer, however, can be freed, if ObjectFile::ClearSymtab
is ever called. This patch makes sure out pointer to the symbol file
is valid before using it.

Added: 
    

Modified: 
    lldb/include/lldb/Symbol/SymbolFile.h
    lldb/source/Symbol/SymbolFile.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Symbol/SymbolFile.h b/lldb/include/lldb/Symbol/SymbolFile.h
index d5fe0331fe5a8..4b5499304664b 100644
--- a/lldb/include/lldb/Symbol/SymbolFile.h
+++ b/lldb/include/lldb/Symbol/SymbolFile.h
@@ -504,7 +504,6 @@ class SymbolFileCommon : public SymbolFile {
                                    // file)
   std::optional<std::vector<lldb::CompUnitSP>> m_compile_units;
   TypeList m_type_list;
-  Symtab *m_symtab = nullptr;
   uint32_t m_abilities = 0;
   bool m_calculated_abilities = false;
   bool m_index_was_loaded_from_cache = false;
@@ -517,6 +516,10 @@ class SymbolFileCommon : public SymbolFile {
 private:
   SymbolFileCommon(const SymbolFileCommon &) = delete;
   const SymbolFileCommon &operator=(const SymbolFileCommon &) = delete;
+
+  /// Do not use m_symtab directly, as it may be freed. Use GetSymtab()
+  /// to access it instead.
+  Symtab *m_symtab = nullptr;
 };
 
 } // namespace lldb_private

diff  --git a/lldb/source/Symbol/SymbolFile.cpp b/lldb/source/Symbol/SymbolFile.cpp
index c7af908543e88..b271efd07bfe3 100644
--- a/lldb/source/Symbol/SymbolFile.cpp
+++ b/lldb/source/Symbol/SymbolFile.cpp
@@ -164,16 +164,15 @@ SymbolFile::RegisterInfoResolver::~RegisterInfoResolver() = default;
 
 Symtab *SymbolFileCommon::GetSymtab() {
   std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
-  if (m_symtab)
-    return m_symtab;
-
   // Fetch the symtab from the main object file.
-  m_symtab = GetMainObjectFile()->GetSymtab();
-
-  // Then add our symbols to it.
-  if (m_symtab)
-    AddSymbols(*m_symtab);
+  auto *symtab = GetMainObjectFile()->GetSymtab();
+  if (m_symtab != symtab) {
+    m_symtab = symtab;
 
+    // Then add our symbols to it.
+    if (m_symtab)
+      AddSymbols(*m_symtab);
+  }
   return m_symtab;
 }
 
@@ -186,8 +185,8 @@ void SymbolFileCommon::SectionFileAddressesChanged() {
   ObjectFile *symfile_objfile = GetObjectFile();
   if (symfile_objfile != module_objfile)
     symfile_objfile->SectionFileAddressesChanged();
-  if (m_symtab)
-    m_symtab->SectionFileAddressesChanged();
+  if (auto *symtab = GetSymtab())
+    symtab->SectionFileAddressesChanged();
 }
 
 uint32_t SymbolFileCommon::GetNumCompileUnits() {