Add specific ppc64le hardware watchpoint handler

[anajuliapc]

Summary

Add hardware watchpoint funcionality for ppc64le

[zturner]

source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp

90–93 ↗

(On Diff #118942)

Can you set these inline in the class body? e.g. when you declare them in the header file:

uint32_t m_max_hwp_supported = 16;
uint32_t m_max_hbp_supported = 16;
bool m_refresh_hwdebug_info = true;

283–295 ↗

(On Diff #118942)

Can you make an enum for these magic values 1, 2, and 3?

302–318 ↗

(On Diff #118942)

How about:

addr_t begin = llvm::alignDown(addr, 8);
addr_t end = llvm::alignUp(addr+size, 8);
size = llvm::NextPowerOf2(end-begin);

327–333 ↗

(On Diff #118942)

Can we use a range-based for here?

DREG *reg = nullptr;
for (auto &dr : m_hwp_regs) {
  if (dr.control & 1) == 0)
    reg = &dr;
  else if (dr.address == addr)
    return LLDB_INVALID_INDEX32;
}

if (!reg)
  return LLDB_INVALID_INDEX32;

reg->real_addr = real_addr;
reg->address = addr;
reg->control = control_value;
reg->mode = rw_mode;

etc

349 ↗

(On Diff #118942)

How about:

m_hwp_regs[wp_index].control &= llvm::maskTrailingZeros<uint32_t>(1);

374 ↗

(On Diff #118942)

reinterpret_cast from long* to long? Is this correct?

389 ↗

(On Diff #118942)

Same as before. Is this correct?

402–413 ↗

(On Diff #118942)

unsigned control = (m_hwp_regs[wp_index].control >> 5) & 0xFF;
assert(llvm::isPowerOf2_32(control + 1));
return llvm::countPopulation(control);

416–417 ↗

(On Diff #118942)

Can this function accept a const DREG& instead of an index?

421–424 ↗

(On Diff #118942)

return !!(m_hwp_regs[wp_index].control & 0x1);

474 ↗

(On Diff #118942)

No need for else here.

source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.h

70–72 ↗

(On Diff #118942)

Can these two functions take const DREG& instead of indices? Also, it seems like they could be raised out of the class and into global static functions in the cpp file.

102 ↗

(On Diff #118942)

How about std::array<DREG, 4> m_hwp_regs;

[clayborg]

I agree with all of Zach's suggestions.

source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.h

98–99 ↗

(On Diff #118942)

As long as this code is only ever compiled native, using "long" and "int" are ok. Just beware of code that isn't always compiled native.

[clayborg]

Feel free to add me as a reviewer. Zach as well.

[gut]

Thanks for supporting this change. I guess @anajuliapc will add you both as reviewer as soon as she updates this patch.

BTW, I agree that patches should be improving code quality but I wanted to highlight that these changes were actually based on the current ARM64 implementation. E.g: this snip and this requested change. But thanks for pointing it out.

source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.h

98–99 ↗

(On Diff #118942)

It's a native only compiled file (see line 13. It defines a #if defined(__powerpc64__) that ends at the end of file)

[zturner]

In D38897#897378, @gut wrote:

Thanks for supporting this change. I guess @anajuliapc will add you both as reviewer as soon as she updates this patch.

BTW, I agree that patches should be improving code quality but I wanted to highlight that these changes were actually based on the current ARM64 implementation. E.g: this snip and this requested change. But thanks for pointing it out.

I'd be equally fine with extracting that logic into a separate function, it looks mostly the same? I don't think we should be duplicating code (or even algorithms written slightly differently)

[anajuliapc]

source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp

374 ↗

(On Diff #118942)

Yes, It's strange but is the only way it works without changing NativeProcessLinux::PtraceWrapper. PPC_PTRACE_DELHWDEBUG works different from other PPC ptrace requests, because it must pass the return of PPC_PTRACE_SETHWDEBUG to ptrace instead of a data structure. If you pass it as long, PtraceWrapper fails because it must receive a void*.

I'll check the other points, thanks for the comments.

[anajuliapc]

• Add values to header file
• Use std::array to declare m_hwp_regs
• Use llvm's mask
• Remove switch and use assert instead
• Remove unnecessary 'else'
• Change return

[anajuliapc]

I applied some of the suggestions. I'm still working on the others, since I may need to change some logic

Thanks for the comments

[anajuliapc]

• Change numeric values to enum
• Use llvm's functions to align addresses

source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp

327–333 ↗

(On Diff #118942)

I don't think it's worth the effort to do it like that. This function returns wp_index anyway, and it's easier to limit the number of watchpoints to PowerPC's actual limit, which is given by ptrace.

416–417 ↗

(On Diff #118942)

No because it's called by NativeProcessLinux., I would have to change it too and I don't think I should since is a common file.

source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.h

70–72 ↗

(On Diff #118942)

I had some ideas while implementing your sugestions... I'm thinking about removing these functions and put these information in m_hwp_regs struct. Do you think I should do it in a new patch?

[clayborg]

source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp

284 ↗

(On Diff #119567)

We should use the lldb::WatchpointKind from lldb-enumerations.h:

//----------------------------------------------------------------------
// Watchpoint Kind
// Indicates what types of events cause the watchpoint to fire.
// Used by Native*Protocol-related classes.
//----------------------------------------------------------------------
FLAGS_ENUM(WatchpointKind){eWatchpointKindRead = (1u << 0),
                           eWatchpointKindWrite = (1u << 1)};

The you look at the bits and do the right thing. "access_mode" used below will just be if read and write are set.

[anajuliapc]

source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp

284 ↗

(On Diff #119567)

Variable watch_flags is set in GDBRemoteCommunicationServerLLGS with read = 2 and write = 1.
Where do you think I should change it?

[clayborg]

source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp

284 ↗

(On Diff #119567)

I would switch them over to use eWatchpointKindRead and eWatchpointKindWrite and modify all code that falls out from that to use these definitions as well.

[anajuliapc]

• Switch over read and write eWatchpointKind to match with watch_flags

[gut]

source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp

292 ↗

(On Diff #119835)

As these enums are bitmasks, the traditional way of checking if more than one is set is to use the bitwise OR operator (|). Mostly because of the semantics, as you're not doing arithmetic there.

[anajuliapc]

• Use bitwise OR operator

[clayborg]

Looks fine. Thanks for doing the changes.

[anajuliapc]

source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp

59 ↗

(On Diff #119847)

I just realized I forgot to remove the enum I've created before. I know you already accepted this patch, but may I update it anyway?

[anajuliapc]

• Remove unused enum

[clayborg]

Yeah, I meant to suggest to remove that. Remove and submit as needed. Thanks again.

[gut]

In D38897#903581, @clayborg wrote:

Looks fine. Thanks for doing the changes.

Hi, it's been already 4 days since this patch was accepted but not merged. Did something happen internally or we just need to wait a little longer?

[gut]

In D38897#908283, @gut wrote:

In D38897#903581, @clayborg wrote:

Looks fine. Thanks for doing the changes.

Hi, it's been already 4 days since this patch was accepted but not merged. Did something happen internally or we just need to wait a little longer?

@eugene : Hi, I wonder if you can commit this accepted review for us. The other 2 reviews from our teams were pushed by you (D36804 D38323)

Thanks in advance.

lldb/trunk/include/lldb/lldb-enumerations.h

enum ExpressionEvaluationPhase {
   eExpressionEvaluationComplete
 };
 
 //----------------------------------------------------------------------
 // Watchpoint Kind
 // Indicates what types of events cause the watchpoint to fire.
 // Used by Native*Protocol-related classes.
 //----------------------------------------------------------------------
-FLAGS_ENUM(WatchpointKind){eWatchpointKindRead = (1u << 0),
-                           eWatchpointKindWrite = (1u << 1)};
+FLAGS_ENUM(WatchpointKind){eWatchpointKindWrite = (1u << 0),
+                           eWatchpointKindRead = (1u << 1)};
 
 enum GdbSignal {
   eGdbSignalBadAccess = 0x91,
   eGdbSignalBadInstruction = 0x92,
   eGdbSignalArithmetic = 0x93,
   eGdbSignalEmulation = 0x94,
   eGdbSignalSoftware = 0x95,
   eGdbSignalBreakpoint = 0x96

lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.h

-//===-- NativeRegisterContextLinux_ppc64le.h ---------------------*- C++ -*-===//
+//===-- NativeRegisterContextLinux_ppc64le.h --------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
public:
 
   Status WriteRegister(const RegisterInfo *reg_info,
                        const RegisterValue &reg_value) override;
 
   Status ReadAllRegisterValues(lldb::DataBufferSP &data_sp) override;
 
   Status WriteAllRegisterValues(const lldb::DataBufferSP &data_sp) override;
 
+  //------------------------------------------------------------------
+  // Hardware watchpoint mangement functions
+  //------------------------------------------------------------------
+
+  uint32_t NumSupportedHardwareWatchpoints() override;
+
+  uint32_t SetHardwareWatchpoint(lldb::addr_t addr, size_t size,
+                                 uint32_t watch_flags) override;
+
+  bool ClearHardwareWatchpoint(uint32_t hw_index) override;
+
+  Status GetWatchpointHitIndex(uint32_t &wp_index,
+                               lldb::addr_t trap_addr) override;
+
+  lldb::addr_t GetWatchpointHitAddress(uint32_t wp_index) override;
+
+  lldb::addr_t GetWatchpointAddress(uint32_t wp_index) override;
+
+  uint32_t GetWatchpointSize(uint32_t wp_index);
+
+  bool WatchpointIsEnabled(uint32_t wp_index);
+
 protected:
   Status DoReadGPR(void *buf, size_t buf_size) override;
 
   Status DoWriteGPR(void *buf, size_t buf_size) override;
 
   void *GetGPRBuffer() override { return &m_gpr_ppc64le; }
 
 private:
   GPR m_gpr_ppc64le; // 64-bit general purpose registers.
 
   bool IsGPR(unsigned reg) const;
+
+  Status ReadHardwareDebugInfo();
+
+  Status WriteHardwareDebugRegs();
+
+  // Debug register info for hardware watchpoints management.
+  struct DREG {
+    lldb::addr_t address;   // Breakpoint/watchpoint address value.
+    lldb::addr_t hit_addr;  // Address at which last watchpoint trigger
+                            // exception occurred.
+    lldb::addr_t real_addr; // Address value that should cause target to stop.
+    uint32_t control;       // Breakpoint/watchpoint control value.
+    uint32_t refcount;      // Serves as enable/disable and reference counter.
+    long slot;              // Saves the value returned from PTRACE_SETHWDEBUG.
+    int mode;               // Defines if watchpoint is read/write/access.
+  };
+
+  std::array<DREG, 4> m_hwp_regs;
+
+  // 16 is just a maximum value, query hardware for actual watchpoint count
+  uint32_t m_max_hwp_supported = 16;
+  uint32_t m_max_hbp_supported = 16;
+  bool m_refresh_hwdebug_info = true;
 };
 
 } // namespace process_linux
 } // namespace lldb_private
 
 #endif // #ifndef lldb_NativeRegisterContextLinux_ppc64le_h
 
 #endif // defined(__powerpc64__)

lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp

NativeRegisterContextLinux_ppc64le::NativeRegisterContextLinux_ppc64le(
     uint32_t concrete_frame_idx)
     : NativeRegisterContextLinux(native_thread, concrete_frame_idx,
                                  new RegisterInfoPOSIX_ppc64le(target_arch)) {
   if (target_arch.GetMachine() != llvm::Triple::ppc64le) {
     llvm_unreachable("Unhandled target architecture.");
   }
 
   ::memset(&m_gpr_ppc64le, 0, sizeof(m_gpr_ppc64le));
+  ::memset(&m_hwp_regs, 0, sizeof(m_hwp_regs));
 }
 
 uint32_t NativeRegisterContextLinux_ppc64le::GetRegisterSetCount() const {
   return k_num_register_sets;
 }
 
 const RegisterSet *
 NativeRegisterContextLinux_ppc64le::GetRegisterSet(uint32_t set_index) const {

 
 Status NativeRegisterContextLinux_ppc64le::DoWriteGPR(
     void *buf, size_t buf_size) {
   int regset = NT_PRSTATUS;
   return NativeProcessLinux::PtraceWrapper(PTRACE_SETREGS, m_thread.GetID(),
                                            &regset, buf, buf_size);
 }
 
+uint32_t NativeRegisterContextLinux_ppc64le::NumSupportedHardwareWatchpoints() {
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_WATCHPOINTS));
+
+  // Read hardware breakpoint and watchpoint information.
+  Status error = ReadHardwareDebugInfo();
+
+  if (error.Fail())
+    return 0;
+
+  LLDB_LOG(log, "{0}", m_max_hwp_supported);
+  return m_max_hwp_supported;
+}
+
+uint32_t NativeRegisterContextLinux_ppc64le::SetHardwareWatchpoint(
+    lldb::addr_t addr, size_t size, uint32_t watch_flags) {
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_WATCHPOINTS));
+  LLDB_LOG(log, "addr: {0:x}, size: {1:x} watch_flags: {2:x}", addr, size,
+           watch_flags);
+
+  // Read hardware breakpoint and watchpoint information.
+  Status error = ReadHardwareDebugInfo();
+
+  if (error.Fail())
+    return LLDB_INVALID_INDEX32;
+
+  uint32_t control_value = 0, wp_index = 0;
+  lldb::addr_t real_addr = addr;
+  uint32_t rw_mode = 0;
+
+  // Check if we are setting watchpoint other than read/write/access
+  // Update watchpoint flag to match ppc64le write-read bit configuration.
+  switch (watch_flags) {
+  case eWatchpointKindWrite:
+    rw_mode = PPC_BREAKPOINT_TRIGGER_WRITE;
+    watch_flags = 2;
+    break;
+  case eWatchpointKindRead:
+    rw_mode = PPC_BREAKPOINT_TRIGGER_READ;
+    watch_flags = 1;
+    break;
+  case (eWatchpointKindRead | eWatchpointKindWrite):
+    rw_mode = PPC_BREAKPOINT_TRIGGER_RW;
+    break;
+  default:
+    return LLDB_INVALID_INDEX32;
+  }
+
+  // Check if size has a valid hardware watchpoint length.
+  if (size != 1 && size != 2 && size != 4 && size != 8)
+    return LLDB_INVALID_INDEX32;
+
+  // Check 8-byte alignment for hardware watchpoint target address.
+  // Below is a hack to recalculate address and size in order to
+  // make sure we can watch non 8-byte alligned addresses as well.
+  if (addr & 0x07) {
+
+    addr_t begin = llvm::alignDown(addr, 8);
+    addr_t end = llvm::alignTo(addr + size, 8);
+    size = llvm::PowerOf2Ceil(end - begin);
+
+    addr = addr & (~0x07);
+  }
+
+  // Setup control value
+  control_value = watch_flags << 3;
+  control_value |= ((1 << size) - 1) << 5;
+  control_value |= (2 << 1) | 1;
+
+  // Iterate over stored watchpoints and find a free wp_index
+  wp_index = LLDB_INVALID_INDEX32;
+  for (uint32_t i = 0; i < m_max_hwp_supported; i++) {
+    if ((m_hwp_regs[i].control & 1) == 0) {
+      wp_index = i; // Mark last free slot
+    } else if (m_hwp_regs[i].address == addr) {
+      return LLDB_INVALID_INDEX32; // We do not support duplicate watchpoints.
+    }
+  }
+
+  if (wp_index == LLDB_INVALID_INDEX32)
+    return LLDB_INVALID_INDEX32;
+
+  // Update watchpoint in local cache
+  m_hwp_regs[wp_index].real_addr = real_addr;
+  m_hwp_regs[wp_index].address = addr;
+  m_hwp_regs[wp_index].control = control_value;
+  m_hwp_regs[wp_index].mode = rw_mode;
+
+  // PTRACE call to set corresponding watchpoint register.
+  error = WriteHardwareDebugRegs();
+
+  if (error.Fail()) {
+    m_hwp_regs[wp_index].address = 0;
+    m_hwp_regs[wp_index].control &= llvm::maskTrailingZeros<uint32_t>(1);
+
+    return LLDB_INVALID_INDEX32;
+  }
+
+  return wp_index;
+}
+
+bool NativeRegisterContextLinux_ppc64le::ClearHardwareWatchpoint(
+    uint32_t wp_index) {
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_WATCHPOINTS));
+  LLDB_LOG(log, "wp_index: {0}", wp_index);
+
+  // Read hardware breakpoint and watchpoint information.
+  Status error = ReadHardwareDebugInfo();
+
+  if (error.Fail())
+    return false;
+
+  if (wp_index >= m_max_hwp_supported)
+    return false;
+
+  // Create a backup we can revert to in case of failure.
+  lldb::addr_t tempAddr = m_hwp_regs[wp_index].address;
+  uint32_t tempControl = m_hwp_regs[wp_index].control;
+  long *tempSlot = reinterpret_cast<long *>(m_hwp_regs[wp_index].slot);
+
+  // Update watchpoint in local cache
+  m_hwp_regs[wp_index].control &= llvm::maskTrailingZeros<uint32_t>(1);
+  m_hwp_regs[wp_index].address = 0;
+  m_hwp_regs[wp_index].slot = 0;
+  m_hwp_regs[wp_index].mode = 0;
+
+  // Ptrace call to update hardware debug registers
+  error = NativeProcessLinux::PtraceWrapper(PPC_PTRACE_DELHWDEBUG,
+                                            m_thread.GetID(), 0, tempSlot);
+
+  if (error.Fail()) {
+    m_hwp_regs[wp_index].control = tempControl;
+    m_hwp_regs[wp_index].address = tempAddr;
+    m_hwp_regs[wp_index].slot = reinterpret_cast<long>(tempSlot);
+
+    return false;
+  }
+
+  return true;
+}
+
+uint32_t
+NativeRegisterContextLinux_ppc64le::GetWatchpointSize(uint32_t wp_index) {
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_WATCHPOINTS));
+  LLDB_LOG(log, "wp_index: {0}", wp_index);
+
+  unsigned control = (m_hwp_regs[wp_index].control >> 5) & 0xff;
+  assert(llvm::isPowerOf2_32(control + 1));
+  return llvm::countPopulation(control);
+}
+
+bool NativeRegisterContextLinux_ppc64le::WatchpointIsEnabled(
+    uint32_t wp_index) {
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_WATCHPOINTS));
+  LLDB_LOG(log, "wp_index: {0}", wp_index);
+
+  return !!((m_hwp_regs[wp_index].control & 0x1) == 0x1);
+}
+
+Status NativeRegisterContextLinux_ppc64le::GetWatchpointHitIndex(
+    uint32_t &wp_index, lldb::addr_t trap_addr) {
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_WATCHPOINTS));
+  LLDB_LOG(log, "wp_index: {0}, trap_addr: {1:x}", wp_index, trap_addr);
+
+  uint32_t watch_size;
+  lldb::addr_t watch_addr;
+
+  for (wp_index = 0; wp_index < m_max_hwp_supported; ++wp_index) {
+    watch_size = GetWatchpointSize(wp_index);
+    watch_addr = m_hwp_regs[wp_index].address;
+
+    if (WatchpointIsEnabled(wp_index) && trap_addr >= watch_addr &&
+        trap_addr <= watch_addr + watch_size) {
+      m_hwp_regs[wp_index].hit_addr = trap_addr;
+      return Status();
+    }
+  }
+
+  wp_index = LLDB_INVALID_INDEX32;
+  return Status();
+}
+
+lldb::addr_t
+NativeRegisterContextLinux_ppc64le::GetWatchpointAddress(uint32_t wp_index) {
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_WATCHPOINTS));
+  LLDB_LOG(log, "wp_index: {0}", wp_index);
+
+  if (wp_index >= m_max_hwp_supported)
+    return LLDB_INVALID_ADDRESS;
+
+  if (WatchpointIsEnabled(wp_index))
+    return m_hwp_regs[wp_index].real_addr;
+  else
+    return LLDB_INVALID_ADDRESS;
+}
+
+lldb::addr_t
+NativeRegisterContextLinux_ppc64le::GetWatchpointHitAddress(uint32_t wp_index) {
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_WATCHPOINTS));
+  LLDB_LOG(log, "wp_index: {0}", wp_index);
+
+  if (wp_index >= m_max_hwp_supported)
+    return LLDB_INVALID_ADDRESS;
+
+  if (WatchpointIsEnabled(wp_index))
+    return m_hwp_regs[wp_index].hit_addr;
+
+  return LLDB_INVALID_ADDRESS;
+}
+
+Status NativeRegisterContextLinux_ppc64le::ReadHardwareDebugInfo() {
+  if (!m_refresh_hwdebug_info) {
+    return Status();
+  }
+
+  ::pid_t tid = m_thread.GetID();
+
+  struct ppc_debug_info hwdebug_info;
+  Status error;
+
+  error = NativeProcessLinux::PtraceWrapper(
+      PPC_PTRACE_GETHWDBGINFO, tid, 0, &hwdebug_info, sizeof(hwdebug_info));
+
+  if (error.Fail())
+    return error;
+
+  m_max_hwp_supported = hwdebug_info.num_data_bps;
+  m_max_hbp_supported = hwdebug_info.num_instruction_bps;
+  m_refresh_hwdebug_info = false;
+
+  return error;
+}
+
+Status NativeRegisterContextLinux_ppc64le::WriteHardwareDebugRegs() {
+  struct ppc_hw_breakpoint reg_state;
+  Status error;
+  long ret;
+
+  for (uint32_t i = 0; i < m_max_hwp_supported; i++) {
+    reg_state.addr = m_hwp_regs[i].address;
+    reg_state.trigger_type = m_hwp_regs[i].mode;
+    reg_state.version = 1;
+    reg_state.addr_mode = PPC_BREAKPOINT_MODE_EXACT;
+    reg_state.condition_mode = PPC_BREAKPOINT_CONDITION_NONE;
+    reg_state.addr2 = 0;
+    reg_state.condition_value = 0;
+
+    error = NativeProcessLinux::PtraceWrapper(PPC_PTRACE_SETHWDEBUG,
+                                              m_thread.GetID(), 0, &reg_state,
+                                              sizeof(reg_state), &ret);
+
+    if (error.Fail())
+      return error;
+
+    m_hwp_regs[i].slot = ret;
+  }
+
+  return error;
+}
+
 #endif // defined(__powerpc64__)

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

GDBRemoteCommunicationClient::GetWatchpointsTriggerAfterInstruction(
     bool &after, const ArchSpec &arch) {
   Status 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.
+    // On targets like MIPS and ppc64le, 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)
+        atype == llvm::Triple::mips64 || atype == llvm::Triple::mips64el ||
+        atype == llvm::Triple::ppc64le)
       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 &&
+    // For MIPS and ppc64le, 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))
+          atype == llvm::Triple::mips64 || atype == llvm::Triple::mips64el)) ||
+        atype == llvm::Triple::ppc64le) {
       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) {