[Lldb-commits] [lldb] [lldb] Prefer exact symbol match when looking up by address (PR #172055)

[via lldb-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Augusto Noronha (augusto2112)

<details>
<summary>Changes</summary>

The current behavior will pick the first symbol that contains the address, this causes LLDB to pick the wrong symbol when looking for swift reflection metadata on Linux, as in that case it is valid for a symbol to completely encompass another one.

Instead, this function should prefer the symbol which is an exact, if it exists. As a bonus, this should also be faster in the vast majority of the cases, as we probably query symbols by their exact address most of the time.

rdar://166344740

---
Full diff: https://github.com/llvm/llvm-project/pull/172055.diff


2 Files Affected:

- (modified) lldb/source/Core/Module.cpp (+13-11) 
- (modified) lldb/unittests/Core/ModuleTest.cpp (+47) 


``````````diff
diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp
index 466340b0e0990..a543d44e25c4a 100644
--- a/lldb/source/Core/Module.cpp
+++ b/lldb/source/Core/Module.cpp
@@ -494,17 +494,19 @@ uint32_t Module::ResolveSymbolContextForAddress(
         !(resolved_flags & eSymbolContextSymbol)) {
       Symtab *symtab = symfile->GetSymtab();
       if (symtab && so_addr.IsSectionOffset()) {
-        Symbol *matching_symbol = nullptr;
-
-        symtab->ForEachSymbolContainingFileAddress(
-            so_addr.GetFileAddress(),
-            [&matching_symbol](Symbol *symbol) -> bool {
-              if (symbol->GetType() != eSymbolTypeInvalid) {
-                matching_symbol = symbol;
-                return false; // Stop iterating
-              }
-              return true; // Keep iterating
-            });
+        addr_t file_address = so_addr.GetFileAddress();
+        Symbol *matching_symbol = symtab->FindSymbolAtFileAddress(file_address);
+
+        if (!matching_symbol) {
+          symtab->ForEachSymbolContainingFileAddress(
+              file_address, [&matching_symbol](Symbol *symbol) -> bool {
+                if (symbol->GetType() != eSymbolTypeInvalid) {
+                  matching_symbol = symbol;
+                  return false; // Stop iterating
+                }
+                return true; // Keep iterating
+              });
+        }
         sc.symbol = matching_symbol;
         if (!sc.symbol && resolve_scope & eSymbolContextFunction &&
             !(resolved_flags & eSymbolContextFunction)) {
diff --git a/lldb/unittests/Core/ModuleTest.cpp b/lldb/unittests/Core/ModuleTest.cpp
index 011554d1b0939..bcaeede367bdd 100644
--- a/lldb/unittests/Core/ModuleTest.cpp
+++ b/lldb/unittests/Core/ModuleTest.cpp
@@ -123,3 +123,50 @@ TEST(ModuleTest, FindFunctionsCppMangledName) {
   ASSERT_EQ(name, "std::vector<int>::size()");
   ASSERT_EQ(result.GetLanguage(), eLanguageTypeC_plus_plus);
 }
+
+TEST(ModuleTest, ResolveSymbolContextForAddressExactMatch) {
+  // Test that ResolveSymbolContextForAddress prefers exact symbol matches
+  // over symbols that merely contain the address.
+  SubsystemRAII<FileSystem, HostInfo, ObjectFileELF, SymbolFileSymtab>
+      subsystems;
+
+  auto ExpectedFile = TestFile::fromYaml(R"(
+--- !ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_DYN
+  Machine:         EM_X86_64
+Sections:
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:         0x1000
+    AddressAlign:    0x10
+    Size:            0x200
+Symbols:
+  - Name:            outer_function
+    Type:            STT_FUNC
+    Section:         .text
+    Value:           0x1000
+    Size:            0x100
+  - Name:            inner_function
+    Type:            STT_FUNC
+    Section:         .text
+    Value:           0x1050
+    Size:            0x10
+...
+)");
+  ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
+
+  auto module_sp = std::make_shared<Module>(ExpectedFile->moduleSpec());
+
+  Address addr(module_sp->GetSectionList()->GetSectionAtIndex(0), 0x50);
+  SymbolContext sc;
+  uint32_t resolved =
+      module_sp->ResolveSymbolContextForAddress(addr, eSymbolContextSymbol, sc);
+
+  ASSERT_TRUE(resolved & eSymbolContextSymbol);
+  ASSERT_NE(sc.symbol, nullptr);
+  EXPECT_STREQ(sc.symbol->GetName().GetCString(), "inner_function");
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/172055