lldb can know architecture-dependent watchpoint behaviors, instead of depending on the remote stub

[jasonmolenda]

Summary

Some CPU architectures will give lldb a watchpoint exception before the instruction has executed (it is rolled back when the address matches), or after the instruction has executed. Currently, lldb learns which type of CPU this is from the gdb remote serial protocol stub though a key in the lldb-extension qHostInfo packet. This is fixed for most architectures, and lldb needs to interop with non-debugserver/non-lldb-server stubs, so moving this into lldb simplifies a lot. This patch still allows the remote stub to override the behavior if it needs to.

There's another lldb extended gdb RSP packet to retrieve the number of watchpoints that this target supports. This needs to be calculated on the target system to be correct, so the stub is the right place to retrieve this knowledge. But lldb needs to interoperate with remote stubs that do not implement this extended packet; we can optionally print the number to the user, but we should not prevent watchpoints from attempting to be set based on this number.

This patch changes lldb to expect these pieces of information -- whether the exceptions are received before/after, and the number of watchpoints -- to not be available from the remote stub, and behave correctly.

The bug driving this was when connected to a gdb RSP stub that did not implement one of these extended packets, and debugging on a target where watchpoint exceptions are received before the instruction executes. lldb would try to fetch whether this machine is a before/after machine, and also fetch the number of watchpoints. The latter packet was not implemented, and Process::GetWatchpointSupportInfo would return error and not provide whether watchpoint exceptions are before/after.

[jasonmolenda]

Ah, I wasn't paying close enough attention and got the logic for MIPS/PPC64 watchpoint exceptions backwards in my update to GDBRemoteCommunicationClient::GetWatchpointReportedAfter, update patch for that.

[jasonmolenda]

tl;dr: watchpoints don't work on things like arm targets with a JTAG gdb stub that doesn't support qWatchpointSupportInfo: or qHostInfo with the watchpoint_exceptions_received key.

Nearly all of this patch is mechanical, and IMO the question is whether I should have stuck with Process:GetWatchpointSupportInfo but added some way to indicate that we cannot fetch the number of watchpoints, but we can report whether the exceptions come before/after.

Also this seems a little weak to have it encoded down in GDBRemoteCommunicationClient; it seems like something we could ask the ArchSpec about tbh. I didn't go that far, but maybe that would be better to do while I'm here.

[DavidSpickett]

I like removing a return by ref and being explicit about what info is wanted where.

So say the stub reports the "watchpoints received" but not the number of slots.

Does lldb just send the watchpoint set packets and rely on the stub to error if it can't handle it (presumably you get a less than helpful error)? As opposed to if you had a number of slots lldb could track how many it had used and give you a nice error instead.

lldb/include/lldb/Target/Process.h

1822–1825

This is a bit muddled.

I guess your point is that number of watchpoints the user can set may be less than the number that the hardware supports. And that this method returns the latter not the former. So perhaps:

This number may be less than the number of watchpoints a user can specify. This is because a single user watchpoint may require multiple watchpoint slots to implement. Due to the size and/or alignment of objects.

1830

I would wave my optional flag here but defining some error value is the way of the land so I will keep it under wraps :)

[jasonmolenda]

David's feedback was spot on, overhaul patch to address.

Change Process::GetWatchpointSlotCount to return an optional count, for when it is available, instead of depending on a magic number.

I've been unhappy with how we calculate if a target reports watchpoint exceptions before or after the instruction has executed. I believe it is target arch specific, I don't think we have any processors that change behavior at runtime. We originally homed this bit of information in debugserver and had it report it in qHostInfo, but it was one of those early decisions that was not correct IMO, and I'm not in favor of perpetuating it. This update to the patch makes a concrete Process::GetWatchpointReportedAfter method that uses the Target architecture to determine it. It also has a call to a DoGetWatchpointReportedAfter method that subclasses can override if they are an environment where the target arch determination is not sufficient - who knows.

This patch separates these into two different Process methods, and sets a default of "before" for arm targets if the qHostInfo packet didn't provide any hints (e.g. watchpoint_exceptions_received:after;), otherwise it returns nullopt.

I removed the before/after calculation from the ProcessWindows plugin because it always returned after; the concrete method in Process already behaves that way.

I need to review & test this patch - it's closer to a WIP at this instant, but I wanted to checkpoint the updates I made so far, and where I'm heading with it.

[jasonmolenda]

I phabracated wrong and the end of the last msg was truncated. I meant to end it with this:

If the number of watchpoints could not be fetched with an lldb-extension packet, Process:GetWatchpointSupportInfo would not report whether breakpoints are received before or after the instruction has executed. This latter is important to know to handle watchpoints; the former doesn't really matter. So when we connect to a non-lldb-server/debugserver stub, and we are targetting an arm cpu (where watchpoint exceptions are received before the instruction executes), lldb would not correctly disable the watchpoint/insn-step/re-enable the watchpoint (v. StopInfoWatchpoint::ShouldStopSynchronous in StopInfo.cpp), you'd hit the watchpoint exception over and over without advancing.

This patch separates these into two different Process methods, and sets a default of "before" for arm targets if the qHostInfo packet didn't provide any hints (e.g. watchpoint_exceptions_received:after;), otherwise it returns nullopt.

I removed the before/after calculation from the ProcessWindows plugin because it always returned after; the concrete method in Process already behaves that way for intel.

I need to review & test this patch - it's closer to a WIP at this instant, but I wanted to checkpoint the updates I made so far, and where I'm heading with it.

[jasonmolenda]

Went over the rewrite one more time, fixing up comments, fixing a couple of logic bugs I wrote, and having tested it with a debugserver I hacked to remove the watchpoint_exceptions_received:before; key in the qHostInfo reply packet, so lldb had to use its knowledge of the target system to determine this.

I have changed ProcessWindows and can't build that to ensure it's correct, but the changes are tiny and I'll keep an eye on the bots when I land it.

I think this is ready for review now.

[jasonmolenda]

If I was going to describe this patch briefly at this point, I would say: "Change lldb from depending on the remote gdb stub to tell it whether watchpoints are received before or after the instruction, to knowing the architectures where watchpoint exceptions are received before the instruction executes, and allow the remote gdb stub to override this behavior in its qHostInfo response". The rest is mechanical change.

[DavidSpickett]

Change lldb from depending on the remote gdb stub to tell it whether watchpoints are received before or after the instruction, to knowing the architectures where watchpoint exceptions are received before the instruction executes, and allow the remote gdb stub to override this behavior in its qHostInfo response.

Sounds good to me. Can you simplify the commit message some now that you've got the changes you want?

Seems like the main things are:

• Split up querying watchpoint reporting style, and number of watchpoint slots.
• Have process determine the reporting style by target, with the option for the remote to override. Meaning this query never fails.
• Only conclude that watchpoints are unsupported if the remote tells us that it has 0 slots.

(the last one I think we always did but good for context)

lldb/source/Target/Process.cpp

2360

Can do if (std::optional<bool> subclass....) {... here.

Also no {} for single line if.

2367

No {} needed here too.

2373

Would this be any clearer with multiple returns? Or one giant return, but the logic is a bit cryptic then.

const ArchSpec &arch = GetTarget().GetArchitecture();
if (!arch.IsValid())
  return true;

llvm::Triple triple = arch.GetTriple();
return !(triple.isMIPS() || triple.isPPC64() || triple.isAArch64() || triple.isArmMClass() || triple.isARM());

Better as:

const ArchSpec &arch = GetTarget().GetArchitecture();
if (!arch.IsValid())
  return false;

llvm::Triple triple = arch.GetTriple();
if (triple.isMIPS() || triple.isPPC64() || triple.isAArch64() || triple.isArmMClass() || triple.isARM())
  return false;

return true;

Also, do we know what RISC-V does?

lldb/source/Target/StopInfo.cpp

833

This is no longer needed because GetWatchpointReportedAfter will fall back to the process' defaults if the GDB layer does not/can not provide a value, correct?

lldb/source/Target/Target.cpp

804

This should just append "Target supports 0 hardware...".

In theory a compiler could figure that out but also it's a bit confusing because it makes the reader wonder what value other than 0 is supposed to end up here.

[DavidSpickett]

lldb/source/Target/Process.cpp

2373

@Emmmer any idea?

[Emmmer]

lldb/source/Target/Process.cpp

2373

It seems standard RISC-V uses the before mode, that is

The action for this trigger will be taken just before the instruction that triggered it is committed, but after all preceding instructions are committed.

If I understand this code correctly (correct me if I was wrong), we should return false for RISC-V.

But in the debug-spec [1], RISC-V seems to allow both before and after modes, which can be detected through CSRs. When debug-spec lands, we may change the code accordingly.

[1]: https://github.com/riscv/riscv-debug-spec/blob/6309972901417a0fa72d812d2ffe89e432f00dff/xml/hwbp_registers.xml#L365

[jasonmolenda]

lldb/source/Target/Process.cpp

2373

Thanks! I'll add isRISCV() to the architectures we default to "watchpoints received before instruction executes". This can be overridden by a gdb remote stub which adds the lldb-extension key to qHostInfo response. although it sounds like it could technically be per *process*??? in which case the key should really be sent in qProcessInfo. The remote stub should be able to send qHostInfo before it has attached to any specific process.

lldb/source/Target/StopInfo.cpp

833

Previously, this would fetch the number of watchpoints available (if the lldb-extended packet was provided) and it would fetch whether watchpoints are received before or after (if the qHostInfo includes the lldb-extended key), and you would get an error if the former was unavailable. (I think the latter would default to "watchpoints are after"). This m_should_stop = true is the behavior when watchpoints are received after the instruction has executed; this is the crux of the bug I was trying to fix, where lldb would not instruction step over the instruction and re-add it, when the packet declaring the number of watchpoints was unimplemented.

lldb/source/Target/Target.cpp

804

It is a bit confusing. The layers that return the optional<uint32_t> will return the actual number of hardware watchpoints available, including 0 if that is the correct value. Or a nullopt to indicate that it could not be retrieved. So in this method, we got back the number of watchpoints available, and it was said to be 0. lldb won't actually check against this number if you try to set one, so even if we got a bogus value here, you could try to set a watchpoint and see if it works. SBProcess::GetNumSupportedHardwareWatchpoints() takes an SBError out arg & returns a uint32_t. It can return 0 to mean "could not get number of watchpoints" or "there are zero watchpoints" but the SBError tells you whether it was the failure case or not.

[jasonmolenda]

Update to fix David's second round of comments, rewrite the Title/Summary to more accurately reflect what I'm doing in this patch. I think we're about done here, but open to any comments.

[DavidSpickett]

One minor comment otherwise LGTM.

[LLDB] tag in the commit title please :)

lldb/source/Target/Process.cpp

2371

Combine these into one and if you feel like it, return false instead.

lldb/include/lldb/Target/Process.h

 #include "lldb/Host/Config.h"
 
 #include <climits>
 
 #include <chrono>
 #include <list>
 #include <memory>
 #include <mutex>
+#include <optional>
 #include <string>
 #include <unordered_set>
 #include <vector>
 
 #include "lldb/Breakpoint/BreakpointSiteList.h"
 #include "lldb/Core/LoadedModuleInfoList.h"
 #include "lldb/Core/PluginInterface.h"
 #include "lldb/Core/ThreadSafeValue.h"

   ///     A vector to contain MemoryRegionInfo objects for all mapped
   ///     ranges.
   ///
   /// \return
   ///     An error value.
   virtual Status
   GetMemoryRegions(lldb_private::MemoryRegionInfos &region_list);
 
-  virtual Status GetWatchpointSupportInfo(uint32_t &num) {
-    Status error;
-    num = 0;
-    error.SetErrorString("Process::GetWatchpointSupportInfo() not supported");
-    return error;
+  /// Get the number of watchpoints supported by this target.
+  ///
+  /// We may be able to determine the number of watchpoints available
+  /// on this target; retrieve this value if possible.
+  ///
+  /// This number may be less than the number of watchpoints a user
+  /// can specify. This is because a single user watchpoint may require
+  /// multiple watchpoint slots to implement. Due to the size
+  /// and/or alignment of objects.
+  ///
+  /// \return
+  ///     Returns the number of watchpoints, if available.
+  virtual std::optional<uint32_t> GetWatchpointSlotCount() {
+    return std::nullopt;
   }
 
-  virtual Status GetWatchpointSupportInfo(uint32_t &num, bool &after) {
-    Status error;
-    num = 0;
-    after = true;
-    error.SetErrorString("Process::GetWatchpointSupportInfo() not supported");
-    return error;
-  }
+  /// Whether lldb will be notified about watchpoints after
+  /// the instruction has completed executing, or if the
+  /// instruction is rolled back and it is notified before it
+  /// executes.
+  /// The default behavior is "exceptions received after instruction
+  /// has executed", except for certain CPU architectures.
+  /// Process subclasses may override this if they have additional
+  /// information.
+  ///
+  /// \return
+  ///     Returns true for targets where lldb is notified after
+  ///     the instruction has completed executing.
+  bool GetWatchpointReportedAfter();
 
   lldb::ModuleSP ReadModuleFromMemory(const FileSpec &file_spec,
                                       lldb::addr_t header_addr,
                                       size_t size_to_read = 512);
 
   /// Attempt to get the attributes for a region of memory in the process.
   ///
   /// It may be possible for the remote debug server to inspect attributes for
protected:
   ///
   /// \return
   ///     An error value.
   virtual Status DoGetMemoryRegionInfo(lldb::addr_t load_addr,
                                        MemoryRegionInfo &range_info) {
     return Status("Process::DoGetMemoryRegionInfo() not supported");
   }
 
+  /// Provide an override value in the subclass for lldb's
+  /// CPU-based logic for whether watchpoint exceptions are
+  /// received before or after an instruction executes.
+  ///
+  /// If a Process subclass needs to override this architecture-based
+  /// result, it may do so by overriding this method.
+  ///
+  /// \return
+  ///     No boolean returned means there is no override of the
+  ///     default architecture-based behavior.
+  ///     true is returned for targets where watchpoints are reported
+  ///     after the instruction has completed.
+  ///     false is returned for targets where watchpoints are reported
+  ///     before the instruction executes.
+  virtual std::optional<bool> DoGetWatchpointReportedAfter() {
+    return std::nullopt;
+  }
+
   lldb::StateType GetPrivateState();
 
   /// The "private" side of resuming a process.  This doesn't alter the state
   /// of m_run_lock, but just causes the process to resume.
   ///
   /// \return
   ///     An Status object describing the success or failure of the resume.
   Status PrivateResume();

lldb/source/API/SBProcess.cpp

 SBProcess::GetNumSupportedHardwareWatchpoints(lldb::SBError &sb_error) const {
   LLDB_INSTRUMENT_VA(this, sb_error);
 
   uint32_t num = 0;
   ProcessSP process_sp(GetSP());
   if (process_sp) {
     std::lock_guard<std::recursive_mutex> guard(
         process_sp->GetTarget().GetAPIMutex());
-    sb_error.SetError(process_sp->GetWatchpointSupportInfo(num));
+    std::optional<uint32_t> actual_num = process_sp->GetWatchpointSlotCount();
+    if (actual_num) {
+      num = *actual_num;
+    } else {
+      sb_error.SetErrorString("Unable to determine number of watchpoints");
+    }
   } else {
     sb_error.SetErrorString("SBProcess is invalid");
   }
   return num;
 }
 
 uint32_t SBProcess::LoadImage(lldb::SBFileSpec &sb_remote_image_spec,
                               lldb::SBError &sb_error) {

lldb/source/Commands/CommandObjectWatchpoint.cpp

public:
     lldb::DescriptionLevel m_level = lldb::eDescriptionLevelBrief;
   };
 
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
     Target *target = &GetSelectedTarget();
 
     if (target->GetProcessSP() && target->GetProcessSP()->IsAlive()) {
-      uint32_t num_supported_hardware_watchpoints;
-      Status error = target->GetProcessSP()->GetWatchpointSupportInfo(
-          num_supported_hardware_watchpoints);
-      if (error.Success())
+      std::optional<uint32_t> num_supported_hardware_watchpoints =
+          target->GetProcessSP()->GetWatchpointSlotCount();
+
+      if (num_supported_hardware_watchpoints)
         result.AppendMessageWithFormat(
             "Number of supported hardware watchpoints: %u\n",
-            num_supported_hardware_watchpoints);
+            *num_supported_hardware_watchpoints);
     }
 
     const WatchpointList &watchpoints = target->GetWatchpointList();
 
     std::unique_lock<std::recursive_mutex> lock;
     target->GetWatchpointList().GetListMutex(lock);
 
     size_t num_watchpoints = watchpoints.GetSize();

lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h

public:
   void OnCreateThread(const HostThread &thread) override;
   void OnExitThread(lldb::tid_t thread_id, uint32_t exit_code) override;
   void OnLoadDll(const ModuleSpec &module_spec,
                  lldb::addr_t module_addr) override;
   void OnUnloadDll(lldb::addr_t module_addr) override;
   void OnDebugString(const std::string &string) override;
   void OnDebuggerError(const Status &error, uint32_t type) override;
 
-  Status GetWatchpointSupportInfo(uint32_t &num) override;
-  Status GetWatchpointSupportInfo(uint32_t &num, bool &after) override;
+  std::optional<uint32_t> GetWatchpointSlotCount() override;
   Status EnableWatchpoint(Watchpoint *wp, bool notify = true) override;
   Status DisableWatchpoint(Watchpoint *wp, bool notify = true) override;
 
 protected:
   ProcessWindows(lldb::TargetSP target_sp, lldb::ListenerSP listener_sp);
 
   Status DoGetMemoryRegionInfo(lldb::addr_t vm_addr,
                                MemoryRegionInfo &info) override;

lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp

if (m_session_data->m_initial_stop_received) {
     LLDB_LOG(
         log,
         "Error {0} occurred launching the process before the initial stop. {1}",
         error.GetError(), error);
     return;
   }
 }
 
-Status ProcessWindows::GetWatchpointSupportInfo(uint32_t &num) {
-  num = RegisterContextWindows::GetNumHardwareBreakpointSlots();
-  return {};
-}
-
-Status ProcessWindows::GetWatchpointSupportInfo(uint32_t &num, bool &after) {
-  num = RegisterContextWindows::GetNumHardwareBreakpointSlots();
-  after = RegisterContextWindows::DoHardwareBreakpointsTriggerAfter();
-  return {};
+std::optional<uint32_t> ProcessWindows::GetWatchpointSlotCount() {
+  return RegisterContextWindows::GetNumHardwareBreakpointSlots();
 }
 
 Status ProcessWindows::EnableWatchpoint(Watchpoint *wp, bool notify) {
   Status error;
 
   if (wp->IsEnabled()) {
     wp->SetEnabled(true, notify);
     return error;

lldb/source/Plugins/Process/Windows/Common/RegisterContextWindows.h

public:
   uint32_t ConvertRegisterKindToRegisterNumber(lldb::RegisterKind kind,
                                                uint32_t num) override;
 
   bool HardwareSingleStep(bool enable) override;
 
   static constexpr uint32_t GetNumHardwareBreakpointSlots() {
     return NUM_HARDWARE_BREAKPOINT_SLOTS;
   }
-  static constexpr bool DoHardwareBreakpointsTriggerAfter() { return true; }
 
   bool AddHardwareBreakpoint(uint32_t slot, lldb::addr_t address, uint32_t size,
                              bool read, bool write);
   bool RemoveHardwareBreakpoint(uint32_t slot);
 
   uint32_t GetTriggeredHardwareBreakpointSlotId();
 
 protected:

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

public:
   lldb::addr_t AllocateMemory(size_t size, uint32_t permissions);
 
   bool DeallocateMemory(lldb::addr_t addr);
 
   Status Detach(bool keep_stopped, lldb::pid_t pid = LLDB_INVALID_PROCESS_ID);
 
   Status GetMemoryRegionInfo(lldb::addr_t addr, MemoryRegionInfo &range_info);
 
-  Status GetWatchpointSupportInfo(uint32_t &num);
+  std::optional<uint32_t> GetWatchpointSlotCount();
 
-  Status GetWatchpointSupportInfo(uint32_t &num, bool &after,
-                                  const ArchSpec &arch);
-
-  Status GetWatchpointsTriggerAfterInstruction(bool &after,
-                                               const ArchSpec &arch);
+  std::optional<bool> GetWatchpointReportedAfter();
 
   const ArchSpec &GetHostArchitecture();
 
   std::chrono::seconds GetHostDefaultPacketTimeout();
 
   const ArchSpec &GetProcessArchitecture();
 
   bool GetProcessStandaloneBinary(UUID &uuid, lldb::addr_t &value,

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

map_node.ForEachChildElement([this](const XMLNode &memory_node) -> bool {
     return true;
   });
 
   m_qXfer_memory_map_loaded = true;
 
   return error;
 }
 
-Status GDBRemoteCommunicationClient::GetWatchpointSupportInfo(uint32_t &num) {
-  Status error;
-
+std::optional<uint32_t> GDBRemoteCommunicationClient::GetWatchpointSlotCount() {
   if (m_supports_watchpoint_support_info == eLazyBoolYes) {
-    num = m_num_supported_hardware_watchpoints;
-    return error;
+    return m_num_supported_hardware_watchpoints;
   }
 
-  // Set num to 0 first.
-  num = 0;
+  std::optional<uint32_t> num;
   if (m_supports_watchpoint_support_info != eLazyBoolNo) {
     StringExtractorGDBRemote response;
     if (SendPacketAndWaitForResponse("qWatchpointSupportInfo:", response) ==
         PacketResult::Success) {
       m_supports_watchpoint_support_info = eLazyBoolYes;
       llvm::StringRef name;
       llvm::StringRef value;
-      bool found_num_field = false;
       while (response.GetNameColonValue(name, value)) {
         if (name.equals("num")) {
           value.getAsInteger(0, m_num_supported_hardware_watchpoints);
           num = m_num_supported_hardware_watchpoints;
-          found_num_field = true;
         }
       }
-      if (!found_num_field) {
+      if (!num) {
         m_supports_watchpoint_support_info = eLazyBoolNo;
       }
     } else {
       m_supports_watchpoint_support_info = eLazyBoolNo;
     }
   }
 
-  if (m_supports_watchpoint_support_info == eLazyBoolNo) {
-    error.SetErrorString("qWatchpointSupportInfo is not supported");
-  }
-  return error;
-}
-
-lldb_private::Status GDBRemoteCommunicationClient::GetWatchpointSupportInfo(
-    uint32_t &num, bool &after, const ArchSpec &arch) {
-  Status error(GetWatchpointSupportInfo(num));
-  if (error.Success())
-    error = GetWatchpointsTriggerAfterInstruction(after, arch);
-  return error;
+  return num;
 }
 
-lldb_private::Status
-GDBRemoteCommunicationClient::GetWatchpointsTriggerAfterInstruction(
-    bool &after, const ArchSpec &arch) {
-  Status error;
-  llvm::Triple triple = arch.GetTriple();
-
-  // 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 and ppc64, watchpoint exceptions are always
-    // generated before the instruction is executed. The connected target may
-    // not support qHostInfo or qWatchpointSupportInfo packets.
-    after = !(triple.isMIPS() || triple.isPPC64());
-  } else {
-    // For MIPS and ppc64, set m_watchpoints_trigger_after_instruction to
-    // eLazyBoolNo if it is not calculated before.
-    if (m_watchpoints_trigger_after_instruction == eLazyBoolCalculate &&
-        (triple.isMIPS() || triple.isPPC64()))
-      m_watchpoints_trigger_after_instruction = eLazyBoolNo;
+std::optional<bool> GDBRemoteCommunicationClient::GetWatchpointReportedAfter() {
+  if (m_qHostInfo_is_valid == eLazyBoolCalculate)
+    GetHostInfo();
 
-    after = (m_watchpoints_trigger_after_instruction != eLazyBoolNo);
+  // Process determines this by target CPU, but allow for the
+  // remote stub to override it via the qHostInfo
+  // watchpoint_exceptions_received key, if it is present.
+  if (m_qHostInfo_is_valid == eLazyBoolYes) {
+    if (m_watchpoints_trigger_after_instruction == eLazyBoolNo)
+      return false;
+    if (m_watchpoints_trigger_after_instruction == eLazyBoolYes)
+      return true;
   }
-  return error;
+
+  return std::nullopt;
 }
 
 int GDBRemoteCommunicationClient::SetSTDIN(const FileSpec &file_spec) {
   if (file_spec) {
     std::string path{file_spec.GetPath(false)};
     StreamString packet;
     packet.PutCString("QSetSTDIN:");
     packet.PutStringAsRawHex8(path);

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

public:
 
   Status DisableBreakpointSite(BreakpointSite *bp_site) override;
 
   // Process Watchpoints
   Status EnableWatchpoint(Watchpoint *wp, bool notify = true) override;
 
   Status DisableWatchpoint(Watchpoint *wp, bool notify = true) override;
 
-  Status GetWatchpointSupportInfo(uint32_t &num) override;
+  std::optional<uint32_t> GetWatchpointSlotCount() override;
 
   llvm::Expected<TraceSupportedResponse> TraceSupported() override;
 
   llvm::Error TraceStop(const TraceStopRequest &request) override;
 
   llvm::Error TraceStart(const llvm::json::Value &request) override;
 
   llvm::Expected<std::string> TraceGetState(llvm::StringRef type) override;
 
   llvm::Expected<std::vector<uint8_t>>
   TraceGetBinaryData(const TraceGetBinaryDataRequest &request) override;
 
-  Status GetWatchpointSupportInfo(uint32_t &num, bool &after) override;
+  std::optional<bool> DoGetWatchpointReportedAfter() override;
 
   bool StartNoticingNewThreads() override;
 
   bool StopNoticingNewThreads() override;
 
   GDBRemoteCommunicationClient &GetGDBRemote() { return m_gdb_comm; }
 
   Status SendEventData(const char *data) override;

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

 
 Status ProcessGDBRemote::DoGetMemoryRegionInfo(addr_t load_addr,
                                                MemoryRegionInfo &region_info) {
 
   Status error(m_gdb_comm.GetMemoryRegionInfo(load_addr, region_info));
   return error;
 }
 
-Status ProcessGDBRemote::GetWatchpointSupportInfo(uint32_t &num) {
-
-  Status error(m_gdb_comm.GetWatchpointSupportInfo(num));
-  return error;
+std::optional<uint32_t> ProcessGDBRemote::GetWatchpointSlotCount() {
+  return m_gdb_comm.GetWatchpointSlotCount();
 }
 
-Status ProcessGDBRemote::GetWatchpointSupportInfo(uint32_t &num, bool &after) {
-  Status error(m_gdb_comm.GetWatchpointSupportInfo(
-      num, after, GetTarget().GetArchitecture()));
-  return error;
+std::optional<bool> ProcessGDBRemote::DoGetWatchpointReportedAfter() {
+  return m_gdb_comm.GetWatchpointReportedAfter();
 }
 
 Status ProcessGDBRemote::DoDeallocateMemory(lldb::addr_t addr) {
   Status error;
   LazyBool supported = m_gdb_comm.SupportsAllocDeallocMemory();
 
   switch (supported) {
   case eLazyBoolCalculate:

lldb/source/Target/Process.cpp

LLDB_LOGF(log,
             "Process::DeallocateMemory(addr=0x%16.16" PRIx64
             ") => err = %s (m_stop_id = %u, m_memory_id = %u)",
             ptr, error.AsCString("SUCCESS"), m_mod_id.GetStopID(),
             m_mod_id.GetMemoryID());
 #endif
   return error;
 }
 
+bool Process::GetWatchpointReportedAfter() {
+  if (std::optional<bool> subclass_override = DoGetWatchpointReportedAfter())
+    return *subclass_override;
+
+  bool reported_after = true;
+  const ArchSpec &arch = GetTarget().GetArchitecture();
+  if (!arch.IsValid())
+    return reported_after;
+  llvm::Triple triple = arch.GetTriple();
+
+  if (triple.isMIPS() || triple.isPPC64() || triple.isRISCV() ||
+      triple.isAArch64() || triple.isArmMClass() || triple.isARM())
+    reported_after = false;
+
+  return reported_after;
+}
+
 ModuleSP Process::ReadModuleFromMemory(const FileSpec &file_spec,
                                        lldb::addr_t header_addr,
                                        size_t size_to_read) {
   Log *log = GetLog(LLDBLog::Host);
   if (log) {
     LLDB_LOGF(log,
               "Process::ReadModuleFromMemory reading %s binary from memory",
               file_spec.GetPath().c_str());

lldb/source/Target/StopInfo.cpp

if (!m_should_stop) {
       return m_should_stop;
     }
     // If this is a system where we need to execute the watchpoint by hand
     // after the hit, queue a thread plan to do that, and then say not to stop.
     // Otherwise, let the async action figure out whether the watchpoint should
     // stop
 
     ProcessSP process_sp = exe_ctx.GetProcessSP();
-    uint32_t num;
-    bool wp_triggers_after;
-
-    if (!process_sp->GetWatchpointSupportInfo(num, wp_triggers_after)
-            .Success()) {
-      m_should_stop_is_valid = true;
-      m_should_stop = true;
-      return m_should_stop;
-    }
-            
+    bool wp_triggers_after = process_sp->GetWatchpointReportedAfter();
     if (!wp_triggers_after) {
       // We have to step over the watchpoint before we know what to do:   
       StopInfoWatchpointSP me_as_siwp_sp 
           = std::static_pointer_cast<StopInfoWatchpoint>(shared_from_this());
       ThreadPlanSP step_over_wp_sp(new ThreadPlanStepOverWatchpoint(
           *(thread_sp.get()), me_as_siwp_sp, wp_sp));
       Status error;
       error = thread_sp->QueueThreadPlan(step_over_wp_sp, false);

lldb/source/Target/Target.cpp

void Target::GetBreakpointNames(std::vector<std::string> &names) {
   llvm::sort(names);
 }
 
 bool Target::ProcessIsValid() {
   return (m_process_sp && m_process_sp->IsAlive());
 }
 
 static bool CheckIfWatchpointsSupported(Target *target, Status &error) {
-  uint32_t num_supported_hardware_watchpoints;
-  Status rc = target->GetProcessSP()->GetWatchpointSupportInfo(
-      num_supported_hardware_watchpoints);
+  std::optional<uint32_t> num_supported_hardware_watchpoints =
+      target->GetProcessSP()->GetWatchpointSlotCount();
 
   // If unable to determine the # of watchpoints available,
   // assume they are supported.
-  if (rc.Fail())
+  if (!num_supported_hardware_watchpoints)
     return true;
 
   if (num_supported_hardware_watchpoints == 0) {
     error.SetErrorStringWithFormat(
         "Target supports (%u) hardware watchpoint slots.\n",
-        num_supported_hardware_watchpoints);
+        *num_supported_hardware_watchpoints);
     return false;
   }
   return true;
 }
 
 // See also Watchpoint::SetWatchpointType(uint32_t type) and the
 // OptionGroupWatchpoint::WatchType enum type.
 WatchpointSP Target::CreateWatchpoint(lldb::addr_t addr, size_t size,