[MIPS] Support standard GDB remote stop reply packet for watchpoint

[jaydeep]

Summary

The patch supports 'TAAwatch:addr' packet.
The patch also sets m_watchpoints_trigger_after_instruction to eLazyBoolNo when qHostInfo or qWatchpointSupportInfo is not supported by the target.

[jaydeep]

Could you please find some time to review this?
Thanks.

[clayborg]

See inlined comments.

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

2475–2476

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...

2485

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...

source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

27

Please use StreamString from:

#include "lldb/Core/StreamString.h"

instead of sstream

2497–2499

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?

3102–3103

I would just pass a "const ArchSpec & arch" instead of just the machine.

[jaydeep]

source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

2497–2499

We need "watchpoint" in the description so that SetThreadStopInfo can create CreateStopReasonWithWatchpointID.

[jaydeep]

Addressed review comments.

[clayborg]

Looks good.

[jaydeep]

Closed by commit rL244865

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h

public:
 
     Error
     GetMemoryRegionInfo (lldb::addr_t addr, MemoryRegionInfo &range_info); 
 
     Error
     GetWatchpointSupportInfo (uint32_t &num); 
 
     Error
-    GetWatchpointSupportInfo (uint32_t &num, bool& after);
+    GetWatchpointSupportInfo (uint32_t &num, bool& after, const ArchSpec &arch);
     
     Error
-    GetWatchpointsTriggerAfterInstruction (bool &after);
+    GetWatchpointsTriggerAfterInstruction (bool &after, const ArchSpec &arch);
 
     const ArchSpec &
     GetHostArchitecture ();
     
     uint32_t
     GetHostDefaultPacketTimeout();
 
     const ArchSpec &

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

GDBRemoteCommunicationClient::GetWatchpointSupportInfo (uint32_t &num)
     if (m_supports_watchpoint_support_info == eLazyBoolNo)
     {
         error.SetErrorString("qWatchpointSupportInfo is not supported");
     }
     return error;
 
 }
 
 lldb_private::Error
-GDBRemoteCommunicationClient::GetWatchpointSupportInfo (uint32_t &num, bool& after)
+GDBRemoteCommunicationClient::GetWatchpointSupportInfo (uint32_t &num, bool& after, const ArchSpec &arch)
 {
     Error error(GetWatchpointSupportInfo(num));
     if (error.Success())
-        error = GetWatchpointsTriggerAfterInstruction(after);
+        error = GetWatchpointsTriggerAfterInstruction(after, arch);
     return error;
 }
 
 lldb_private::Error
-GDBRemoteCommunicationClient::GetWatchpointsTriggerAfterInstruction (bool &after)
+GDBRemoteCommunicationClient::GetWatchpointsTriggerAfterInstruction (bool &after, const ArchSpec &arch)
 {
     Error error;
+    llvm::Triple::ArchType atype = arch.GetMachine();
     
     // we assume watchpoints will happen after running the relevant opcode
     // and we only want to override this behavior if we have explicitly
     // received a qHostInfo telling us otherwise
     if (m_qHostInfo_is_valid != eLazyBoolYes)
+    {
+        // On targets like MIPS, watchpoint exceptions are always generated 
+        // before the instruction is executed. The connected target may not 
+        // support qHostInfo or qWatchpointSupportInfo packets.
+        if (atype == llvm::Triple::mips || atype == llvm::Triple::mipsel
+            || atype == llvm::Triple::mips64 || atype == llvm::Triple::mips64el)
+            after = false;
+        else
             after = true;
+    }
     else
+    {
+        // For MIPS, set m_watchpoints_trigger_after_instruction to eLazyBoolNo 
+        // if it is not calculated before.
+        if (m_watchpoints_trigger_after_instruction == eLazyBoolCalculate &&
+            (atype == llvm::Triple::mips || atype == llvm::Triple::mipsel
+            || atype == llvm::Triple::mips64 || atype == llvm::Triple::mips64el))
+            m_watchpoints_trigger_after_instruction = eLazyBoolNo;
+
         after = (m_watchpoints_trigger_after_instruction != eLazyBoolNo);
+    }
     return error;
 }
 
 int
 GDBRemoteCommunicationClient::SetSTDIN(const FileSpec &file_spec)
 {
     if (file_spec)
     {

source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

 #include <sys/stat.h>
 #include <sys/types.h>
 #include <time.h>
 
 // C++ Includes
 #include <algorithm>
 #include <map>
 #include <mutex>
 
 #include "lldb/Breakpoint/Watchpoint.h"
 #include "lldb/Interpreter/Args.h"
 #include "lldb/Core/ArchSpec.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Host/ConnectionFileDescriptor.h"
 #include "lldb/Host/FileSpec.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
case 'S':
                             const size_t byte_size = bytes.GetStringRef().size()/2;
                             DataBufferSP data_buffer_sp(new DataBufferHeap(byte_size, 0));
                             const size_t bytes_copied = bytes.GetHexBytes (data_buffer_sp->GetBytes(), byte_size, 0);
                             if (bytes_copied == byte_size)
                                 m_memory_cache.AddL1CacheData(mem_cache_addr, data_buffer_sp);
                         }
                     }
                 }
+                else if (key.compare("watch") == 0 || key.compare("rwatch") == 0 || key.compare("awatch") == 0)
+                {
+                    // Support standard GDB remote stop reply packet 'TAAwatch:addr'
+                    lldb::addr_t wp_addr = StringConvert::ToUInt64 (value.c_str(), LLDB_INVALID_ADDRESS, 16);
+                    WatchpointSP wp_sp = GetTarget().GetWatchpointList().FindByAddress(wp_addr);
+                    uint32_t wp_index = LLDB_INVALID_INDEX32;
+
+                    if (wp_sp)
+                        wp_index = wp_sp->GetHardwareIndex();
+
+                    reason = "watchpoint";
+                    StreamString ostr;
+                    ostr.Printf("%" PRIu64 " %" PRIu32, wp_addr, wp_index);
+                    description = ostr.GetString().c_str();
+                }
                 else if (key.size() == 2 && ::isxdigit(key[0]) && ::isxdigit(key[1]))
                 {
                     uint32_t reg = StringConvert::ToUInt32 (key.c_str(), UINT32_MAX, 16);
                     if (reg != UINT32_MAX)
                         expedited_register_map[reg] = std::move(value);
                 }
             }
 
ProcessGDBRemote::GetWatchpointSupportInfo (uint32_t &num)
     
     Error error (m_gdb_comm.GetWatchpointSupportInfo (num));
     return error;
 }
 
 Error
 ProcessGDBRemote::GetWatchpointSupportInfo (uint32_t &num, bool& after)
 {
-    Error error (m_gdb_comm.GetWatchpointSupportInfo (num, after));
+    Error error (m_gdb_comm.GetWatchpointSupportInfo (num, after, GetTarget().GetArchitecture()));
     return error;
 }
 
 Error
 ProcessGDBRemote::DoDeallocateMemory (lldb::addr_t addr)
 {
     Error error; 
     LazyBool supported = m_gdb_comm.SupportsAllocDeallocMemory();