PR 5270

* main.cxx: Restored pre-10-08 version: moved uprobes build to
buildrun.cxx.
* buildrun.cxx: Reworked uprobes build so that the resulting
Module.symvers can be used in building the stap-generated
module.  If user isn't root, call verify_uprobes_uptodate()
rather than trying (and failing) to rebuild uprobes.ko.
* buildrun.h: uprobes_enabled() and make_uprobes() are no
longer extern.
* runtime/uprobes/Makefile: Added uprobes.ko target for use
by verify_uprobes_uptodate().

5 files changed, 109 insertions, 39 deletions

diff --git a/ChangeLog b/ChangeLog
index 95dff80c..95cc25ff 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2007-11-13  Jim Keniston  <jkenisto@us.ibm.com>
+
+	PR 5270
+	* main.cxx: Restored pre-10-08 version: moved uprobes build to
+	buildrun.cxx.
+	* buildrun.cxx: Reworked uprobes build so that the resulting
+	Module.symvers can be used in building the stap-generated
+	module.  If user isn't root, call verify_uprobes_uptodate()
+	rather than trying (and failing) to rebuild uprobes.ko.
+	* buildrun.h: uprobes_enabled() and make_uprobes() are no
+	longer extern.
+	* runtime/uprobes/Makefile: Added uprobes.ko target for use
+	by verify_uprobes_uptodate().
+
 2007-11-12  Martin Hunt  <hunt@redhat.com>
 
 	* stap.1.in: Replaced references to the log() function.
diff --git a/buildrun.cxx b/buildrun.cxx
index 3eeae794..2f7c358d 100644
--- a/buildrun.cxx
+++ b/buildrun.cxx
@@ -29,6 +29,8 @@ extern "C" {
 
 using namespace std;
 
+static int uprobes_pass (systemtap_session& s);
+
 /* Adjust and run make_cmd to build a kernel module. */
 static int
 run_make_cmd(systemtap_session& s, string& make_cmd)
@@ -59,10 +61,13 @@ run_make_cmd(systemtap_session& s, string& make_cmd)
 int
 compile_pass (systemtap_session& s)
 {
+  int rc = uprobes_pass (s);
+  if (rc)
+    return rc;
+
   // fill in a quick Makefile
   string makefile_nm = s.tmpdir + "/Makefile";
   ofstream o (makefile_nm.c_str());
-  int rc = 0;
 
   // Create makefile
 
@@ -132,40 +137,101 @@ compile_pass (systemtap_session& s)
   return rc;
 }
 
+static const string uprobes_home = string(PKGDATADIR "/runtime/uprobes");
 
-bool
-uprobes_enabled (void)
+/*
+ * If uprobes was built as part of the kernel build (either built-in
+ * or as a module), the uprobes exports should show up in
+ * /lib/modules/`uname -r`/build/Module.symvers.  Return true if so.
+ */
+static bool
+kernel_built_uprobes (systemtap_session& s)
 {
-  int rc = system ("/bin/grep -q unregister_uprobe /proc/kallsyms");
+  string grep_cmd = string ("/bin/grep -q unregister_uprobe /lib/modules/")
+    + s.kernel_release + string ("/build/Module.symvers");
+  int rc = system (grep_cmd.c_str());
   return (rc == 0);
 }
 
-int
-make_uprobes (systemtap_session& s)
+static bool
+verify_uprobes_uptodate (systemtap_session& s)
 {
-  string uprobes_home = string(PKGDATADIR "/runtime/uprobes");
+  if (s.verbose)
+    clog << "Pass 4, preamble: "
+	 << "verifying that SystemTap's version of uprobes is up to date."
+	 << endl;
+
+  string make_cmd = string("make -q -C ") + uprobes_home
+    + string(" uprobes.ko");
+  int rc = run_make_cmd(s, make_cmd);
+  if (rc) {
+    clog << "SystemTap's version of uprobes is out of date." << endl;
+    clog << "As root, run \"make\" in " << uprobes_home << "." << endl;
+  }
 
-  // Quietly skip the build if the Makefile has been removed.
-  string makefile = uprobes_home + string("/Makefile");
-  struct stat buf;
-  if (stat(makefile.c_str(), &buf) != 0)
-  	return 2;	// make's exit value for No such file or directory.
+  return rc;
+}
 
+static int
+make_uprobes (systemtap_session& s)
+{
   if (s.verbose)
-    clog << "Pass 4, overtime: "
+    clog << "Pass 4, preamble: "
 	 << "(re)building SystemTap's version of uprobes."
 	 << endl;
 
   string make_cmd = string("make -C ") + uprobes_home;
   int rc = run_make_cmd(s, make_cmd);
-  if (rc && s.verbose)
-    clog << "Uprobes build failed. "
-    	 << "Hope uprobes is available at run time."
-	 << endl;
+  if (s.verbose) {
+    if (rc)
+      clog << "Uprobes (re)build failed." << endl;
+    else
+      clog << "Uprobes (re)build complete." << endl;
+  }
 
   return rc;
 }
 
+/*
+ * Copy uprobes' exports (in Module.symvers) into the temporary directory
+ * so the script-module build can find them.
+ */
+static int
+copy_uprobes_symbols (systemtap_session& s)
+{
+  string cp_cmd = string("/bin/cp ") + uprobes_home +
+    string("/Module.symvers ") + s.tmpdir;
+  int rc = system (cp_cmd.c_str());
+  return rc;
+}
+
+static int
+uprobes_pass (systemtap_session& s)
+{
+  if (!s.need_uprobes || kernel_built_uprobes(s))
+    return 0;
+  /*
+   * We need to use the version of uprobes that comes with SystemTap, so
+   * we may need to rebuild uprobes.ko there.  Unfortunately, this is
+   * never a no-op; e.g., the modpost step gets run every time.  We don't
+   * want non-root users modifying uprobes, so we keep the uprobes
+   * directory writable only by root.  But that means a non-root member
+   * of group stapdev can't run the make even if everything's up to date.
+   *
+   * So for non-root users, we just use "make -q" with a fake target to
+   * verify that uprobes doesn't need to be rebuilt.  If that's not so,
+   * stap must fail.
+   */
+  int rc;
+  if (geteuid() == 0) {
+    rc = make_uprobes(s);
+    if (rc == 0)
+      rc = copy_uprobes_symbols(s);
+  } else
+    rc = verify_uprobes_uptodate(s);
+  return rc;
+}
+
 int
 run_pass (systemtap_session& s)
 {
diff --git a/buildrun.h b/buildrun.h
index 54e78c1a..88a63a5e 100644
--- a/buildrun.h
+++ b/buildrun.h
@@ -12,8 +12,6 @@
 #include "elaborate.h"
 
 int compile_pass (systemtap_session& s);
-bool uprobes_enabled (void);
-int make_uprobes (systemtap_session& s);
 int run_pass (systemtap_session& s);
 
 
diff --git a/main.cxx b/main.cxx
index 97f089a8..7d4b8133 100644
--- a/main.cxx
+++ b/main.cxx
@@ -225,7 +225,6 @@ main (int argc, char * const argv [])
   s.symtab = false;
   s.use_cache = true;
   s.tapset_compile_coverage = false;
-  s.need_uprobes = false;
 
   const char* s_p = getenv ("SYSTEMTAP_TAPSET");
   if (s_p != NULL)  
@@ -729,13 +728,12 @@ main (int argc, char * const argv [])
       // See if we can use cached source/module.
       if (get_from_cache(s))
         {
-	  // If our last pass isn't 5, and we don't need to build
-	  // uprobes, we're done (since passes 3 and 4 just generate
-	  // what we just pulled out of the cache).
-	  if (s.last_pass < 4) goto cleanup;
+	  // If our last pass isn't 5, we're done (since passes 3 and
+	  // 4 just generate what we just pulled out of the cache).
+	  if (s.last_pass < 5) goto cleanup;
 
-	  // Short-circuit to pass 4.5.
-	  goto pass_4point5;
+	  // Short-circuit to pass 5.
+	  goto pass_5;
 	}
     }
 
@@ -812,20 +810,8 @@ main (int argc, char * const argv [])
 	}
     }
 
-  if (rc) goto cleanup;
+  if (rc || s.last_pass == 4) goto cleanup;
 
-  // PASS 4.5: BUILD SYSTEMTAP'S VERSION OF UPROBES (IF NECESSARY)
-pass_4point5:
-  if (s.need_uprobes)
-    {
-      if (s.last_pass == 5 && uprobes_enabled())
-	// Uprobes symbols are currently available in the kernel,
-	// so staprun won't use what we'd build anyway.
-	goto pass_5;
-
-	(void) make_uprobes(s);
-    }
-  if (s.last_pass == 4) goto cleanup;
 
   // PASS 5: RUN
 pass_5:
diff --git a/runtime/uprobes/Makefile b/runtime/uprobes/Makefile
index 806f7c48..40af7aa2 100644
--- a/runtime/uprobes/Makefile
+++ b/runtime/uprobes/Makefile
@@ -1,10 +1,16 @@
 obj-m := uprobes.o
 KDIR := /lib/modules/$(shell uname -r)/build
 PWD := $(shell pwd)
+DEPENDENCIES := $(shell echo uprobes_arch.[ch] uprobes.[ch] uprobes_*.[ch])
+DEPENDENCIES += Makefile $(KDIR)/Module.symvers
 
 default:
 	$(MAKE) -C $(KDIR) SUBDIRS=$(PWD) modules
 
+# This target is used with "make -q" to see whether a "real" build is needed.
+uprobes.ko: $(DEPENDENCIES)
+	@echo uprobes.ko is not a valid target.  See Makefile.
+
 clean:
 	rm -f *.mod.c *.ko *.o .*.cmd *~
 	rm -rf .tmp_versions