Skip to content

[LLDB] Fix Android debugging #98581

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

Closed
wants to merge 3 commits into from
Closed

[LLDB] Fix Android debugging #98581

wants to merge 3 commits into from

Conversation

splhack
Copy link
Contributor

@splhack splhack commented Jul 12, 2024

Goal

Fix Android debugging

What

This pull request reverts these two commits because it broke Android debugging as explained below.

This pull requests also reverts this commit to avoid conflicts on the revert.

Breakage state

LLDB fails to resolve executable on remote-android platform process attach.

platform select remote-android
platform connect unix-connect://localhost:NNNN/
platform process attach -p NNN

Using logging lldb all

log enable -f /tmp/lldb.log lldb all

Without this pull request,

intern-state     DynamicLoaderPOSIXDYLD::ResolveExecutableModule - got executable by pid 576: /system/bin/foo
intern-state     DynamicLoaderPOSIXDYLD::ResolveExecutableModule - failed to resolve executable with module spec "file = '/system/bin/foo', arch = x86_64-unknown-linux-android": '/system/bin/foo' does not exist

With this pull request,

intern-state     DynamicLoaderPOSIXDYLD::ResolveExecutableModule - got executable by pid 576: /system/bin/foo
intern-state     0x000055C038956E30 Communication::Write (src = 0x0000151FEC010C80, src_len = 135) connection = 0x000055C0384F3AC0
intern-state     0x55c0384f3ac0 ConnectionFileDescriptor::Write (src = 0x151fec010c80, src_len = 135)
intern-state     0x55c038946b80 Socket::Write() (socket = 10, src = 0x151fec010c80, src_len = 135, flags = 0) => 135 (error = (null))
intern-state     0x55c0384f3ac0 ConnectionFileDescriptor::Write(fd = 10, src = 0x151fec010c80, src_len = 135) => 135 (error = (null))
intern-state     this = 0x000055C038956E30, dst = 0x0000151FFA7FB770, dst_len = 8192, timeout = 5000000 us, connection = 0x000055C0384F3AC0
intern-state     this = 0x000055C0384F3AC0, timeout = 5000000 us
intern-state     0x55c038946b80 Socket::Read() (socket = 10, src = 0x151ffa7fb770, src_len = 253, flags = 0) => 253 (error = (null))
intern-state     0x55c0384f3ac0 ConnectionFileDescriptor::Read()  fd = 10, dst = 0x151ffa7fb770, dst_len = 8192) => 253, error = (null)
intern-state     PlatformRemoteGDBServer::GetModuleSpec - got module info for (/system/bin/foo:x86_64-unknown-linux-android) : file = '/system/bin/foo', arch = x86_64-unknown-linux-android, uuid = NNNNNN-NNNN-NNNN-NNNN-NNNNNNNN, object size = NNNN

Breakage detail

From the logs,

intern-state     DynamicLoaderPOSIXDYLD::ResolveExecutableModule - got executable by pid 576: /system/bin/foo
intern-state     DynamicLoaderPOSIXDYLD::ResolveExecutableModule - failed to resolve executable with module spec "file = '/system/bin/foo', arch = x86_64-unknown-linux-android": '/system/bin/foo' does not exist

LLDB failed to resolve executable between this log

LLDB_LOGF(
log, "DynamicLoaderPOSIXDYLD::%s - got executable by pid %" PRIu64 ": %s",
__FUNCTION__, m_process->GetID(),
process_info.GetExecutableFile().GetPath().c_str());

and this log

LLDB_LOGF(log,
"DynamicLoaderPOSIXDYLD::%s - failed to resolve executable "
"with module spec \"%s\": %s",
__FUNCTION__, stream.GetData(), error.AsCString());

Therefore, the problem is in platform_sp->ResolveExecutable.

auto error = platform_sp->ResolveExecutable(
module_spec, module_sp,
!executable_search_paths.IsEmpty() ? &executable_search_paths : nullptr);

According to the error message '/system/bin/foo' does not exist,
most likely platform_sp->ResolveExecutable tried to access the path in the local file system in the host that runs LLDB. It is supposed to access the path in the target file system via lldb-server.

@splhack splhack requested a review from JDevlieghere as a code owner July 12, 2024 02:25
@llvmbot llvmbot added the lldb label Jul 12, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 12, 2024

@llvm/pr-subscribers-lldb

Author: Kazuki Sakamoto (splhack)

Changes

Goal

Fix Android debugging

What

This pull request reverts these two commits because it broke Android debugging as explained below.

This pull requests also reverts this commit to avoid conflicts on the revert.

Breakage state

LLDB fails to resolve executable on remote-android platform process attach.

platform select remote-android
platform connect unix-connect://localhost:NNNN/
platform process attach -p NNN

Using logging lldb all

log enable -f /tmp/lldb.log lldb all

Without this pull request,

intern-state     DynamicLoaderPOSIXDYLD::ResolveExecutableModule - got executable by pid 576: /system/bin/foo
intern-state     DynamicLoaderPOSIXDYLD::ResolveExecutableModule - failed to resolve executable with module spec "file = '/system/bin/foo', arch = x86_64-unknown-linux-android": '/system/bin/foo' does not exist

With this pull request,

intern-state     DynamicLoaderPOSIXDYLD::ResolveExecutableModule - got executable by pid 576: /system/bin/foo
intern-state     0x000055C038956E30 Communication::Write (src = 0x0000151FEC010C80, src_len = 135) connection = 0x000055C0384F3AC0
intern-state     0x55c0384f3ac0 ConnectionFileDescriptor::Write (src = 0x151fec010c80, src_len = 135)
intern-state     0x55c038946b80 Socket::Write() (socket = 10, src = 0x151fec010c80, src_len = 135, flags = 0) => 135 (error = (null))
intern-state     0x55c0384f3ac0 ConnectionFileDescriptor::Write(fd = 10, src = 0x151fec010c80, src_len = 135) => 135 (error = (null))
intern-state     this = 0x000055C038956E30, dst = 0x0000151FFA7FB770, dst_len = 8192, timeout = 5000000 us, connection = 0x000055C0384F3AC0
intern-state     this = 0x000055C0384F3AC0, timeout = 5000000 us
intern-state     0x55c038946b80 Socket::Read() (socket = 10, src = 0x151ffa7fb770, src_len = 253, flags = 0) => 253 (error = (null))
intern-state     0x55c0384f3ac0 ConnectionFileDescriptor::Read()  fd = 10, dst = 0x151ffa7fb770, dst_len = 8192) => 253, error = (null)
intern-state     PlatformRemoteGDBServer::GetModuleSpec - got module info for (/system/bin/foo:x86_64-unknown-linux-android) : file = '/system/bin/foo', arch = x86_64-unknown-linux-android, uuid = NNNNNN-NNNN-NNNN-NNNN-NNNNNNNN, object size = NNNN

Breakage detail

From the logs,

intern-state     DynamicLoaderPOSIXDYLD::ResolveExecutableModule - got executable by pid 576: /system/bin/foo
intern-state     DynamicLoaderPOSIXDYLD::ResolveExecutableModule - failed to resolve executable with module spec "file = '/system/bin/foo', arch = x86_64-unknown-linux-android": '/system/bin/foo' does not exist

LLDB failed to resolve executable between this log

LLDB_LOGF(
log, "DynamicLoaderPOSIXDYLD::%s - got executable by pid %" PRIu64 ": %s",
__FUNCTION__, m_process->GetID(),
process_info.GetExecutableFile().GetPath().c_str());

and this log

LLDB_LOGF(log,
"DynamicLoaderPOSIXDYLD::%s - failed to resolve executable "
"with module spec \"%s\": %s",
__FUNCTION__, stream.GetData(), error.AsCString());

Therefore, the problem is in platform_sp->ResolveExecutable.

auto error = platform_sp->ResolveExecutable(
module_spec, module_sp,
!executable_search_paths.IsEmpty() ? &executable_search_paths : nullptr);

According to the error message '/system/bin/foo' does not exist,
most likely platform_sp->ResolveExecutable tried to access the path in the local file system in the host that runs LLDB. It is supposed to access the path in the target file system via lldb-server.


Patch is 29.43 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/98581.diff

14 Files Affected:

  • (modified) lldb/include/lldb/Symbol/ObjectFile.h (-1)
  • (modified) lldb/include/lldb/Target/Platform.h (+6-1)
  • (modified) lldb/include/lldb/Target/RemoteAwarePlatform.h (+4-5)
  • (modified) lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp (+75)
  • (modified) lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h (+4)
  • (modified) lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp (+65)
  • (modified) lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h (+4)
  • (modified) lldb/source/Symbol/ObjectFile.cpp (-9)
  • (modified) lldb/source/Target/Platform.cpp (+82-45)
  • (modified) lldb/source/Target/RemoteAwarePlatform.cpp (+127-11)
  • (modified) lldb/test/API/assert_messages_test/TestAssertMessages.py (+1-1)
  • (modified) lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py (+1-1)
  • (modified) lldb/test/Shell/ObjectFile/PECOFF/invalid-export-table.yaml (+1-1)
  • (modified) lldb/unittests/Target/RemoteAwarePlatformTest.cpp (+6-7)
diff --git a/lldb/include/lldb/Symbol/ObjectFile.h b/lldb/include/lldb/Symbol/ObjectFile.h
index 8592323322e38..6348d8103f85d 100644
--- a/lldb/include/lldb/Symbol/ObjectFile.h
+++ b/lldb/include/lldb/Symbol/ObjectFile.h
@@ -178,7 +178,6 @@ class ObjectFile : public std::enable_shared_from_this<ObjectFile>,
                                         lldb::offset_t file_offset,
                                         lldb::offset_t file_size,
                                         lldb_private::ModuleSpecList &specs);
-  static bool IsObjectFile(lldb_private::FileSpec file_spec);
   /// Split a path into a file path with object name.
   ///
   /// For paths like "/tmp/foo.a(bar.o)" we often need to split a path up into
diff --git a/lldb/include/lldb/Target/Platform.h b/lldb/include/lldb/Target/Platform.h
index 5ed2fc33356d9..d2329c95daf61 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -124,7 +124,7 @@ class Platform : public PluginInterface {
   ///     Returns \b true if this Platform plug-in was able to find
   ///     a suitable executable, \b false otherwise.
   virtual Status ResolveExecutable(const ModuleSpec &module_spec,
-                                   lldb::ModuleSP &exe_module_sp,
+                                   lldb::ModuleSP &module_sp,
                                    const FileSpecList *module_search_paths_ptr);
 
   /// Find a symbol file given a symbol file module specification.
@@ -989,6 +989,11 @@ class Platform : public PluginInterface {
 
   virtual const char *GetCacheHostname();
 
+  virtual Status
+  ResolveRemoteExecutable(const ModuleSpec &module_spec,
+                          lldb::ModuleSP &exe_module_sp,
+                          const FileSpecList *module_search_paths_ptr);
+
 private:
   typedef std::function<Status(const ModuleSpec &)> ModuleResolver;
 
diff --git a/lldb/include/lldb/Target/RemoteAwarePlatform.h b/lldb/include/lldb/Target/RemoteAwarePlatform.h
index fb2eecfaa23a8..0b9d79f9ff038 100644
--- a/lldb/include/lldb/Target/RemoteAwarePlatform.h
+++ b/lldb/include/lldb/Target/RemoteAwarePlatform.h
@@ -20,14 +20,13 @@ class RemoteAwarePlatform : public Platform {
 public:
   using Platform::Platform;
 
-  virtual Status
-  ResolveExecutable(const ModuleSpec &module_spec,
-                    lldb::ModuleSP &exe_module_sp,
-                    const FileSpecList *module_search_paths_ptr) override;
-
   bool GetModuleSpec(const FileSpec &module_file_spec, const ArchSpec &arch,
                      ModuleSpec &module_spec) override;
 
+  Status
+  ResolveExecutable(const ModuleSpec &module_spec, lldb::ModuleSP &module_sp,
+                    const FileSpecList *module_search_paths_ptr) override;
+
   lldb::user_id_t OpenFile(const FileSpec &file_spec, File::OpenOptions flags,
                            uint32_t mode, Status &error) override;
 
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
index c27a34b7201a2..0c4b566b7d535 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
@@ -383,6 +383,81 @@ PlatformSP PlatformAppleSimulator::CreateInstance(
   return PlatformSP();
 }
 
+Status PlatformAppleSimulator::ResolveExecutable(
+    const ModuleSpec &module_spec, lldb::ModuleSP &exe_module_sp,
+    const FileSpecList *module_search_paths_ptr) {
+  Status error;
+  // Nothing special to do here, just use the actual file and architecture
+
+  ModuleSpec resolved_module_spec(module_spec);
+
+  // If we have "ls" as the exe_file, resolve the executable loation based on
+  // the current path variables
+  // TODO: resolve bare executables in the Platform SDK
+  //    if (!resolved_exe_file.Exists())
+  //        resolved_exe_file.ResolveExecutableLocation ();
+
+  // Resolve any executable within a bundle on MacOSX
+  // TODO: verify that this handles shallow bundles, if not then implement one
+  // ourselves
+  Host::ResolveExecutableInBundle(resolved_module_spec.GetFileSpec());
+
+  if (FileSystem::Instance().Exists(resolved_module_spec.GetFileSpec())) {
+    if (resolved_module_spec.GetArchitecture().IsValid()) {
+      error = ModuleList::GetSharedModule(resolved_module_spec, exe_module_sp,
+                                          NULL, NULL, NULL);
+
+      if (exe_module_sp && exe_module_sp->GetObjectFile())
+        return error;
+      exe_module_sp.reset();
+    }
+    // No valid architecture was specified or the exact ARM slice wasn't found
+    // so ask the platform for the architectures that we should be using (in
+    // the correct order) and see if we can find a match that way
+    StreamString arch_names;
+    llvm::ListSeparator LS;
+    ArchSpec platform_arch;
+    for (const ArchSpec &arch : GetSupportedArchitectures({})) {
+      resolved_module_spec.GetArchitecture() = arch;
+
+      // Only match x86 with x86 and x86_64 with x86_64...
+      if (!module_spec.GetArchitecture().IsValid() ||
+          module_spec.GetArchitecture().GetCore() ==
+              resolved_module_spec.GetArchitecture().GetCore()) {
+        error = ModuleList::GetSharedModule(resolved_module_spec, exe_module_sp,
+                                            NULL, NULL, NULL);
+        // Did we find an executable using one of the
+        if (error.Success()) {
+          if (exe_module_sp && exe_module_sp->GetObjectFile())
+            break;
+          else
+            error.SetErrorToGenericError();
+        }
+
+        arch_names << LS << platform_arch.GetArchitectureName();
+      }
+    }
+
+    if (error.Fail() || !exe_module_sp) {
+      if (FileSystem::Instance().Readable(resolved_module_spec.GetFileSpec())) {
+        error.SetErrorStringWithFormatv(
+            "'{0}' doesn't contain any '{1}' platform architectures: {2}",
+            resolved_module_spec.GetFileSpec(), GetPluginName(),
+            arch_names.GetString());
+      } else {
+        error.SetErrorStringWithFormat(
+            "'%s' is not readable",
+            resolved_module_spec.GetFileSpec().GetPath().c_str());
+      }
+    }
+  } else {
+    error.SetErrorStringWithFormat("'%s' does not exist",
+                                   module_spec.GetFileSpec().GetPath().c_str());
+  }
+
+  return error;
+}
+
 Status PlatformAppleSimulator::GetSymbolFile(const FileSpec &platform_file,
                                              const UUID *uuid_ptr,
                                              FileSpec &local_file) {
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h
index 7fcf2c502ca6a..e17e7b6992ee5 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h
@@ -87,6 +87,10 @@ class PlatformAppleSimulator : public PlatformDarwin {
   std::vector<ArchSpec>
   GetSupportedArchitectures(const ArchSpec &process_host_arch) override;
 
+  Status
+  ResolveExecutable(const ModuleSpec &module_spec, lldb::ModuleSP &module_sp,
+                    const FileSpecList *module_search_paths_ptr) override;
+
   Status GetSharedModule(const ModuleSpec &module_spec, Process *process,
                          lldb::ModuleSP &module_sp,
                          const FileSpecList *module_search_paths_ptr,
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
index 9323b66181c7c..a244ee35976b6 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp
@@ -63,6 +63,71 @@ void PlatformRemoteDarwinDevice::GetStatus(Stream &strm) {
   }
 }
 
+Status PlatformRemoteDarwinDevice::ResolveExecutable(
+    const ModuleSpec &ms, lldb::ModuleSP &exe_module_sp,
+    const FileSpecList *module_search_paths_ptr) {
+  Status error;
+  // Nothing special to do here, just use the actual file and architecture
+
+  ModuleSpec resolved_module_spec(ms);
+
+  // Resolve any executable within a bundle on MacOSX
+  // TODO: verify that this handles shallow bundles, if not then implement one
+  // ourselves
+  Host::ResolveExecutableInBundle(resolved_module_spec.GetFileSpec());
+
+  if (FileSystem::Instance().Exists(resolved_module_spec.GetFileSpec())) {
+    if (resolved_module_spec.GetArchitecture().IsValid() ||
+        resolved_module_spec.GetUUID().IsValid()) {
+      error = ModuleList::GetSharedModule(resolved_module_spec, exe_module_sp,
+                                          nullptr, nullptr, nullptr);
+
+      if (exe_module_sp && exe_module_sp->GetObjectFile())
+        return error;
+      exe_module_sp.reset();
+    }
+    // No valid architecture was specified or the exact ARM slice wasn't found
+    // so ask the platform for the architectures that we should be using (in
+    // the correct order) and see if we can find a match that way
+    StreamString arch_names;
+    llvm::ListSeparator LS;
+    ArchSpec process_host_arch;
+    for (const ArchSpec &arch : GetSupportedArchitectures(process_host_arch)) {
+      resolved_module_spec.GetArchitecture() = arch;
+      error = ModuleList::GetSharedModule(resolved_module_spec, exe_module_sp,
+                                          nullptr, nullptr, nullptr);
+      // Did we find an executable using one of the
+      if (error.Success()) {
+        if (exe_module_sp && exe_module_sp->GetObjectFile())
+          break;
+        else
+          error.SetErrorToGenericError();
+      }
+
+      arch_names << LS << arch.GetArchitectureName();
+    }
+
+    if (error.Fail() || !exe_module_sp) {
+      if (FileSystem::Instance().Readable(resolved_module_spec.GetFileSpec())) {
+        error.SetErrorStringWithFormatv(
+            "'{0}' doesn't contain any '{1}' platform architectures: {2}",
+            resolved_module_spec.GetFileSpec(), GetPluginName(),
+            arch_names.GetData());
+      } else {
+        error.SetErrorStringWithFormat(
+            "'%s' is not readable",
+            resolved_module_spec.GetFileSpec().GetPath().c_str());
+      }
+    }
+  } else {
+    error.SetErrorStringWithFormat(
+        "'%s' does not exist",
+        resolved_module_spec.GetFileSpec().GetPath().c_str());
+  }
+
+  return error;
+}
+
 bool PlatformRemoteDarwinDevice::GetFileInSDK(const char *platform_file_path,
                                      uint32_t sdk_idx,
                                      lldb_private::FileSpec &local_file) {
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h
index 557f4876e91ab..c604866040995 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h
@@ -40,6 +40,10 @@ class PlatformRemoteDarwinDevice : public PlatformDarwinDevice {
   ~PlatformRemoteDarwinDevice() override;
 
   // Platform functions
+  Status
+  ResolveExecutable(const ModuleSpec &module_spec, lldb::ModuleSP &module_sp,
+                    const FileSpecList *module_search_paths_ptr) override;
+
   void GetStatus(Stream &strm) override;
 
   virtual Status GetSymbolFile(const FileSpec &platform_file,
diff --git a/lldb/source/Symbol/ObjectFile.cpp b/lldb/source/Symbol/ObjectFile.cpp
index 2608a9c5fb79a..d890ad92e8312 100644
--- a/lldb/source/Symbol/ObjectFile.cpp
+++ b/lldb/source/Symbol/ObjectFile.cpp
@@ -184,15 +184,6 @@ ObjectFileSP ObjectFile::FindPlugin(const lldb::ModuleSP &module_sp,
   return object_file_sp;
 }
 
-bool ObjectFile::IsObjectFile(lldb_private::FileSpec file_spec) {
-  DataBufferSP data_sp;
-  offset_t data_offset = 0;
-  ModuleSP module_sp = std::make_shared<Module>(file_spec);
-  return static_cast<bool>(ObjectFile::FindPlugin(
-      module_sp, &file_spec, 0, FileSystem::Instance().GetByteSize(file_spec),
-      data_sp, data_offset));
-}
-
 size_t ObjectFile::GetModuleSpecifications(const FileSpec &file,
                                            lldb::offset_t file_offset,
                                            lldb::offset_t file_size,
diff --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
index bb90c377d86b2..c39cc3f1203c7 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/lldb/source/Target/Platform.cpp
@@ -732,6 +732,42 @@ Status
 Platform::ResolveExecutable(const ModuleSpec &module_spec,
                             lldb::ModuleSP &exe_module_sp,
                             const FileSpecList *module_search_paths_ptr) {
+  Status error;
+
+  if (FileSystem::Instance().Exists(module_spec.GetFileSpec())) {
+    if (module_spec.GetArchitecture().IsValid()) {
+      error = ModuleList::GetSharedModule(module_spec, exe_module_sp,
+                                          module_search_paths_ptr, nullptr,
+                                          nullptr);
+    } else {
+      // No valid architecture was specified, ask the platform for the
+      // architectures that we should be using (in the correct order) and see
+      // if we can find a match that way
+      ModuleSpec arch_module_spec(module_spec);
+      ArchSpec process_host_arch;
+      for (const ArchSpec &arch :
+           GetSupportedArchitectures(process_host_arch)) {
+        arch_module_spec.GetArchitecture() = arch;
+        error = ModuleList::GetSharedModule(arch_module_spec, exe_module_sp,
+                                            module_search_paths_ptr, nullptr,
+                                            nullptr);
+        // Did we find an executable using one of the
+        if (error.Success() && exe_module_sp)
+          break;
+      }
+    }
+  } else {
+    error.SetErrorStringWithFormat(
+        "'%s' does not exist", module_spec.GetFileSpec().GetPath().c_str());
+  }
+  return error;
+}
+
+Status
+Platform::ResolveRemoteExecutable(const ModuleSpec &module_spec,
+                            lldb::ModuleSP &exe_module_sp,
+                            const FileSpecList *module_search_paths_ptr) {
+  Status error;
 
   // We may connect to a process and use the provided executable (Don't use
   // local $PATH).
@@ -740,57 +776,57 @@ Platform::ResolveExecutable(const ModuleSpec &module_spec,
   // Resolve any executable within a bundle on MacOSX
   Host::ResolveExecutableInBundle(resolved_module_spec.GetFileSpec());
 
-  if (!FileSystem::Instance().Exists(resolved_module_spec.GetFileSpec()) &&
-      !module_spec.GetUUID().IsValid())
-    return Status::createWithFormat("'{0}' does not exist",
-                                    resolved_module_spec.GetFileSpec());
+  if (FileSystem::Instance().Exists(resolved_module_spec.GetFileSpec()) ||
+      module_spec.GetUUID().IsValid()) {
+    if (resolved_module_spec.GetArchitecture().IsValid() ||
+        resolved_module_spec.GetUUID().IsValid()) {
+      error = ModuleList::GetSharedModule(resolved_module_spec, exe_module_sp,
+                                          module_search_paths_ptr, nullptr,
+                                          nullptr);
 
-  if (resolved_module_spec.GetArchitecture().IsValid() ||
-      resolved_module_spec.GetUUID().IsValid()) {
-    Status error =
-        ModuleList::GetSharedModule(resolved_module_spec, exe_module_sp,
-                                    module_search_paths_ptr, nullptr, nullptr);
-
-    if (exe_module_sp && exe_module_sp->GetObjectFile())
-      return error;
-    exe_module_sp.reset();
-  }
-  // No valid architecture was specified or the exact arch wasn't found.
-  // Ask the platform for the architectures that we should be using (in the
-  // correct order) and see if we can find a match that way.
-  StreamString arch_names;
-  llvm::ListSeparator LS;
-  ArchSpec process_host_arch;
-  Status error;
-  for (const ArchSpec &arch : GetSupportedArchitectures(process_host_arch)) {
-    resolved_module_spec.GetArchitecture() = arch;
-    error =
-        ModuleList::GetSharedModule(resolved_module_spec, exe_module_sp,
-                                    module_search_paths_ptr, nullptr, nullptr);
-    if (error.Success()) {
       if (exe_module_sp && exe_module_sp->GetObjectFile())
-        break;
-      error.SetErrorToGenericError();
+        return error;
+      exe_module_sp.reset();
     }
+    // No valid architecture was specified or the exact arch wasn't found so
+    // ask the platform for the architectures that we should be using (in the
+    // correct order) and see if we can find a match that way
+    StreamString arch_names;
+    llvm::ListSeparator LS;
+    ArchSpec process_host_arch;
+    for (const ArchSpec &arch : GetSupportedArchitectures(process_host_arch)) {
+      resolved_module_spec.GetArchitecture() = arch;
+      error = ModuleList::GetSharedModule(resolved_module_spec, exe_module_sp,
+                                          module_search_paths_ptr, nullptr,
+                                          nullptr);
+      // Did we find an executable using one of the
+      if (error.Success()) {
+        if (exe_module_sp && exe_module_sp->GetObjectFile())
+          break;
+        else
+          error.SetErrorToGenericError();
+      }
 
-    arch_names << LS << arch.GetArchitectureName();
-  }
-
-  if (exe_module_sp && error.Success())
-    return {};
-
-  if (!FileSystem::Instance().Readable(resolved_module_spec.GetFileSpec()))
-    return Status::createWithFormat("'{0}' is not readable",
-                                    resolved_module_spec.GetFileSpec());
+      arch_names << LS << arch.GetArchitectureName();
+    }
 
-  if (!ObjectFile::IsObjectFile(resolved_module_spec.GetFileSpec()))
-    return Status::createWithFormat("'{0}' is not a valid executable",
+    if (error.Fail() || !exe_module_sp) {
+      if (FileSystem::Instance().Readable(resolved_module_spec.GetFileSpec())) {
+        error.SetErrorStringWithFormatv(
+            "'{0}' doesn't contain any '{1}' platform architectures: {2}",
+            resolved_module_spec.GetFileSpec(), GetPluginName(),
+            arch_names.GetData());
+      } else {
+        error.SetErrorStringWithFormatv("'{0}' is not readable",
+                                        resolved_module_spec.GetFileSpec());
+      }
+    }
+  } else {
+    error.SetErrorStringWithFormatv("'{0}' does not exist",
                                     resolved_module_spec.GetFileSpec());
+  }
 
-  return Status::createWithFormat(
-      "'{0}' doesn't contain any '{1}' platform architectures: {2}",
-      resolved_module_spec.GetFileSpec(), GetPluginName(),
-      arch_names.GetData());
+  return error;
 }
 
 Status Platform::ResolveSymbolFile(Target &target, const ModuleSpec &sym_spec,
@@ -1446,7 +1482,8 @@ Platform::GetCachedExecutable(ModuleSpec &module_spec,
   Status error = GetRemoteSharedModule(
       module_spec, nullptr, module_sp,
       [&](const ModuleSpec &spec) {
-        return ResolveExecutable(spec, module_sp, module_search_paths_ptr);
+        return ResolveRemoteExecutable(spec, module_sp,
+                                       module_search_paths_ptr);
       },
       nullptr);
   if (error.Success()) {
diff --git a/lldb/source/Target/RemoteAwarePlatform.cpp b/lldb/source/Target/RemoteAwarePlatform.cpp
index 5fc2d63876b92..9a41a423cadd3 100644
--- a/lldb/source/Target/RemoteAwarePlatform.cpp
+++ b/lldb/source/Target/RemoteAwarePlatform.cpp
@@ -30,26 +30,142 @@ bool RemoteAwarePlatform::GetModuleSpec(const FileSpec &module_file_spec,
 }
 
 Status RemoteAwarePlatform::ResolveExecutable(
-    const ModuleSpec &module_spec, lldb::ModuleSP &exe_module_sp,
+    const ModuleSpec &module_spec, ModuleSP &exe_module_sp,
     const FileSpecList *module_search_paths_ptr) {
+  Status error;
+  // Nothing special to do here, just use the actual file and architecture
+
+  char exe_path[PATH_MAX];
   ModuleSpec resolved_module_spec(module_spec);
 
-  // The host platform can resolve the path more aggressively.
   if (IsHost()) {
-    FileSpec &resolved_file_spec = resolved_module_spec.GetFileSpec();
-
-    if (!FileSystem::Instance().Exists(resolved_file_spec)) {
-      resolved_module_spec.GetFileSpec().SetFile(resolved_file_spec.GetPath(),
+    // If we have "ls" as the exe_file, resolve the executable location based
+    // on the current path variables
+    if ...
[truncated]

Copy link

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff a853fe25df1cda0117055c0d836e7b107d98c791 8602d97a8a9b1f810000b6776bede708eeb54bc3 --extensions h,cpp -- lldb/include/lldb/Symbol/ObjectFile.h lldb/include/lldb/Target/Platform.h lldb/include/lldb/Target/RemoteAwarePlatform.h lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp lldb/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.h lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.cpp lldb/source/Plugins/Platform/MacOSX/PlatformRemoteDarwinDevice.h lldb/source/Symbol/ObjectFile.cpp lldb/source/Target/Platform.cpp lldb/source/Target/RemoteAwarePlatform.cpp lldb/unittests/Target/RemoteAwarePlatformTest.cpp
View the diff from clang-format here.
diff --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
index c39cc3f120..31e1bda7bc 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/lldb/source/Target/Platform.cpp
@@ -757,16 +757,16 @@ Platform::ResolveExecutable(const ModuleSpec &module_spec,
       }
     }
   } else {
-    error.SetErrorStringWithFormat(
-        "'%s' does not exist", module_spec.GetFileSpec().GetPath().c_str());
+    error.SetErrorStringWithFormat("'%s' does not exist",
+                                   module_spec.GetFileSpec().GetPath().c_str());
   }
   return error;
 }
 
 Status
 Platform::ResolveRemoteExecutable(const ModuleSpec &module_spec,
-                            lldb::ModuleSP &exe_module_sp,
-                            const FileSpecList *module_search_paths_ptr) {
+                                  lldb::ModuleSP &exe_module_sp,
+                                  const FileSpecList *module_search_paths_ptr) {
   Status error;
 
   // We may connect to a process and use the provided executable (Don't use
diff --git a/lldb/source/Target/RemoteAwarePlatform.cpp b/lldb/source/Target/RemoteAwarePlatform.cpp
index 9a41a423ca..5c1979c979 100644
--- a/lldb/source/Target/RemoteAwarePlatform.cpp
+++ b/lldb/source/Target/RemoteAwarePlatform.cpp
@@ -93,11 +93,12 @@ Status RemoteAwarePlatform::ResolveExecutable(
   if (error.Success()) {
     if (resolved_module_spec.GetArchitecture().IsValid()) {
       error = ModuleList::GetSharedModule(resolved_module_spec, exe_module_sp,
-                                          module_search_paths_ptr, nullptr, nullptr);
+                                          module_search_paths_ptr, nullptr,
+                                          nullptr);
       if (error.Fail()) {
         // If we failed, it may be because the vendor and os aren't known. If
-	// that is the case, try setting them to the host architecture and give
-	// it another try.
+        // that is the case, try setting them to the host architecture and give
+        // it another try.
         llvm::Triple &module_triple =
             resolved_module_spec.GetArchitecture().GetTriple();
         bool is_vendor_specified =
@@ -113,8 +114,9 @@ Status RemoteAwarePlatform::ResolveExecutable(
           if (!is_os_specified)
             module_triple.setOSName(host_triple.getOSName());
 
-          error = ModuleList::GetSharedModule(resolved_module_spec,
-                                              exe_module_sp, module_search_paths_ptr, nullptr, nullptr);
+          error = ModuleList::GetSharedModule(
+              resolved_module_spec, exe_module_sp, module_search_paths_ptr,
+              nullptr, nullptr);
         }
       }
 
@@ -137,7 +139,8 @@ Status RemoteAwarePlatform::ResolveExecutable(
            GetSupportedArchitectures(process_host_arch)) {
         resolved_module_spec.GetArchitecture() = arch;
         error = ModuleList::GetSharedModule(resolved_module_spec, exe_module_sp,
-                                            module_search_paths_ptr, nullptr, nullptr);
+                                            module_search_paths_ptr, nullptr,
+                                            nullptr);
         // Did we find an executable using one of the
         if (error.Success()) {
           if (exe_module_sp && exe_module_sp->GetObjectFile())

@dzhidzhoev
Copy link
Member

dzhidzhoev commented Jul 12, 2024

Could you check if this solves your issue?
#98623

@splhack
Copy link
Contributor Author

splhack commented Jul 12, 2024

@dzhidzhoev Yes, #98623 fixed the ResolveExecutableModule issue, thank you!

@dzhidzhoev
Copy link
Member

@dzhidzhoev Yes, #98623 fixed the ResolveExecutableModule issue, thank you!

May I ask you: are there any public builders available for remote Android lldb testing? It will come in handy to know if there are any.

@clayborg
Copy link
Collaborator

We really need there to be some tests somehow or somewhere for this. Without tests we can't verify this doesn't get broken in the future. If we can't test with true android we could create a test platform layer that is only available in debug builds that could allow us to simulate things.

@splhack splhack deleted the fix-android branch November 20, 2024 22:11
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