[lldb/Target] Add GetStartSymbol method to DynamicLoader plugins

[medismailben]

This patch introduces a new method to the dynamic loader plugin, to fetch its start symbol.

This can be useful to resolve the start symbol address for instance.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Med Ismail Bennani (medismailben)

Changes

This patch introduces a new method to the dynamic loader plugin, to fetch its start symbol.

This can be useful to resolve the start symbol address for instance.

---

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

3 Files Affected:

• (modified) lldb/include/lldb/Target/DynamicLoader.h (+7-1)
• (modified) lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp (+15)
• (modified) lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h (+2)

diff --git a/lldb/include/lldb/Target/DynamicLoader.h b/lldb/include/lldb/Target/DynamicLoader.h
index e508d192d2759..4ce3e053c5b1a 100644
--- a/lldb/include/lldb/Target/DynamicLoader.h
+++ b/lldb/include/lldb/Target/DynamicLoader.h
@@ -10,9 +10,11 @@
 #define LLDB_TARGET_DYNAMICLOADER_H
 
 #include "lldb/Core/PluginInterface.h"
+#include "lldb/Symbol/Symbol.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/UUID.h"
+#include "lldb/Utility/UnimplementedError.h"
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-forward.h"
 #include "lldb/lldb-private-enumerations.h"
@@ -24,7 +26,6 @@ namespace lldb_private {
 class ModuleList;
 class Process;
 class SectionList;
-class Symbol;
 class SymbolContext;
 class SymbolContextList;
 class Thread;
@@ -329,6 +330,11 @@ class DynamicLoader : public PluginInterface {
   /// safe to call certain APIs or SPIs.
   virtual bool IsFullyInitialized() { return true; }
 
+  /// Return the `start` function \b symbol in the dynamic loader module.
+  virtual llvm::Expected<lldb_private::Symbol> GetStartSymbol() {
+    return llvm::make_error<UnimplementedError>();
+  }
+
 protected:
   // Utility methods for derived classes
 
diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
index 0e17d57fa9c4f..f4a61fd02d93c 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -609,6 +609,21 @@ void DynamicLoaderDarwin::UpdateDYLDImageInfoFromNewImageInfo(
   }
 }
 
+llvm::Expected<lldb_private::Symbol> DynamicLoaderDarwin::GetStartSymbol() {
+  ModuleSP dyld_sp = GetDYLDModule();
+  if (!dyld_sp)
+    return llvm::createStringError(
+        "Couldn't retrieve DYLD module. Cannot get `start` symbol.");
+
+  const Symbol *symbol =
+      dyld_sp->FindFirstSymbolWithNameAndType(ConstString("_dyld_start"));
+  if (!symbol)
+    return llvm::createStringError(
+        "Cannot find `start` symbol in DYLD module.");
+
+  return *symbol;
+}
+
 void DynamicLoaderDarwin::SetDYLDModule(lldb::ModuleSP &dyld_module_sp) {
   m_dyld_module_wp = dyld_module_sp;
 }
diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h
index 8f9a29c94173f..0cdaaf6532972 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h
@@ -67,6 +67,8 @@ class DynamicLoaderDarwin : public lldb_private::DynamicLoader {
   // Clear method for classes derived from this one
   virtual void DoClear() = 0;
 
+  llvm::Expected<lldb_private::Symbol> GetStartSymbol() override;
+
   void SetDYLDModule(lldb::ModuleSP &dyld_module_sp);
 
   lldb::ModuleSP GetDYLDModule();

[bulbazord]

Looks fine to me, but you probably want @jasonmolenda to sign off to be sure.

[jasonmolenda]

Should this be an llvm::Expected? It's only going to be defined for DynamicLoaderDarwin today. If some generic code wanted to use this, where it may not be applicable or defined for another DynamicLoader plugin, it'll need to consume the UnimplementedError. Given the optionality of it, a std::optional seems more natural to me maybe?

[medismailben]

In my understanding, every dynamic loader should have an equivalent to a start symbol, that's why I went with Expected instead of optional. I believe that long term, each dynamic loader plugin would have to implement this method for their own platform and only the base class would have the UnimplementedError.

[jasonmolenda]

I would DEFINITELY not bank on this assumption. The world of environments a debugger needs to support is vast.

[jasonmolenda]

LGTM.

[jasonmolenda]

We can remove this header file include now.

[medismailben]

Yup!

[labath]

I'd like to note that, at least in the ELF world, there's no guarantee that there will be an actual symbol at the address where the process starts executing. The convention is to call this _start but nothing in the system actually relies on that. The ELF header contains the address of the first instruction, the OS jumps to that, and that's it.

So, I think that a more generic interface would be to return an Address (which the caller can try to resolve to a symbol if it wants to).

[jimingham]

I agree with Pavel, we don't for most purposes care whether there's a symbol at the start address (for instance we can use it for the "expression evaluation return breakpoint" as an address. So we shouldn't require the symbol. Jim

…

On Jul 22, 2024, at 12:04 AM, Pavel Labath ***@***.***> wrote: I'd like to note that, at least in the ELF world, there's no guarantee that there will be an actual symbol at the address where the process starts executing. The convention is to call this _start but nothing in the system actually relies on that. The ELF header contains the address of the first instruction, the OS jumps to that, and that's it. So, I think that a more generic interface would be to return an Address (which the caller can try to resolve to a symbol if it wants to). — Reply to this email directly, view it on GitHub <#99673 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADUPVW7ZFE2ZXN3SJI7ZSMTZNSVILAVCNFSM6AAAAABLE7M55SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENBSGIZTKNJYGU>. You are receiving this because you are on a team that was mentioned.

[medismailben]

@labath @jimingham I've addressed your comment in #99909