[lldb] add --platform-available-ports option to the dotest.py

[dlav-sc]

This patch adds --platform-available-ports option to the dotest.py script to avoid hardcoded gdb ports in lldb testsuite.

Currently, this option could be helpful in GdbRemoteTestCases (e.g. TestLldbGdbServer, TestNonStop, TestGdbRemoteThreadsInStopReply, TestGdbRemotePlatformFile, TestGdbRemote_vCont)

[llvmbot]

@llvm/pr-subscribers-lldb

Author: None (dlav-sc)

Changes

This patch adds --platform-available-ports option to the dotest.py script to avoid hardcoded gdb ports in lldb testsuite.

Currently, this option could be helpful in GdbRemoteTestCases (e.g. TestLldbGdbServer, TestNonStop, TestGdbRemoteThreadsInStopReply, TestGdbRemotePlatformFile, TestGdbRemote_vCont)

---

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

5 Files Affected:

• (modified) lldb/packages/Python/lldbsuite/test/configuration.py (+1)
• (modified) lldb/packages/Python/lldbsuite/test/dotest.py (+2)
• (modified) lldb/packages/Python/lldbsuite/test/dotest_args.py (+7)
• (modified) lldb/packages/Python/lldbsuite/test/lldbtest.py (+4)
• (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py (+3)

diff --git a/lldb/packages/Python/lldbsuite/test/configuration.py b/lldb/packages/Python/lldbsuite/test/configuration.py
index 1bacd74a968c31..c54f8d6a3962f8 100644
--- a/lldb/packages/Python/lldbsuite/test/configuration.py
+++ b/lldb/packages/Python/lldbsuite/test/configuration.py
@@ -99,6 +99,7 @@
 lldb_platform_name = None
 lldb_platform_url = None
 lldb_platform_working_dir = None
+lldb_platform_available_ports = None
 
 # Apple SDK
 apple_sdk = None
diff --git a/lldb/packages/Python/lldbsuite/test/dotest.py b/lldb/packages/Python/lldbsuite/test/dotest.py
index 681ea1638f2d6f..7cc8f2985043e6 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -419,6 +419,8 @@ def parseOptionsAndInitTestdirs():
         configuration.lldb_platform_url = args.lldb_platform_url
     if args.lldb_platform_working_dir:
         configuration.lldb_platform_working_dir = args.lldb_platform_working_dir
+    if args.lldb_platform_available_ports:
+        configuration.lldb_platform_available_ports = args.lldb_platform_available_ports
     if platform_system == "Darwin" and args.apple_sdk:
         configuration.apple_sdk = args.apple_sdk
     if args.test_build_dir:
diff --git a/lldb/packages/Python/lldbsuite/test/dotest_args.py b/lldb/packages/Python/lldbsuite/test/dotest_args.py
index a80428ebec5891..18047fdca2a921 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest_args.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest_args.py
@@ -292,6 +292,13 @@ def create_parser():
         metavar="platform-working-dir",
         help="The directory to use on the remote platform.",
     )
+    group.add_argument(
+        "--platform-available-ports",
+        dest="lldb_platform_available_ports",
+        type=lambda ports: [int(port.strip()) for port in ports.split(":")],
+        metavar="platform-available-ports",
+        help="Ports available for connection to a lldb server on the remote platform",
+    )
 
     # Test-suite behaviour
     group = parser.add_argument_group("Runtime behaviour options")
diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 8884ef5933ada8..112f139518f393 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -747,6 +747,10 @@ def getSourcePath(self, name):
         """Return absolute path to a file in the test's source directory."""
         return os.path.join(self.getSourceDir(), name)
 
+    def getPlatformAvailablePorts(self):
+        """Return ports available for connection to a lldb server on the remote platform."""
+        return configuration.lldb_platform_available_ports
+
     @classmethod
     def setUpCommands(cls):
         commands = [
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
index 8c8e4abed0b454..f7516d26d01d79 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
@@ -185,6 +185,9 @@ def setUpServerLogging(self, is_llgs):
             ]
 
     def get_next_port(self):
+        available_ports = self.getPlatformAvailablePorts()
+        if available_ports:
+            return random.choice(available_ports)
         return 12000 + random.randint(0, 3999)
 
     def reset_test_sequence(self):

[DavidSpickett]

@dlav-sc do you still have an interest in this? If not, I will close this PR.

[dlav-sc]

@dlav-sc do you still have an interest in this? If not, I will close this PR.

Hi @DavidSpickett, I believe I still need this patch for two reasons:

1. I don't like the hardcoded ports used in the GdbRemoteTestCases for remote debugging (check out the get_next_port() function in lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py). These ports might be unavailable or not properly forwarded, which can lead to failures.

2. In our CI it is possible to have a situation when parallel testing of different builds make certain ports unavailable and qemu is not able to forward those. To avoid this issue, we can try forwarding different ports and using them instead. However, in this case, dotest.py needs to know which ports are available. The option I've added does just that - it tells dotest.py about the ports it can use during testing.

[DavidSpickett]

Yeah I didn't realise it generated them randomly, though I don't know what else it was going to do :)

Do you need a CMake variable for this too? I wonder how you are getting the argument to dotest.

For example https://lab.llvm.org/buildbot/#/builders/195/builds/5784/steps/6/logs/stdio uses CMake variables.

No harm adding one for this, as I see the other platform_ ones already there.

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[dlav-sc]

I don't know what else it was going to do :)

Yeah, me too, so I've decided to retain the old behavior if --platform-available-ports isn’t specified.

Do you need a CMake variable for this too? I wonder how you are getting the argument to dotest.

I don't think I really need a separate CMake flag here. Currently, the --platform-name option for dotest.py is passed in through the LLDB_TEST_USER_ARGS CMake variable, so I can also include --platform-available-ports there too.

[DavidSpickett]

This change is fine now I just want to understand whether you'll still get test cases competing for ports even with this option.