Skip to content

Conversation

@dzhidzhoev
Copy link
Member

Seemingly, #96256 removed the only call to
Platform::GetCachedExecutable, which broke the resolution of executable modules in the remote debugging mode
(#97410).

This commit fixes that.

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

llvmbot commented Jul 12, 2024

@llvm/pr-subscribers-lldb

Author: Vladislav Dzhidzhoev (dzhidzhoev)

Changes

Seemingly, #96256 removed the only call to
Platform::GetCachedExecutable, which broke the resolution of executable modules in the remote debugging mode
(#97410).

This commit fixes that.


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

2 Files Affected:

  • (modified) lldb/source/Target/Platform.cpp (+1-1)
  • (modified) lldb/source/Target/RemoteAwarePlatform.cpp (+3)
diff --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
index bb90c377d86b2..1900898db6494 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/lldb/source/Target/Platform.cpp
@@ -1446,7 +1446,7 @@ 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 Platform::ResolveExecutable(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..f3aafb87149c8 100644
--- a/lldb/source/Target/RemoteAwarePlatform.cpp
+++ b/lldb/source/Target/RemoteAwarePlatform.cpp
@@ -46,6 +46,9 @@ Status RemoteAwarePlatform::ResolveExecutable(
 
     if (!FileSystem::Instance().Exists(resolved_file_spec))
       FileSystem::Instance().ResolveExecutableLocation(resolved_file_spec);
+  } else if (m_remote_platform_sp) {
+    return GetCachedExecutable(resolved_module_spec, exe_module_sp,
+        module_search_paths_ptr);
   }
 
   return Platform::ResolveExecutable(resolved_module_spec, exe_module_sp,

@github-actions
Copy link

github-actions bot commented Jul 12, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Seemingly, llvm#96256 removed the only call to
Platform::GetCachedExecutable, which broke the resolution of executable
modules in the remote debugging mode
(llvm#97410).

This commit fixes that.
@dzhidzhoev dzhidzhoev force-pushed the lldb/resolve-module-fix branch from 3f8a416 to b86d6f9 Compare July 12, 2024 12:28
@JDevlieghere
Copy link
Member

That was an oversight. Thanks for restoring this. LGTM.

@JDevlieghere
Copy link
Member

CC @clayborg as this probably explains the Android debugging regression.

@splhack
Copy link
Contributor

splhack commented Jul 12, 2024

Verified this fixed the ResolveExecutableModule issue with Android target reported in #98581

@dzhidzhoev dzhidzhoev merged commit 73dad7a into llvm:main Jul 12, 2024
aaryanshukla pushed a commit to aaryanshukla/llvm-project that referenced this pull request Jul 14, 2024
Seemingly, llvm#96256 removed the only call to
Platform::GetCachedExecutable, which broke the resolution of executable
modules in the remote debugging mode
(llvm#97410).

This commit fixes that.
@clayborg
Copy link
Collaborator

Very nice. We really need to get an android test in for this, or at least a test that simulates what android does in some way to verify we don't regress this in the future.

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.

[lldb] Some LLDB API tests are broken in the cross build after #96256

5 participants