[lldb] [gdb-remote client] Remove breakpoints in forked processes

[mgorny]

Summary

Remove software breakpoints from forked processes in order to restore
the original program code before detaching it.

[mgorny]

(TODO: vfork support)

[mgorny]

Now uses a helper function to switch breakpoints. Might not seem necessary here but the next patch makes heavy use of it.

[mgorny]

Rebase.

[mgorny]

Fix regression in tests.

[mgorny]

Rebased.

[mgorny]

Rebased

[krytarowski]

Looks correct.

[augusto2112]

Hi. It looks like the fork-follow-parent-softbp test is failing on CI.

[mgorny]

In D100263#2975272, @augusto2112 wrote:

Hi. It looks like the fork-follow-parent-softbp test is failing on CI.

Thanks for the ping. Shouldn't the bot have mailed me? I'm wondering if there's a problem on my end.

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

struct ModuleCacheInfo {
     }
   };
 
   llvm::DenseMap<ModuleCacheKey, ModuleSpec, ModuleCacheInfo>
       m_cached_module_specs;
 
   ProcessGDBRemote(const ProcessGDBRemote &) = delete;
   const ProcessGDBRemote &operator=(const ProcessGDBRemote &) = delete;
+
+  // fork helpers
+  void DidForkSwitchSoftwareBreakpoints(bool enable);
 };
 
 } // namespace process_gdb_remote
 } // namespace lldb_private
 
 #endif // LLDB_SOURCE_PLUGINS_PROCESS_GDB_REMOTE_PROCESSGDBREMOTE_H

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

 
 CommandObject *ProcessGDBRemote::GetPluginCommandObject() {
   if (!m_command_sp)
     m_command_sp = std::make_shared<CommandObjectMultiwordProcessGDBRemote>(
         GetTarget().GetDebugger().GetCommandInterpreter());
   return m_command_sp.get();
 }
 
+void ProcessGDBRemote::DidForkSwitchSoftwareBreakpoints(bool enable) {
+  GetBreakpointSiteList().ForEach([this, enable](BreakpointSite *bp_site) {
+    if (bp_site->IsEnabled() &&
+        (bp_site->GetType() == BreakpointSite::eSoftware ||
+         bp_site->GetType() == BreakpointSite::eExternal)) {
+      m_gdb_comm.SendGDBStoppointTypePacket(
+          eBreakpointSoftware, enable, bp_site->GetLoadAddress(),
+          bp_site->GetTrapOpcodeMaxByteSize());
+    }
+  });
+}
+
 void ProcessGDBRemote::DidFork(lldb::pid_t child_pid, lldb::tid_t child_tid) {
   Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
 
+  lldb::pid_t parent_pid = m_gdb_comm.GetCurrentProcessID();
+  // Any valid TID will suffice, thread-relevant actions will set a proper TID
+  // anyway.
+  lldb::tid_t parent_tid = m_thread_ids.front();
+
+  if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointSoftware)) {
+    // Switch to the new process to clear breakpoints there.
+    if (!m_gdb_comm.SetCurrentThread(child_tid, child_pid)) {
+      LLDB_LOG(log, "ProcessGDBRemote::DidFork() unable to set pid/tid");
+      return;
+    }
+
+    // Disable all software breakpoints in the forked process.
+    DidForkSwitchSoftwareBreakpoints(false);
+
+    // Reset gdb-remote to the original process.
+    if (!m_gdb_comm.SetCurrentThread(parent_tid, parent_pid)) {
+      LLDB_LOG(log, "ProcessGDBRemote::DidFork() unable to reset pid/tid");
+      return;
+    }
+  }
+
   LLDB_LOG(log, "Detaching forked child {0}", child_pid);
   Status error = m_gdb_comm.Detach(false, child_pid);
   if (error.Fail()) {
-      LLDB_LOG(log,
-               "ProcessGDBRemote::DidFork() detach packet send failed: {0}",
-                error.AsCString() ? error.AsCString() : "<unknown error>");
-      return;
+    LLDB_LOG(log, "ProcessGDBRemote::DidFork() detach packet send failed: {0}",
   }
 }
 
 void ProcessGDBRemote::DidVFork(lldb::pid_t child_pid, lldb::tid_t child_tid) {
   Log *log(ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS));
 
   LLDB_LOG(log, "Detaching forked child {0}", child_pid);
   Status error = m_gdb_comm.Detach(false, child_pid);
   if (error.Fail()) {
       LLDB_LOG(log,
                "ProcessGDBRemote::DidFork() detach packet send failed: {0}",
                 error.AsCString() ? error.AsCString() : "<unknown error>");
       return;
   }
 }

lldb/test/API/functionalities/gdb_remote_client/gdbclientutils.py

def respond(self, packet):
             return self.qC()
         if packet == "QEnableErrorStrings":
             return self.QEnableErrorStrings()
         if packet == "?":
             return self.haltReason()
         if packet == "s":
             return self.haltReason()
         if packet[0] == "H":
-            return self.selectThread(packet[1], int(packet[2:], 16))
+            tid = packet[2:]
+            if "." in tid:
+                assert tid.startswith("p")
+                # TODO: do we want to do anything with PID?
+                tid = tid.split(".", 1)[1]
+            return self.selectThread(packet[1], int(tid, 16))
         if packet[0:6] == "qXfer:":
             obj, read, annex, location = packet[6:].split(":")
             offset, length = [int(x, 16) for x in location.split(',')]
             data, has_more = self.qXferRead(obj, annex, offset, length)
             if data is not None:
                 return self._qXferResponse(data, has_more)
             return ""
         if packet.startswith("vAttach;"):

lldb/test/Shell/Subprocess/clone-follow-parent-softbp.test

+# REQUIRES: native && (system-linux || system-netbsd)
+# clone() tests fails on arm64 Linux, PR #49899
+# UNSUPPORTED: system-linux && target-aarch64
+# RUN: %clangxx_host %p/Inputs/fork.cpp -DTEST_CLONE -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+b parent_func
+b child_func
+process launch
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = breakpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0

lldb/test/Shell/Subprocess/fork-follow-parent-softbp.test

+# REQUIRES: native
+# UNSUPPORTED: system-windows
+# RUN: %clangxx_host %p/Inputs/fork.cpp -DTEST_FORK=fork -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+b parent_func
+b child_func
+process launch
+# CHECK-NOT: function run in parent
+# CHECK: stop reason = breakpoint
+continue
+# CHECK: function run in parent
+# CHECK: child exited: 0