labath created this revision. labath added reviewers: JDevlieghere, clayborg. Herald added a subscriber: kristof.beyls. labath requested review of this revision. Herald added a project: LLDB.
We have a plugin.process.gdb-remote.packet-timeout setting, which can be used to control how long the lldb client is willing to wait before declaring the server side dead. Our test suite makes use of this feature, and sets the setting value fairly high, as the low default value can cause flaky tests, particularly on slower bots. After fixing TestPlatformConnect (one of the few tests exercising the remote platform capabilities of lldb) in 4b284b9ca <https://reviews.llvm.org/rG4b284b9ca8098e284b8d965a633b71bd283043d6>, it immediately started being flaky on the arm bots. It turns out this is because the packet-timeout setting is not being applied to platform connections. This patch makes the platform connections also respect the value of this setting. This does create a slightly weird dependency, where one plugin depends on the setting value from another plugin (of a completely different kind). However, it seemed to me that creating a separate setting for platform connections would be even more confusing. I'm open to other ideas, though. I also add a test which checks that the timeout value is being honored. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D97769 Files: lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h lldb/test/API/commands/platform/connect/TestPlatformConnect.py lldb/test/API/functionalities/gdb_remote_client/TestPlatformClient.py
Index: lldb/test/API/functionalities/gdb_remote_client/TestPlatformClient.py =================================================================== --- lldb/test/API/functionalities/gdb_remote_client/TestPlatformClient.py +++ lldb/test/API/functionalities/gdb_remote_client/TestPlatformClient.py @@ -1,6 +1,7 @@ import lldb import binascii import os +import time from lldbsuite.test.lldbtest import * from lldbsuite.test.decorators import * from gdbclientutils import * @@ -64,3 +65,41 @@ substrs=["error: no processes were found on the \"remote-linux\" platform"]) finally: self.dbg.GetSelectedPlatform().DisconnectRemote() + + class TimeoutResponder(MockGDBServerResponder): + """A mock server, which takes a very long time to compute the working + directory.""" + def __init__(self): + MockGDBServerResponder.__init__(self) + + def qGetWorkingDir(self): + time.sleep(10) + return hexlify("/foo/bar") + + def test_no_timeout(self): + """Test that we honor the timeout setting. With a large enough timeout, + we should get the CWD successfully.""" + + self.server.responder = TestPlatformClient.TimeoutResponder() + self.runCmd("settings set plugin.process.gdb-remote.packet-timeout 30") + plat = lldb.SBPlatform("remote-linux") + try: + self.assertSuccess(plat.ConnectRemote(lldb.SBPlatformConnectOptions("connect://" + + self.server.get_connect_address()))) + self.assertEqual(plat.GetWorkingDirectory(), "/foo/bar") + finally: + plat.DisconnectRemote() + + def test_timeout(self): + """Test that we honor the timeout setting. With a small timeout, CWD + retrieval should fail.""" + + self.server.responder = TestPlatformClient.TimeoutResponder() + self.runCmd("settings set plugin.process.gdb-remote.packet-timeout 3") + plat = lldb.SBPlatform("remote-linux") + try: + self.assertSuccess(plat.ConnectRemote(lldb.SBPlatformConnectOptions("connect://" + + self.server.get_connect_address()))) + self.assertIsNone(plat.GetWorkingDirectory()) + finally: + plat.DisconnectRemote() Index: lldb/test/API/commands/platform/connect/TestPlatformConnect.py =================================================================== --- lldb/test/API/commands/platform/connect/TestPlatformConnect.py +++ lldb/test/API/commands/platform/connect/TestPlatformConnect.py @@ -13,7 +13,6 @@ @expectedFailureAll(hostoslist=["windows"], triple='.*-android') @skipIfWindows # lldb-server does not terminate correctly @skipIfDarwin # lldb-server not found correctly - @skipIf(oslist=["linux"], archs=["arm", "aarch64"]) # Fails randomly @add_test_categories(["lldb-server"]) def test_platform_process_connect(self): self.build() Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h =================================================================== --- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h +++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h @@ -68,6 +68,8 @@ static const char *GetPluginDescriptionStatic(); + static std::chrono::seconds GetPacketTimeout(); + // Check if a given Process bool CanDebug(lldb::TargetSP target_sp, bool plugin_specified_by_name) override; Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp =================================================================== --- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -213,6 +213,10 @@ return process_sp; } +std::chrono::seconds ProcessGDBRemote::GetPacketTimeout() { + return std::chrono::seconds(GetGlobalPluginProperties()->GetPacketTimeout()); +} + bool ProcessGDBRemote::CanDebug(lldb::TargetSP target_sp, bool plugin_specified_by_name) { if (plugin_specified_by_name) Index: lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp =================================================================== --- lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp +++ lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp @@ -30,6 +30,7 @@ #include "lldb/Utility/UriParser.h" #include "Plugins/Process/Utility/GDBRemoteSignals.h" +#include "Plugins/Process/gdb-remote/ProcessGDBRemote.h" using namespace lldb; using namespace lldb_private; @@ -202,7 +203,10 @@ /// Default Constructor PlatformRemoteGDBServer::PlatformRemoteGDBServer() : Platform(false), // This is a remote platform - m_gdb_client() {} + m_gdb_client() { + m_gdb_client.SetPacketTimeout( + process_gdb_remote::ProcessGDBRemote::GetPacketTimeout()); +} /// Destructor. ///
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits