llvmbot wrote:

<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Sergei Druzhkov (DrSergei)

<details>
<summary>Changes</summary>

Added unknown request handler to avoid crash. Returning error in this case 
looks better than stopping entire debug session.
```
0.      Program arguments: /home/sergei/llvm-project/build/bin/lldb-dap
 #<!-- -->0 0x0000603918a0ed92 
llvm::sys::PrintStackTrace(llvm::raw_ostream&amp;, int) 
(/home/sergei/llvm-project/build/bin/lldb-dap+0x6ed92)
 #<!-- -->1 0x0000603918a0c29f llvm::sys::RunSignalHandlers() 
(/home/sergei/llvm-project/build/bin/lldb-dap+0x6c29f)
 #<!-- -->2 0x0000603918a0c3ec SignalHandler(int, siginfo_t*, void*) 
Signals.cpp:0:0
 #<!-- -->3 0x00007b487ba45330 (/lib/x86_64-linux-gnu/libc.so.6+0x45330)
 #<!-- -->4 0x00007b487ba9eb2c __pthread_kill_implementation 
./nptl/pthread_kill.c:44:76
 #<!-- -->5 0x00007b487ba9eb2c __pthread_kill_internal 
./nptl/pthread_kill.c:78:10
 #<!-- -->6 0x00007b487ba9eb2c pthread_kill ./nptl/pthread_kill.c:89:10
 #<!-- -->7 0x00007b487ba4527e raise ./signal/../sysdeps/posix/raise.c:27:6
 #<!-- -->8 0x00007b487ba288ff abort ./stdlib/abort.c:81:7
 #<!-- -->9 0x00007b487bea5ff5 (/lib/x86_64-linux-gnu/libstdc++.so.6+0xa5ff5)
#<!-- -->10 0x00007b487bebb0da (/lib/x86_64-linux-gnu/libstdc++.so.6+0xbb0da)
#<!-- -->11 0x00007b487bea5a55 std::unexpected() 
(/lib/x86_64-linux-gnu/libstdc++.so.6+0xa5a55)
#<!-- -->12 0x0000603918a27cdd 
(/home/sergei/llvm-project/build/bin/lldb-dap+0x87cdd)
#<!-- -->13 0x00006039189c6b7d main 
(/home/sergei/llvm-project/build/bin/lldb-dap+0x26b7d)
#<!-- -->14 0x00007b487ba2a1ca __libc_start_call_main 
./csu/../sysdeps/nptl/libc_start_call_main.h:74:3
#<!-- -->15 0x00007b487ba2a28b call_init ./csu/../csu/libc-start.c:128:20
#<!-- -->16 0x00007b487ba2a28b __libc_start_main ./csu/../csu/libc-start.c:347:5
#<!-- -->17 0x00006039189c80d5 _start 
(/home/sergei/llvm-project/build/bin/lldb-dap+0x280d5)
```

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


9 Files Affected:

- (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py 
(+8) 
- (added) lldb/test/API/tools/lldb-dap/launch/TestDAP_launch_unknown_request.py 
(+25) 
- (modified) lldb/tools/lldb-dap/CMakeLists.txt (+1) 
- (modified) lldb/tools/lldb-dap/DAP.cpp (+6-8) 
- (modified) lldb/tools/lldb-dap/DAP.h (+2) 
- (modified) lldb/tools/lldb-dap/Handler/RequestHandler.h (+9) 
- (added) lldb/tools/lldb-dap/Handler/UnknownRequestHandler.cpp (+19) 
- (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.h (+5) 
- (modified) llvm/utils/gn/secondary/lldb/tools/lldb-dap/BUILD.gn (+1) 


``````````diff
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index fdccc9eae9fe4..7bbad52147747 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -1677,6 +1677,14 @@ def request_testGetTargetBreakpoints(self):
         }
         return self._send_recv(command_dict)
 
+    def request_unknown(self):
+        command_dict = {
+            "command": "unknown",
+            "type": "request",
+            "arguments": {},
+        }
+        return self._send_recv(command_dict)
+
     def terminate(self):
         self.send.close()
         if self._recv_thread.is_alive():
diff --git 
a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch_unknown_request.py 
b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch_unknown_request.py
new file mode 100644
index 0000000000000..8a0f8b4a80590
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch_unknown_request.py
@@ -0,0 +1,25 @@
+"""
+Test lldb-dap launch request.
+"""
+
+from lldbsuite.test.decorators import expectedFailureWindows
+import lldbdap_testcase
+
+
+class TestDAP_launch_unknown_request(lldbdap_testcase.DAPTestCaseBase):
+    """
+    Tests handling of unknown request.
+    """
+
+    @expectedFailureWindows(
+        bugnumber="https://github.com/llvm/llvm-project/issues/137599";
+    )
+    def test(self):
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program)
+        
+        response = self.dap_server.request_unknown()
+        self.assertFalse(response["success"])
+        self.assertEqual(response["body"]["error"]["format"], "Unknown 
request")
+
+        self.continue_to_exit()
diff --git a/lldb/tools/lldb-dap/CMakeLists.txt 
b/lldb/tools/lldb-dap/CMakeLists.txt
index 8566c663fcc06..1bb7a2e498b9b 100644
--- a/lldb/tools/lldb-dap/CMakeLists.txt
+++ b/lldb/tools/lldb-dap/CMakeLists.txt
@@ -64,6 +64,7 @@ add_lldb_library(lldbDAP
   Handler/StepOutRequestHandler.cpp
   Handler/TestGetTargetBreakpointsRequestHandler.cpp
   Handler/ThreadsRequestHandler.cpp
+  Handler/UnknownRequestHandler.cpp
   Handler/VariablesRequestHandler.cpp
   Handler/WriteMemoryRequestHandler.cpp
 
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index b76b05c5d1459..0a0d3c3903409 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -849,15 +849,11 @@ bool DAP::HandleObject(const Message &M) {
     auto handler_pos = request_handlers.find(req->command);
     dispatcher.Set("client_data",
                    llvm::Twine("request_command:", req->command).str());
-    if (handler_pos != request_handlers.end()) {
+    if (handler_pos != request_handlers.end())
       handler_pos->second->Run(*req);
-      return true; // Success
-    }
-
-    dispatcher.Set("error",
-                   llvm::Twine("unhandled-command:" + req->command).str());
-    DAP_LOG(log, "error: unhandled command '{0}'", req->command);
-    return false; // Fail
+    else
+      unknown_request_handler->Run(*req);
+    return true; // Success
   }
 
   if (const auto *resp = std::get_if<Response>(&M)) {
@@ -1577,6 +1573,8 @@ void DAP::RegisterRequests() {
   RegisterRequest<VariablesRequestHandler>();
   RegisterRequest<WriteMemoryRequestHandler>();
 
+  unknown_request_handler = std::make_unique<UnknownRequestHandler>(*this);
+
   // Custom requests
   RegisterRequest<CompileUnitsRequestHandler>();
   RegisterRequest<ModulesRequestHandler>();
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 34d6a29b3c110..f2e1162eb3a61 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -482,6 +482,8 @@ struct DAP final : public DAPTransport::MessageHandler {
   llvm::StringMap<std::unique_ptr<BaseRequestHandler>> request_handlers;
   /// @}
 
+  std::unique_ptr<BaseRequestHandler> unknown_request_handler;
+
   /// Event threads.
   /// @{
   void ProgressEventThread();
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h 
b/lldb/tools/lldb-dap/Handler/RequestHandler.h
index 9feb636fd5c28..4b4f1435e7acb 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.h
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h
@@ -658,6 +658,15 @@ class WriteMemoryRequestHandler final
   Run(const protocol::WriteMemoryArguments &args) const override;
 };
 
+class UnknownRequestHandler final
+    : public RequestHandler<protocol::UnknownArguments,
+                            protocol::UnknownResponseBody> {
+public:
+  using RequestHandler::RequestHandler;
+  static llvm::StringLiteral GetCommand() { return "unknown"; }
+  llvm::Error Run(const protocol::UnknownArguments &args) const override;
+};
+
 } // namespace lldb_dap
 
 #endif
diff --git a/lldb/tools/lldb-dap/Handler/UnknownRequestHandler.cpp 
b/lldb/tools/lldb-dap/Handler/UnknownRequestHandler.cpp
new file mode 100644
index 0000000000000..c576256a48265
--- /dev/null
+++ b/lldb/tools/lldb-dap/Handler/UnknownRequestHandler.cpp
@@ -0,0 +1,19 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "DAPError.h"
+#include "Protocol/ProtocolRequests.h"
+#include "RequestHandler.h"
+#include "llvm/Support/Error.h"
+
+using namespace lldb_dap;
+using namespace lldb_dap::protocol;
+
+llvm::Error UnknownRequestHandler::Run(const UnknownArguments &args) const {
+  return llvm::make_error<DAPError>("Unknown request");
+}
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h 
b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
index 28c9f48200e0c..91fc2a4db6f57 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
@@ -1312,6 +1312,11 @@ struct StackTraceResponseBody {
 };
 llvm::json::Value toJSON(const StackTraceResponseBody &);
 
+/// Arguments for unknown request.
+using UnknownArguments = EmptyArguments;
+/// Response to unknowns request.
+using UnknownResponseBody = VoidResponse;
+
 } // namespace lldb_dap::protocol
 
 #endif
diff --git a/llvm/utils/gn/secondary/lldb/tools/lldb-dap/BUILD.gn 
b/llvm/utils/gn/secondary/lldb/tools/lldb-dap/BUILD.gn
index 896da7df95bb1..edcbc5e31f4f1 100644
--- a/llvm/utils/gn/secondary/lldb/tools/lldb-dap/BUILD.gn
+++ b/llvm/utils/gn/secondary/lldb/tools/lldb-dap/BUILD.gn
@@ -68,6 +68,7 @@ static_library("lib") {
     "Handler/StepOutRequestHandler.cpp",
     "Handler/TestGetTargetBreakpointsRequestHandler.cpp",
     "Handler/ThreadsRequestHandler.cpp",
+    "Handler/UnknownRequestHandler.cpp",
     "Handler/VariablesRequestHandler.cpp",
     "Handler/WriteMemoryRequestHandler.cpp",
     "InstructionBreakpoint.cpp",

``````````

</details>


https://github.com/llvm/llvm-project/pull/181109
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to