PR4225 and PR6826: expand & canonicalize executable path names process probes

diff --git a/ChangeLog b/ChangeLog
index 47b52f173dce0e1940e2ff22c1f9cbf24ab62d49..b312bc3453d87b8b0662eff036348a042aef1824 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2008-08-09  Frank Ch. Eigler  <fche@elastic.org>
+
+       * main.cxx (main): Don't override $PATH etc.
+       * tapsets.cxx (base_query ctor, dwarf_builder::build,
+       itrace_builder, utrace_derived_probe): Use find_executable()
+       to resolve FOO path in process("FOO").
+       * util.cxx (find_executable): Duplicate execvp logic.
+
 2008-08-09  Frank Ch. Eigler  <fche@elastic.org>
 
        * util.cxx (find_executable): Reorganize, simplify, canonicalize.

diff --git a/main.cxx b/main.cxx
index 8c99cf7e96f0bb7b1fc00b66704de22c04b2bed4..a2ceb6561751d1701c22c891339d376910593043 100644 (file)
--- a/main.cxx
+++ b/main.cxx
@@ -696,25 +696,11 @@ main (int argc, char * const argv [])
 
   int rc = 0;
 
-  // override PATH and LC_ALL
-  const char *path = "/bin:/sbin:/usr/bin:/usr/sbin";
-  rc = setenv("PATH", path, 1) || setenv("LC_ALL", "C", 1);
-  if (rc)
-    {
-      const char* e = strerror (errno);
-      cerr << "setenv (\"PATH=" << path << "\" + \"LC_ALL=C\"): "
-           << e << endl;
-    }
-
-  // Get rid of a few standard environment variables (which might
-  // cause us to do unintended things).
-  rc = unsetenv("IFS") || unsetenv("CDPATH") || unsetenv("ENV")
-      || unsetenv("BASH_ENV");
-  if (rc)
-    {
-      const char* e = strerror (errno);
-      cerr << "unsetenv failed: " << e << endl;
-    }
+  // For PR1477, we used to override $PATH and $LC_ALL and other stuff
+  // here.  We seem to use complete pathnames in
+  // buildrun.cxx/tapsets.cxx now, so this is not necessary.  Further,
+  // it interferes with util.cxx:find_executable(), used for $PATH
+  // resolution.
 
   s.kernel_base_release.assign(s.kernel_release, 0, s.kernel_release.find('-'));
 
@@ -993,6 +979,7 @@ main (int argc, char * const argv [])
   if (rc || s.last_pass == 3 || pending_interrupts) goto cleanup;
 
   // PASS 4: COMPILATION
+
   times (& tms_before);
   gettimeofday (&tv_before, NULL);
   rc = compile_pass (s);

diff --git a/tapsets.cxx b/tapsets.cxx
index ab5713b0f60e87be3678b6f547cee1f8981a8943..66c9a7ac127baf0439b13c720f95d214e181568e 100644 (file)
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -2357,7 +2357,11 @@ base_query::base_query(systemtap_session & sess,
   if (has_module)
     has_process = false;
   else
-    has_process = get_string_param(params, TOK_PROCESS, module_val);
+    {
+      has_process = get_string_param(params, TOK_PROCESS, module_val);
+      if (has_process) 
+        module_val = find_executable (module_val);
+    }
 
   assert (has_kernel || has_process || has_module);
 }
@@ -5001,6 +5005,8 @@ dwarf_builder::build(systemtap_session & sess,
     }
   else if (get_param (parameters, TOK_PROCESS, module_name))
     {
+      module_name = find_executable (module_name); // canonicalize it
+
       // user-space target; we use one dwflpp instance per module name
       // (= program or shared library)
       if (user_dw.find(module_name) == user_dw.end())
@@ -5536,41 +5542,11 @@ struct itrace_builder: public derived_probe_builder
 
     // If we have a path, we need to validate it.
     if (has_path)
-    {
-       string::size_type start_pos, end_pos;
-       string component;
-
-       // Make sure it starts with '/'.
-       if (path[0] != '/')
-           throw semantic_error ("process path must start with a '/'",
-                                 location->tok);
-
-       start_pos = 1;                  // get past the initial '/'
-       while ((end_pos = path.find('/', start_pos)) != string::npos)
-        {
-           component = path.substr(start_pos, end_pos - start_pos);
-           // Make sure it isn't empty.
-           if (component.size() == 0)
-               throw semantic_error ("process path component cannot be empty",
-                                     location->tok);
-           // Make sure it isn't relative.
-           else if (component == "." || component == "..")
-               throw semantic_error ("process path cannot be relative (and contain '.' or '..')", location->tok);
-
-           start_pos = end_pos + 1;
-       }
-       component = path.substr(start_pos);
-       // Make sure it doesn't end with '/'.
-       if (component.size() == 0)
-           throw semantic_error ("process path cannot end with a '/'", location->tok);
-       // Make sure it isn't relative.
-       else if (component == "." || component == "..")
-           throw semantic_error ("process path cannot be relative (and contain '.' or '..')", location->tok);
-    }
+      path = find_executable (path);
 
     finished_results.push_back(new itrace_derived_probe(sess, base, location,
                                                         has_path, path, pid,
-                                                                                                                                       single_step
+                                                        single_step
                                                        ));
   }
 };
@@ -5809,13 +5785,54 @@ utrace_derived_probe::utrace_derived_probe (systemtap_session &s,
                                             probe* p, probe_point* l,
                                             bool hp, string &pn, int64_t pd,
                                            enum utrace_derived_probe_flags f):
-  derived_probe(p, l), has_path(hp), path(pn), pid(pd), flags(f),
+  derived_probe (p, new probe_point (*l) /* .components soon rewritten */ ),
+  has_path(hp), path(pn), pid(pd), flags(f),
   target_symbol_seen(false)
 {
   // Make a local-variable-expanded copy of the probe body
   utrace_var_expanding_copy_visitor v (s, name, flags);
   require <statement*> (&v, &(this->body), base->body);
   target_symbol_seen = v.target_symbol_seen;
+
+  // Reset the sole element of the "locations" vector as a
+  // "reverse-engineered" form of the incoming (q.base_loc) probe
+  // point.  This allows a user to see what program etc.
+  // number any particular match of the wildcards.
+
+  vector<probe_point::component*> comps;
+  if (hp)
+    comps.push_back (new probe_point::component(TOK_PROCESS, new literal_string(path)));
+  else
+    comps.push_back (new probe_point::component(TOK_PROCESS, new literal_number(pid)));    
+  switch (flags)
+    {
+    case UDPF_THREAD_BEGIN:
+      comps.push_back (new probe_point::component(TOK_THREAD));
+      comps.push_back (new probe_point::component(TOK_BEGIN));
+      break;
+    case UDPF_THREAD_END:
+      comps.push_back (new probe_point::component(TOK_THREAD));
+      comps.push_back (new probe_point::component(TOK_END));
+      break;
+    case UDPF_SYSCALL:
+      comps.push_back (new probe_point::component(TOK_SYSCALL));
+      break;
+    case UDPF_SYSCALL_RETURN:
+      comps.push_back (new probe_point::component(TOK_SYSCALL));
+      comps.push_back (new probe_point::component(TOK_RETURN));
+      break;
+    case UDPF_BEGIN:
+      comps.push_back (new probe_point::component(TOK_BEGIN));
+      break;
+    case UDPF_END:
+      comps.push_back (new probe_point::component(TOK_END));
+      break;
+    default: 
+      assert (0);
+    }
+
+  // Overwrite it.
+  this->sole_location()->components = comps;
 }
 
 
@@ -5897,6 +5914,7 @@ struct utrace_builder: public derived_probe_builder
     bool has_path = get_param (parameters, TOK_PROCESS, path);
     bool has_pid = get_param (parameters, TOK_PROCESS, pid);
     enum utrace_derived_probe_flags flags = UDPF_NONE;
+
     assert (has_path || has_pid);
 
     if (has_null_param (parameters, TOK_THREAD))
@@ -5921,39 +5939,7 @@ struct utrace_builder: public derived_probe_builder
     // If we have a path, we need to validate it.
     if (has_path)
     {
-       string::size_type start_pos, end_pos;
-       string component;
-
-        // XXX: these checks should be done in terms of filesystem
-        // operations.
-
-       // Make sure it starts with '/'.
-       if (path[0] != '/')
-           throw semantic_error ("process path must start with a '/'",
-                                 location->tok);
-
-       start_pos = 1;                  // get past the initial '/'
-       while ((end_pos = path.find('/', start_pos)) != string::npos)
-        {
-           component = path.substr(start_pos, end_pos - start_pos);
-           // Make sure it isn't empty.
-           if (component.size() == 0)
-               throw semantic_error ("process path component cannot be empty",
-                                     location->tok);
-           // Make sure it isn't relative.
-           else if (component == "." || component == "..")
-               throw semantic_error ("process path cannot be relative (and contain '.' or '..')", location->tok);
-
-           start_pos = end_pos + 1;
-       }
-       component = path.substr(start_pos);
-       // Make sure it doesn't end with '/'.
-       if (component.size() == 0)
-           throw semantic_error ("process path cannot end with a '/'", location->tok);
-       // Make sure it isn't relative.
-       else if (component == "." || component == "..")
-           throw semantic_error ("process path cannot be relative (and contain '.' or '..')", location->tok);
-
+      path = find_executable (path);
       sess.unwindsym_modules.insert (path);
     }
 

diff --git a/util.cxx b/util.cxx
index 8be4ea5d4e4715877237a74e82c69ecc93f9d4bf..ab0a1a913816dbdc0f6cd60663c6953fc3fc9ec7 100644 (file)
--- a/util.cxx
+++ b/util.cxx
@@ -154,7 +154,10 @@ tokenize(const string& str, vector<string>& tokens,
 }
 
 
-//  Find an executable by name in $PATH.
+// Resolve an executable name to a canonical full path name, with the
+// same policy as execvp().  A program name not containing a slash
+// will be searched along the $PATH.
+
 string find_executable(const string& name)
 {
   string retpath;
@@ -162,9 +165,13 @@ string find_executable(const string& name)
   if (name.size() == 0)
     return name;
 
-  if (name[0] == '/') // already fully qualified
-    retpath = name;
-  else // need to search along $PATH
+  struct stat st;
+
+  if (name.find('/') != string::npos) // slash in the path already?
+    {
+      retpath = name;
+    }
+  else // Nope, search $PATH.
     {
       char *path = getenv("PATH");
       if (path)
@@ -178,13 +185,11 @@ string find_executable(const string& name)
             {
               string fname = *i + "/" + name;
               const char *f = fname.c_str();
-              struct stat st1, st2;
               
               // Look for a normal executable file.
               if (access(f, X_OK) == 0
-                  && lstat(f, &st1) == 0
-                  && stat(f, &st2) == 0
-                  && S_ISREG(st2.st_mode))
+                  && stat(f, &st) == 0
+                  && S_ISREG(st.st_mode))
                 {
                   retpath = fname;
                   break;
@@ -193,6 +198,12 @@ string find_executable(const string& name)
         }
     }
 
+  
+  // Could not find the program on the $PATH.  We'll just fall back to
+  // the unqualified name, which our caller will probably fail with.
+  if (retpath == "")
+    retpath = name;
+
   // Canonicalize the path name.
   char *cf = canonicalize_file_name (retpath.c_str());
   if (cf)