Add 'FindFirstSymbolWithNameAndType()' to ModuleList.

[mbucko]

No description provided.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Miro Bucko (mbucko)

Changes

---

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

2 Files Affected:

• (modified) lldb/include/lldb/Core/ModuleList.h (+4)
• (modified) lldb/source/Core/ModuleList.cpp (+13)

diff --git a/lldb/include/lldb/Core/ModuleList.h b/lldb/include/lldb/Core/ModuleList.h
index 43d931a8447406..29881a967b7a61 100644
--- a/lldb/include/lldb/Core/ModuleList.h
+++ b/lldb/include/lldb/Core/ModuleList.h
@@ -353,6 +353,10 @@ class ModuleList {
 
   lldb::ModuleSP FindFirstModule(const ModuleSpec &module_spec) const;
 
+  const Symbol *
+  FindFirstSymbolWithNameAndType(ConstString name,
+                                 lldb::SymbolType symbol_type) const;
+
   void FindSymbolsWithNameAndType(ConstString name,
                                   lldb::SymbolType symbol_type,
                                   SymbolContextList &sc_list) const;
diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index 2b8ccab2406c6b..3b0ee6bd87b334 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -524,6 +524,19 @@ void ModuleList::FindGlobalVariables(const RegularExpression &regex,
     module_sp->FindGlobalVariables(regex, max_matches, variable_list);
 }
 
+const Symbol *
+ModuleList::FindFirstSymbolWithNameAndType(ConstString name,
+                                           lldb::SymbolType symbol_type) const {
+  std::lock_guard<std::recursive_mutex> guard(m_modules_mutex);
+  for (const ModuleSP &module_sp : m_modules) {
+    const Symbol *symbol =
+        module_sp->FindFirstSymbolWithNameAndType(name, symbol_type);
+    if (symbol)
+      return symbol;
+  }
+  return nullptr;
+}
+
 void ModuleList::FindSymbolsWithNameAndType(ConstString name,
                                             SymbolType symbol_type,
                                             SymbolContextList &sc_list) const {

[Michael137]

Do you have a link to a PR that exercises this new API?

[mbucko]

Do you have a link to a PR that exercises this new API?

I'm only using it in an internal branch

[Michael137]

I'm only using it in an internal branch

Not sure what the protocol is for this (CC @JDevlieghere @labath). At the very least we should have some coverage for it in the test-suite. There's precedent for this, e.g., APIs that only get exercised on the Swift branch, but at least those are on a public fork and tested on public build-bots.

[medismailben]

I'm only using it in an internal branch

Not sure what the protocol is for this (CC @JDevlieghere @labath). At the very least we should have some coverage for it in the test-suite. There's precedent for this, e.g., APIs that only get exercised on the Swift branch, but at least those are on a public fork and tested on public build-bots.

+1, given that the SBAPI needs to remain stable we try to avoid hoisting APIs from (downstream) forks unless there's a use for them upstream.

[labath]

I'm only using it in an internal branch

Not sure what the protocol is for this (CC @JDevlieghere @labath). At the very least we should have some coverage for it in the test-suite. There's precedent for this, e.g., APIs that only get exercised on the Swift branch, but at least those are on a public fork and tested on public build-bots.

I don't really know how to answer this generally, but I'm not particularly concerned about this change, since its just a wrapper over another API and it fits in with the general design of our other APIs. A test would definitely be nice, but I don't know how easy it is to set up (given its an internal API, it would need to go through a unit test, etc.).

I think the easiest way to motivate this change and satisfy the testing requirement would be to find upstream use case(s) for this function -- and it looks like this is a perfect candidate for the simplification of MemoryHistoryAsan::CreateInstance.

[clayborg]

This would be easy to test if we expose this via the public API in SBTarget. We don't expose a FindFirstSymbolWithNameAndType, but we can expose maybe:

lldb::SBSymbol SBTarget::FindFirstSymbol(const char *name, lldb::SymbolType type = eSymbolTypeAny);

Since the target has a module list internally, this can use this new function and test it.

To test:

• create two binaries each with a code symbol and a data symbol with the same name by compiling with clang.
• Create a SBModule via SBModule(const SBModuleSpec &module_spec); for each binary
• create an empty SBTarget and call bool SBTarget::AddModule(lldb::SBModule &module); first with binary 1, and then binary 2, and call the API and sure you get the one from the binary 1 by checking the SBSymbol's SBAddress and getting the module and verifying it is the right one.
• do the same thing as above but add binary 2 first and then binary 1. Make sure you get the symbol from binary 2.