Skip to content

[lldb] Change GetStartSymbol to GetStartAddress in DynamicLoader #99909

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 22, 2024

Conversation

medismailben
Copy link
Member

@medismailben medismailben commented Jul 22, 2024

On linux, the start address doesn't necessarily have a symbol attached to it.

This is why this patch replaces DynamicLoader::GetStartSymbol with DynamicLoader::GetStartAddress instead to make it more generic.

On linux, the start address doesn't necessarily have a symbol attached
to it.

This is why this patch replaces `DynamicLoader::GetStartSymbol` with
`DynamicLoader::GetStartAddress` instead.

Signed-off-by: Med Ismail Bennani <[email protected]>
@llvmbot
Copy link
Member

llvmbot commented Jul 22, 2024

@llvm/pr-subscribers-lldb

Author: Med Ismail Bennani (medismailben)

Changes

On linux, the start address doesn't necessarily have a symbol attached to it.

This is why this patch replaces DynamicLoader::GetStartSymbol with DynamicLoader::GetStartAddress instead.


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

3 Files Affected:

  • (modified) lldb/include/lldb/Target/DynamicLoader.h (+5-4)
  • (modified) lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp (+2-2)
  • (modified) lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h (+2-2)
diff --git a/lldb/include/lldb/Target/DynamicLoader.h b/lldb/include/lldb/Target/DynamicLoader.h
index 15384245194b0..0629e2faae7e9 100644
--- a/lldb/include/lldb/Target/DynamicLoader.h
+++ b/lldb/include/lldb/Target/DynamicLoader.h
@@ -9,8 +9,8 @@
 #ifndef LLDB_TARGET_DYNAMICLOADER_H
 #define LLDB_TARGET_DYNAMICLOADER_H
 
+#include "lldb/Core/Address.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"
@@ -25,6 +25,7 @@ namespace lldb_private {
 class ModuleList;
 class Process;
 class SectionList;
+class Symbol;
 class SymbolContext;
 class SymbolContextList;
 class Thread;
@@ -329,10 +330,10 @@ 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.
-  /// This is the symbol the process will begin executing with
+  /// Return the `start` \b address in the dynamic loader module.
+  /// This is the address the process will begin executing with
   /// `process launch --stop-at-entry`.
-  virtual std::optional<lldb_private::Symbol> GetStartSymbol() {
+  virtual std::optional<lldb_private::Address> GetStartAddress() {
     return std::nullopt;
   }
 
diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
index 5c6331735bde8..3863b6b3520db 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.cpp
@@ -609,7 +609,7 @@ void DynamicLoaderDarwin::UpdateDYLDImageInfoFromNewImageInfo(
   }
 }
 
-std::optional<lldb_private::Symbol> DynamicLoaderDarwin::GetStartSymbol() {
+std::optional<lldb_private::Address> DynamicLoaderDarwin::GetStartAddress() {
   Log *log = GetLog(LLDBLog::DynamicLoader);
 
   auto log_err = [log](llvm::StringLiteral err_msg) -> std::nullopt_t {
@@ -626,7 +626,7 @@ std::optional<lldb_private::Symbol> DynamicLoaderDarwin::GetStartSymbol() {
   if (!symbol)
     return log_err("Cannot find `start` symbol in DYLD module.");
 
-  return *symbol;
+  return symbol->GetAddress();
 }
 
 void DynamicLoaderDarwin::SetDYLDModule(lldb::ModuleSP &dyld_module_sp) {
diff --git a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h
index 4ac55fdf6f3af..3613c4c29b178 100644
--- a/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h
+++ b/lldb/source/Plugins/DynamicLoader/MacOSX-DYLD/DynamicLoaderDarwin.h
@@ -56,6 +56,8 @@ class DynamicLoaderDarwin : public lldb_private::DynamicLoader {
 
   virtual bool NeedToDoInitialImageFetch() = 0;
 
+  std::optional<lldb_private::Address> GetStartAddress() override;
+
 protected:
   void PrivateInitialize(lldb_private::Process *process);
 
@@ -67,8 +69,6 @@ class DynamicLoaderDarwin : public lldb_private::DynamicLoader {
   // Clear method for classes derived from this one
   virtual void DoClear() = 0;
 
-  std::optional<lldb_private::Symbol> GetStartSymbol() override;
-
   void SetDYLDModule(lldb::ModuleSP &dyld_module_sp);
 
   lldb::ModuleSP GetDYLDModule();

@medismailben medismailben merged commit bb8a740 into llvm:main Jul 22, 2024
6 of 7 checks passed
medismailben added a commit to medismailben/llvm-project that referenced this pull request Jul 22, 2024
…m#99909)

On linux, the start address doesn't necessarily have a symbol attached
to it.

This is why this patch replaces `DynamicLoader::GetStartSymbol` with
`DynamicLoader::GetStartAddress` instead to make it more generic.

Signed-off-by: Med Ismail Bennani <[email protected]>
(cherry picked from commit bb8a740)
Copy link
Collaborator

@labath labath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
)

Summary:
On linux, the start address doesn't necessarily have a symbol attached
to it.

This is why this patch replaces `DynamicLoader::GetStartSymbol` with
`DynamicLoader::GetStartAddress` instead to make it more generic.

Signed-off-by: Med Ismail Bennani <[email protected]>

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251309
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants