[lldb][ELF] Move address class map into the symbol table

Code in llvm#90622 exposed a situation
where a symbol table may be loaded via a second copy of the same object.

This resulted in the first copy having its address class map updated, but
not the second. And it was that second one we used for symbol lookups,
since it was found by a plugin and we assume those to be more specific.

(the plugins returning the same file may itself be a bug)

I tried fixing this by returning the address map changes from the
symbol table parsing. Which works but it's awkward to make sure
both object's maps get updated.

Instead, this change moves the address class map into the symbol table,
which is shared between the objects already. ObjectFileELF::GetAddressClass
was already checking whether the symbol table existed anyway,
so we are ok to use it.

Author: DavidSpickett

lldb/include/lldb/Symbol/Symtab.h

@@ -23,6 +23,8 @@ class Symtab {
 public:
   typedef std::vector<uint32_t> IndexCollection;
   typedef UniqueCStringMap<uint32_t> NameToIndexMap;
+  typedef std::map<lldb::addr_t, lldb_private::AddressClass>
+      FileAddressToAddressClassMap;
 
   enum Debug {
     eDebugNo,  // Not a debug symbol
@@ -239,6 +241,16 @@ class Symtab {
   }
   /// \}
 
+  /// Set the address class for the given address.
+  void SetAddressClass(lldb::addr_t addr,
+                       lldb_private::AddressClass addr_class);
+
+  /// Lookup the address class of the given address.
+  ///
+  /// \return
+  ///   The address' class, if it is known, otherwise AddressClass::eCode.
+  lldb_private::AddressClass GetAddressClass(lldb::addr_t addr);
+
 protected:
   typedef std::vector<Symbol> collection;
   typedef collection::iterator iterator;
@@ -274,6 +286,9 @@ class Symtab {
   collection m_symbols;
   FileRangeToIndexMap m_file_addr_to_index;
 
+  /// The address class for each symbol in the elf file
+  FileAddressToAddressClassMap m_address_class_map;
+
   /// Maps function names to symbol indices (grouped by FunctionNameTypes)
   std::map<lldb::FunctionNameType, UniqueCStringMap<uint32_t>>
       m_name_to_symbol_indices;

lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp

@@ -769,17 +769,7 @@ AddressClass ObjectFileELF::GetAddressClass(addr_t file_addr) {
   if (res != AddressClass::eCode)
     return res;
 
-  auto ub = m_address_class_map.upper_bound(file_addr);
-  if (ub == m_address_class_map.begin()) {
-    // No entry in the address class map before the address. Return default
-    // address class for an address in a code section.
-    return AddressClass::eCode;
-  }
-
-  // Move iterator to the address class entry preceding address
-  --ub;
-
-  return ub->second;
+  return symtab->GetAddressClass(file_addr);
 }
 
 size_t ObjectFileELF::SectionIndex(const SectionHeaderCollIter &I) {
@@ -2213,18 +2203,18 @@ unsigned ObjectFileELF::ParseSymbols(Symtab *symtab, user_id_t start_id,
             switch (mapping_symbol) {
             case 'a':
               // $a[.<any>]* - marks an ARM instruction sequence
-              m_address_class_map[symbol.st_value] = AddressClass::eCode;
+              symtab->SetAddressClass(symbol.st_value, AddressClass::eCode);
               break;
             case 'b':
             case 't':
               // $b[.<any>]* - marks a THUMB BL instruction sequence
               // $t[.<any>]* - marks a THUMB instruction sequence
-              m_address_class_map[symbol.st_value] =
-                  AddressClass::eCodeAlternateISA;
+              symtab->SetAddressClass(symbol.st_value,
+                                      AddressClass::eCodeAlternateISA);
               break;
             case 'd':
               // $d[.<any>]* - marks a data item sequence (e.g. lit pool)
-              m_address_class_map[symbol.st_value] = AddressClass::eData;
+              symtab->SetAddressClass(symbol.st_value, AddressClass::eData);
               break;
             }
           }
@@ -2238,11 +2228,11 @@ unsigned ObjectFileELF::ParseSymbols(Symtab *symtab, user_id_t start_id,
             switch (mapping_symbol) {
             case 'x':
               // $x[.<any>]* - marks an A64 instruction sequence
-              m_address_class_map[symbol.st_value] = AddressClass::eCode;
+              symtab->SetAddressClass(symbol.st_value, AddressClass::eCode);
               break;
             case 'd':
               // $d[.<any>]* - marks a data item sequence (e.g. lit pool)
-              m_address_class_map[symbol.st_value] = AddressClass::eData;
+              symtab->SetAddressClass(symbol.st_value, AddressClass::eData);
               break;
             }
           }
@@ -2260,11 +2250,11 @@ unsigned ObjectFileELF::ParseSymbols(Symtab *symtab, user_id_t start_id,
             // conjunction with symbol.st_value to produce the final
             // symbol_value that we store in the symtab.
             symbol_value_offset = -1;
-            m_address_class_map[symbol.st_value ^ 1] =
-                AddressClass::eCodeAlternateISA;
+            symtab->SetAddressClass(symbol.st_value ^ 1,
+                                    AddressClass::eCodeAlternateISA);
           } else {
             // This address is ARM
-            m_address_class_map[symbol.st_value] = AddressClass::eCode;
+            symtab->SetAddressClass(symbol.st_value, AddressClass::eCode);
           }
         }
       }
@@ -2285,17 +2275,19 @@ unsigned ObjectFileELF::ParseSymbols(Symtab *symtab, user_id_t start_id,
       */
       if (arch.IsMIPS()) {
         if (IS_MICROMIPS(symbol.st_other))
-          m_address_class_map[symbol.st_value] = AddressClass::eCodeAlternateISA;
+          symtab->SetAddressClass(symbol.st_value,
+                                  AddressClass::eCodeAlternateISA);
         else if ((symbol.st_value & 1) && (symbol_type == eSymbolTypeCode)) {
           symbol.st_value = symbol.st_value & (~1ull);
-          m_address_class_map[symbol.st_value] = AddressClass::eCodeAlternateISA;
+          symtab->SetAddressClass(symbol.st_value,
+                                  AddressClass::eCodeAlternateISA);
         } else {
           if (symbol_type == eSymbolTypeCode)
-            m_address_class_map[symbol.st_value] = AddressClass::eCode;
+            symtab->SetAddressClass(symbol.st_value, AddressClass::eCode);
           else if (symbol_type == eSymbolTypeData)
-            m_address_class_map[symbol.st_value] = AddressClass::eData;
+            symtab->SetAddressClass(symbol.st_value, AddressClass::eData);
           else
-            m_address_class_map[symbol.st_value] = AddressClass::eUnknown;
+            symtab->SetAddressClass(symbol.st_value, AddressClass::eUnknown);
         }
       }
     }
@@ -3060,10 +3052,10 @@ void ObjectFileELF::ParseSymtab(Symtab &lldb_symtab) {
       if (arch.GetMachine() == llvm::Triple::arm &&
           (entry_point_file_addr & 1)) {
         symbol.GetAddressRef().SetOffset(entry_point_addr.GetOffset() ^ 1);
-        m_address_class_map[entry_point_file_addr ^ 1] =
-            AddressClass::eCodeAlternateISA;
+        lldb_symtab.SetAddressClass(entry_point_file_addr ^ 1,
+                                    AddressClass::eCodeAlternateISA);
       } else {
-        m_address_class_map[entry_point_file_addr] = AddressClass::eCode;
+        lldb_symtab.SetAddressClass(entry_point_file_addr, AddressClass::eCode);
       }
       lldb_symtab.AddSymbol(symbol);
     }

lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.h

@@ -187,9 +187,6 @@ class ObjectFileELF : public lldb_private::ObjectFile {
   typedef DynamicSymbolColl::iterator DynamicSymbolCollIter;
   typedef DynamicSymbolColl::const_iterator DynamicSymbolCollConstIter;
 
-  typedef std::map<lldb::addr_t, lldb_private::AddressClass>
-      FileAddressToAddressClassMap;
-
   /// Version of this reader common to all plugins based on this class.
   static const uint32_t m_plugin_version = 1;
   static const uint32_t g_core_uuid_magic;
@@ -227,9 +224,6 @@ class ObjectFileELF : public lldb_private::ObjectFile {
   /// The architecture detected from parsing elf file contents.
   lldb_private::ArchSpec m_arch_spec;
 
-  /// The address class for each symbol in the elf file
-  FileAddressToAddressClassMap m_address_class_map;
-
   /// Returns the index of the given section header.
   size_t SectionIndex(const SectionHeaderCollIter &I);
 

lldb/source/Symbol/Symtab.cpp

@@ -1372,3 +1372,22 @@ bool Symtab::LoadFromCache() {
     SetWasLoadedFromCache();
   return result;
 }
+
+void Symtab::SetAddressClass(lldb::addr_t addr,
+                             lldb_private::AddressClass addr_class) {
+  m_address_class_map.insert_or_assign(addr, addr_class);
+}
+
+lldb_private::AddressClass Symtab::GetAddressClass(lldb::addr_t addr) {
+  auto ub = m_address_class_map.upper_bound(addr);
+  if (ub == m_address_class_map.begin()) {
+    // No entry in the address class map before the address. Return default
+    // address class for an address in a code section.
+    return AddressClass::eCode;
+  }
+
+  // Move iterator to the address class entry preceding address
+  --ub;
+
+  return ub->second;
+}