Skip to content

Commit 6542cf1

Browse files
authored
[lldb/platform-gdb] Do not assume a persistent connection (llvm#131736)
After https://reviews.llvm.org/D116539, when `m_gdb_client_up` in `PlatformRemoteGDBServer` is not null, the connection to a server is expected to exist. However, `PlatformRemoteGDBServer::DisconnectRemote()` is not the only way to close the connection; `GDBRemoteCommunication::WaitForPacketNoLock()` can disconnect if the server stops responding, and in this case `m_gdb_client_up` is not cleared. The patch removes this assumption and checks the connection status directly.
1 parent 33e5d01 commit 6542cf1

File tree

4 files changed

+77
-5
lines changed

4 files changed

+77
-5
lines changed

lldb/source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -206,11 +206,7 @@ bool PlatformRemoteGDBServer::SetRemoteWorkingDirectory(
206206
}
207207

208208
bool PlatformRemoteGDBServer::IsConnected() const {
209-
if (m_gdb_client_up) {
210-
assert(m_gdb_client_up->IsConnected());
211-
return true;
212-
}
213-
return false;
209+
return m_gdb_client_up && m_gdb_client_up->IsConnected();
214210
}
215211

216212
Status PlatformRemoteGDBServer::ConnectRemote(Args &args) {

lldb/unittests/Platform/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,4 @@ add_lldb_unittest(LLDBPlatformTests
1515
)
1616

1717
add_subdirectory(Android)
18+
add_subdirectory(gdb-server)
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
add_lldb_unittest(PlatformGdbRemoteTests
2+
PlatformRemoteGDBServerTest.cpp
3+
4+
LINK_LIBS
5+
lldbPluginPlatformGDB
6+
LLVMTestingSupport
7+
)
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
//===-- PlatformRemoteGDBServerTest.cpp -----------------------------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h"
10+
#include "lldb/Utility/Connection.h"
11+
#include "gmock/gmock.h"
12+
13+
using namespace lldb;
14+
using namespace lldb_private;
15+
using namespace lldb_private::platform_gdb_server;
16+
using namespace lldb_private::process_gdb_remote;
17+
using namespace testing;
18+
19+
namespace {
20+
21+
class PlatformRemoteGDBServerHack : public PlatformRemoteGDBServer {
22+
public:
23+
void
24+
SetGDBClient(std::unique_ptr<GDBRemoteCommunicationClient> gdb_client_up) {
25+
m_gdb_client_up = std::move(gdb_client_up);
26+
}
27+
};
28+
29+
class MockConnection : public lldb_private::Connection {
30+
public:
31+
MOCK_METHOD(lldb::ConnectionStatus, Connect,
32+
(llvm::StringRef url, Status *error_ptr), (override));
33+
MOCK_METHOD(lldb::ConnectionStatus, Disconnect, (Status * error_ptr),
34+
(override));
35+
MOCK_METHOD(bool, IsConnected, (), (const, override));
36+
MOCK_METHOD(size_t, Read,
37+
(void *dst, size_t dst_len, const Timeout<std::micro> &timeout,
38+
lldb::ConnectionStatus &status, Status *error_ptr),
39+
(override));
40+
MOCK_METHOD(size_t, Write,
41+
(const void *dst, size_t dst_len, lldb::ConnectionStatus &status,
42+
Status *error_ptr),
43+
(override));
44+
MOCK_METHOD(std::string, GetURI, (), (override));
45+
MOCK_METHOD(bool, InterruptRead, (), (override));
46+
};
47+
48+
} // namespace
49+
50+
TEST(PlatformRemoteGDBServerTest, IsConnected) {
51+
bool is_connected = true;
52+
53+
auto connection = std::make_unique<NiceMock<MockConnection>>();
54+
ON_CALL(*connection, IsConnected())
55+
.WillByDefault(ReturnPointee(&is_connected));
56+
57+
auto client = std::make_unique<GDBRemoteCommunicationClient>();
58+
client->SetConnection(std::move(connection));
59+
60+
PlatformRemoteGDBServerHack server;
61+
EXPECT_FALSE(server.IsConnected());
62+
63+
server.SetGDBClient(std::move(client));
64+
EXPECT_TRUE(server.IsConnected());
65+
66+
is_connected = false;
67+
EXPECT_FALSE(server.IsConnected());
68+
}

0 commit comments

Comments
 (0)