Reduce code duplication in ProcessGDBRemote

[labath]

Summary

SetThreadStopInfo was checking for a breakpoint at the current PC several times. This merges the
identical code into a separate function. I've left one breakpoint check alone, as it was doing
more complicated stuff, and it did not see a way to merge that without making the interface
complicated. NFC.

[chying]

TestRegisters.py and TestDebugBreak.py are broken after this change. Could you take a look?
http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/13090

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

private:
     // For ProcessGDBRemote only
     //------------------------------------------------------------------
     static bool
     NewThreadNotifyBreakpointHit (void *baton,
                          StoppointCallbackContext *context,
                          lldb::user_id_t break_id,
                          lldb::user_id_t break_loc_id);
 
+    bool
+    SetThreadStopReasonIfAtBreakpoint(Thread &thread);
+
     DISALLOW_COPY_AND_ASSIGN (ProcessGDBRemote);
 };
 
 } // namespace process_gdb_remote
 } // namespace lldb_private
 
 #endif // liblldb_ProcessGDBRemote_h_

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

if (tid != LLDB_INVALID_THREAD_ID)
                 else
                 {
                     bool handled = false;
                     bool did_exec = false;
                     if (!reason.empty())
                     {
                         if (reason.compare("trace") == 0)
                         {
-                            addr_t pc = thread_sp->GetRegisterContext()->GetPC();
-                            lldb::BreakpointSiteSP bp_site_sp = thread_sp->GetProcess()->GetBreakpointSiteList().FindByAddress(pc);
-
-                            // If the current pc is a breakpoint site then the StopInfo should be set to Breakpoint
-                            // Otherwise, it will be set to Trace.
-                            if (bp_site_sp && bp_site_sp->ValidForThisThread(thread_sp.get()))
-                            {
-                                thread_sp->SetStopInfo(
-                                    StopInfo::CreateStopReasonWithBreakpointSiteID(*thread_sp, bp_site_sp->GetID()));
-                            }
-                            else
-                              thread_sp->SetStopInfo (StopInfo::CreateStopReasonToTrace (*thread_sp));
+                            if (!SetThreadStopReasonIfAtBreakpoint(*thread_sp))
+                                thread_sp->SetStopInfo(StopInfo::CreateStopReasonToTrace(*thread_sp));
                             handled = true;
                         }
                         else if (reason.compare("breakpoint") == 0)
                         {
-                            addr_t pc = thread_sp->GetRegisterContext()->GetPC();
-                            lldb::BreakpointSiteSP bp_site_sp = thread_sp->GetProcess()->GetBreakpointSiteList().FindByAddress(pc);
-                            if (bp_site_sp)
-                            {
-                                // If the breakpoint is for this thread, then we'll report the hit, but if it is for another thread,
-                                // we can just report no reason.  We don't need to worry about stepping over the breakpoint here, that
-                                // will be taken care of when the thread resumes and notices that there's a breakpoint under the pc.
-                                handled = true;
-                                if (bp_site_sp->ValidForThisThread (thread_sp.get()))
-                                {
-                                    thread_sp->SetStopInfo (StopInfo::CreateStopReasonWithBreakpointSiteID (*thread_sp, bp_site_sp->GetID()));
-                                }
-                                else
-                                {
-                                    StopInfoSP invalid_stop_info_sp;
-                                    thread_sp->SetStopInfo (invalid_stop_info_sp);
-                                }
-                            }
+                            if (!SetThreadStopReasonIfAtBreakpoint(*thread_sp))
+                                thread_sp->SetStopInfo(StopInfoSP());
                         }
                         else if (reason.compare("trap") == 0)
                         {
                             // Let the trap just use the standard signal stop reason below...
                         }
                         else if (reason.compare("watchpoint") == 0)
                         {
                             StringExtractor desc_extractor(description.c_str());
if (tid != LLDB_INVALID_THREAD_ID)
                         {
                             did_exec = true;
                             thread_sp->SetStopInfo (StopInfo::CreateStopReasonWithExec(*thread_sp));
                             handled = true;
                         }
                     }
                     else if (!signo)
                     {
-                        addr_t pc = thread_sp->GetRegisterContext()->GetPC();
-                        lldb::BreakpointSiteSP bp_site_sp =
-                            thread_sp->GetProcess()->GetBreakpointSiteList().FindByAddress(pc);
-
                         // If the current pc is a breakpoint site then the StopInfo should be set to Breakpoint
                         // even though the remote stub did not set it as such. This can happen when
                         // the thread is involuntarily interrupted (e.g. due to stops on other
                         // threads) just as it is about to execute the breakpoint instruction.
-                        if (bp_site_sp && bp_site_sp->ValidForThisThread(thread_sp.get()))
-                        {
-                            thread_sp->SetStopInfo(
-                                StopInfo::CreateStopReasonWithBreakpointSiteID(*thread_sp, bp_site_sp->GetID()));
+                        if (SetThreadStopReasonIfAtBreakpoint(*thread_sp))
                             handled = true;
-                        }
-                    }
 
                     if (!handled && signo && did_exec == false)
                     {
                         if (signo == SIGTRAP)
                         {
                             // Currently we are going to assume SIGTRAP means we are either
                             // hitting a breakpoint or hardware single stepping.
                             handled = true;
ProcessGDBRemote::ModulesDidLoad (ModuleList &module_list)
     // We must call the lldb_private::Process::ModulesDidLoad () first before we do anything
     Process::ModulesDidLoad (module_list);
 
     // After loading shared libraries, we can ask our remote GDB server if
     // it needs any symbols.
     m_gdb_comm.ServeSymbolLookups(this);
 }
 
+bool
+ProcessGDBRemote::SetThreadStopReasonIfAtBreakpoint(Thread &thread)
+{
+    const addr_t pc = thread.GetRegisterContext()->GetPC();
+    lldb::BreakpointSiteSP bp_site_sp = thread.GetProcess()->GetBreakpointSiteList().FindByAddress(pc);
+    if (!bp_site_sp || !bp_site_sp->ValidForThisThread(&thread))
+        return false;
+
+    thread.SetStopInfo(StopInfo::CreateStopReasonWithBreakpointSiteID(thread, bp_site_sp->GetID()));
+    return true;
+}
 
 class CommandObjectProcessGDBRemoteSpeedTest: public CommandObjectParsed
 {
 public:
     CommandObjectProcessGDBRemoteSpeedTest(CommandInterpreter &interpreter) :
         CommandObjectParsed (interpreter,
                              "process plugin packet speed-test",
                              "Tests packet speeds of various sizes to determine the performance characteristics of the GDB remote connection. ",