[Lldb-commits] [PATCH] D11747: [MIPS] Support standard GDB remote stop reply packet for watchpoint

[Greg Clayton via lldb-commits]

clayborg added a subscriber: clayborg.
clayborg requested changes to this revision.
clayborg added a reviewer: clayborg.
clayborg added a comment.
This revision now requires changes to proceed.

See inlined comments.


================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:2475-2476
@@ -2474,4 +2474,4 @@
 
 lldb_private::Error
-GDBRemoteCommunicationClient::GetWatchpointSupportInfo (uint32_t &num, bool& after)
+GDBRemoteCommunicationClient::GetWatchpointSupportInfo (uint32_t &num, bool& after, llvm::Triple::ArchType atype)
 {
----------------
Change to the last argument: "llvm::Triple::ArchType atype" to "const ArchSpec &arch" in case we need to check the vendor or OS for some reason...

================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:2485
@@ -2484,3 +2484,3 @@
 lldb_private::Error
-GDBRemoteCommunicationClient::GetWatchpointsTriggerAfterInstruction (bool &after)
+GDBRemoteCommunicationClient::GetWatchpointsTriggerAfterInstruction (bool &after, llvm::Triple::ArchType atype)
 {
----------------
Change to the last argument: "llvm::Triple::ArchType atype" to "const ArchSpec &arch" in case we need to check the vendor or OS for some reason...

================
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:27
@@ -26,2 +26,3 @@
 #include <mutex>
+#include <sstream>
 
----------------
Please use StreamString from:

```
#include "lldb/Core/StreamString.h"
```

instead of sstream

================
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:2498-2500
@@ +2497,5 @@
+                    reason = "watchpoint";
+                    std::ostringstream ostr;
+                    ostr << wp_addr << " " << wp_index;
+                    description = ostr.str();
+                }
----------------
Not sure if you need to do this, try not setting this and see how the description comes out. The watchpoint stop info should set this correctly?

================
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:3103-3104
@@ -3086,3 +3102,4 @@
 {
-    Error error (m_gdb_comm.GetWatchpointSupportInfo (num, after));
+    const ArchSpec &target_arch = GetTarget().GetArchitecture();
+    Error error (m_gdb_comm.GetWatchpointSupportInfo (num, after, target_arch.GetMachine()));
     return error;
----------------
I would just pass a "const ArchSpec & arch" instead of just the machine.


Repository:
  rL LLVM

http://reviews.llvm.org/D11747