Author: Ebuka Ezike Date: 2025-06-18T22:48:24+01:00 New Revision: 51aa6a4993ea18c968a087352d1cf569c840c41f
URL: https://github.com/llvm/llvm-project/commit/51aa6a4993ea18c968a087352d1cf569c840c41f DIFF: https://github.com/llvm/llvm-project/commit/51aa6a4993ea18c968a087352d1cf569c840c41f.diff LOG: [lldb-dap] Use protocol types for ReadMemory request (#144552) Read memory from process instead of target. Added: Modified: lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py lldb/tools/lldb-dap/Handler/ReadMemoryRequestHandler.cpp lldb/tools/lldb-dap/Handler/RequestHandler.h lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp lldb/tools/lldb-dap/Protocol/ProtocolRequests.h lldb/unittests/DAP/ProtocolTypesTest.cpp Removed: ################################################################################ diff --git a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py index 74062f3ab2164..55fb4a961e783 100644 --- a/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py +++ b/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py @@ -111,8 +111,17 @@ def test_readMemory(self): # VS Code sends those in order to check if a `memoryReference` can actually be dereferenced. mem = self.dap_server.request_readMemory(memref, 0, 0) self.assertEqual(mem["success"], True) - self.assertEqual(mem["body"]["data"], "") + self.assertNotIn( + "data", mem["body"], f"expects no data key in response: {mem!r}" + ) + + # Reads at offset 0x0 return unreadable bytes + bytes_to_read = 6 + mem = self.dap_server.request_readMemory("0x0", 0, bytes_to_read) + self.assertEqual(mem["body"]["unreadableBytes"], bytes_to_read) + + # Reads with invalid address fails. + mem = self.dap_server.request_readMemory("-3204", 0, 10) + self.assertFalse(mem["success"], "expect fail on reading memory.") - # Reads at offset 0x0 fail - mem = self.dap_server.request_readMemory("0x0", 0, 6) - self.assertEqual(mem["success"], False) + self.continue_to_exit() diff --git a/lldb/tools/lldb-dap/Handler/ReadMemoryRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/ReadMemoryRequestHandler.cpp index 891c2af4f2f28..7065b6a24b554 100644 --- a/lldb/tools/lldb-dap/Handler/ReadMemoryRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/ReadMemoryRequestHandler.cpp @@ -7,136 +7,47 @@ //===----------------------------------------------------------------------===// #include "DAP.h" -#include "EventHelper.h" #include "JSONUtils.h" #include "RequestHandler.h" #include "llvm/ADT/StringExtras.h" -#include "llvm/Support/Base64.h" namespace lldb_dap { -// "ReadMemoryRequest": { -// "allOf": [ { "$ref": "#/definitions/Request" }, { -// "type": "object", -// "description": "Reads bytes from memory at the provided location. Clients -// should only call this request if the corresponding -// capability `supportsReadMemoryRequest` is true.", -// "properties": { -// "command": { -// "type": "string", -// "enum": [ "readMemory" ] -// }, -// "arguments": { -// "$ref": "#/definitions/ReadMemoryArguments" -// } -// }, -// "required": [ "command", "arguments" ] -// }] -// }, -// "ReadMemoryArguments": { -// "type": "object", -// "description": "Arguments for `readMemory` request.", -// "properties": { -// "memoryReference": { -// "type": "string", -// "description": "Memory reference to the base location from which data -// should be read." -// }, -// "offset": { -// "type": "integer", -// "description": "Offset (in bytes) to be applied to the reference -// location before reading data. Can be negative." -// }, -// "count": { -// "type": "integer", -// "description": "Number of bytes to read at the specified location and -// offset." -// } -// }, -// "required": [ "memoryReference", "count" ] -// }, -// "ReadMemoryResponse": { -// "allOf": [ { "$ref": "#/definitions/Response" }, { -// "type": "object", -// "description": "Response to `readMemory` request.", -// "properties": { -// "body": { -// "type": "object", -// "properties": { -// "address": { -// "type": "string", -// "description": "The address of the first byte of data returned. -// Treated as a hex value if prefixed with `0x`, or -// as a decimal value otherwise." -// }, -// "unreadableBytes": { -// "type": "integer", -// "description": "The number of unreadable bytes encountered after -// the last successfully read byte.\nThis can be -// used to determine the number of bytes that should -// be skipped before a subsequent -// `readMemory` request succeeds." -// }, -// "data": { -// "type": "string", -// "description": "The bytes read from memory, encoded using base64. -// If the decoded length of `data` is less than the -// requested `count` in the original `readMemory` -// request, and `unreadableBytes` is zero or -// omitted, then the client should assume it's -// reached the end of readable memory." -// } -// }, -// "required": [ "address" ] -// } -// } -// }] -// }, -void ReadMemoryRequestHandler::operator()( - const llvm::json::Object &request) const { - llvm::json::Object response; - FillResponse(request, response); - auto *arguments = request.getObject("arguments"); +// Reads bytes from memory at the provided location. +// +// Clients should only call this request if the corresponding capability +// `supportsReadMemoryRequest` is true +llvm::Expected<protocol::ReadMemoryResponseBody> +ReadMemoryRequestHandler::Run(const protocol::ReadMemoryArguments &args) const { + const lldb::addr_t raw_address = args.memoryReference + args.offset; - llvm::StringRef memoryReference = - GetString(arguments, "memoryReference").value_or(""); - auto addr_opt = DecodeMemoryReference(memoryReference); - if (!addr_opt.has_value()) { - response["success"] = false; - response["message"] = - "Malformed memory reference: " + memoryReference.str(); - dap.SendJSON(llvm::json::Value(std::move(response))); - return; - } - lldb::addr_t addr_int = *addr_opt; - addr_int += GetInteger<uint64_t>(arguments, "offset").value_or(0); - const uint64_t count_requested = - GetInteger<uint64_t>(arguments, "count").value_or(0); + lldb::SBProcess process = dap.target.GetProcess(); + if (!lldb::SBDebugger::StateIsStoppedState(process.GetState())) + return llvm::make_error<NotStoppedError>(); + const uint64_t count_read = std::max<uint64_t>(args.count, 1); // We also need support reading 0 bytes // VS Code sends those requests to check if a `memoryReference` // can be dereferenced. - const uint64_t count_read = std::max<uint64_t>(count_requested, 1); - std::vector<uint8_t> buf; - buf.resize(count_read); + protocol::ReadMemoryResponseBody response; + std::vector<std::byte> &buffer = response.data; + buffer.resize(count_read); + lldb::SBError error; - lldb::SBAddress addr{addr_int, dap.target}; - size_t count_result = - dap.target.ReadMemory(addr, buf.data(), count_read, error); - if (count_result == 0) { - response["success"] = false; - EmplaceSafeString(response, "message", error.GetCString()); - dap.SendJSON(llvm::json::Value(std::move(response))); - return; + const size_t memory_count = dap.target.GetProcess().ReadMemory( + raw_address, buffer.data(), buffer.size(), error); + + response.address = "0x" + llvm::utohexstr(raw_address); + + // reading memory may fail for multiple reasons. memory not readable, + // reading out of memory range and gaps in memory. return from + // the last readable byte. + if (error.Fail() && (memory_count < count_read)) { + response.unreadableBytes = count_read - memory_count; } - buf.resize(std::min<size_t>(count_result, count_requested)); - llvm::json::Object body; - std::string formatted_addr = "0x" + llvm::utohexstr(addr_int); - body.try_emplace("address", formatted_addr); - body.try_emplace("data", llvm::encodeBase64(buf)); - response.try_emplace("body", std::move(body)); - dap.SendJSON(llvm::json::Value(std::move(response))); + buffer.resize(std::min<size_t>(memory_count, args.count)); + return response; } } // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h index 054cc7a321316..e35b9830ab60f 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.h +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h @@ -564,14 +564,17 @@ class DisassembleRequestHandler final Run(const protocol::DisassembleArguments &args) const override; }; -class ReadMemoryRequestHandler : public LegacyRequestHandler { +class ReadMemoryRequestHandler final + : public RequestHandler<protocol::ReadMemoryArguments, + llvm::Expected<protocol::ReadMemoryResponseBody>> { public: - using LegacyRequestHandler::LegacyRequestHandler; + using RequestHandler::RequestHandler; static llvm::StringLiteral GetCommand() { return "readMemory"; } FeatureSet GetSupportedFeatures() const override { return {protocol::eAdapterFeatureReadMemoryRequest}; } - void operator()(const llvm::json::Object &request) const override; + llvm::Expected<protocol::ReadMemoryResponseBody> + Run(const protocol::ReadMemoryArguments &args) const override; }; class CancelRequestHandler : public RequestHandler<protocol::CancelArguments, diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp index e6ba54ed4dcd6..9bd84a6c898f9 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp @@ -7,9 +7,11 @@ //===----------------------------------------------------------------------===// #include "Protocol/ProtocolRequests.h" +#include "JSONUtils.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/StringMap.h" #include "llvm/ADT/StringRef.h" +#include "llvm/Support/Base64.h" #include "llvm/Support/JSON.h" #include <utility> @@ -480,4 +482,39 @@ json::Value toJSON(const DisassembleResponseBody &DRB) { return json::Object{{"instructions", DRB.instructions}}; } +bool fromJSON(const json::Value &Params, ReadMemoryArguments &RMA, + json::Path P) { + json::ObjectMapper O(Params, P); + + const json::Object *rma_obj = Params.getAsObject(); + constexpr llvm::StringRef ref_key = "memoryReference"; + const std::optional<llvm::StringRef> memory_ref = rma_obj->getString(ref_key); + if (!memory_ref) { + P.field(ref_key).report("missing value"); + return false; + } + + const std::optional<lldb::addr_t> addr_opt = + DecodeMemoryReference(*memory_ref); + if (!addr_opt) { + P.field(ref_key).report("Malformed memory reference"); + return false; + } + + RMA.memoryReference = *addr_opt; + + return O && O.map("count", RMA.count) && O.mapOptional("offset", RMA.offset); +} + +json::Value toJSON(const ReadMemoryResponseBody &RMR) { + json::Object result{{"address", RMR.address}}; + + if (RMR.unreadableBytes != 0) + result.insert({"unreadableBytes", RMR.unreadableBytes}); + if (!RMR.data.empty()) + result.insert({"data", llvm::encodeBase64(RMR.data)}); + + return result; +} + } // namespace lldb_dap::protocol diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h index 01b8f2445c9fa..7d9a99fdacce6 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h @@ -839,6 +839,42 @@ bool fromJSON(const llvm::json::Value &, DisassembleResponseBody &, llvm::json::Path); llvm::json::Value toJSON(const DisassembleResponseBody &); +/// Arguments for `readMemory` request. +struct ReadMemoryArguments { + /// Memory reference to the base location from which data should be read. + lldb::addr_t memoryReference; + + /// Offset (in bytes) to be applied to the reference location before reading + /// data. Can be negative. + int64_t offset = 0; + + /// Number of bytes to read at the specified location and offset. + uint64_t count; +}; +bool fromJSON(const llvm::json::Value &, ReadMemoryArguments &, + llvm::json::Path); + +/// Response to `readMemory` request. +struct ReadMemoryResponseBody { + /// The address of the first byte of data returned. + /// Treated as a hex value if prefixed with `0x`, or as a decimal value + /// otherwise. + std::string address; + + /// The number of unreadable bytes encountered after the last successfully + /// read byte. + /// This can be used to determine the number of bytes that should be skipped + /// before a subsequent `readMemory` request succeeds. + uint64_t unreadableBytes = 0; + + /// The bytes read from memory, encoded using base64. If the decoded length + /// of `data` is less than the requested `count` in the original `readMemory` + /// request, and `unreadableBytes` is zero or omitted, then the client should + /// assume it's reached the end of readable memory. + std::vector<std::byte> data; +}; +llvm::json::Value toJSON(const ReadMemoryResponseBody &); + } // namespace lldb_dap::protocol #endif diff --git a/lldb/unittests/DAP/ProtocolTypesTest.cpp b/lldb/unittests/DAP/ProtocolTypesTest.cpp index 46a09f090fea2..9c93eb8c94b05 100644 --- a/lldb/unittests/DAP/ProtocolTypesTest.cpp +++ b/lldb/unittests/DAP/ProtocolTypesTest.cpp @@ -765,3 +765,36 @@ TEST(ProtocolTypesTest, StepInTarget) { EXPECT_EQ(target.endLine, deserialized_target->endLine); EXPECT_EQ(target.endColumn, deserialized_target->endColumn); } + +TEST(ProtocolTypesTest, ReadMemoryArguments) { + ReadMemoryArguments args; + args.count = 20; + args.memoryReference = 43962; + args.offset = 0; + + llvm::Expected<ReadMemoryArguments> expected = + parse<ReadMemoryArguments>(R"({"memoryReference":"-4000", "count": 20})"); + ASSERT_THAT_EXPECTED(expected, llvm::Failed()); + expected = parse<ReadMemoryArguments>( + R"({"memoryReference":"0xabba", "count": 20})"); + ASSERT_THAT_EXPECTED(expected, llvm::Succeeded()); + + EXPECT_EQ(args.count, expected->count); + EXPECT_EQ(args.memoryReference, expected->memoryReference); + EXPECT_EQ(args.offset, expected->offset); +} + +TEST(ProtocolTypesTest, ReadMemoryResponseBody) { + ReadMemoryResponseBody response; + response.address = "0xdeadbeef"; + const std::string data_str = "hello world!"; + std::transform(data_str.begin(), data_str.end(), + std::back_inserter(response.data), + [](char letter) { return std::byte(letter); }); + response.unreadableBytes = 1; + + Expected<Value> expected = json::parse( + R"({ "address": "0xdeadbeef", "data": "aGVsbG8gd29ybGQh", "unreadableBytes": 1})"); + ASSERT_THAT_EXPECTED(expected, llvm::Succeeded()); + EXPECT_EQ(pp(*expected), pp(response)); +} \ No newline at end of file _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits