[lldb-dap] Allow empty memory reference in disassemble arguments

[DrSergei]

This patch implements a workaround for a VSCode bug that causes it to send disassemble requests with empty memory reference. You can find more detailed description here. I propose to allow empty memory reference and return invalid instructions when this occurs.

Error log example:

1759923554.517830610 (stdio) --> {"command":"disassemble","arguments":{"memoryReference":"","offset":0,"instructionOffset":-50,"instructionCount":50,"resolveSymbols":true},"type":"request","seq":3}
1759923554.518007517 (stdio) queued (command=disassemble seq=3)
1759923554.518254757 (stdio) <-- {"body":{"error":{"format":"invalid arguments for request 'disassemble': malformed memory reference at arguments.memoryReference\n{\n  \"instructionCount\": 50,\n  \"instructionOffset\": -50,\n  \"memoryReference\": /* error: malformed memory reference */ \"\",\n  \"offset\": 0,\n  \"resolveSymbols\": true\n}","id":3,"showUser":true}},"command":"disassemble","request_seq":3,"seq":0,"success":false,"type":"response"}

I am not sure that we should add workaround here when bug on VSCode side, but I think this bug affects our users. WDYT?

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Sergei Druzhkov (DrSergei)

Changes

This patch implements a workaround for a VSCode bug that causes it to send disassemble requests with empty memory reference. You can find more detailed description here. I propose to allow empty memory reference and return invalid instructions when this occurs.

Error log example:

1759923554.517830610 (stdio) --> {"command":"disassemble","arguments":{"memoryReference":"","offset":0,"instructionOffset":-50,"instructionCount":50,"resolveSymbols":true},"type":"request","seq":3}
1759923554.518007517 (stdio) queued (command=disassemble seq=3)
1759923554.518254757 (stdio) <-- {"body":{"error":{"format":"invalid arguments for request 'disassemble': malformed memory reference at arguments.memoryReference\n{\n  \"instructionCount\": 50,\n  \"instructionOffset\": -50,\n  \"memoryReference\": /* error: malformed memory reference */ \"\",\n  \"offset\": 0,\n  \"resolveSymbols\": true\n}","id":3,"showUser":true}},"command":"disassemble","request_seq":3,"seq":0,"success":false,"type":"response"}

I am not sure that we should add workaround here when bug on VSCode side, but I think this bug affects our users. WDYT?

---

Full diff: https://github.com/llvm/llvm-project/pull/162517.diff

5 Files Affected:

• (modified) lldb/test/API/tools/lldb-dap/disassemble/TestDAP_disassemble.py (+22)
• (modified) lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp (+5)
• (modified) lldb/tools/lldb-dap/JSONUtils.cpp (+6-1)
• (modified) lldb/tools/lldb-dap/JSONUtils.h (+4-1)
• (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp (+1-1)

diff --git a/lldb/test/API/tools/lldb-dap/disassemble/TestDAP_disassemble.py b/lldb/test/API/tools/lldb-dap/disassemble/TestDAP_disassemble.py
index 0562f20335a23..6c41c86ff9ae5 100644
--- a/lldb/test/API/tools/lldb-dap/disassemble/TestDAP_disassemble.py
+++ b/lldb/test/API/tools/lldb-dap/disassemble/TestDAP_disassemble.py
@@ -91,3 +91,25 @@ def test_disassemble_backwards(self):
         # clear breakpoints
         self.set_source_breakpoints(source, [])
         self.continue_to_exit()
+
+    def test_disassemble_empty_memory_reference(self):
+        """
+        Tests the 'disassemble' request with empty memory reference.
+        """
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program)
+        source = "main.c"
+        bp_line_no = line_number(source, "// breakpoint 1")
+        self.set_source_breakpoints(source, [bp_line_no])
+        self.continue_to_next_stop()
+
+        instructions = self.dap_server.request_disassemble(
+            memoryReference="", instructionOffset=0, instructionCount=50
+        )
+        self.assertEqual(len(instructions), 50)
+        for instruction in instructions:
+            self.assertEqual(instruction["presentationHint"], "invalid")
+
+        # clear breakpoints
+        self.set_source_breakpoints(source, [])
+        self.continue_to_exit()
diff --git a/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp
index 9542f091b055e..6d2eb74a9634c 100644
--- a/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp
@@ -182,6 +182,11 @@ static DisassembledInstruction ConvertSBInstructionToDisassembledInstruction(
 /// `supportsDisassembleRequest` is true.
 llvm::Expected<DisassembleResponseBody>
 DisassembleRequestHandler::Run(const DisassembleArguments &args) const {
+  if (args.memoryReference == LLDB_INVALID_ADDRESS) {
+    std::vector<DisassembledInstruction> invalid_instructions(
+        args.instructionCount, GetInvalidInstruction());
+    return DisassembleResponseBody{std::move(invalid_instructions)};
+  }
   const lldb::addr_t addr_ptr = args.memoryReference + args.offset;
   lldb::SBAddress addr(addr_ptr, dap.target);
   if (!addr.IsValid())
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 4f26599a49bac..c00e39c86b92a 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -122,7 +122,7 @@ DecodeMemoryReference(llvm::StringRef memoryReference) {
 
 bool DecodeMemoryReference(const llvm::json::Value &v, llvm::StringLiteral key,
                            lldb::addr_t &out, llvm::json::Path path,
-                           bool required) {
+                           bool required, bool allow_empty) {
   const llvm::json::Object *v_obj = v.getAsObject();
   if (!v_obj) {
     path.report("expected object");
@@ -145,6 +145,11 @@ bool DecodeMemoryReference(const llvm::json::Value &v, llvm::StringLiteral key,
     return false;
   }
 
+  if (allow_empty && mem_ref_str->empty()) {
+    out = LLDB_INVALID_ADDRESS;
+    return true;
+  }
+
   const std::optional<lldb::addr_t> addr_opt =
       DecodeMemoryReference(*mem_ref_str);
   if (!addr_opt) {
diff --git a/lldb/tools/lldb-dap/JSONUtils.h b/lldb/tools/lldb-dap/JSONUtils.h
index e9094f67b94ec..b8187fe988350 100644
--- a/lldb/tools/lldb-dap/JSONUtils.h
+++ b/lldb/tools/lldb-dap/JSONUtils.h
@@ -156,11 +156,14 @@ DecodeMemoryReference(llvm::StringRef memoryReference);
 ///    Indicates if the key is required to be present, otherwise report an error
 ///    if the key is missing.
 ///
+/// \param[in] allow_empty
+///    Interpret empty string as a valid value, don't report an error.
+///
 /// \return
 ///    Returns \b true if the address was decoded successfully.
 bool DecodeMemoryReference(const llvm::json::Value &v, llvm::StringLiteral key,
                            lldb::addr_t &out, llvm::json::Path path,
-                           bool required);
+                           bool required, bool allow_empty = false);
 
 /// Extract an array of strings for the specified key from an object.
 ///
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
index b455112cd37d9..fc046d18825ec 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
@@ -511,7 +511,7 @@ bool fromJSON(const llvm::json::Value &Params, DisassembleArguments &DA,
   json::ObjectMapper O(Params, P);
   return O &&
          DecodeMemoryReference(Params, "memoryReference", DA.memoryReference, P,
-                               /*required=*/true) &&
+                               /*required=*/true, /*allow_empty*/ true) &&
          O.mapOptional("offset", DA.offset) &&
          O.mapOptional("instructionOffset", DA.instructionOffset) &&
          O.map("instructionCount", DA.instructionCount) &&