[Lldb-commits] [PATCH] D70847: [lldb] Set executable module when adding modules to the Target

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

So it might be better to create a ELF file for a remote platform, then run "obj2yaml" on it. Then the test will run "yaml2obj" on it, there are many test cases that do this, so look for "*.yaml" in the test directories. Then the test can check for a hard coded target triple, and also verify that the platform was changed. I will add inline comments for clarification in the test python file.



================
Comment at: lldb/packages/Python/lldbsuite/test/commands/target/set-exec/TestSetExecutable.py:16
+        """Test adding images to the target."""
+        self.build()
+
----------------
Create a yaml file and put it in the same directory as this file. Change this line to be:
```
        src_dir = self.getSourceDir()
        yaml_path = os.path.join(src_dir, "a.yaml")
        obj_path = self.getBuildArtifact("a.out")
        self.yaml2obj(yaml_path, obj_path)
```
Make an ELF file that that is for a remote platform where the triple and platform won't match typical hosts.


================
Comment at: lldb/packages/Python/lldbsuite/test/commands/target/set-exec/TestSetExecutable.py:21
+        self.assertTrue(target, VALID_TARGET)
+        self.assertEqual(len(target.GetTriple()), 0)
+
----------------
Don't know if I would run this assert here. I could see a target that is created with nothing possibly using the host architecture and host platform by default. So maybe remote this assert where the target triple is not set.


================
Comment at: lldb/packages/Python/lldbsuite/test/commands/target/set-exec/TestSetExecutable.py:26
+        target.AddModule(self.getBuildArtifact("a.out"), None, None)
+        self.assertNotEqual(len(target.GetTriple()), 0)
----------------
Check the exact triple of the remote executable you end up adding.

Also add a check that the platform gets updated. Platforms must be compatible with the architecture of the target's main executable, so it is good to verify that the platform gets updated as well. Something like:

```
platform = target.GetPlatform()
set.assertEqual(platform.GetName(), "remote-linux")
```

This will make this test complete. If the platform isn't right we will need to fix this as well.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70847/new/

https://reviews.llvm.org/D70847