provide common interfaces for continue and detach requests (#253)

oi/OIDebugger.cpp

@@ -445,25 +445,52 @@ bool OIDebugger::isExtendedWait(int status) {
   return (getExtendedWaitEventType(status) != 0);
 }
 
+/*
+ * Continue the main traced thread either by a detach request (default)
+ * or via a continue. No provision is made for passing a signal.
+ */
 bool OIDebugger::contTargetThread(bool detach) const {
-  VLOG(4) << "contTargetThread: About to PTRACE_CONT pid " << std::dec
+  VLOG(4) << "contTargetThread: About to " << (detach ? "detach" : "continue")
-          << traceePid << " detach = " << detach;
+          << " pid " << std::dec << traceePid;
 
-  if (detach) {
+  if (!detach) {
-    if ((ptrace(PTRACE_DETACH, traceePid, nullptr, nullptr)) < 0) {
+    return contTargetThread(traceePid);
-      LOG(ERROR) << "contTargetThread failed to detach pid " << traceePid << " "
+  }
-                 << strerror(errno) << "(" << errno << ")";
+
-    }
+  errno = 0;
-  } else {
+  if ((ptrace(PTRACE_DETACH, traceePid, nullptr, nullptr)) < 0) {
-    if ((ptrace(PTRACE_CONT, traceePid, nullptr, nullptr)) < 0) {
+    LOG(ERROR) << "contTargetThread failed to detach pid " << traceePid << " "
-      LOG(ERROR) << "contTargetThread failed to continue pid " << traceePid
+               << strerror(errno) << "(" << errno << ")";
-                 << " " << strerror(errno) << "(" << errno << ")";
+    return false;
-    }
   }
 
   return true;
 }
 
+/*
+ * Continue the thread specified in 'targetPid' passing an optional
+ * signal via the optional 'data' parameter. The ptrace(2) data parameter
+ * is a 'void*' but used for many purposes. Here the sole pupose in detach
+ * or continue is to pass a signal value for the target thread and although
+ * it's not explicitly stated in the man page, a value of 0 (default) is
+ * treated as no signal passed.
+ */
+bool OIDebugger::contTargetThread(pid_t targetPid, unsigned long data) const {
+  bool ret = true;
+
+  VLOG(4) << "contTargetThread: About to continue pid " << std::dec
+          << targetPid;
+
+  errno = 0;
+  if ((ptrace(PTRACE_CONT, targetPid, nullptr, data)) < 0) {
+    LOG(ERROR) << "contTargetThread failed to continue pid " << targetPid << " "
+               << strerror(errno) << "(" << errno << ")";
+    ret = false;
+  }
+
+  return ret;
+}
+
 bool OIDebugger::replayTrappedInstr(const trapInfo& t,
                                     pid_t pid,
                                     struct user_regs_struct& regs,
@@ -590,12 +617,7 @@ OIDebugger::processTrapRet OIDebugger::processFuncTrap(
     LOG(ERROR) << "Failed to locate all objects addresses. Aborting...";
     replayTrappedInstr(*t, pid, t->savedRegs, t->savedFPregs);
 
-    errno = 0;
+    contTargetThread(pid);
-    VLOG(4) << "processFuncEntry about to PTRACE_CONT pid " << std::dec << pid;
-    if (ptrace(PTRACE_CONT, pid, nullptr, nullptr) < 0) {
-      LOG(ERROR) << "processTrap: Error in PTRACE_CONT for pid " << pid << " "
-                 << strerror(errno) << "(" << errno << ")";
-    }
 
     return OIDebugger::OID_ERR;
   }
@@ -686,12 +708,7 @@ OIDebugger::processTrapRet OIDebugger::processFuncTrap(
   VLOG(4) << "Inserting Trapinfo for pid " << std::dec << pid;
   threadTrapState.insert_or_assign(pid, std::move(t));
 
-  errno = 0;
+  contTargetThread(pid);
-  VLOG(4) << "processFuncTrap about to PTRACE_CONT pid " << std::dec << pid;
-  if (ptrace(PTRACE_CONT, pid, nullptr, nullptr) < 0) {
-    LOG(ERROR) << "processTrap: Error in PTRACE_CONT for pid " << pid << " "
-               << strerror(errno) << "(" << errno << ")";
-  }
 
   VLOG(1) << "Finished Function Trap processing\n";
 
@@ -764,14 +781,9 @@ OIDebugger::processTrapRet OIDebugger::processJitCodeRet(
     VLOG(4) << "Erasing Trapinfo for pid " << std::dec << iter->first;
     threadTrapState.erase(iter);
 
-    VLOG(4) << "processTrap2 processing PTRACE_CONT pid " << pid;
-
     jitTrapProcessTime.stop();
 
-    if (ptrace(PTRACE_CONT, pid, nullptr, nullptr) < 0) {
+    contTargetThread(pid);
-      LOG(ERROR) << "processTrap2: Error in PTRACE_CONT for pid " << pid << " "
-                 << strerror(errno) << "(" << errno << ")";
-    }
 
     if (++count == 1 || isInterrupted()) {
       VLOG(1) << "count: " << count << " oid done";
@@ -882,11 +894,7 @@ bool OIDebugger::processGlobal(const std::string& varName) {
     LOG(ERROR) << "Execute: Couldn't restore registers: " << strerror(errno);
   }
 
-  errno = 0;
+  contTargetThread(traceePid);
-  if (ptrace(PTRACE_CONT, traceePid, nullptr, nullptr) < 0) {
-    VLOG(1) << "processTrap: Error in PTRACE_CONT for pid " << traceePid << " "
-            << strerror(errno) << "(" << errno << ")";
-  }
 
   return true;
 }
@@ -1013,16 +1021,11 @@ OIDebugger::processTrapRet OIDebugger::processTrap(pid_t pid,
         VLOG(4) << "child was stopped with: " << WSTOPSIG(tstatus);
       }
 
-      VLOG(4) << "About to PTRACE_CONT pid " << std::dec << childPid;
-
       ptrace(PTRACE_SETOPTIONS, childPid, NULL,
              PTRACE_O_TRACESYSGOOD | PTRACE_O_TRACEFORK | PTRACE_O_TRACECLONE |
                  PTRACE_O_TRACEVFORK);
 
-      if (ptrace(PTRACE_CONT, childPid, nullptr, nullptr) < 0) {
+      contTargetThread(childPid);
-        VLOG(1) << "processTrap2: Error in PTRACE_CONT for pid " << childPid
-                << " " << strerror(errno) << "(" << errno << ")";
-      }
     } else if (type == PTRACE_EVENT_EXIT) {
       VLOG(4) << "Thread exiting!! pid " << std::dec << newpid;
       threadList.erase(
@@ -1034,11 +1037,7 @@ OIDebugger::processTrapRet OIDebugger::processTrap(pid_t pid,
       VLOG(4) << "unhandled extended wait event type: " << type;
     }
 
-    VLOG(4) << "extended wait About to cont pid " << newpid;
+    contTargetThread(newpid);
-    if (ptrace(PTRACE_CONT, newpid, nullptr, nullptr) < 0) {
-      VLOG(4) << "processTrap2: Error in PTRACE_CONT for pid " << newpid << " "
-              << strerror(errno) << "(" << errno << ")";
-    }
 
     return ret;
   }
@@ -1053,10 +1052,7 @@ OIDebugger::processTrapRet OIDebugger::processTrap(pid_t pid,
        */
     case SIGALRM: {
       VLOG(4) << "SIGALRM received!!";
-      if (ptrace(PTRACE_CONT, newpid, nullptr, SIGALRM) < 0) {
+      contTargetThread(newpid, SIGALRM);
-        VLOG(4) << "processTrap2: Error in PTRACE_CONT SIGALRM for pid "
-                << newpid << " " << strerror(errno) << "(" << errno << ")";
-      }
       break;
     }
 
@@ -1098,18 +1094,10 @@ OIDebugger::processTrapRet OIDebugger::processTrap(pid_t pid,
         replayTrappedInstr(*t, newpid, t->savedRegs, t->savedFPregs);
         threadTrapState.erase(newpid);
 
-        errno = 0;
+        contTargetThread(newpid);
-        if (ptrace(PTRACE_CONT, newpid, nullptr, nullptr) < 0) {
-          VLOG(4) << "processTrap: Error in PTRACE_CONT for pid " << newpid
-                  << " " << strerror(errno) << "(" << errno << ")";
-        }
       } else {
         // We are not the source of the SIGSEGV so forward it to the target.
-        errno = 0;
+        contTargetThread(newpid, sig);
-        if (ptrace(PTRACE_CONT, newpid, nullptr, sig) < 0) {
-          VLOG(4) << "processTrap: Error in PTRACE_CONT for pid " << newpid
-                  << " " << strerror(errno) << "(" << errno << ")";
-        }
       }
       return OIDebugger::OID_DONE;
     }
@@ -1122,10 +1110,7 @@ OIDebugger::processTrapRet OIDebugger::processTrap(pid_t pid,
 
     case SIGCHLD: {
       VLOG(4) << "SIGCHLD processing for pid " << newpid;
-      if (ptrace(PTRACE_CONT, newpid, nullptr, SIGCHLD) < 0) {
+      contTargetThread(newpid, sig);
-        VLOG(4) << "processTrap2: Error in PTRACE_CONT SIGCHLD for pid " << pid
-                << " " << strerror(errno) << "(" << errno << ")";
-      }
 
       break;
     }
@@ -1190,11 +1175,7 @@ OIDebugger::processTrapRet OIDebugger::processTrap(pid_t pid,
           }
 
           replayTrappedInstr(*tInfo, newpid, regs, fpregs);
-
+          contTargetThread(newpid);
-          if (ptrace(PTRACE_CONT, newpid, nullptr, nullptr) < 0) {
-            LOG(ERROR) << "processTrap: Error in PTRACE_CONT for pid " << newpid
-                       << " " << strerror(errno) << "(" << errno << ")";
-          }
 
           break;
         }
@@ -1221,11 +1202,7 @@ OIDebugger::processTrapRet OIDebugger::processTrap(pid_t pid,
                      << newpid << " " << strerror(errno);
         }
 
-        errno = 0;
+        contTargetThread(newpid);
-        if (ptrace(PTRACE_CONT, newpid, nullptr, nullptr) < 0) {
-          LOG(ERROR) << "processTrap: Error in PTRACE_CONT for pid " << newpid
-                     << " " << strerror(errno) << "(" << errno << ")";
-        }
       }
 
       break;
@@ -1242,10 +1219,7 @@ OIDebugger::processTrapRet OIDebugger::processTrap(pid_t pid,
       VLOG(4) << "contAndExecute: Explictly unhandled signal. Forwarding "
               << strsignal(WSTOPSIG(status));
 
-      if (ptrace(PTRACE_CONT, newpid, nullptr, sig) < 0) {
+      contTargetThread(newpid, sig);
-        VLOG(4) << "processTrap2: Error in PTRACE_CONT SIGALRM for pid " << pid
-                << " " << strerror(errno) << "(" << errno << ")";
-      }
       break;
   }
 
@@ -1834,12 +1808,10 @@ bool OIDebugger::removeTraps(pid_t pid) {
   }
 
   /* Resume the main thread now, so it doesn't have to wait on restoreState */
-  errno = 0;
+  if (!contTargetThread(targetPid)) {
-  if (ptrace(PTRACE_CONT, targetPid, nullptr, nullptr) < 0) {
-    LOG(ERROR) << "Couldn't continue target pid " << targetPid
-               << " (Reason: " << strerror(errno) << ")";
     return false;
   }
+
   return true;
 }
 

oi/OIDebugger.h

@@ -235,6 +235,7 @@ class OIDebugger {
   static bool isExtendedWait(int);
   void dumpAlltaskStates(void);
   std::optional<std::vector<uintptr_t>> findRetLocs(FuncDesc&);
+  bool contTargetThread(pid_t, unsigned long = 0) const;
 
   OICompiler::Config compilerConfig{};
   const OICodeGen::Config& generatorConfig;