<rdar://problem/11782789> Changes to the watchpoint implementation on ARM so that we single-step before stopping at the WP. This is necessary because on ARM the WP triggers before the opcode is actually executed, so we would be unable to continue since we would keep hitting the WP. We work around this by disabling the WP, single stepping and then putting the WP back in place.

llvm-svn: 160199

9 files changed, 105 insertions, 5 deletions

diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index 5ca697e1c7e..c876d75248d 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -2786,6 +2786,14 @@ public:
         return error;
     }
 
+    virtual Error
+    GetWatchpointSupportInfo (uint32_t &num, bool& after)
+    {
+        Error error;
+        error.SetErrorString ("Process::GetWatchpointSupportInfo() not supported");
+        return error;
+    }
+    
     lldb::ModuleSP
     ReadModuleFromMemory (const FileSpec& file_spec, 
                           lldb::addr_t header_addr,
diff --git a/lldb/include/lldb/Target/StopInfo.h b/lldb/include/lldb/Target/StopInfo.h
index 3ed6d804c24..a4933af6312 100644
--- a/lldb/include/lldb/Target/StopInfo.h
+++ b/lldb/include/lldb/Target/StopInfo.h
@@ -161,14 +161,15 @@ protected:
     // N.B. running to evaluate a user expression does not count. 
     bool HasTargetRunSinceMe ();
 
-private:
-    friend class Thread;
-    
-    // MakeStopInfoValid is necessary to allow saved stop infos to resurrect themselves as valid.  It should
-    // only need to be called by Thread::RestoreThreadStateFromCheckpoint.
+    // MakeStopInfoValid is necessary to allow saved stop infos to resurrect themselves as valid.
+    // It should only be used by Thread::RestoreThreadStateFromCheckpoint and to make sure the one-step
+    // needed for before-the-fact watchpoints does not prevent us from stopping
     void
     MakeStopInfoValid ();
     
+private:
+    friend class Thread;
+    
     DISALLOW_COPY_AND_ASSIGN (StopInfo);
 };
 
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 83d2843309a..4fac83d334c 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -49,6 +49,7 @@ GDBRemoteCommunicationClient::GDBRemoteCommunicationClient(bool is_platform) :
     m_supports_alloc_dealloc_memory (eLazyBoolCalculate),
     m_supports_memory_region_info  (eLazyBoolCalculate),
     m_supports_watchpoint_support_info  (eLazyBoolCalculate),
+    m_watchpoints_trigger_after_instruction(eLazyBoolCalculate),
     m_supports_qProcessInfoPID (true),
     m_supports_qfProcessInfo (true),
     m_supports_qUserName (true),
@@ -1019,6 +1020,17 @@ GDBRemoteCommunicationClient::GetHostInfo (bool force)
                         if (m_os_version_major != UINT32_MAX)
                             ++num_keys_decoded;
                     }
+                    else if (name.compare("watchpoint_exceptions_received") == 0)
+                    {
+                        ++num_keys_decoded;
+                        if (strcmp(value.c_str(),"before") == 0)
+                            m_watchpoints_trigger_after_instruction = eLazyBoolNo;
+                        else if (strcmp(value.c_str(),"after") == 0)
+                            m_watchpoints_trigger_after_instruction = eLazyBoolYes;
+                        else
+                            --num_keys_decoded;
+                    }
+
                 }
                 
                 if (num_keys_decoded > 0)
@@ -1352,6 +1364,30 @@ GDBRemoteCommunicationClient::GetWatchpointSupportInfo (uint32_t &num)
 
 }
 
+lldb_private::Error
+GDBRemoteCommunicationClient::GetWatchpointSupportInfo (uint32_t &num, bool& after)
+{
+    Error error(GetWatchpointSupportInfo(num));
+    if (error.Success())
+        error = GetWatchpointsTriggerAfterInstruction(after);
+    return error;
+}
+
+lldb_private::Error
+GDBRemoteCommunicationClient::GetWatchpointsTriggerAfterInstruction (bool &after)
+{
+    Error error;
+    
+    // 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)
+        after = true;
+    else
+        after = (m_watchpoints_trigger_after_instruction != eLazyBoolNo);
+    return error;
+}
+
 int
 GDBRemoteCommunicationClient::SetSTDIN (char const *path)
 {
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
index fb578001dc3..a283389d220 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -209,6 +209,12 @@ public:
     lldb_private::Error
     GetWatchpointSupportInfo (uint32_t &num); 
 
+    lldb_private::Error
+    GetWatchpointSupportInfo (uint32_t &num, bool& after);
+    
+    lldb_private::Error
+    GetWatchpointsTriggerAfterInstruction (bool &after);
+
     const lldb_private::ArchSpec &
     GetHostArchitecture ();
     
@@ -358,6 +364,7 @@ protected:
     lldb_private::LazyBool m_supports_alloc_dealloc_memory;
     lldb_private::LazyBool m_supports_memory_region_info;
     lldb_private::LazyBool m_supports_watchpoint_support_info;
+    lldb_private::LazyBool m_watchpoints_trigger_after_instruction;
 
     bool
         m_supports_qProcessInfoPID:1,
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
index 3f8677d1c7e..70d73d3c38a 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
@@ -223,6 +223,11 @@ GDBRemoteCommunicationServer::Handle_qHostInfo (StringExtractorGDBRemote &packet
     if (sub != LLDB_INVALID_CPUTYPE)
         response.Printf ("cpusubtype:%u;", sub);
     
+    if (cpu == CPU_TYPE_ARM)
+        response.Printf("watchpoint_exceptions_received:before;");   // On armv7 we use "synchronous" watchpoints which means the exception is delivered before the instruction executes.
+    else
+        response.Printf("watchpoint_exceptions_received:after;");
+    
     switch (lldb::endian::InlHostByteOrder())
     {
     case eByteOrderBig:     response.PutCString ("endian:big;"); break;
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index e0d22360156..06d19e40033 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -2015,6 +2015,13 @@ ProcessGDBRemote::GetWatchpointSupportInfo (uint32_t &num)
 }
 
 Error
+ProcessGDBRemote::GetWatchpointSupportInfo (uint32_t &num, bool& after)
+{
+    Error error (m_gdb_comm.GetWatchpointSupportInfo (num, after));
+    return error;
+}
+
+Error
 ProcessGDBRemote::DoDeallocateMemory (lldb::addr_t addr)
 {
     Error error; 
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
index 04d6ff2e7a8..563b89963fa 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
@@ -207,6 +207,9 @@ public:
     virtual lldb_private::Error
     GetWatchpointSupportInfo (uint32_t &num);
     
+    virtual lldb_private::Error
+    GetWatchpointSupportInfo (uint32_t &num, bool& after);
+    
     virtual bool
     StartNoticingNewThreads();    
 
diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 113aa57aacb..5b782beb4bd 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -461,6 +461,36 @@ public:
         if (wp_sp)
         {
             ExecutionContext exe_ctx (m_thread.GetStackFrameAtIndex(0));
+            {
+                // check if this process is running on an architecture where watchpoints trigger
+				// before the associated instruction runs. if so, disable the WP, single-step and then
+				// re-enable the watchpoint
+                Process* process = exe_ctx.GetProcessPtr();
+                if (process)
+                {
+                    uint32_t num; bool wp_triggers_after;
+                    if (process->GetWatchpointSupportInfo(num, wp_triggers_after).Success())
+                    {
+                        if (!wp_triggers_after)
+                        {
+                            process->DisableWatchpoint(wp_sp.get());
+                            
+                            ThreadPlan *new_plan = m_thread.QueueThreadPlanForStepSingleInstruction(false, // step-over
+                                                                                                    false, // abort_other_plans
+                                                                                                    true); // stop_other_threads
+                            new_plan->SetIsMasterPlan (true);
+                            new_plan->SetOkayToDiscard (false);
+                            process->GetThreadList().SetSelectedThreadByID (m_thread.GetID());
+                            process->Resume ();
+                            process->WaitForProcessToStop (NULL);
+                            process->GetThreadList().SetSelectedThreadByID (m_thread.GetID());
+                            MakeStopInfoValid(); // make sure we do not fail to stop because of the single-step taken above
+                            
+                            process->EnableWatchpoint(wp_sp.get());
+                        }
+                    }
+                }
+            }
             StoppointCallbackContext context (event_ptr, exe_ctx, false);
             bool stop_requested = wp_sp->InvokeCallback (&context);
             // Also make sure that the callback hasn't continued the target.  
diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp
index d1c66150d58..c66e6f6c93f 100644
--- a/lldb/tools/debugserver/source/RNBRemote.cpp
+++ b/lldb/tools/debugserver/source/RNBRemote.cpp
@@ -3587,10 +3587,13 @@ RNBRemote::HandlePacket_qHostInfo (const char *p)
     if (cputype == CPU_TYPE_ARM)
     {
         strm << "ostype:ios;";
+        // On armv7 we use "synchronous" watchpoints which means the exception is delivered before the instruction executes.
+        strm << "watchpoint_exceptions_received:before;";
     }
     else
     {
         strm << "ostype:macosx;";
+        strm << "watchpoint_exceptions_received:after;";
     }
 //    char ostype[64];
 //    len = sizeof(ostype);