mgorny updated this revision to Diff 371307.
mgorny added a comment.
Add fallback notes, as requested.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107811/new/
https://reviews.llvm.org/D107811
Files:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
Index: lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
===================================================================
--- lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py
@@ -156,6 +156,38 @@
finally:
self.dbg.GetSelectedPlatform().DisconnectRemote()
+ def test_file_permissions_fallback(self):
+ """Test 'platform get-permissions' fallback to fstat"""
+
+ class Responder(MockGDBServerResponder):
+ def vFile(self, packet):
+ if packet.startswith("vFile:open:"):
+ return "F5"
+ elif packet.startswith("vFile:fstat:"):
+ return "F40;" + 8 * "\0\0\1\xA4" + 4 * "\0" + 52 * "\0"
+ if packet.startswith("vFile:close:"):
+ return "F0"
+ return ""
+
+ self.server.responder = Responder()
+
+ try:
+ self.runCmd("platform select remote-gdb-server")
+ self.runCmd("platform connect connect://" +
+ self.server.get_connect_address())
+ self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+ self.match("platform get-permissions /some/file.txt",
+ [r"File permissions of /some/file\.txt \(remote\): 0o0644"])
+ self.assertPacketLogContains([
+ "vFile:mode:2f736f6d652f66696c652e747874",
+ "vFile:open:2f736f6d652f66696c652e747874,00000000,00000000",
+ "vFile:fstat:5",
+ "vFile:close:5",
+ ])
+ finally:
+ self.dbg.GetSelectedPlatform().DisconnectRemote()
+
def test_file_exists(self):
"""Test 'platform file-exists'"""
@@ -201,3 +233,58 @@
])
finally:
self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+ def test_file_exists_fallback(self):
+ """Test 'platform file-exists' fallback to open"""
+
+ class Responder(MockGDBServerResponder):
+ def vFile(self, packet):
+ if packet.startswith("vFile:open:"):
+ return "F5"
+ if packet.startswith("vFile:close:"):
+ return "F0"
+ return ""
+
+ self.server.responder = Responder()
+
+ try:
+ self.runCmd("platform select remote-gdb-server")
+ self.runCmd("platform connect connect://" +
+ self.server.get_connect_address())
+ self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+ self.match("platform file-exists /some/file.txt",
+ [r"File /some/file\.txt \(remote\) exists"])
+ self.assertPacketLogContains([
+ "vFile:exists:2f736f6d652f66696c652e747874",
+ "vFile:open:2f736f6d652f66696c652e747874,00000000,00000000",
+ "vFile:close:5",
+ ])
+ finally:
+ self.dbg.GetSelectedPlatform().DisconnectRemote()
+
+ def test_file_exists_not_fallback(self):
+ """Test 'platform file-exists' fallback to open with non-existing file"""
+
+ class Responder(MockGDBServerResponder):
+ def vFile(self, packet):
+ if packet.startswith("vFile:open:"):
+ return "F-1,2"
+ return ""
+
+ self.server.responder = Responder()
+
+ try:
+ self.runCmd("platform select remote-gdb-server")
+ self.runCmd("platform connect connect://" +
+ self.server.get_connect_address())
+ self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected())
+
+ self.match("platform file-exists /some/file.txt",
+ [r"File /some/file\.txt \(remote\) does not exist"])
+ self.assertPacketLogContains([
+ "vFile:exists:2f736f6d652f66696c652e747874",
+ "vFile:open:2f736f6d652f66696c652e747874,00000000,00000000",
+ ])
+ finally:
+ self.dbg.GetSelectedPlatform().DisconnectRemote()
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -600,7 +600,8 @@
m_supports_QEnvironment : 1, m_supports_QEnvironmentHexEncoded : 1,
m_supports_qSymbol : 1, m_qSymbol_requests_done : 1,
m_supports_qModuleInfo : 1, m_supports_jThreadsInfo : 1,
- m_supports_jModulesInfo : 1, m_supports_vFileSize : 1;
+ m_supports_jModulesInfo : 1, m_supports_vFileSize : 1,
+ m_supports_vFileMode : 1, m_supports_vFileExists : 1;
/// Current gdb remote protocol process identifier for all other operations
lldb::pid_t m_curr_pid = LLDB_INVALID_PROCESS_ID;
Index: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===================================================================
--- lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -65,7 +65,8 @@
m_supports_QEnvironmentHexEncoded(true), m_supports_qSymbol(true),
m_qSymbol_requests_done(false), m_supports_qModuleInfo(true),
m_supports_jThreadsInfo(true), m_supports_jModulesInfo(true),
- m_supports_vFileSize(true),
+ m_supports_vFileSize(true), m_supports_vFileMode(true),
+ m_supports_vFileExists(true),
m_host_arch(), m_process_arch(), m_os_build(), m_os_kernel(),
m_hostname(), m_gdb_server_name(), m_default_packet_timeout(0),
@@ -3176,37 +3177,52 @@
Status
GDBRemoteCommunicationClient::GetFilePermissions(const FileSpec &file_spec,
uint32_t &file_permissions) {
- std::string path{file_spec.GetPath(false)};
- Status error;
- lldb_private::StreamString stream;
- stream.PutCString("vFile:mode:");
- stream.PutStringAsRawHex8(path);
- StringExtractorGDBRemote response;
- if (SendPacketAndWaitForResponse(stream.GetString(), response) ==
- PacketResult::Success) {
- if (response.GetChar() != 'F') {
- error.SetErrorStringWithFormat("invalid response to '%s' packet",
+ if (m_supports_vFileMode) {
+ std::string path{file_spec.GetPath(false)};
+ Status error;
+ lldb_private::StreamString stream;
+ stream.PutCString("vFile:mode:");
+ stream.PutStringAsRawHex8(path);
+ StringExtractorGDBRemote response;
+ if (SendPacketAndWaitForResponse(stream.GetString(), response) !=
+ PacketResult::Success) {
+ error.SetErrorStringWithFormat("failed to send '%s' packet",
stream.GetData());
- } else {
- const uint32_t mode = response.GetS32(-1, 16);
- if (static_cast<int32_t>(mode) == -1) {
- if (response.GetChar() == ',') {
- int response_errno = response.GetS32(-1, 16);
- if (response_errno > 0)
- error.SetError(response_errno, lldb::eErrorTypePOSIX);
- else
- error.SetErrorToGenericError();
- } else
- error.SetErrorToGenericError();
+ return error;
+ }
+ if (!response.IsUnsupportedResponse()) {
+ if (response.GetChar() != 'F') {
+ error.SetErrorStringWithFormat("invalid response to '%s' packet",
+ stream.GetData());
} else {
- file_permissions = mode & (S_IRWXU | S_IRWXG | S_IRWXO);
+ const uint32_t mode = response.GetS32(-1, 16);
+ if (static_cast<int32_t>(mode) == -1) {
+ if (response.GetChar() == ',') {
+ int response_errno = response.GetS32(-1, 16);
+ if (response_errno > 0)
+ error.SetError(response_errno, lldb::eErrorTypePOSIX);
+ else
+ error.SetErrorToGenericError();
+ } else
+ error.SetErrorToGenericError();
+ } else {
+ file_permissions = mode & (S_IRWXU | S_IRWXG | S_IRWXO);
+ }
}
+ return error;
+ } else { // response.IsUnsupportedResponse()
+ m_supports_vFileMode = false;
}
- } else {
- error.SetErrorStringWithFormat("failed to send '%s' packet",
- stream.GetData());
}
- return error;
+
+ // Fallback to fstat. NB: unlike vFile:mode, this does not work if the server
+ // is unable to open the file.
+ llvm::Optional<GDBRemoteFStatData> st = Stat(file_spec);
+ if (st) {
+ file_permissions = st->gdb_st_mode & (S_IRWXU | S_IRWXG | S_IRWXO);
+ return Status();
+ }
+ return Status("fstat failed");
}
uint64_t GDBRemoteCommunicationClient::ReadFile(lldb::user_id_t fd,
@@ -3349,21 +3365,34 @@
// Extension of host I/O packets to get whether a file exists.
bool GDBRemoteCommunicationClient::GetFileExists(
const lldb_private::FileSpec &file_spec) {
- std::string path(file_spec.GetPath(false));
- lldb_private::StreamString stream;
- stream.PutCString("vFile:exists:");
- stream.PutStringAsRawHex8(path);
- StringExtractorGDBRemote response;
- if (SendPacketAndWaitForResponse(stream.GetString(), response) ==
- PacketResult::Success) {
- if (response.GetChar() != 'F')
- return false;
- if (response.GetChar() != ',')
+ if (m_supports_vFileExists) {
+ std::string path(file_spec.GetPath(false));
+ lldb_private::StreamString stream;
+ stream.PutCString("vFile:exists:");
+ stream.PutStringAsRawHex8(path);
+ StringExtractorGDBRemote response;
+ if (SendPacketAndWaitForResponse(stream.GetString(), response) !=
+ PacketResult::Success)
return false;
- bool retcode = (response.GetChar() != '0');
- return retcode;
+ if (!response.IsUnsupportedResponse()) {
+ if (response.GetChar() != 'F')
+ return false;
+ if (response.GetChar() != ',')
+ return false;
+ bool retcode = (response.GetChar() != '0');
+ return retcode;
+ } else
+ m_supports_vFileExists = false;
}
- return false;
+
+ // Fallback to open. NB: unlike vFile:exists, this returns file if the file
+ // cannot be opened.
+ Status error;
+ lldb::user_id_t fd = OpenFile(file_spec, File::eOpenOptionReadOnly, 0, error);
+ if (fd == UINT64_MAX)
+ return false;
+ CloseFile(fd, error);
+ return true;
}
bool GDBRemoteCommunicationClient::CalculateMD5(
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits