[Lldb-commits] [lldb] [LLDB] Fix Android debugging (PR #98581)

Kazuki Sakamoto via lldb-commits lldb-commits at lists.llvm.org
Thu Jul 11 19:25:09 PDT 2024


https://github.com/splhack created https://github.com/llvm/llvm-project/pull/98581

# Goal

Fix Android debugging

# What

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

- https://github.com/llvm/llvm-project/commit/bf3e3289d67cb0fe136b0660cac39c24c9f65069
- https://github.com/llvm/llvm-project/commit/c3fe1c4472e72a3832be911e8fa9098fa84762a0

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

- https://github.com/llvm/llvm-project/commit/ed7e46877dc7f09b5d194f87c87bfc2bebfcf27a

# 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

https://github.com/llvm/llvm-project/blob/90abdf83e273586a43e1270e5f0a11de5cc35383/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp#L839-L842

and this log

https://github.com/llvm/llvm-project/blob/90abdf83e273586a43e1270e5f0a11de5cc35383/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp#L857-L860

Therefore, the problem is in `platform_sp->ResolveExecutable`.

https://github.com/llvm/llvm-project/blob/90abdf83e273586a43e1270e5f0a11de5cc35383/lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp#L850-L852

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.

>From 8a4152b0dbfbe2733fa8b359fe991bf335f6f530 Mon Sep 17 00:00:00 2001
From: Kazuki Sakamoto <sakamoto at splhack.org>
Date: Thu, 11 Jul 2024 18:37:17 -0700
Subject: [PATCH 1/3] Revert "[lldb] Improve error message for unrecognized
 executables (#97490)"

This reverts commit ed7e46877dc7f09b5d194f87c87bfc2bebfcf27a.
---
 lldb/include/lldb/Symbol/ObjectFile.h         |  1 -
 lldb/source/Symbol/ObjectFile.cpp             |  9 --
 lldb/source/Target/Platform.cpp               | 87 +++++++++----------
 .../PECOFF/invalid-export-table.yaml          |  2 +-
 4 files changed, 44 insertions(+), 55 deletions(-)

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/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..c06abd3070f31 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/lldb/source/Target/Platform.cpp
@@ -732,6 +732,7 @@ Status
 Platform::ResolveExecutable(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 +741,55 @@ 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 (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 (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 (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.
+    // 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);
+      if (error.Success()) {
+        if (exe_module_sp && exe_module_sp->GetObjectFile())
+          break;
+        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,
diff --git a/lldb/test/Shell/ObjectFile/PECOFF/invalid-export-table.yaml b/lldb/test/Shell/ObjectFile/PECOFF/invalid-export-table.yaml
index e6564661b96fd..389261ad9b10a 100644
--- a/lldb/test/Shell/ObjectFile/PECOFF/invalid-export-table.yaml
+++ b/lldb/test/Shell/ObjectFile/PECOFF/invalid-export-table.yaml
@@ -4,7 +4,7 @@
 # RUN: yaml2obj %s -o %t.exe
 # RUN: %lldb %t.exe 2>&1 | FileCheck %s
 
-# CHECK: error: '{{.*}}' is not a valid executable
+# CHECK: error: '{{.*}}' doesn't contain any {{.*}} platform architectures
 --- !COFF
 OptionalHeader:
   AddressOfEntryPoint: 4096

>From 7a253c2070f04d4abb5c85c249675274a35d4e86 Mon Sep 17 00:00:00 2001
From: Kazuki Sakamoto <sakamoto at splhack.org>
Date: Thu, 11 Jul 2024 18:37:26 -0700
Subject: [PATCH 2/3] Revert "[lldb] Resolve executables more aggressively on
 the host"

This reverts commit c3fe1c4472e72a3832be911e8fa9098fa84762a0.
---
 .../include/lldb/Target/RemoteAwarePlatform.h |  5 ----
 lldb/source/Target/RemoteAwarePlatform.cpp    | 23 -------------------
 2 files changed, 28 deletions(-)

diff --git a/lldb/include/lldb/Target/RemoteAwarePlatform.h b/lldb/include/lldb/Target/RemoteAwarePlatform.h
index fb2eecfaa23a8..6fbeec7888a98 100644
--- a/lldb/include/lldb/Target/RemoteAwarePlatform.h
+++ b/lldb/include/lldb/Target/RemoteAwarePlatform.h
@@ -20,11 +20,6 @@ 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;
 
diff --git a/lldb/source/Target/RemoteAwarePlatform.cpp b/lldb/source/Target/RemoteAwarePlatform.cpp
index 5fc2d63876b92..63243d6e71307 100644
--- a/lldb/source/Target/RemoteAwarePlatform.cpp
+++ b/lldb/source/Target/RemoteAwarePlatform.cpp
@@ -29,29 +29,6 @@ bool RemoteAwarePlatform::GetModuleSpec(const FileSpec &module_file_spec,
   return false;
 }
 
-Status RemoteAwarePlatform::ResolveExecutable(
-    const ModuleSpec &module_spec, lldb::ModuleSP &exe_module_sp,
-    const FileSpecList *module_search_paths_ptr) {
-  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(),
-                                                 FileSpec::Style::native);
-      FileSystem::Instance().Resolve(resolved_file_spec);
-    }
-
-    if (!FileSystem::Instance().Exists(resolved_file_spec))
-      FileSystem::Instance().ResolveExecutableLocation(resolved_file_spec);
-  }
-
-  return Platform::ResolveExecutable(resolved_module_spec, exe_module_sp,
-                                     module_search_paths_ptr);
-}
-
 Status RemoteAwarePlatform::RunShellCommand(
     llvm::StringRef command, const FileSpec &working_dir, int *status_ptr,
     int *signo_ptr, std::string *command_output,

>From 8602d97a8a9b1f810000b6776bede708eeb54bc3 Mon Sep 17 00:00:00 2001
From: Kazuki Sakamoto <sakamoto at splhack.org>
Date: Thu, 11 Jul 2024 18:37:32 -0700
Subject: [PATCH 3/3] Revert "[lldb] Unify Platform::ResolveExecutable
 (#96256)"

This reverts commit bf3e3289d67cb0fe136b0660cac39c24c9f65069.
---
 lldb/include/lldb/Target/Platform.h           |   7 +-
 .../include/lldb/Target/RemoteAwarePlatform.h |   4 +
 .../MacOSX/PlatformAppleSimulator.cpp         |  75 ++++++++++
 .../Platform/MacOSX/PlatformAppleSimulator.h  |   4 +
 .../MacOSX/PlatformRemoteDarwinDevice.cpp     |  65 ++++++++
 .../MacOSX/PlatformRemoteDarwinDevice.h       |   4 +
 lldb/source/Target/Platform.cpp               |  48 +++++-
 lldb/source/Target/RemoteAwarePlatform.cpp    | 139 ++++++++++++++++++
 .../TestAssertMessages.py                     |   2 +-
 .../tools/lldb-dap/launch/TestDAP_launch.py   |   2 +-
 .../Target/RemoteAwarePlatformTest.cpp        |  13 +-
 11 files changed, 348 insertions(+), 15 deletions(-)

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 6fbeec7888a98..0b9d79f9ff038 100644
--- a/lldb/include/lldb/Target/RemoteAwarePlatform.h
+++ b/lldb/include/lldb/Target/RemoteAwarePlatform.h
@@ -23,6 +23,10 @@ class RemoteAwarePlatform : public Platform {
   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/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
index c06abd3070f31..c39cc3f1203c7 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/lldb/source/Target/Platform.cpp
@@ -734,6 +734,41 @@ Platform::ResolveExecutable(const ModuleSpec &module_spec,
                             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).
   ModuleSpec resolved_module_spec(module_spec);
@@ -753,9 +788,9 @@ Platform::ResolveExecutable(const ModuleSpec &module_spec,
         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.
+    // 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;
@@ -764,10 +799,12 @@ Platform::ResolveExecutable(const ModuleSpec &module_spec,
       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;
-        error.SetErrorToGenericError();
+        else
+          error.SetErrorToGenericError();
       }
 
       arch_names << LS << arch.GetArchitectureName();
@@ -1445,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 63243d6e71307..9a41a423cadd3 100644
--- a/lldb/source/Target/RemoteAwarePlatform.cpp
+++ b/lldb/source/Target/RemoteAwarePlatform.cpp
@@ -29,6 +29,145 @@ bool RemoteAwarePlatform::GetModuleSpec(const FileSpec &module_file_spec,
   return false;
 }
 
+Status RemoteAwarePlatform::ResolveExecutable(
+    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);
+
+  if (IsHost()) {
+    // If we have "ls" as the exe_file, resolve the executable location based
+    // on the current path variables
+    if (!FileSystem::Instance().Exists(resolved_module_spec.GetFileSpec())) {
+      resolved_module_spec.GetFileSpec().GetPath(exe_path, sizeof(exe_path));
+      resolved_module_spec.GetFileSpec().SetFile(exe_path,
+                                                 FileSpec::Style::native);
+      FileSystem::Instance().Resolve(resolved_module_spec.GetFileSpec());
+    }
+
+    if (!FileSystem::Instance().Exists(resolved_module_spec.GetFileSpec()))
+      FileSystem::Instance().ResolveExecutableLocation(
+          resolved_module_spec.GetFileSpec());
+
+    // Resolve any executable within a bundle on MacOSX
+    Host::ResolveExecutableInBundle(resolved_module_spec.GetFileSpec());
+
+    if (FileSystem::Instance().Exists(resolved_module_spec.GetFileSpec()))
+      error.Clear();
+    else {
+      const uint32_t permissions = FileSystem::Instance().GetPermissions(
+          resolved_module_spec.GetFileSpec());
+      if (permissions && (permissions & eFilePermissionsEveryoneR) == 0)
+        error.SetErrorStringWithFormat(
+            "executable '%s' is not readable",
+            resolved_module_spec.GetFileSpec().GetPath().c_str());
+      else
+        error.SetErrorStringWithFormat(
+            "unable to find executable for '%s'",
+            resolved_module_spec.GetFileSpec().GetPath().c_str());
+    }
+  } else {
+    if (m_remote_platform_sp) {
+      return GetCachedExecutable(resolved_module_spec, exe_module_sp,
+                                 module_search_paths_ptr);
+    }
+
+    // We may connect to a process and use the provided executable (Don't use
+    // local $PATH).
+
+    // Resolve any executable within a bundle on MacOSX
+    Host::ResolveExecutableInBundle(resolved_module_spec.GetFileSpec());
+
+    if (FileSystem::Instance().Exists(resolved_module_spec.GetFileSpec()))
+      error.Clear();
+    else
+      error.SetErrorStringWithFormat("the platform is not currently "
+                                     "connected, and '%s' doesn't exist in "
+                                     "the system root.",
+                                     exe_path);
+  }
+
+  if (error.Success()) {
+    if (resolved_module_spec.GetArchitecture().IsValid()) {
+      error = ModuleList::GetSharedModule(resolved_module_spec, exe_module_sp,
+                                          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.
+        llvm::Triple &module_triple =
+            resolved_module_spec.GetArchitecture().GetTriple();
+        bool is_vendor_specified =
+            (module_triple.getVendor() != llvm::Triple::UnknownVendor);
+        bool is_os_specified =
+            (module_triple.getOS() != llvm::Triple::UnknownOS);
+        if (!is_vendor_specified || !is_os_specified) {
+          const llvm::Triple &host_triple =
+              HostInfo::GetArchitecture(HostInfo::eArchKindDefault).GetTriple();
+
+          if (!is_vendor_specified)
+            module_triple.setVendorName(host_triple.getVendorName());
+          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);
+        }
+      }
+
+      // TODO find out why exe_module_sp might be NULL
+      if (error.Fail() || !exe_module_sp || !exe_module_sp->GetObjectFile()) {
+        exe_module_sp.reset();
+        error.SetErrorStringWithFormat(
+            "'%s' doesn't contain the architecture %s",
+            resolved_module_spec.GetFileSpec().GetPath().c_str(),
+            resolved_module_spec.GetArchitecture().GetArchitectureName());
+      }
+    } 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
+      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 (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());
+        }
+      }
+    }
+  }
+
+  return error;
+}
+
 Status RemoteAwarePlatform::RunShellCommand(
     llvm::StringRef command, const FileSpec &working_dir, int *status_ptr,
     int *signo_ptr, std::string *command_output,
diff --git a/lldb/test/API/assert_messages_test/TestAssertMessages.py b/lldb/test/API/assert_messages_test/TestAssertMessages.py
index 3c54b9379d4c1..173dfd14e2a35 100644
--- a/lldb/test/API/assert_messages_test/TestAssertMessages.py
+++ b/lldb/test/API/assert_messages_test/TestAssertMessages.py
@@ -30,7 +30,7 @@ def test_createTestTarget(self):
         except AssertionError as e:
             self.assertIn(
                 "Couldn't create target for path 'doesnt_exist': "
-                "error: 'doesnt_exist' does not exist",
+                "error: unable to find executable for 'doesnt_exist'",
                 str(e),
             )
 
diff --git a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
index b1b3d05ed4548..cde2d0e583d44 100644
--- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
+++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
@@ -447,7 +447,7 @@ def test_failing_launch_commands(self):
         # Verify all "launchCommands" were founc in console output
         # The launch should fail due to the invalid command.
         self.verify_commands("launchCommands", output, launchCommands)
-        self.assertRegex(output, r"bad/path/.*does not exist")
+        self.assertRegex(output, r"unable to find executable for '/bad/path/")
 
     @skipIfWindows
     @skipIfNetBSD  # Hangs on NetBSD as well
diff --git a/lldb/unittests/Target/RemoteAwarePlatformTest.cpp b/lldb/unittests/Target/RemoteAwarePlatformTest.cpp
index d7810b20af95d..c36bd35c819dd 100644
--- a/lldb/unittests/Target/RemoteAwarePlatformTest.cpp
+++ b/lldb/unittests/Target/RemoteAwarePlatformTest.cpp
@@ -32,15 +32,14 @@ class RemoteAwarePlatformTester : public RemoteAwarePlatform {
                ProcessSP(ProcessAttachInfo &, Debugger &, Target *, Status &));
   MOCK_METHOD0(CalculateTrapHandlerSymbolNames, void());
 
-  MOCK_METHOD2(ResolveExecutable,
+  MOCK_METHOD2(ResolveRemoteExecutable,
                std::pair<Status, ModuleSP>(const ModuleSpec &,
                                            const FileSpecList *));
-  Status
-  ResolveExecutable(const ModuleSpec &module_spec,
-                    lldb::ModuleSP &exe_module_sp,
-                    const FileSpecList *module_search_paths_ptr) /*override*/
+  Status ResolveRemoteExecutable(
+      const ModuleSpec &module_spec, lldb::ModuleSP &exe_module_sp,
+      const FileSpecList *module_search_paths_ptr) /*override*/
   { // NOLINT(modernize-use-override)
-    auto pair = ResolveExecutable(module_spec, module_search_paths_ptr);
+    auto pair = ResolveRemoteExecutable(module_spec, module_search_paths_ptr);
     exe_module_sp = pair.second;
     return pair.first;
   }
@@ -80,7 +79,7 @@ TEST_F(RemoteAwarePlatformTest, TestResolveExecutabelOnClientByPlatform) {
   static const ArchSpec process_host_arch;
   EXPECT_CALL(platform, GetSupportedArchitectures(process_host_arch))
       .WillRepeatedly(Return(std::vector<ArchSpec>()));
-  EXPECT_CALL(platform, ResolveExecutable(_, _))
+  EXPECT_CALL(platform, ResolveRemoteExecutable(_, _))
       .WillRepeatedly(Return(std::make_pair(Status(), expected_executable)));
 
   platform.SetRemotePlatform(std::make_shared<TargetPlatformTester>(false));



More information about the lldb-commits mailing list