Report watchpoint hits during single stepping.

[chaoren]

Summary

Reorganized NativeProcessLinux::MonitorSIGTRAP to check for watchpoint hits on
TRAP_TRACE.

Added test for stepping over watchpoints.

https://llvm.org/bugs/show_bug.cgi?id=22814

[ovyalov]

source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp

1085 ↗

(On Diff #22148)

Do we need to return failed error here?

test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py

1 ↗

(On Diff #22148)

Did you run this test remotely?

[clayborg]

Should probably init the references values to something invalid as mentioned in inlined comments.

source/Host/common/NativeRegisterContext.cpp

309 ↗

(On Diff #22148)

Set "is_hit" to false?

315 ↗

(On Diff #22148)

Set "wp_index" to UINT32_MAX?

321 ↗

(On Diff #22148)

Set "is_vacant" to false?

[labath]

test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py

51 ↗

(On Diff #22148)

This test relies on step-over being implemented using (instruction) single-stepping. If that is ever changed to do a "run to temporary breakpoint", the test will be ineffective. Could the check be implemented using "thread step-inst" in a loop? If not then at least make a note of this somewhere.

[tberghammer]

source/Plugins/Process/Linux/NativeProcessLinux.cpp

2254–2278 ↗

(On Diff #22148)

The logic for TRAP_TRACE is very similar to the one in NativeThreadLinux::SetStoppedByWatchpoint. Can you merge to common part of the two?

2268–2275 ↗

(On Diff #22148)

What happens if it is a conditional breakpoint where the break condition isn't met? I haven't checked it but I guess the thread will be continued even though it should be in stopped by trace state.

[jingham]

test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py

37–38 ↗

(On Diff #22148)

Please don't use self.runCmd("breakpoint set --name main") bare like this in test cases. This makes a breakpoint but never checks that the breakpoint gets set. If something goes wrong with that you'll get some confusing error message later on which won't tell you what really happened.

lldbutil has a bunch of utility functions for setting breakpoints that set the breakpoint, and then check that it actually was set - so you can easily set breakpoints without having to do all the checking by hand. run_break_set_by_symbol is the one you want in this case.

[chaoren]

source/Plugins/Process/Linux/NativeProcessLinux.cpp

2254–2278 ↗

(On Diff #22148)

Yeah, I think we can remove SetStoppedByTrace() in NativeThreadLinux::SetStoppedByWatchpoint if we just combine TRAP_TRACE and TRAP_HWBKPT.

2268–2275 ↗

(On Diff #22148)

:( Yeah, that does happen. Commenting that out for now. I'll revisit once I figure out how to check the condition there. And add a test case in lang/c/stepping/TestStepAndBreakpoints.py.

source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp

1085 ↗

(On Diff #22148)

I think whether a watchpoint hit is expected or not depends on the calling context. It's conceivable that the caller expects the possiblilty that no watchpoint has been hit. In that case, the caller will need to do some extra checking to determine if it needs to propagate the error.

[chaoren]

• Made reference values invalid for unimplemented.
• Using step-inst loop.
• Merging TRAP_TRACE & NativeThreadLinux::SetStoppedByWatchpoint logic.
• Using run_break_set_by_symbol.
• Commenting out breakpoint detection until I figure out how to handle conditional breakpoints.
• Remote test fails. The test looked simple enough (no file transfers, special env vars), that I didn't suspect there would be a difference. Investigating...

[tberghammer]

source/Plugins/Process/Linux/NativeProcessLinux.cpp

2253 ↗

(On Diff #22218)

Can you add a comment why we have this case here? (I don't know. If you don't know it either then just leave it as it is now)

2263–2264 ↗

(On Diff #22218)

Please pass down the watchpoint index to MonitorWatchpoint and then to SetStoppedByWatchpoint so it don't have to look it up again.

2267–2276 ↗

(On Diff #22218)

AFAIK it is impossible for LLGS to detect if a conditional breakpoint was hit or not because it don't know anything about the condition. I suggest to remove this comment and just create a bug for it (with possibly a test case) because this part of the code will never fix this issue. I think the proper solution will require lldb to do the check if a breakpoint was hit or not when it receive a stop by trace packet.

We should also consider what is the expected behavior when a watchpoint and a breakpoint is hit with the same instruction. I don't really care about what we display to the user but both the hit count of the breakpoint and the hit count of the watchpoint should be incremented. This behavior can be implemented also if you move the breakpoint detection into lldb.

source/Plugins/Process/Linux/NativeThreadLinux.cpp

400–402 ↗

(On Diff #22218)

Please add an assert and set stop reason to some meaningful default value (e.g.: Invalid) if wp_index == LLDB_INVALID_INDEX32 as this case shouldn't happen

[chaoren]

source/Plugins/Process/Linux/NativeProcessLinux.cpp

2253 ↗

(On Diff #22218)

No clue.

source/Plugins/Process/Linux/NativeThreadLinux.cpp

400–402 ↗

(On Diff #22218)

I think just an assert would be enough.

[chaoren]

• Pass down the watchpoint index to MonitorWatchpoint and then to SetStoppedByWatchpoint as per tberghammer's suggestion.
• Rewrote test using Python API as per jingham's suggestion.

[clayborg]

lgtm

[chaoren]

test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py

1 ↗

(On Diff #22148)

Remote test fails for VMware target, but not real machine (Vince's workstation). PTRACE_SINGLESTEP doesn't trigger watchpoints at all. Could this be a bug in VMware?

lldb/trunk/include/lldb/Host/common/NativeRegisterContext.h

public:
 
     virtual bool
     ClearHardwareWatchpoint (uint32_t hw_index);
 
     virtual Error
     ClearAllHardwareWatchpoints ();
 
     virtual Error
-    IsWatchpointHit (uint8_t wp_index);
+    IsWatchpointHit(uint32_t wp_index, bool &is_hit);
 
     virtual Error
-    IsWatchpointVacant (uint32_t wp_index);
+    GetWatchpointHitIndex(uint32_t &wp_index);
+
+    virtual Error
+    IsWatchpointVacant (uint32_t wp_index, bool &is_vacant);
 
     virtual lldb::addr_t
     GetWatchpointAddress (uint32_t wp_index);
 
     virtual bool
     HardwareSingleStep (bool enable);
 
     virtual Error

lldb/trunk/source/Host/common/NativeRegisterContext.cpp

 
 Error
 NativeRegisterContext::ClearAllHardwareWatchpoints ()
 {
     return Error ("not implemented");
 }
 
 Error
-NativeRegisterContext::IsWatchpointHit (uint8_t wp_index)
+NativeRegisterContext::IsWatchpointHit(uint32_t wp_index, bool &is_hit)
 {
+    is_hit = false;
     return Error ("not implemented");
 }
 
 Error
-NativeRegisterContext::IsWatchpointVacant (uint32_t wp_index)
+NativeRegisterContext::GetWatchpointHitIndex(uint32_t &wp_index)
 {
+    wp_index = LLDB_INVALID_INDEX32;
+    return Error ("not implemented");
+}
+
+Error
+NativeRegisterContext::IsWatchpointVacant (uint32_t wp_index, bool &is_vacant)
+{
+    is_vacant = false;
     return Error ("not implemented");
 }
 
 lldb::addr_t
 NativeRegisterContext::GetWatchpointAddress (uint32_t wp_index)
 {
     return LLDB_INVALID_ADDRESS;
 }

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

private:
         static bool
         MonitorCallback(void *callback_baton,
                 lldb::pid_t pid, bool exited, int signal, int status);
 
         void
         MonitorSIGTRAP(const siginfo_t *info, lldb::pid_t pid);
 
         void
+        MonitorTrace(lldb::pid_t pid, NativeThreadProtocolSP thread_sp);
+
+        void
+        MonitorBreakpoint(lldb::pid_t pid, NativeThreadProtocolSP thread_sp);
+
+        void
+        MonitorWatchpoint(lldb::pid_t pid, NativeThreadProtocolSP thread_sp, uint32_t wp_index);
+
+        void
         MonitorSignal(const siginfo_t *info, lldb::pid_t pid, bool exited);
 
 #if 0
         static ::ProcessMessage::CrashReason
         GetCrashReasonForSIGSEGV(const siginfo_t *info);
 
         static ::ProcessMessage::CrashReason
         GetCrashReasonForSIGILL(const siginfo_t *info);

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

 #include "lldb/Host/common/NativeRegisterContext.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Host/HostNativeThread.h"
 #include "lldb/Host/ThreadLauncher.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/ProcessLaunchInfo.h"
+#include "lldb/Utility/LLDBAssert.h"
 #include "lldb/Utility/PseudoTerminal.h"
 
 #include "Plugins/Process/POSIX/ProcessPOSIXLog.h"
 #include "Plugins/Process/Utility/LinuxSignals.h"
 #include "Utility/StringExtractor.h"
 #include "NativeThreadLinux.h"
 #include "ProcFileReader.h"
 #include "Procfs.h"
case (SIGTRAP | (PTRACE_EVENT_EXIT << 8)):
                                                    return Resume (tid_to_resume, (supress_signal) ? LLDB_INVALID_SIGNAL_NUMBER : signo);
                                                },
                                                CoordinatorErrorHandler);
 
         break;
     }
 
     case 0:
-    case TRAP_TRACE:
-        // We receive this on single stepping.
+    case TRAP_TRACE:  // We receive this on single stepping.
+    case TRAP_HWBKPT: // We receive this on watchpoint hit
+        if (thread_sp)
+        {
+            // If a watchpoint was hit, report it
+            uint32_t wp_index;
+            Error error = thread_sp->GetRegisterContext()->GetWatchpointHitIndex(wp_index);
+            if (error.Fail() && log)
+                log->Printf("NativeProcessLinux::%s() "
+                            "received error while checking for watchpoint hits, "
+                            "pid = %" PRIu64 " error = %s",
+                            __FUNCTION__, pid, error.AsCString());
+            if (wp_index != LLDB_INVALID_INDEX32)
+            {
+                MonitorWatchpoint(pid, thread_sp, wp_index);
+                break;
+            }
+        }
+        // Otherwise, report step over
+        MonitorTrace(pid, thread_sp);
+        break;
+
+    case SI_KERNEL:
+    case TRAP_BRKPT:
+        MonitorBreakpoint(pid, thread_sp);
+        break;
+
+    case SIGTRAP:
+    case (SIGTRAP | 0x80):
         if (log)
-            log->Printf ("NativeProcessLinux::%s() received trace event, pid = %" PRIu64 " (single stepping)", __FUNCTION__, pid);
+            log->Printf ("NativeProcessLinux::%s() received unknown SIGTRAP system call stop event, pid %" PRIu64 "tid %" PRIu64 ", resuming", __FUNCTION__, GetID (), pid);
 
+        // This thread is currently stopped.
+        NotifyThreadStop (pid);
         if (thread_sp)
+            std::static_pointer_cast<NativeThreadLinux> (thread_sp)->SetStoppedBySignal (SIGTRAP);
+
+            
+        // Ignore these signals until we know more about them.
+        m_coordinator_up->RequestThreadResume (pid,
+                                               [=](lldb::tid_t tid_to_resume, bool supress_signal)
                                                {
-            std::static_pointer_cast<NativeThreadLinux> (thread_sp)->SetStoppedByTrace ();
-        }
+                                                   std::static_pointer_cast<NativeThreadLinux> (thread_sp)->SetRunning ();
+                                                   return Resume (tid_to_resume, LLDB_INVALID_SIGNAL_NUMBER);
+                                               },
+                                               CoordinatorErrorHandler);
+        break;
+
+    default:
+        assert(false && "Unexpected SIGTRAP code!");
+        if (log)
+            log->Printf ("NativeProcessLinux::%s() pid %" PRIu64 "tid %" PRIu64 " received unhandled SIGTRAP code: 0x%" PRIx64, __FUNCTION__, GetID (), pid, static_cast<uint64_t> (SIGTRAP | (PTRACE_EVENT_CLONE << 8)));
+        break;
+        
+}
+
+void
+NativeProcessLinux::MonitorTrace(lldb::pid_t pid, NativeThreadProtocolSP thread_sp)
+{
+    Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS));
+    if (log)
+        log->Printf("NativeProcessLinux::%s() received trace event, pid = %" PRIu64 " (single stepping)",
+                __FUNCTION__, pid);
+
+    if (thread_sp)
+        std::static_pointer_cast<NativeThreadLinux>(thread_sp)->SetStoppedByTrace();
 
-        // This thread is currently stopped.
-        NotifyThreadStop (pid);
+    NotifyThreadStop(pid);
 
-        // Here we don't have to request the rest of the threads to stop or request a deferred stop.
-        // This would have already happened at the time the Resume() with step operation was signaled.
-        // At this point, we just need to say we stopped, and the deferred notifcation will fire off
-        // once all running threads have checked in as stopped.
-        SetCurrentThreadID (pid);
-        // Tell the process we have a stop (from software breakpoint).
-        CallAfterRunningThreadsStop (pid,
-                                     [=] (lldb::tid_t signaling_tid)
-                                     {
-                                         SetState (StateType::eStateStopped, true);
-                                     });
-        break;
+    SetCurrentThreadID(pid);
+    CallAfterRunningThreadsStop(pid,
+                                [=](lldb::tid_t signaling_tid)
+                                   SetState(StateType::eStateStopped, true);
+}
 
-    case SI_KERNEL:
-    case TRAP_BRKPT:
-        if (log)
-            log->Printf ("NativeProcessLinux::%s() received breakpoint event, pid = %" PRIu64, __FUNCTION__, pid);
+void
+NativeProcessLinux::MonitorBreakpoint(lldb::pid_t pid, NativeThreadProtocolSP thread_sp)
+{
+    Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS | LIBLLDB_LOG_BREAKPOINTS));
+        log->Printf("NativeProcessLinux::%s() received breakpoint event, pid = %" PRIu64,
+                __FUNCTION__, pid);
 
-        // This thread is currently stopped.
-        NotifyThreadStop (pid);
+    NotifyThreadStop(pid);
 
-        // Mark the thread as stopped at breakpoint.
-        if (thread_sp)
-        {
-            std::static_pointer_cast<NativeThreadLinux> (thread_sp)->SetStoppedByBreakpoint ();
-            Error error = FixupBreakpointPCAsNeeded (thread_sp);
-            if (error.Fail ())
-            {
-                if (log)
-                    log->Printf ("NativeProcessLinux::%s() pid = %" PRIu64 " fixup: %s", __FUNCTION__, pid, error.AsCString ());
-            }
-        }
-        else
-        {
-            if (log)
-                log->Printf ("NativeProcessLinux::%s()  pid = %" PRIu64 ": warning, cannot process software breakpoint since no thread metadata", __FUNCTION__, pid);
-        }
+        std::static_pointer_cast<NativeThreadLinux>(thread_sp)->SetStoppedByBreakpoint();
+        Error error = FixupBreakpointPCAsNeeded(thread_sp);
+        if (error.Fail())
+                log->Printf("NativeProcessLinux::%s() pid = %" PRIu64 " fixup: %s",
+                        __FUNCTION__, pid, error.AsCString());
+            log->Printf("NativeProcessLinux::%s()  pid = %" PRIu64 ": "
+                    "warning, cannot process software breakpoint since no thread metadata",
+                    __FUNCTION__, pid);
 
 
-        // We need to tell all other running threads before we notify the delegate about this stop.
-        CallAfterRunningThreadsStop (pid,
-                                     [=](lldb::tid_t deferred_notification_tid)
-                                     {
-                                         SetCurrentThreadID (deferred_notification_tid);
-                                         // Tell the process we have a stop (from software breakpoint).
-                                         SetState (StateType::eStateStopped, true);
-                                     });
-        break;
+    CallAfterRunningThreadsStop(pid,
+                                    SetCurrentThreadID(deferred_notification_tid);
+                                    SetState(StateType::eStateStopped, true);
+}
 
-    case TRAP_HWBKPT:
-        if (log)
-            log->Printf ("NativeProcessLinux::%s() received watchpoint event, pid = %" PRIu64, __FUNCTION__, pid);
+void
+NativeProcessLinux::MonitorWatchpoint(lldb::pid_t pid, NativeThreadProtocolSP thread_sp, uint32_t wp_index)
+{
+    Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS | LIBLLDB_LOG_WATCHPOINTS));
+        log->Printf("NativeProcessLinux::%s() received watchpoint event, "
+                    "pid = %" PRIu64 ", wp_index = %" PRIu32,
+                    __FUNCTION__, pid, wp_index);
 
-        // This thread is currently stopped.
-        NotifyThreadStop (pid);
+    NotifyThreadStop(pid);
 
-        // Mark the thread as stopped at watchpoint.
-        // The address is at (lldb::addr_t)info->si_addr if we need it.
-        if (thread_sp)
-            std::static_pointer_cast<NativeThreadLinux> (thread_sp)->SetStoppedByWatchpoint ();
-        else
-        {
-            if (log)
-                log->Printf ("NativeProcessLinux::%s() pid %" PRIu64 " tid %" PRIu64 ": warning, cannot process hardware breakpoint since no thread metadata", __FUNCTION__, GetID (), pid);
-        }
+    lldbassert(thread_sp && "thread_sp cannot be NULL");
+    std::static_pointer_cast<NativeThreadLinux>(thread_sp)->SetStoppedByWatchpoint(wp_index);
 
-        // We need to tell all other running threads before we notify the delegate about this stop.
-        CallAfterRunningThreadsStop (pid,
-                                     [=](lldb::tid_t deferred_notification_tid)
-                                     {
-                                         SetCurrentThreadID (deferred_notification_tid);
-                                         // Tell the process we have a stop (from hardware breakpoint).
-                                         SetState (StateType::eStateStopped, true);
-                                     });
-        break;
-
-    case SIGTRAP:
-    case (SIGTRAP | 0x80):
-        if (log)
-            log->Printf ("NativeProcessLinux::%s() received unknown SIGTRAP system call stop event, pid %" PRIu64 "tid %" PRIu64 ", resuming", __FUNCTION__, GetID (), pid);
-
-        // This thread is currently stopped.
-        NotifyThreadStop (pid);
-        if (thread_sp)
-            std::static_pointer_cast<NativeThreadLinux> (thread_sp)->SetStoppedBySignal (SIGTRAP);
-
-            
-        // Ignore these signals until we know more about them.
-        m_coordinator_up->RequestThreadResume (pid,
-                                               [=](lldb::tid_t tid_to_resume, bool supress_signal)
-                                               {
-                                                   std::static_pointer_cast<NativeThreadLinux> (thread_sp)->SetRunning ();
-                                                   return Resume (tid_to_resume, LLDB_INVALID_SIGNAL_NUMBER);
-                                               },
-                                               CoordinatorErrorHandler);
-        break;
-
-    default:
-        assert(false && "Unexpected SIGTRAP code!");
-        if (log)
-            log->Printf ("NativeProcessLinux::%s() pid %" PRIu64 "tid %" PRIu64 " received unhandled SIGTRAP code: 0x%" PRIx64, __FUNCTION__, GetID (), pid, static_cast<uint64_t> (SIGTRAP | (PTRACE_EVENT_CLONE << 8)));
-        break;
-        
-    }
+    CallAfterRunningThreadsStop(pid,
+                                    SetCurrentThreadID(deferred_notification_tid);
+                                    // Tell the process we have a stop (from watchpoint).
+                                    SetState(StateType::eStateStopped, true);
 }
 
 void
 NativeProcessLinux::MonitorSignal(const siginfo_t *info, lldb::pid_t pid, bool exited)
 {
     assert (info && "null info");
     if (!info)
         return;

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

public:
 
         Error
         ReadAllRegisterValues (lldb::DataBufferSP &data_sp) override;
 
         Error
         WriteAllRegisterValues (const lldb::DataBufferSP &data_sp) override;
 
         Error
-        IsWatchpointHit (uint8_t wp_index) override;
+        IsWatchpointHit (uint32_t wp_index, bool &is_hit) override;
 
         Error
-        IsWatchpointVacant (uint32_t wp_index) override;
+        IsWatchpointVacant (uint32_t wp_index, bool &is_vacant) override;
 
         bool
         ClearHardwareWatchpoint (uint32_t wp_index) override;
 
         Error
         ClearAllHardwareWatchpoints () override;
 
         Error

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

 NativeRegisterContextLinux_mips64::WriteAllRegisterValues (const lldb::DataBufferSP &data_sp)
 {
     Error error;
     error.SetErrorString ("MIPS TODO: NativeRegisterContextLinux_mips64::WriteAllRegisterValues not implemented");
     return error;
 }
 
 Error
-NativeRegisterContextLinux_mips64::IsWatchpointHit (uint8_t wp_index)
+NativeRegisterContextLinux_mips64::IsWatchpointHit (uint32_t wp_index, bool &is_hit)
 {
-    Error error;
-    error.SetErrorString ("MIPS TODO: NativeRegisterContextLinux_mips64::IsWatchpointHit not implemented");
-    return error;
+    is_hit = false;
+    return Error("MIPS TODO: NativeRegisterContextLinux_mips64::IsWatchpointHit not implemented");
 }
 
 Error
-NativeRegisterContextLinux_mips64::IsWatchpointVacant (uint32_t wp_index)
+NativeRegisterContextLinux_mips64::IsWatchpointVacant (uint32_t wp_index, bool &is_vacant)
 {
-    Error error;
-    error.SetErrorString ("MIPS TODO: NativeRegisterContextLinux_mips64::IsWatchpointVacant not implemented");
-    return error;
+    is_vacant = false;
+    return Error("MIPS TODO: NativeRegisterContextLinux_mips64::IsWatchpointVacant not implemented");
 }
 
 bool
 NativeRegisterContextLinux_mips64::ClearHardwareWatchpoint (uint32_t wp_index)
 {
     return false;
 }
 

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

public:
 
         Error
         ReadAllRegisterValues (lldb::DataBufferSP &data_sp) override;
 
         Error
         WriteAllRegisterValues (const lldb::DataBufferSP &data_sp) override;
 
         Error
-        IsWatchpointHit(uint8_t wp_index) override;
+        IsWatchpointHit(uint32_t wp_index, bool &is_hit) override;
 
         Error
-        IsWatchpointVacant(uint32_t wp_index) override;
+        GetWatchpointHitIndex(uint32_t &wp_index) override;
+
+        Error
+        IsWatchpointVacant(uint32_t wp_index, bool &is_vacant) override;
 
         bool
         ClearHardwareWatchpoint(uint32_t wp_index) override;
 
         Error
         ClearAllHardwareWatchpoints () override;
 
         Error

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

 //===-- NativeRegisterContextLinux_x86_64.cpp ---------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "NativeRegisterContextLinux_x86_64.h"
 
+#include "lldb/Core/Log.h"
 #include "lldb/lldb-private-forward.h"
 #include "lldb/Core/DataBufferHeap.h"
 #include "lldb/Core/Error.h"
 #include "lldb/Core/RegisterValue.h"
 #include "lldb/Host/common/NativeProcessProtocol.h"
 #include "lldb/Host/common/NativeThreadProtocol.h"
 #include "Plugins/Process/Linux/NativeProcessLinux.h"
 
NativeRegisterContextLinux_x86_64::WriteGPR()
     if (!process_sp)
         return false;
     NativeProcessLinux *const process_p = reinterpret_cast<NativeProcessLinux*> (process_sp.get ());
 
     return process_p->WriteGPR (m_thread.GetID (), &m_gpr_x86_64, GetRegisterInfoInterface ().GetGPRSize ()).Success();
 }
 
 Error
-NativeRegisterContextLinux_x86_64::IsWatchpointHit(uint8_t wp_index)
+NativeRegisterContextLinux_x86_64::IsWatchpointHit(uint32_t wp_index, bool &is_hit)
 {
     if (wp_index >= NumSupportedHardwareWatchpoints())
-        return Error ("Watchpoint index out of range");
+        return Error("Watchpoint index out of range");
 
     RegisterValue reg_value;
     Error error = ReadRegisterRaw(m_reg_info.first_dr + 6, reg_value);
-    if (error.Fail()) return error;
+    if (error.Fail())
+    {
+        is_hit = false;
+        return error;
+    }
 
     uint64_t status_bits = reg_value.GetAsUInt64();
 
-    bool is_hit = status_bits & (1 << wp_index);
+    is_hit = status_bits & (1 << wp_index);
 
-    error.SetError (!is_hit, lldb::eErrorTypeInvalid);
+    return error;
+}
 
+Error
+NativeRegisterContextLinux_x86_64::GetWatchpointHitIndex(uint32_t &wp_index) {
+    uint32_t num_hw_wps = NumSupportedHardwareWatchpoints();
+    for (wp_index = 0; wp_index < num_hw_wps; ++wp_index)
+    {
+        bool is_hit;
+        Error error = IsWatchpointHit(wp_index, is_hit);
+        if (error.Fail()) {
+            wp_index = LLDB_INVALID_INDEX32;
             return error;
+        } else if (is_hit) {
+            return error;
+        }
+    }
+    wp_index = LLDB_INVALID_INDEX32;
+    return Error();
 }
 
 Error
-NativeRegisterContextLinux_x86_64::IsWatchpointVacant(uint32_t wp_index)
+NativeRegisterContextLinux_x86_64::IsWatchpointVacant(uint32_t wp_index, bool &is_vacant)
 {
     if (wp_index >= NumSupportedHardwareWatchpoints())
         return Error ("Watchpoint index out of range");
 
     RegisterValue reg_value;
     Error error = ReadRegisterRaw(m_reg_info.first_dr + 7, reg_value);
-    if (error.Fail()) return error;
+    if (error.Fail())
+    {
+        is_vacant = false;
+        return error;
+    }
 
     uint64_t control_bits = reg_value.GetAsUInt64();
 
-    bool is_vacant = !(control_bits & (1 << (2 * wp_index)));
-
-    error.SetError (!is_vacant, lldb::eErrorTypeInvalid);
+    is_vacant = !(control_bits & (1 << (2 * wp_index)));
 
     return error;
 }
 
 Error
 NativeRegisterContextLinux_x86_64::SetHardwareWatchpointWithIndex(
         lldb::addr_t addr, size_t size, uint32_t watch_flags, uint32_t wp_index) {
 
     if (wp_index >= NumSupportedHardwareWatchpoints())
         return Error ("Watchpoint index out of range");
 
     if (watch_flags != 0x1 && watch_flags != 0x3)
         return Error ("Invalid read/write bits for watchpoint");
 
     if (size != 1 && size != 2 && size != 4 && size != 8)
         return Error ("Invalid size for watchpoint");
 
-    Error error = IsWatchpointVacant (wp_index);
+    bool is_vacant;
+    Error error = IsWatchpointVacant (wp_index, is_vacant);
     if (error.Fail()) return error;
+    if (!is_vacant) return Error("Watchpoint index not vacant");
 
     RegisterValue reg_value;
     error = ReadRegisterRaw(m_reg_info.first_dr + 7, reg_value);
     if (error.Fail()) return error;
 
     // for watchpoints 0, 1, 2, or 3, respectively,
     // set bits 1, 3, 5, or 7
     uint64_t enable_bit = 1 << (2 * wp_index);
NativeRegisterContextLinux_x86_64::ClearAllHardwareWatchpoints()
     uint64_t control_bits = reg_value.GetAsUInt64() & ~bit_mask;
     return WriteRegister(m_reg_info.first_dr + 7, RegisterValue(control_bits));
 }
 
 uint32_t
 NativeRegisterContextLinux_x86_64::SetHardwareWatchpoint(
         lldb::addr_t addr, size_t size, uint32_t watch_flags)
 {
+    Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_WATCHPOINTS));
     const uint32_t num_hw_watchpoints = NumSupportedHardwareWatchpoints();
     for (uint32_t wp_index = 0; wp_index < num_hw_watchpoints; ++wp_index)
-        if (IsWatchpointVacant(wp_index).Success())
-        {
-            if (SetHardwareWatchpointWithIndex(addr, size, watch_flags, wp_index).Fail())
-                continue;
+        bool is_vacant;
+        Error error = IsWatchpointVacant(wp_index, is_vacant);
+        if (is_vacant)
+        {
+            error = SetHardwareWatchpointWithIndex(addr, size, watch_flags, wp_index);
+            if (error.Success())
                 return wp_index;
         }
+        if (error.Fail() && log)
+        {
+            log->Printf("NativeRegisterContextLinux_x86_64::%s Error: %s",
+                    __FUNCTION__, error.AsCString());
+        }
+    }
     return LLDB_INVALID_INDEX32;
 }
 
 lldb::addr_t
 NativeRegisterContextLinux_x86_64::GetWatchpointAddress(uint32_t wp_index)
 {
     if (wp_index >= NumSupportedHardwareWatchpoints())
         return LLDB_INVALID_ADDRESS;

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

private:
 
         void
         SetStoppedByExec ();
 
         void
         SetStoppedByBreakpoint ();
 
         void
-        SetStoppedByWatchpoint ();
+        SetStoppedByWatchpoint (uint32_t wp_index);
 
         bool
         IsStoppedAtBreakpoint ();
 
         bool
         IsStoppedAtWatchpoint ();
 
         void

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

 #include "NativeRegisterContextLinux_x86_64.h"
 #include "NativeRegisterContextLinux_mips64.h"
 
 #include "lldb/Core/Log.h"
 #include "lldb/Core/State.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Host/HostNativeThread.h"
+#include "lldb/Utility/LLDBAssert.h"
 #include "lldb/lldb-enumerations.h"
 
 #include "llvm/ADT/SmallString.h"
 
 #include "Plugins/Process/POSIX/CrashReason.h"
 
 #include "Plugins/Process/Utility/RegisterContextLinux_arm64.h"
 #include "Plugins/Process/Utility/RegisterContextLinux_i386.h"
NativeThreadLinux::SetStoppedByBreakpoint ()
     m_state = new_state;
 
     m_stop_info.reason = StopReason::eStopReasonBreakpoint;
     m_stop_info.details.signal.signo = SIGTRAP;
     m_stop_description.clear();
 }
 
 void
-NativeThreadLinux::SetStoppedByWatchpoint ()
+NativeThreadLinux::SetStoppedByWatchpoint (uint32_t wp_index)
 {
-    Log *log (GetLogIfAllCategoriesSet (LIBLLDB_LOG_THREAD));
-    lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
-    if (log)
-    {
-        NativeProcessProtocolSP process_sp = m_process_wp.lock ();
-        if (process_sp)
-            pid = process_sp->GetID ();
-    }
-
     const StateType new_state = StateType::eStateStopped;
     MaybeLogStateChange (new_state);
     m_state = new_state;
-
-    NativeRegisterContextSP reg_ctx = GetRegisterContext ();
-    const uint32_t num_hw_watchpoints = reg_ctx->NumSupportedHardwareWatchpoints ();
-
     m_stop_description.clear ();
-    for (uint32_t wp_index = 0; wp_index < num_hw_watchpoints; ++wp_index)
-    {
-        if (reg_ctx->IsWatchpointHit (wp_index).Success())
-        {
-            if (log)
-                log->Printf ("NativeThreadLinux:%s (pid=%" PRIu64 ", tid=%" PRIu64 ") watchpoint found with idx: %u",
-                             __FUNCTION__, pid, GetID (), wp_index);
+
+    lldbassert(wp_index != LLDB_INVALID_INDEX32 &&
+               "wp_index cannot be invalid");
 
-            std::ostringstream ostr;
-            ostr << reg_ctx->GetWatchpointAddress (wp_index) << " " << wp_index;
-            m_stop_description = ostr.str();
+    ostr << GetRegisterContext()->GetWatchpointAddress(wp_index) << " ";
+    ostr << wp_index;
 
-            m_stop_info.reason = StopReason::eStopReasonWatchpoint;
-            m_stop_info.details.signal.signo = SIGTRAP;
-            return;
-        }
-    }
-
-    // The process reported a watchpoint was hit, but we haven't found the
-    // watchpoint. Assume that a stopped by trace is reported as a hardware
-    // watchpoint what happens on some linux kernels (e.g.: android-arm64
-    // platfrom-21).
-
-    if (log)
-        log->Printf ("NativeThreadLinux:%s (pid=%" PRIu64 ", tid=%" PRIu64 ") none of the watchpoint was hit.",
-                     __FUNCTION__, pid, GetID ());
-
-    SetStoppedByTrace ();
 }
 
 bool
 NativeThreadLinux::IsStoppedAtBreakpoint ()
 {
     return GetState () == StateType::eStateStopped &&
         m_stop_info.reason == StopReason::eStopReasonBreakpoint;
 }

lldb/trunk/test/functionalities/watchpoint/step_over_watchpoint/Makefile

+LEVEL = ../../../make
+
+C_SOURCES := main.c
+
+include $(LEVEL)/Makefile.rules

lldb/trunk/test/functionalities/watchpoint/step_over_watchpoint/TestStepOverWatchpoint.py

+"""Test stepping over watchpoints."""
+
+import unittest2
+import lldb
+import lldbutil
+from lldbtest import *
+
+
+class TestStepOverWatchpoint(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    def getCategories(self):
+        return ['basic_process']
+
+    @unittest2.skipUnless(sys.platform.startswith("darwin"), "requires Darwin")
+    @dsym_test
+    def test_with_dsym(self):
+        """Test stepping over watchpoints."""
+        self.buildDsym()
+        self.step_over_watchpoint()
+
+    @dwarf_test
+    def test_with_dwarf(self):
+        """Test stepping over watchpoints."""
+        self.buildDwarf()
+        self.step_over_watchpoint()
+
+    def setUp(self):
+        TestBase.setUp(self)
+
+    def step_inst_for_watchpoint(self, wp_id):
+        watchpoint_hit = False
+        current_line = self.frame().GetLineEntry().GetLine()
+        while self.frame().GetLineEntry().GetLine() == current_line:
+            self.thread().StepInstruction(False)  # step_over=False
+            stop_reason = self.thread().GetStopReason()
+            if stop_reason == lldb.eStopReasonWatchpoint:
+                self.assertFalse(watchpoint_hit, "Watchpoint already hit.")
+                expected_stop_desc = "watchpoint %d" % wp_id
+                actual_stop_desc = self.thread().GetStopDescription(20)
+                self.assertTrue(actual_stop_desc == expected_stop_desc,
+                                "Watchpoint ID didn't match.")
+                watchpoint_hit = True
+            else:
+                self.assertTrue(stop_reason == lldb.eStopReasonPlanComplete,
+                                STOPPED_DUE_TO_STEP_IN)
+        self.assertTrue(watchpoint_hit, "Watchpoint never hit.")
+
+    def step_over_watchpoint(self):
+        """Test stepping over watchpoints."""
+        exe = os.path.join(os.getcwd(), 'a.out')
+
+        target = self.dbg.CreateTarget(exe)
+        self.assertTrue(self.target, VALID_TARGET)
+
+        lldbutil.run_break_set_by_symbol(self, 'main')
+
+        process = target.LaunchSimple(None, None,
+                                      self.get_process_working_directory())
+        self.assertTrue(process.IsValid(), PROCESS_IS_VALID)
+        self.assertTrue(process.GetState() == lldb.eStateStopped,
+                        PROCESS_STOPPED)
+
+        thread = lldbutil.get_stopped_thread(process,
+                                             lldb.eStopReasonBreakpoint)
+        self.assertTrue(thread.IsValid(), "Failed to get thread.")
+
+        frame = thread.GetFrameAtIndex(0)
+        self.assertTrue(frame.IsValid(), "Failed to get frame.")
+
+        read_value = frame.FindValue('g_watch_me_read',
+                                     lldb.eValueTypeVariableGlobal)
+        self.assertTrue(read_value.IsValid(), "Failed to find read value.")
+
+        error = lldb.SBError()
+
+        # resolve_location=True, read=True, write=False
+        read_watchpoint = read_value.Watch(True, True, False, error)
+        self.assertTrue(error.Success(),
+                        "Error while setting watchpoint: %s" %
+                        error.GetCString())
+        self.assertTrue(read_watchpoint, "Failed to set read watchpoint.")
+
+        write_value = frame.FindValue('g_watch_me_write',
+                                      lldb.eValueTypeVariableGlobal)
+        self.assertTrue(write_value, "Failed to find write value.")
+
+        # resolve_location=True, read=False, write=True
+        write_watchpoint = write_value.Watch(True, False, True, error)
+        self.assertTrue(read_watchpoint, "Failed to set write watchpoint.")
+        self.assertTrue(error.Success(),
+                        "Error while setting watchpoint: %s" %
+                        error.GetCString())
+
+        thread.StepOver()
+        self.assertTrue(thread.GetStopReason() == lldb.eStopReasonWatchpoint,
+                        STOPPED_DUE_TO_WATCHPOINT)
+        self.assertTrue(thread.GetStopDescription(20) == 'watchpoint 1')
+
+        process.Continue()
+        self.assertTrue(process.GetState() == lldb.eStateStopped,
+                        PROCESS_STOPPED)
+        self.assertTrue(thread.GetStopDescription(20) == 'step over')
+
+        self.step_inst_for_watchpoint(1)
+
+        thread.StepOver()
+        self.assertTrue(thread.GetStopReason() == lldb.eStopReasonWatchpoint,
+                        STOPPED_DUE_TO_WATCHPOINT)
+        self.assertTrue(thread.GetStopDescription(20) == 'watchpoint 2')
+
+        process.Continue()
+        self.assertTrue(process.GetState() == lldb.eStateStopped,
+                        PROCESS_STOPPED)
+        self.assertTrue(thread.GetStopDescription(20) == 'step over')
+
+        self.step_inst_for_watchpoint(2)
+
+if __name__ == '__main__':
+    import atexit
+    lldb.SBDebugger.Initialize()
+    atexit.register(lambda: lldb.SBDebugger.Terminate())
+    unittest2.main()

lldb/trunk/test/functionalities/watchpoint/step_over_watchpoint/main.c

+char g_watch_me_read;
+char g_watch_me_write;
+char g_temp;
+
+void watch_read() {
+    g_temp = g_watch_me_read;
+}
+
+void watch_write() {
+    g_watch_me_write = g_temp;
+}
+
+int main() {
+    watch_read();
+    g_temp = g_watch_me_read;
+    watch_write();
+    g_watch_me_write = g_temp;
+    return 0;
+}