PR10424: Consider each tracepoint header separately

With the current monolithic tracepoint query module, a failure in any of
the discovered tracepoint headers means that you can't use any of the
others either.  This patch creates a separate query module for each
header so they can pass or fail independently.

* buildrun.cxx (make_tracequery): take a single header name instead of
  globbing for everything we can find.
* hash.cxx (find_tracequery_hash): name the header file we're hashing.
* tapsets.cxx (tracepoint_query::handle_query_func): make sure we don't
  duplicate tracepoints found through different headers.
  (tracepoint_builder::get_tracequery_module): get a header's module
  (tracepoint_builder::init_dw): glob for all tracepoint headers, and
  feed all their modules into dwflpp.

5 files changed, 83 insertions, 52 deletions

diff --git a/buildrun.cxx b/buildrun.cxx
index 5967066e..e4b4b7bf 100644
--- a/buildrun.cxx
+++ b/buildrun.cxx
@@ -27,7 +27,6 @@ extern "C" {
 #include <unistd.h>
 #include <string.h>
 #include <errno.h>
-#include <glob.h>
 }
 
 
@@ -382,10 +381,15 @@ run_pass (systemtap_session& s)
 
 // Build a tiny kernel module to query tracepoints
 int
-make_tracequery(systemtap_session& s, string& name, const vector<string>& extra_headers)
+make_tracequery(systemtap_session& s, string& name,
+                const std::string& header,
+                const vector<string>& extra_headers)
 {
+  static unsigned tick = 0;
+  string basename("tracequery_kmod_" + lex_cast<string>(++tick));
+
   // create a subdirectory for the module
-  string dir(s.tmpdir + "/tracequery");
+  string dir(s.tmpdir + "/" + basename);
   if (create_dir(dir.c_str()) != 0)
     {
       if (! s.suppress_warnings)
@@ -393,18 +397,18 @@ make_tracequery(systemtap_session& s, string& name, const vector<string>& extra_
       return 1;
     }
 
-  name = dir + "/tracequery.ko";
+  name = dir + "/" + basename + ".ko";
 
   // create a simple Makefile
   string makefile(dir + "/Makefile");
   ofstream omf(makefile.c_str());
   // force debuginfo generation, and relax implicit functions
   omf << "EXTRA_CFLAGS := -g -Wno-implicit-function-declaration" << endl;
-  omf << "obj-m := tracequery.o" << endl;
+  omf << "obj-m := " + basename + ".o" << endl;
   omf.close();
 
   // create our source file
-  string source(dir + "/tracequery.c");
+  string source(dir + "/" + basename + ".c");
   ofstream osrc(source.c_str());
   osrc << "#ifdef CONFIG_TRACEPOINTS" << endl;
   osrc << "#include <linux/tracepoint.h>" << endl;
@@ -424,37 +428,8 @@ make_tracequery(systemtap_session& s, string& name, const vector<string>& extra_
   for (unsigned z=0; z<extra_headers.size(); z++)
     osrc << "#include <" << extra_headers[z] << ">\n";
 
-  // dynamically pull in all tracepoint headers from include/trace/
-  glob_t trace_glob;
-  string globs[] = {
-      "/include/trace/*.h",
-      "/include/trace/events/*.h",
-      "/source/include/trace/*.h",
-      "/source/include/trace/events/*.h",
-  };
-  for (unsigned z = 0; z < sizeof(globs) / sizeof(globs[0]); z++)
-    {
-      string glob_str(s.kernel_build_tree + globs[z]);
-      glob(glob_str.c_str(), 0, NULL, &trace_glob);
-      for (unsigned i = 0; i < trace_glob.gl_pathc; ++i)
-        {
-          string header(trace_glob.gl_pathv[i]);
-          size_t root_pos = header.rfind("/include/");
-          assert(root_pos != string::npos);
-          header.erase(0, root_pos + 9);
-
-          // filter out a few known "internal-only" headers
-          if (header.find("/ftrace.h") != string::npos)
-            continue;
-          if (header.find("/trace_events.h") != string::npos)
-            continue;
-          if (header.find("_event_types.h") != string::npos)
-            continue;
-
-          osrc << "#include <" << header << ">" << endl;
-        }
-      globfree(&trace_glob);
-    }
+  // add the requested tracepoint header
+  osrc << "#include <" << header << ">" << endl;
 
   // finish up the module source
   osrc << "#endif /* CONFIG_TRACEPOINTS */" << endl;
diff --git a/buildrun.h b/buildrun.h
index 7fa4ccfc..bec81aca 100644
--- a/buildrun.h
+++ b/buildrun.h
@@ -15,6 +15,7 @@ int compile_pass (systemtap_session& s);
 int run_pass (systemtap_session& s);
 
 int make_tracequery(systemtap_session& s, std::string& name,
+                    const std::string& header,
                     const std::vector<std::string>& extra_headers);
 int make_typequery(systemtap_session& s, std::string& module);
 
diff --git a/hash.cxx b/hash.cxx
index a0608c03..eac22f48 100644
--- a/hash.cxx
+++ b/hash.cxx
@@ -257,12 +257,13 @@ find_hash (systemtap_session& s, const string& script)
 
 
 string
-find_tracequery_hash (systemtap_session& s)
+find_tracequery_hash (systemtap_session& s, const string& header)
 {
   hash h;
   get_base_hash(s, h);
 
-  // The basic hash should be good enough for the tracepoint query module
+  // Add the tracepoint header to the computed hash
+  h.add_file(header);
 
   // Get the directory path to store our cached module
   string result, hashdir;
diff --git a/hash.h b/hash.h
index a6397a52..f52fd89e 100644
--- a/hash.h
+++ b/hash.h
@@ -36,7 +36,7 @@ public:
 };
 
 void find_hash (systemtap_session& s, const std::string& script);
-std::string find_tracequery_hash (systemtap_session& s);
+std::string find_tracequery_hash (systemtap_session& s, const std::string& header);
 std::string find_typequery_hash (systemtap_session& s, const std::string& name);
 
 /* vim: set sw=2 ts=8 cino=>4,n-2,{2,^-2,t0,(0,u0,w1,M1 : */
diff --git a/tapsets.cxx b/tapsets.cxx
index bb6f0daa..b9684641 100644
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -5603,6 +5603,7 @@ struct tracepoint_query : public base_query
   probe * base_probe;
   probe_point * base_loc;
   vector<derived_probe *> & results;
+  set<string> probed_names;
 
   void handle_query_module();
   int handle_query_cu(Dwarf_Die * cudie);
@@ -5639,6 +5640,12 @@ tracepoint_query::handle_query_func(Dwarf_Die * func)
 
   assert(dw.function_name.compare(0, 10, "stapprobe_") == 0);
   string tracepoint_instance = dw.function_name.substr(10);
+
+  // check for duplicates -- sometimes tracepoint headers may be indirectly
+  // included in more than one of our tracequery modules.
+  if (!probed_names.insert(tracepoint_instance).second)
+    return DWARF_CB_OK;
+
   derived_probe *dp = new tracepoint_derived_probe (dw.sess, dw, *func,
                                                     tracepoint_instance,
                                                     base_probe, base_loc);
@@ -5670,6 +5677,7 @@ struct tracepoint_builder: public derived_probe_builder
 private:
   dwflpp *dw;
   bool init_dw(systemtap_session& s);
+  string get_tracequery_module(systemtap_session& s, const string& header);
 
 public:
 
@@ -5691,17 +5699,15 @@ public:
 };
 
 
-bool
-tracepoint_builder::init_dw(systemtap_session& s)
+string
+tracepoint_builder::get_tracequery_module(systemtap_session& s,
+                                          const string& header)
 {
-  if (dw != NULL)
-    return true;
-
   string tracequery_path;
   if (s.use_cache)
     {
       // see if the cached module exists
-      tracequery_path = find_tracequery_hash(s);
+      tracequery_path = find_tracequery_hash(s, header);
       if (!tracequery_path.empty())
         {
           int fd = open(tracequery_path.c_str(), O_RDONLY);
@@ -5709,19 +5715,23 @@ tracepoint_builder::init_dw(systemtap_session& s)
             {
               if (s.verbose > 2)
                 clog << "Pass 2: using cached " << tracequery_path << endl;
-
-              dw = new dwflpp(s, tracequery_path, false);
               close(fd);
-              return true;
+              return tracequery_path;
             }
         }
     }
 
   // no cached module, time to make it
+
+  size_t root_pos = header.rfind("/include/");
+  string short_header = (root_pos != string::npos) ?
+    header.substr(root_pos + 9) : header;
+
   string tracequery_ko;
-  int rc = make_tracequery(s, tracequery_ko, tracepoint_extra_headers());
+  int rc = make_tracequery(s, tracequery_ko, short_header,
+                           tracepoint_extra_headers());
   if (rc != 0)
-    return false;
+    return "";
 
   if (s.use_cache)
     {
@@ -5734,8 +5744,52 @@ tracepoint_builder::init_dw(systemtap_session& s)
         cerr << "Copy failed (\"" << tracequery_ko << "\" to \""
              << tracequery_path << "\"): " << strerror(errno) << endl;
     }
+  return tracequery_ko;
+}
+
+
+bool
+tracepoint_builder::init_dw(systemtap_session& s)
+{
+  if (dw != NULL)
+    return true;
+
+  vector<string> tracequery_modules;
+
+  glob_t trace_glob;
+  string globs[] = {
+      "/include/trace/*.h",
+      "/include/trace/events/*.h",
+      "/source/include/trace/*.h",
+      "/source/include/trace/events/*.h",
+  };
+  for (unsigned z = 0; z < sizeof(globs) / sizeof(globs[0]); z++)
+    {
+      string glob_str(s.kernel_build_tree + globs[z]);
+      glob(glob_str.c_str(), 0, NULL, &trace_glob);
+      for (unsigned i = 0; i < trace_glob.gl_pathc; ++i)
+        {
+          string header(trace_glob.gl_pathv[i]);
+
+          // filter out a few known "internal-only" headers
+          if (header.find("/ftrace.h") != string::npos)
+            continue;
+          if (header.find("/trace_events.h") != string::npos)
+            continue;
+          if (header.find("_event_types.h") != string::npos)
+            continue;
+
+          string tracequery_path = get_tracequery_module(s, header);
+          if (!tracequery_path.empty())
+            tracequery_modules.push_back(tracequery_path);
+        }
+      globfree(&trace_glob);
+    }
+
+  // TODO: consider other sources of tracepoint headers too, like from
+  // a command-line parameter or some environment or .systemtaprc
 
-  dw = new dwflpp(s, tracequery_ko, false);
+  dw = new dwflpp(s, tracequery_modules);
   return true;
 }