Skip to content

[lldb] Fixed the TestCompletion test running on a remote target #92281

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 2 commits into from
May 15, 2024

Conversation

slydiman
Copy link
Contributor

Install the image to the remote target if necessary. Platform::LoadImage() uses the following logic before DoLoadImage()

if (IsRemote() || local_file != remote_file) {
  error = Install(local_file, remote_file);
  ...
}

The FileSpec for the local path may be resolved, so it is necessary to use the condition if lldb.remote_platform:.

Install the image to the remote target if necessary. Platform::LoadImage() uses the following logic before DoLoadImage()
```
if (IsRemote() || local_file != remote_file) {
  error = Install(local_file, remote_file);
  ...
}
```
The FileSpec for the local path may be resolved, so it is necessary to use the condition `if lldb.remote_platform:`.
@slydiman slydiman requested a review from JDevlieghere as a code owner May 15, 2024 15:22
@slydiman slydiman requested a review from DavidSpickett May 15, 2024 15:22
@llvmbot llvmbot added the lldb label May 15, 2024
@slydiman slydiman requested a review from labath May 15, 2024 15:23
@llvmbot
Copy link
Member

llvmbot commented May 15, 2024

@llvm/pr-subscribers-lldb

Author: Dmitry Vasilyev (slydiman)

Changes

Install the image to the remote target if necessary. Platform::LoadImage() uses the following logic before DoLoadImage()

if (IsRemote() || local_file != remote_file) {
  error = Install(local_file, remote_file);
  ...
}

The FileSpec for the local path may be resolved, so it is necessary to use the condition if lldb.remote_platform:.


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

1 Files Affected:

  • (modified) lldb/test/API/functionalities/completion/TestCompletion.py (+14-3)
diff --git a/lldb/test/API/functionalities/completion/TestCompletion.py b/lldb/test/API/functionalities/completion/TestCompletion.py
index 0d6907e0c3d22..9959c7363aa2b 100644
--- a/lldb/test/API/functionalities/completion/TestCompletion.py
+++ b/lldb/test/API/functionalities/completion/TestCompletion.py
@@ -107,9 +107,20 @@ def test_process_unload(self):
             self, "// Break here", lldb.SBFileSpec("main.cpp")
         )
         err = lldb.SBError()
-        self.process().LoadImage(
-            lldb.SBFileSpec(self.getBuildArtifact("libshared.so")), err
-        )
+        if lldb.remote_platform:
+            self.process().LoadImage(
+                lldb.SBFileSpec(self.getBuildArtifact("libshared.so")),
+                lldb.SBFileSpec(
+                    lldbutil.append_to_process_working_directory(self, "libshared.so"),
+                    False,
+                ),
+                err,
+            )
+        else:
+            self.process().LoadImage(
+                lldb.SBFileSpec(self.getBuildArtifact("libshared.so")),
+                err,
+            )
         self.assertSuccess(err)
 
         self.complete_from_to("process unload ", "process unload 0")

Comment on lines 110 to 123
if lldb.remote_platform:
self.process().LoadImage(
lldb.SBFileSpec(self.getBuildArtifact("libshared.so")),
lldb.SBFileSpec(
lldbutil.append_to_process_working_directory(self, "libshared.so"),
False,
),
err,
)
else:
self.process().LoadImage(
lldb.SBFileSpec(self.getBuildArtifact("libshared.so")),
err,
)
Copy link
Member

Choose a reason for hiding this comment

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

I think this can be simplified to:

err = lldb.SBError()
local_spec = lldb.SBFileSpec(self.getBuildArtifact("libshared.so"))
remote_spec = lldb.SBFileSpec(lldbutil.append_to_process_working_directory(self, "libshared.so"), false) if lldb.remote_platform else lldb.SBFileSpec()
self.process().LoadImage(local_spec, remote_spec)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have updated the patch. Thanks.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Unfortunately, this ends up modifying the source tree. I'm going to revert this back to the intermediate version.

Copy link
Member

@JDevlieghere JDevlieghere left a comment

Choose a reason for hiding this comment

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

Thanks. LGTM!

@slydiman slydiman merged commit eb822dc into llvm:main May 15, 2024
4 checks passed
labath added a commit that referenced this pull request May 17, 2024
This was a side-effect of the "optimization" in #92281. Deoptimize the
code slightly.
@slydiman slydiman deleted the fix-lldb-test-TestCompletion branch July 25, 2024 21:31
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