[lldb-dap] Give attach test binaries unique names

[JDevlieghere]

Give the test binaries used for attaching unique names to avoid accidentally attaching to the wrong binary.

Fixes #138197

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

Changes

Give the test binaries used for attaching unique names to avoid accidentally attaching to the wrong binary.

Fixes #138197

---

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

3 Files Affected:

• (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py (+2-2)
• (modified) lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py (+17-29)
• (modified) lldb/test/API/tools/lldb-dap/attach/TestDAP_attachByPortNum.py (+13-9)

diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
index ee5272850b9a8..8d407b8e1a17e 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
@@ -28,8 +28,8 @@ def create_debug_adapter(self, lldbDAPEnv=None, connection=None):
             env=lldbDAPEnv,
         )
 
-    def build_and_create_debug_adapter(self, lldbDAPEnv=None):
-        self.build()
+    def build_and_create_debug_adapter(self, lldbDAPEnv=None, dictionary=None):
+        self.build(dictionary=dictionary)
         self.create_debug_adapter(lldbDAPEnv)
 
     def set_source_breakpoints(self, source_path, lines, data=None):
diff --git a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py
index 6f70316821c8c..17885ef953ea3 100644
--- a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py
+++ b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py
@@ -2,13 +2,13 @@
 Test lldb-dap attach request
 """
 
-
 import dap_server
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 import lldbdap_testcase
 import os
+import uuid
 import shutil
 import subprocess
 import tempfile
@@ -25,7 +25,7 @@ def spawn_and_wait(program, delay):
     process.wait()
 
 
-@skipIf
+@skip
 class TestDAP_attach(lldbdap_testcase.DAPTestCaseBase):
     def set_and_hit_breakpoint(self, continueToExit=True):
         source = "main.c"
@@ -45,8 +45,9 @@ def test_by_pid(self):
         """
         Tests attaching to a process by process ID.
         """
-        self.build_and_create_debug_adapter()
-        program = self.getBuildArtifact("a.out")
+        unique_name = str(uuid.uuid4())
+        self.build_and_create_debug_adapter(dictionary={"EXE": unique_name})
+        program = self.getBuildArtifact(unique_name)
         self.process = subprocess.Popen(
             [program],
             stdin=subprocess.PIPE,
@@ -61,34 +62,17 @@ def test_by_name(self):
         """
         Tests attaching to a process by process name.
         """
-        self.build_and_create_debug_adapter()
-        orig_program = self.getBuildArtifact("a.out")
-        # Since we are going to attach by process name, we need a unique
-        # process name that has minimal chance to match a process that is
-        # already running. To do this we use tempfile.mktemp() to give us a
-        # full path to a location where we can copy our executable. We then
-        # run this copy to ensure we don't get the error "more that one
-        # process matches 'a.out'".
-        program = tempfile.mktemp()
-        shutil.copyfile(orig_program, program)
-        shutil.copymode(orig_program, program)
+        unique_name = str(uuid.uuid4())
+        self.build_and_create_debug_adapter(dictionary={"EXE": unique_name})
+        program = self.getBuildArtifact(unique_name)
 
         # Use a file as a synchronization point between test and inferior.
         pid_file_path = lldbutil.append_to_process_working_directory(
             self, "pid_file_%d" % (int(time.time()))
         )
 
-        def cleanup():
-            if os.path.exists(program):
-                os.unlink(program)
-            self.run_platform_command("rm %s" % (pid_file_path))
-
-        # Execute the cleanup function during test case tear down.
-        self.addTearDownHook(cleanup)
-
         popen = self.spawnSubprocess(program, [pid_file_path])
-
-        pid = lldbutil.wait_for_file_on_target(self, pid_file_path)
+        lldbutil.wait_for_file_on_target(self, pid_file_path)
 
         self.attach(program=program)
         self.set_and_hit_breakpoint(continueToExit=True)
@@ -136,8 +120,10 @@ def test_commands(self):
         "terminateCommands" are a list of LLDB commands that get executed when
         the debugger session terminates.
         """
-        self.build_and_create_debug_adapter()
-        program = self.getBuildArtifact("a.out")
+        unique_name = str(uuid.uuid4())
+        self.build_and_create_debug_adapter(dictionary={"EXE": unique_name})
+        program = self.getBuildArtifact(unique_name)
+
         # Here we just create a target and launch the process as a way to test
         # if we are able to use attach commands to create any kind of a target
         # and use it for debugging
@@ -209,8 +195,10 @@ def test_terminate_commands(self):
         Tests that the "terminateCommands", that can be passed during
         attach, are run when the debugger is disconnected.
         """
-        self.build_and_create_debug_adapter()
-        program = self.getBuildArtifact("a.out")
+        unique_name = str(uuid.uuid4())
+        self.build_and_create_debug_adapter(dictionary={"EXE": unique_name})
+        program = self.getBuildArtifact(unique_name)
+
         # Here we just create a target and launch the process as a way to test
         # if we are able to use attach commands to create any kind of a target
         # and use it for debugging
diff --git a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attachByPortNum.py b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attachByPortNum.py
index 51f62b79f3f4f..96d427979b15b 100644
--- a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attachByPortNum.py
+++ b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attachByPortNum.py
@@ -2,7 +2,6 @@
 Test lldb-dap "port" configuration to "attach" request
 """
 
-
 import dap_server
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
@@ -11,6 +10,7 @@
 from lldbgdbserverutils import Pipe
 import lldbdap_testcase
 import os
+import uuid
 import shutil
 import subprocess
 import tempfile
@@ -59,8 +59,9 @@ def test_by_port(self):
         """
         Tests attaching to a process by port.
         """
-        self.build_and_create_debug_adapter()
-        program = self.getBuildArtifact("a.out")
+        unique_name = str(uuid.uuid4())
+        self.build_and_create_debug_adapter(dictionary={"EXE": unique_name})
+        program = self.getBuildArtifact(unique_name)
 
         debug_server_tool = self.getBuiltinDebugServerTool()
 
@@ -91,8 +92,9 @@ def test_by_port_and_pid(self):
         """
         Tests attaching to a process by process ID and port number.
         """
-        self.build_and_create_debug_adapter()
-        program = self.getBuildArtifact("a.out")
+        unique_name = str(uuid.uuid4())
+        self.build_and_create_debug_adapter(dictionary={"EXE": unique_name})
+        program = self.getBuildArtifact(unique_name)
 
         # It is not necessary to launch "lldb-server" to obtain the actual port and pid for attaching.
         # However, when providing the port number and pid directly, "lldb-dap" throws an error message, which is expected.
@@ -119,8 +121,9 @@ def test_by_invalid_port(self):
         """
         Tests attaching to a process by invalid port number 0.
         """
-        self.build_and_create_debug_adapter()
-        program = self.getBuildArtifact("a.out")
+        unique_name = str(uuid.uuid4())
+        self.build_and_create_debug_adapter(dictionary={"EXE": unique_name})
+        program = self.getBuildArtifact(unique_name)
 
         port = 0
         response = self.attach(
@@ -138,8 +141,9 @@ def test_by_illegal_port(self):
         """
         Tests attaching to a process by illegal/greater port number 65536
         """
-        self.build_and_create_debug_adapter()
-        program = self.getBuildArtifact("a.out")
+        unique_name = str(uuid.uuid4())
+        self.build_and_create_debug_adapter(dictionary={"EXE": unique_name})
+        program = self.getBuildArtifact(unique_name)
 
         port = 65536
         args = [program]

[ashgti]

Does attach normally only check the basename of the executable? Or is it supposed to match the entire path?

I see a comment in TestDAP_attach.test_by_name that mentions making the path more unique, so I'm wondering if something else has changed or if that previous attempt at making the name more unique was a little off the mark.

[JDevlieghere]

I see a comment in TestDAP_attach.test_by_name that mentions making the path more unique, so I'm wondering if something else has changed or if that previous attempt at making the name more unique was a little off the mark.

Excellent question. We are indeed only using the executable name for the attach. Note how the AttachRequestHandler isn't setting the path in the attach info. We only use the program to create the target, which means we go down this path in Target::Attach:

  // If no process info was specified, then use the target executable name as
  // the process to attach to by default
  if (!attach_info.ProcessInfoSpecified()) {
    if (old_exec_module_sp)
      attach_info.GetExecutableFile().SetFilename(
            old_exec_module_sp->GetPlatformFileSpec().GetFilename());
    [...]

  }

I still think the unique test names are preferable over the old logic to change the directory, but we should fix this too.

[JDevlieghere]

#138557