PR10544: clean up stap child process error handling

* util.cxx (stap_system): Take extra verbosity value.  Standardize
  error handling / tracing.
* util.h: Corresponding changes.
* buildrun.cxx, main.cxx, modsign.cxx: Update callers.

6 files changed, 40 insertions, 41 deletions

diff --git a/buildrun.cxx b/buildrun.cxx
index be3d89ed..796663e0 100644
--- a/buildrun.cxx
+++ b/buildrun.cxx
@@ -63,11 +63,7 @@ run_make_cmd(systemtap_session& s, string& make_cmd)
   else
     make_cmd += " -s >/dev/null";
 
-  if (s.verbose > 1) clog << "Running " << make_cmd << endl;
-  rc = stap_system (make_cmd.c_str());
-  if (rc && s.verbose > 1)
-    clog << "Error " << rc << " " << strerror(rc) << endl;
-  return rc;
+  return stap_system (s.verbose, make_cmd);
 }
 
 static void
@@ -253,10 +249,8 @@ kernel_built_uprobes (systemtap_session& s)
 {
   string grep_cmd = string ("/bin/grep -q unregister_uprobe ") + 
     s.kernel_build_tree + string ("/Module.symvers");
-  int rc = stap_system (grep_cmd.c_str());
-  if (rc && s.verbose > 1)
-    clog << "Error " << rc << " " << strerror(rc) << endl;
-  return (rc == 0);
+
+  return (stap_system (s.verbose, grep_cmd) == 0);
 }
 
 static bool
@@ -306,10 +300,8 @@ copy_uprobes_symbols (systemtap_session& s)
   string uprobes_home = s.runtime_path + "/uprobes";
   string cp_cmd = string("/bin/cp ") + uprobes_home +
     string("/Module.symvers ") + s.tmpdir;
-  int rc = stap_system (cp_cmd.c_str());
-  if (rc && s.verbose > 1)
-    clog << "Error " << rc << " " << strerror(rc) << endl;
-  return rc;
+
+  return stap_system (s.verbose, cp_cmd);
 }
 
 static int
@@ -342,8 +334,6 @@ uprobes_pass (systemtap_session& s)
 int
 run_pass (systemtap_session& s)
 {
-  int rc = 0;
-
   // for now, just spawn staprun
   string staprun_cmd = string(getenv("SYSTEMTAP_STAPRUN") ?: BINDIR "/staprun")
     + " "
@@ -371,12 +361,7 @@ run_pass (systemtap_session& s)
 
   staprun_cmd += s.tmpdir + "/" + s.module_name + ".ko";
 
-  if (s.verbose>1) clog << "Running " << staprun_cmd << endl;
-
-  rc = stap_system (staprun_cmd.c_str ());
-  if (rc && s.verbose > 1)
-    clog << "Error " << rc << " " << strerror(rc) << endl;
-  return rc;
+  return stap_system (s.verbose, staprun_cmd);
 }
 
 
@@ -512,10 +497,7 @@ make_typequery_umod(systemtap_session& s, const string& header, string& name)
      + name + " -xc /dev/null -include " + header;
   if (s.verbose < 4)
     cmd += " >/dev/null 2>&1";
-  int rc = stap_system (cmd.c_str());
-  if (rc && s.verbose > 1)
-    clog << "Error " << rc << " " << strerror(rc) << endl;
-  return rc;
+  return stap_system (s.verbose, cmd);
 }
 
 
diff --git a/main.cxx b/main.cxx
index a3a7e56e..bc0d7389 100644
--- a/main.cxx
+++ b/main.cxx
@@ -1330,10 +1330,8 @@ pass_5:
 	  // Remove the temporary directory.
           string cleanupcmd = "rm -rf ";
           cleanupcmd += s.tmpdir;
-          if (s.verbose>1) clog << "Running " << cleanupcmd << endl;
-	  int status = stap_system (cleanupcmd.c_str());
-	  if (status != 0 && s.verbose>1)
-	    clog << "Cleanup command failed, status: " << status << endl;
+
+	  (void) stap_system (s.verbose, cleanupcmd);
         }
     }
 
diff --git a/modsign.cxx b/modsign.cxx
index 8f29dab1..cacd5699 100644
--- a/modsign.cxx
+++ b/modsign.cxx
@@ -284,14 +284,14 @@ init_cert_db_path (const string &cert_db_path) {
 
   // Generate the certificate and database.
   string cmd = BINDIR "/stap-gen-cert " + cert_db_path;
-  rc = stap_system (cmd.c_str()) == 0;
+  rc = stap_system (0, cmd) == 0;
 
   // If we are root, authorize the new certificate as a trusted
   // signer. It is not an error if this fails.
   if (geteuid () == 0)
     {
       cmd = BINDIR "/stap-authorize-signing-cert " + cert_db_path + "/stap.cert";
-      stap_system (cmd.c_str());
+      stap_system (0, cmd);
     }
 
   return rc;
diff --git a/tapsets.cxx b/tapsets.cxx
index f94bea99..ddb5696d 100644
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -4431,7 +4431,7 @@ uprobe_derived_probe_group::emit_module_decls (systemtap_session& s)
       s.op->line() << " .finder = {";
       if (p->pid != 0)
         s.op->line() << " .pid=" << p->pid;
-      else if (p->section == ".absolute")
+      else if (p->section == ".absolute") // proxy for ET_EXEC -> exec()'d program
         s.op->line() << " .procname=" << lex_cast_qstring(p->module) << ", ";
       // else ".dynamic" gets procname=0, pid=0, activating task_finder "global tracing"
       s.op->line() << "},";
diff --git a/util.cxx b/util.cxx
index 495d2f5a..057cc7ab 100644
--- a/util.cxx
+++ b/util.cxx
@@ -326,22 +326,41 @@ static pid_t spawned_pid = 0;
 
 // Runs a command with a saved PID, so we can kill it from the signal handler
 int
-stap_system(const char *command)
+stap_system(int verbose, const std::string& command)
 {
-  STAP_PROBE1(stap, stap_system__start, command);
-  const char * argv[] = { "sh", "-c", command, NULL };
+  const char *cmd = command.c_str();
+  STAP_PROBE1(stap, stap_system__start, cmd);
+  char const * const argv[] = { "sh", "-c", cmd, NULL };
   int ret, status;
 
   spawned_pid = 0;
-  ret = posix_spawn(&spawned_pid, "/bin/sh", NULL, NULL,
-                    const_cast<char **>(argv), environ);
+
+  if (verbose > 1) 
+    clog << "Running " << command << endl;
+
+  ret = posix_spawn(&spawned_pid, "/bin/sh", NULL, NULL, const_cast<char * const *>(argv), environ);
   STAP_PROBE2(stap, stap_system__spawn, ret, spawned_pid);
   if (ret == 0)
     {
-      if (waitpid(spawned_pid, &status, 0) == spawned_pid)
-        ret = WIFEXITED(status) ? WEXITSTATUS(status) : 128 + WTERMSIG(status);
+      ret = waitpid(spawned_pid, &status, 0);
+      if (ret == spawned_pid)
+        {
+          ret = WIFEXITED(status) ? WEXITSTATUS(status) : 128 + WTERMSIG(status);
+          if (verbose > 2) 
+            clog << "Spawn waitpid result (0x" << ios::hex << status << ios::dec << "): " << ret << endl;
+        }
       else
-        ret = errno;
+        {
+          if (verbose > 1) 
+            clog << "Spawn waitpid error (" << ret << "): " << strerror(errno) << endl;
+          ret = -1;
+        }
+    }
+  else
+    {
+      if (verbose > 1) 
+        clog << "Spawn error (" << ret << "): " << strerror(ret) << endl;
+      ret = -1;
     }
   STAP_PROBE1(stap, stap_system__complete, ret);
   spawned_pid = 0;
diff --git a/util.h b/util.h
index 4921cef7..1577bb54 100644
--- a/util.h
+++ b/util.h
@@ -16,7 +16,7 @@ void tokenize(const std::string& str, std::vector<std::string>& tokens,
 std::string find_executable(const std::string& name);
 const std::string cmdstr_quoted(const std::string& cmd);
 std::string git_revision(const std::string& path);
-int stap_system(const char *command);
+int stap_system(int verbose, const std::string& command);
 int kill_stap_spawn(int sig);