diff --git a/lldb/docs/lldb-platform-packets.txt b/lldb/docs/lldb-platform-packets.txt --- a/lldb/docs/lldb-platform-packets.txt +++ b/lldb/docs/lldb-platform-packets.txt @@ -369,7 +369,7 @@ // are the constant values in enum OpenOptions from lldb's File.h // 3. mode bits, base 16 // -// response is F followed by the opened file descriptor in base 10. +// response is F followed by the opened file descriptor in base 16. // "F-1,errno" with the errno if an error occurs. // //---------------------------------------------------------------------- @@ -383,7 +383,7 @@ // receive: vFile:close:7 // send: F0 // -// File descriptor is in base 10. +// File descriptor is in base 16. // "F-1,errno" with the errno if an error occurs. @@ -399,17 +399,12 @@ // send: F4;a'b\00 // // request packet has the fields: -// 1. file descriptor, base 10 -// 2. number of bytes to be read, base 10 -// 3. offset into file to start from, base 10 +// 1. file descriptor, base 16 +// 2. number of bytes to be read, base 16 +// 3. offset into file to start from, base 16 // -// Response is F, followed by the number of bytes read (base 10), a +// Response is F, followed by the number of bytes read (base 16), a // semicolon, followed by the data in the binary-escaped-data encoding. -// -// COMPATIBILITY -// The gdb-remote serial protocol documentation says that numbers -// in "vFile:" packets should be hexadecimal. Instead lldb uses -// decimal. //---------------------------------------------------------------------- @@ -424,16 +419,11 @@ // send: F1024 // // request packet has the fields: -// 1. file descriptor, base 10 -// 2. offset into file to start from, base 10 +// 1. file descriptor, base 16 +// 2. offset into file to start from, base 16 // 3. binary-escaped-data to be written // -// Response is F, followed by the number of bytes written (base 10) -// -// COMPATIBILITY -// The gdb-remote serial protocol documentation says that numbers -// in "vFile:" packets should be hexadecimal. Instead lldb uses -// decimal. +// Response is F, followed by the number of bytes written (base 16) diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -2959,7 +2959,7 @@ if (response.GetChar() != 'F') return Status("invalid response to '%s' packet", packet.str().c_str()); - return Status(response.GetU32(UINT32_MAX), eErrorTypePOSIX); + return Status(response.GetHexMaxU32(false, UINT32_MAX), eErrorTypePOSIX); } Status @@ -2980,7 +2980,7 @@ if (response.GetChar() != 'F') return Status("invalid response to '%s' packet", stream.GetData()); - return Status(response.GetU32(UINT32_MAX), eErrorTypePOSIX); + return Status(response.GetHexMaxU32(false, UINT32_MAX), eErrorTypePOSIX); } static uint64_t ParseHostIOPacketResponse(StringExtractorGDBRemote &response, @@ -2988,11 +2988,11 @@ response.SetFilePos(0); if (response.GetChar() != 'F') return fail_result; - int32_t result = response.GetS32(-2); + int32_t result = response.GetS32(-2, 16); if (result == -2) return fail_result; if (response.GetChar() == ',') { - int result_errno = response.GetS32(-2); + int result_errno = response.GetS32(-2, 16); if (result_errno != -2) error.SetError(result_errno, eErrorTypePOSIX); else @@ -3026,7 +3026,7 @@ bool GDBRemoteCommunicationClient::CloseFile(lldb::user_id_t fd, Status &error) { lldb_private::StreamString stream; - stream.Printf("vFile:close:%i", (int)fd); + stream.Printf("vFile:close:%x", (int)fd); StringExtractorGDBRemote response; if (SendPacketAndWaitForResponse(stream.GetString(), response) == PacketResult::Success) { @@ -3093,10 +3093,10 @@ error.SetErrorStringWithFormat("invalid response to '%s' packet", stream.GetData()); } else { - const uint32_t mode = response.GetS32(-1); + const uint32_t mode = response.GetS32(-1, 16); if (static_cast(mode) == -1) { if (response.GetChar() == ',') { - int response_errno = response.GetS32(-1); + int response_errno = response.GetS32(-1, 16); if (response_errno > 0) error.SetError(response_errno, lldb::eErrorTypePOSIX); else @@ -3119,16 +3119,23 @@ uint64_t dst_len, Status &error) { lldb_private::StreamString stream; - stream.Printf("vFile:pread:%i,%" PRId64 ",%" PRId64, (int)fd, dst_len, + stream.Printf("vFile:pread:%x,%" PRIx64 ",%" PRIx64, (int)fd, dst_len, offset); StringExtractorGDBRemote response; if (SendPacketAndWaitForResponse(stream.GetString(), response) == PacketResult::Success) { if (response.GetChar() != 'F') return 0; - uint32_t retcode = response.GetHexMaxU32(false, UINT32_MAX); - if (retcode == UINT32_MAX) - return retcode; + int64_t retcode = response.GetS64(-1, 16); + if (retcode == -1) { + error.SetErrorToGenericError(); + if (response.GetChar() == ',') { + int response_errno = response.GetS32(-1, 16); + if (response_errno > 0) + error.SetError(response_errno, lldb::eErrorTypePOSIX); + } + return -1; + } const char next = (response.Peek() ? *response.Peek() : 0); if (next == ',') return 0; @@ -3153,7 +3160,7 @@ uint64_t src_len, Status &error) { lldb_private::StreamGDBRemote stream; - stream.Printf("vFile:pwrite:%i,%" PRId64 ",", (int)fd, offset); + stream.Printf("vFile:pwrite:%x,%" PRIx64 ",", (int)fd, offset); stream.PutEscapedBytes(src, src_len); StringExtractorGDBRemote response; if (SendPacketAndWaitForResponse(stream.GetString(), response) == @@ -3162,15 +3169,15 @@ error.SetErrorStringWithFormat("write file failed"); return 0; } - uint64_t bytes_written = response.GetU64(UINT64_MAX); - if (bytes_written == UINT64_MAX) { + int64_t bytes_written = response.GetS64(-1, 16); + if (bytes_written == -1) { error.SetErrorToGenericError(); if (response.GetChar() == ',') { - int response_errno = response.GetS32(-1); + int response_errno = response.GetS32(-1, 16); if (response_errno > 0) error.SetError(response_errno, lldb::eErrorTypePOSIX); } - return 0; + return -1; } return bytes_written; } else { @@ -3194,11 +3201,11 @@ if (SendPacketAndWaitForResponse(stream.GetString(), response) == PacketResult::Success) { if (response.GetChar() == 'F') { - uint32_t result = response.GetU32(UINT32_MAX); + uint32_t result = response.GetHexMaxU32(false, UINT32_MAX); if (result != 0) { error.SetErrorToGenericError(); if (response.GetChar() == ',') { - int response_errno = response.GetS32(-1); + int response_errno = response.GetS32(-1, 16); if (response_errno > 0) error.SetError(response_errno, lldb::eErrorTypePOSIX); } @@ -3225,11 +3232,11 @@ if (SendPacketAndWaitForResponse(stream.GetString(), response) == PacketResult::Success) { if (response.GetChar() == 'F') { - uint32_t result = response.GetU32(UINT32_MAX); + uint32_t result = response.GetHexMaxU32(false, UINT32_MAX); if (result != 0) { error.SetErrorToGenericError(); if (response.GetChar() == ',') { - int response_errno = response.GetS32(-1); + int response_errno = response.GetS32(-1, 16); if (response_errno > 0) error.SetError(response_errno, lldb::eErrorTypePOSIX); } diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp @@ -509,22 +509,21 @@ // Do not close fd. auto file = FileSystem::Instance().Open(path_spec, flags, mode, false); - int save_errno = 0; + StreamString response; + response.PutChar('F'); + int descriptor = File::kInvalidDescriptor; if (file) { descriptor = file.get()->GetDescriptor(); + response.Printf("%x", descriptor); } else { + response.PutCString("-1"); std::error_code code = errorToErrorCode(file.takeError()); if (code.category() == std::system_category()) { - save_errno = code.value(); + response.Printf(",%x", code.value()); } } - StreamString response; - response.PutChar('F'); - response.Printf("%i", descriptor); - if (save_errno) - response.Printf(",%i", save_errno); return SendPacketNoLock(response.GetString()); } } @@ -536,7 +535,7 @@ GDBRemoteCommunicationServerCommon::Handle_vFile_Close( StringExtractorGDBRemote &packet) { packet.SetFilePos(::strlen("vFile:close:")); - int fd = packet.GetS32(-1); + int fd = packet.GetS32(-1, 16); int err = -1; int save_errno = 0; if (fd >= 0) { @@ -549,9 +548,9 @@ } StreamString response; response.PutChar('F'); - response.Printf("%i", err); + response.Printf("%x", err); if (save_errno) - response.Printf(",%i", save_errno); + response.Printf(",%x", save_errno); return SendPacketNoLock(response.GetString()); } @@ -560,28 +559,29 @@ StringExtractorGDBRemote &packet) { StreamGDBRemote response; packet.SetFilePos(::strlen("vFile:pread:")); - int fd = packet.GetS32(-1); + int fd = packet.GetS32(-1, 16); if (packet.GetChar() == ',') { - size_t count = packet.GetU64(SIZE_MAX); + size_t count = packet.GetHexMaxU64(false, SIZE_MAX); if (packet.GetChar() == ',') { - off_t offset = packet.GetU64(UINT32_MAX); + off_t offset = packet.GetHexMaxU32(false, UINT32_MAX); if (count == SIZE_MAX) { - response.Printf("F-1:%i", EINVAL); + response.Printf("F-1:%x", EINVAL); return SendPacketNoLock(response.GetString()); } std::string buffer(count, 0); NativeFile file(fd, File::eOpenOptionReadOnly, false); Status error = file.Read(static_cast(&buffer[0]), count, offset); - const ssize_t bytes_read = error.Success() ? count : -1; const int save_errno = error.GetError(); response.PutChar('F'); - response.Printf("%zi", bytes_read); - if (save_errno) - response.Printf(",%i", save_errno); - else { + if (error.Success()) { + response.Printf("%zx", count); response.PutChar(';'); - response.PutEscapedBytes(&buffer[0], bytes_read); + response.PutEscapedBytes(&buffer[0], count); + } else { + response.PutCString("-1"); + if (save_errno) + response.Printf(",%x", save_errno); } return SendPacketNoLock(response.GetString()); } @@ -597,9 +597,9 @@ StreamGDBRemote response; response.PutChar('F'); - int fd = packet.GetU32(UINT32_MAX); + int fd = packet.GetS32(-1, 16); if (packet.GetChar() == ',') { - off_t offset = packet.GetU64(UINT32_MAX); + off_t offset = packet.GetHexMaxU32(false, UINT32_MAX); if (packet.GetChar() == ',') { std::string buffer; if (packet.GetEscapedBinaryData(buffer)) { @@ -607,13 +607,16 @@ size_t count = buffer.size(); Status error = file.Write(static_cast(&buffer[0]), count, offset); - const ssize_t bytes_written = error.Success() ? count : -1; const int save_errno = error.GetError(); - response.Printf("%zi", bytes_written); - if (save_errno) - response.Printf(",%i", save_errno); + if (error.Success()) + response.Printf("%zx", count); + else { + response.PutCString("-1"); + if (save_errno) + response.Printf(",%x", save_errno); + } } else { - response.Printf("-1,%i", EINVAL); + response.Printf("-1,%x", EINVAL); } return SendPacketNoLock(response.GetString()); } @@ -655,9 +658,9 @@ std::error_code ec; const uint32_t mode = FileSystem::Instance().GetPermissions(file_spec, ec); StreamString response; - response.Printf("F%u", mode); + response.Printf("F%x", mode); if (mode == 0 || ec) - response.Printf(",%i", (int)Status(ec).GetError()); + response.Printf(",%x", (int)Status(ec).GetError()); return SendPacketNoLock(response.GetString()); } return SendErrorResponse(23); @@ -697,7 +700,7 @@ Status error = FileSystem::Instance().Symlink(src_spec, FileSpec(dst)); StreamString response; - response.Printf("F%u,%u", error.GetError(), error.GetError()); + response.Printf("F%x,%x", error.GetError(), error.GetError()); return SendPacketNoLock(response.GetString()); } @@ -709,7 +712,7 @@ packet.GetHexByteString(path); Status error(llvm::sys::fs::remove(path)); StreamString response; - response.Printf("F%u,%u", error.GetError(), error.GetError()); + response.Printf("F%x,%x", error.GetError(), error.GetError()); return SendPacketNoLock(response.GetString()); } @@ -791,7 +794,7 @@ Status error(llvm::sys::fs::create_directory(path, mode)); StreamGDBRemote response; - response.Printf("F%u", error.GetError()); + response.Printf("F%x", error.GetError()); return SendPacketNoLock(response.GetString()); } @@ -811,7 +814,7 @@ Status error(llvm::sys::fs::setPermissions(path, perms)); StreamGDBRemote response; - response.Printf("F%u", error.GetError()); + response.Printf("F%x", error.GetError()); return SendPacketNoLock(response.GetString()); } diff --git a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py --- a/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py +++ b/lldb/test/API/functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py @@ -2,12 +2,20 @@ class TestGDBRemotePlatformFile(GDBRemoteTestBase): - def test_file_open(self): - """Test mock-opening a remote file""" + def test_file(self): + """Test mock operations on a remote file""" class Responder(MockGDBServerResponder): def vFile(self, packet): - return "F10" + if packet.startswith("vFile:open:"): + return "F10" + elif packet.startswith("vFile:pread:"): + return "Fd;frobnicator" + elif packet.startswith("vFile:pwrite:"): + return "Fa" + elif packet.startswith("vFile:close:"): + return "F0" + return "F-1,16" self.server.responder = Responder() @@ -17,9 +25,54 @@ self.server.get_connect_address()) self.assertTrue(self.dbg.GetSelectedPlatform().IsConnected()) - self.runCmd("platform file open /some/file.txt -v 0755") + self.match("platform file open /some/file.txt -v 0755", + [r"File Descriptor = 16"]) + self.match("platform file read 16 -o 11 -c 13", + [r"Return = 11\nData = \"frobnicator\""]) + self.match("platform file write 16 -o 11 -d teststring", + [r"Return = 10"]) + self.match("platform file close 16", + [r"file 16 closed."]) self.assertPacketLogContains([ - "vFile:open:2f736f6d652f66696c652e747874,0000020a,000001ed" + "vFile:open:2f736f6d652f66696c652e747874,0000020a,000001ed", + "vFile:pread:10,d,b", + "vFile:pwrite:10,b,teststring", + "vFile:close:10", + ]) + finally: + self.dbg.GetSelectedPlatform().DisconnectRemote() + + def test_file_fail(self): + """Test mocked failures of remote operations""" + + class Responder(MockGDBServerResponder): + def vFile(self, packet): + return "F-1,16" + + 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 open /some/file.txt -v 0755", + [r"error: Invalid argument"], + error=True) + # TODO: fix the commands to fail on unsuccessful result + self.match("platform file read 16 -o 11 -c 13", + [r"Return = -1\nData = \"\""]) + self.match("platform file write 16 -o 11 -d teststring", + [r"Return = -1"]) + self.match("platform file close 16", + [r"error: Invalid argument"], + error=True) + self.assertPacketLogContains([ + "vFile:open:2f736f6d652f66696c652e747874,0000020a,000001ed", + "vFile:pread:10,d,b", + "vFile:pwrite:10,b,teststring", + "vFile:close:10", ]) finally: self.dbg.GetSelectedPlatform().DisconnectRemote() diff --git a/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py b/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py new file mode 100644 --- /dev/null +++ b/lldb/test/API/tools/lldb-server/TestGdbRemotePlatformFile.py @@ -0,0 +1,215 @@ +from __future__ import print_function + +# lldb test suite imports +from lldbsuite.test.decorators import * +from lldbsuite.test.lldbtest import TestBase + +# gdb-remote-specific imports +import lldbgdbserverutils +from gdbremote_testcase import GdbRemoteTestCaseBase + +import binascii +import stat +import tempfile + + +class TestGdbRemotePlatformFile(GdbRemoteTestCaseBase): + + mydir = TestBase.compute_mydir(__file__) + + def test_platform_file_rdonly(self): + self.vFile_test(read=True) + + def test_platform_file_wronly(self): + self.vFile_test(write=True) + + def test_platform_file_rdwr(self): + self.vFile_test(read=True, write=True) + + def test_platform_file_wronly_append(self): + self.vFile_test(write=True, append=True) + + def test_platform_file_rdwr_append(self): + self.vFile_test(read=True, write=True, append=True) + + def test_platform_file_wronly_trunc(self): + self.vFile_test(write=True, trunc=True) + + def test_platform_file_rdwr_trunc(self): + self.vFile_test(read=True, write=True, trunc=True) + + def test_platform_file_wronly_creat(self): + self.vFile_test(write=True, creat=True) + + def test_platform_file_wronly_creat_excl(self): + self.vFile_test(write=True, creat=True, excl=True) + + def test_platform_file_wronly_fail(self): + server = self.connect_to_debug_monitor() + self.assertIsNotNone(server) + + # create a temporary directory + with tempfile.TemporaryDirectory() as temp_dir: + temp_path = os.path.join(temp_dir, "test") + self.assertFalse(os.path.exists(temp_path)) + + # attempt to open the file without O_CREAT + self.do_handshake() + self.test_sequence.add_log_lines( + ["read packet: $vFile:open:%s,1,0#00" % ( + binascii.b2a_hex(temp_path.encode()).decode(),), + {"direction": "send", + "regex": r"^\$F-1,[0-9a-fA-F]+#[0-9a-fA-F]{2}$"}], + True) + self.expect_gdbremote_sequence() + + def test_platform_file_wronly_creat_excl_fail(self): + server = self.connect_to_debug_monitor() + self.assertIsNotNone(server) + + with tempfile.NamedTemporaryFile() as temp_file: + # attempt to open the file with O_CREAT|O_EXCL + self.do_handshake() + self.test_sequence.add_log_lines( + ["read packet: $vFile:open:%s,a01,0#00" % ( + binascii.b2a_hex(temp_file.name.encode()).decode(),), + {"direction": "send", + "regex": r"^\$F-1,[0-9a-fA-F]+#[0-9a-fA-F]{2}$"}], + True) + self.expect_gdbremote_sequence() + + def expect_error(self): + self.test_sequence.add_log_lines( + [{"direction": "send", + "regex": r"^\$F-1,[0-9a-fA-F]+#[0-9a-fA-F]{2}$"}], + True) + self.expect_gdbremote_sequence() + + def vFile_test(self, read=False, write=False, append=False, trunc=False, + creat=False, excl=False): + if read and write: + mode = 2 + elif write: + mode = 1 + else: # read + mode = 0 + if append: + mode |= 8 + if creat: + mode |= 0x200 + if trunc: + mode |= 0x400 + if excl: + mode |= 0x800 + + server = self.connect_to_debug_monitor() + self.assertIsNotNone(server) + + # create a temporary file with some data + test_data = 'some test data longer than 16 bytes\n' + if creat: + temp_dir = tempfile.TemporaryDirectory() + else: + temp_file = tempfile.NamedTemporaryFile() + + try: + if creat: + temp_path = os.path.join(temp_dir.name, "test") + self.assertFalse(os.path.exists(temp_path)) + else: + temp_file.write(test_data.encode()) + temp_file.flush() + temp_path = temp_file.name + + # open the file for reading + self.do_handshake() + self.test_sequence.add_log_lines( + ["read packet: $vFile:open:%s,%x,1a0#00" % ( + binascii.b2a_hex(temp_path.encode()).decode(), + mode), + {"direction": "send", + "regex": r"^\$F([0-9a-fA-F]+)#[0-9a-fA-F]{2}$", + "capture": {1: "fd"}}], + True) + + context = self.expect_gdbremote_sequence() + self.assertIsNotNone(context) + fd = int(context["fd"], 16) + + # read data from the file + self.reset_test_sequence() + self.test_sequence.add_log_lines( + ["read packet: $vFile:pread:%x,11,10#00" % (fd,)], + True) + if read: + self.test_sequence.add_log_lines( + [{"direction": "send", + "regex": r"^\$F([0-9a-fA-F]+);(.*)#[0-9a-fA-F]{2}$", + "capture": {1: "size", 2: "data"}}], + True) + context = self.expect_gdbremote_sequence() + self.assertIsNotNone(context) + if trunc: + self.assertEqual(context["size"], "0") + self.assertEqual(context["data"], "") + else: + self.assertEqual(context["size"], "11") # hex + self.assertEqual(context["data"], test_data[0x10:0x10 + 0x11]) + else: + self.expect_error() + + # another offset + if read and not trunc: + self.reset_test_sequence() + self.test_sequence.add_log_lines( + ["read packet: $vFile:pread:%x,6,3#00" % (fd,), + {"direction": "send", + "regex": r"^\$F([0-9a-fA-F]+);(.+)#[0-9a-fA-F]{2}$", + "capture": {1: "size", 2: "data"}}], + True) + context = self.expect_gdbremote_sequence() + self.assertIsNotNone(context) + self.assertEqual(context["size"], "6") # hex + self.assertEqual(context["data"], test_data[3:3 + 6]) + + # write data to the file + self.reset_test_sequence() + self.test_sequence.add_log_lines( + ["read packet: $vFile:pwrite:%x,6,somedata#00" % (fd,)], + True) + if write: + self.test_sequence.add_log_lines( + ["send packet: $F8#00"], + True) + self.expect_gdbremote_sequence() + else: + self.expect_error() + + # close the file + self.reset_test_sequence() + self.test_sequence.add_log_lines( + ["read packet: $vFile:close:%x#00" % (fd,), + "send packet: $F0#00"], + True) + self.expect_gdbremote_sequence() + + if write: + # check if the data was actually written + if creat: + temp_file = open(temp_path, "rb") + self.assertEqual(os.fstat(temp_file.fileno()).st_mode & 0o7777, + 0o640) + temp_file.seek(0) + data = test_data.encode() + if trunc or creat: + data = b"\0" * 6 + b"somedata" + elif append: + data += b"somedata" + else: + data = data[:6] + b"somedata" + data[6 + 8:] + self.assertEqual(temp_file.read(), data) + finally: + if creat: + temp_dir.cleanup() + else: + temp_file.close()