2005-09-27 Frank Ch. Eigler <fche@elastic.org>

	PR 1368.
	* translate.cxx (emit_common_header): Move some MAX* definitions out ...
	(translate_pass): ... to here.  Fix probe_start API impedance mismatch.
	(emit_module_init, exit): Tolerate registration errors, such as absence
	of kretprobes support.

diff --git a/ChangeLog b/ChangeLog
index 7025ddfa2316584d34f1f09ed74c9e671e02afcc..04062c05c6c751dde3c41b242a2f0d82f6253f6f 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
 2005-09-27  Frank Ch. Eigler  <fche@elastic.org>
 
+       PR 1368.
+       * translate.cxx (emit_common_header): Move some MAX* definitions out ...
+       (translate_pass): ... to here.  Fix probe_start API impedance mismatch.
+       (emit_module_init, exit): Tolerate registration errors, such as absence
+       of kretprobes support.
+
+2005-09-27  Frank Ch. Eigler  <fche@elastic.org>
        PR 1311.
        * tapsets.cxx (target_variable_flavour_calculating_visitor::
        visit_target_symbol): Print verbose error.

diff --git a/tapsets.cxx b/tapsets.cxx
index d3194d8f9721885491aad6bb94617d8ab9ecbcde..7eaf11f4654ef33f58a14d4997d75b78af3a35ab 100644 (file)
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -2307,11 +2307,15 @@ dwarf_derived_probe::emit_registrations (translator_output* o,
 
   if (has_return)
     {
+      o->newline() << "#ifdef ARCH_SUPPORTS_KRETPROBES";
       o->newline() << probe_name << ".handler = &" << func_name << ";";
       o->newline() << probe_name << ".maxactive = 1;";
       // XXX: pending PR 1289
       // o->newline() << probe_name << ".kp_fault_handler = &stap_kprobe_fault_handler;";
       o->newline() << "rc = register_kretprobe (&(" << probe_name << "));";
+      o->newline() << "#else";
+      o->newline() << "rc = 1;";
+      o->newline() << "#endif";
     }
   else
     {
@@ -2328,7 +2332,13 @@ dwarf_derived_probe::emit_registrations (translator_output* o,
   o->newline() << "if (unlikely (rc)) while (--i > 0)";
   o->indent(1);
   if (has_return)
-    o->newline() << "unregister_kretprobe (&(" << probe_name << "));";
+    {
+      o->newline() << "#ifdef ARCH_SUPPORTS_KRETPROBES";
+      o->newline() << "unregister_kretprobe (&(" << probe_name << "));";
+      o->newline() << "#else";
+      o->newline() << ";";
+      o->newline() << "#endif";
+    }
   else
     o->newline() << "unregister_kprobe (&(" << probe_name << "));";
   o->newline(-2) << "}";
@@ -2344,7 +2354,13 @@ dwarf_derived_probe::emit_deregistrations (translator_output* o, unsigned proben
   string probe_name = struct_kprobe_array_name(probenum) + "[i]";
   o->indent(1);
   if (has_return)
-    o->newline() << "unregister_kretprobe (&(" << probe_name << "));"; 
+    {
+      o->newline() << "#ifdef ARCH_SUPPORTS_KRETPROBES";
+      o->newline() << "unregister_kretprobe (&(" << probe_name << "));";
+      o->newline() << "#else";
+      o->newline() << ";";
+      o->newline() << "#endif";
+    }
   else
     o->newline() << "unregister_kprobe (&(" << probe_name << "));";
   o->indent(-1);
@@ -2388,10 +2404,13 @@ dwarf_derived_probe::emit_probe_entries (translator_output* o,
   assert(locations.size() == probe_points.size());
 
   if (has_return)
+    {
+      o->newline() << "#ifdef ARCH_SUPPORTS_KRETPROBES";
       o->newline() << "static struct kretprobe "
                    << probe_array
                   << "[" << probe_points.size() << "]"
                    << "= {";
+    }
   else
       o->newline() << "static struct kprobe "
                    << probe_array
@@ -2412,6 +2431,9 @@ dwarf_derived_probe::emit_probe_entries (translator_output* o,
     }
   o->newline(-1) << "};";
 
+  if (has_return)
+    o->newline() << "#endif /* ARCH_SUPPORTS_KRETPROBES */";
+
   o->newline();
 
   // This is somewhat gross, but it should work: we allocate a
@@ -2443,6 +2465,8 @@ dwarf_derived_probe::emit_probe_entries (translator_output* o,
   // Construct a single entry function, and a struct kprobe pointing into
   // the entry function. The entry function will call the probe function.
   o->newline();
+  if (has_return)
+    o->newline() << "#ifdef ARCH_SUPPORTS_KRETPROBES";
   o->newline() << "static int ";
   o->newline() << function_name(probenum) << " (";
   if (has_return)
@@ -2492,6 +2516,8 @@ dwarf_derived_probe::emit_probe_entries (translator_output* o,
   o->newline() << "atomic_dec (& c->busy);";
   o->newline() << "return 0;";
   o->newline(-1) << "}" << endl;
+  if (has_return)
+    o->newline() << "#endif /* ARCH_SUPPORTS_KRETPROBES */";
 
   o->newline();
 }

diff --git a/translate.cxx b/translate.cxx
index cc1d97bff8f96ec0885499d4203e1f3aedd4bcb4..cc4b58c596fac31ccf5423d999359e379788dfcb 100644 (file)
--- a/translate.cxx
+++ b/translate.cxx
@@ -577,14 +577,6 @@ c_unparser::emit_common_header ()
 {
   // XXX: tapsets.cxx should be able to add additional definitions
 
-  o->newline() << "#include \"loc2c-runtime.h\" ";
-  o->newline() << "#define MAXNESTING 30";
-  o->newline() << "#define MAXCONCURRENCY NR_CPUS";
-  o->newline() << "#define MAXSTRINGLEN 128";
-  o->newline() << "#define MAXTRYLOCK 20";
-  o->newline() << "#define MAXACTION 1000";
-  o->newline() << "#define MAXMAPENTRIES 2048";
-  o->newline();
   o->newline() << "typedef char string_t[MAXSTRINGLEN];";
   o->newline() << "typedef struct { } stats_t;";
   o->newline();
@@ -743,6 +735,12 @@ c_unparser::emit_module_init ()
       // probe from attempting to run.
       o->newline(1) << "atomic_set (&session_state, STAP_SESSION_ERROR);";
 
+      // XXX: would be nice to print failing probe point
+      o->newline() << "_stp_error (\"probe " << i << " registration failed"
+                   << ", rc=%d, %s\\n\", rc, "
+                   << lex_cast_qstring(*session->probes[i]->tok)
+                   << ");";
+
       // We need to deregister any already probes set up - this is
       // essential for kprobes.
       if (i > 0)
@@ -757,10 +755,13 @@ c_unparser::emit_module_init ()
   // otherwise act like probe insertion was a success.
   o->newline() << "if (atomic_read (&session_state) == STAP_SESSION_STARTING)";
   o->newline(1) << "atomic_set (&session_state, STAP_SESSION_RUNNING);";
-  // XXX: else maybe set anyrc and thus return a failure from module_init?
   o->newline(-1) << "goto out;";
 
-  // recovery code for partially successful registration (rc != 0)
+  // Recovery code for partially successful registration (rc != 0)
+  // XXX: Do we need to delay here to ensure any triggered probes have
+  // terminated?  Probably not much, as they should all test for
+  // SESSION_STARTING state right at the top and return.  ("begin"
+  // probes don't count, as they return synchronously.)
   o->newline();
   for (int i=session->probes.size()-2; i >= 0; i--) // NB: -2
     {
@@ -772,6 +773,15 @@ c_unparser::emit_module_init ()
     }  
   o->newline();
 
+  // If any registrations failed, we will need to deregister the globals,
+  // as this is our only chance.
+  for (unsigned i=0; i<session->globals.size(); i++)
+    {
+      vardecl* v = session->globals[i];      
+      if (v->index_types.size() > 0)
+       o->newline() << getmap (v).fini();
+    }
+
   o->newline(-1) << "out:";
   o->newline(1) << "return rc;";
   o->newline(-1) << "}" << endl;
@@ -785,6 +795,13 @@ c_unparser::emit_module_exit ()
   o->newline() << "void systemtap_module_exit () {";
   // rc?
   o->newline(1) << "int holdon;";
+
+  // If we aborted startup, then everything has been cleaned up already, and
+  // module_exit shouldn't even have been called.  But since it might be, let's
+  // beat a hasty retreat to avoid double uninitialization.
+  o->newline() << "if (atomic_read (&session_state) == STAP_SESSION_STARTING)";
+  o->newline(1) << "return;";
+  o->indent(-1);
   
   o->newline() << "if (atomic_read (&session_state) == STAP_SESSION_RUNNING)";
   // NB: only other valid state value is ERROR, in which case we don't 
@@ -2356,18 +2373,30 @@ translate_pass (systemtap_session& s)
 
   try
     {
+      // This is at the very top of the file.
       s.op->line() << "#define TEST_MODE " << (s.test_mode ? 1 : 0) << endl;
 
+      s.op->newline() << "#define MAXNESTING 30";
+      s.op->newline() << "#define MAXCONCURRENCY NR_CPUS";
+      s.op->newline() << "#define MAXSTRINGLEN 128";
+      s.op->newline() << "#define MAXTRYLOCK 20";
+      s.op->newline() << "#define MAXACTION 1000";
+      s.op->newline() << "#define MAXMAPENTRIES 2048";
+
+      // impedance mismatch
+      s.op->newline() << "#define STP_STRING_SIZE MAXSTRINGLEN";
+      s.op->newline() << "#define STP_NUM_STRINGS 1";
+
       s.op->newline() << "#if TEST_MODE";
       s.op->newline() << "#include \"runtime.h\"";
       s.op->newline() << "#else";
-      s.op->newline() << "#define STP_NUM_STRINGS 1";
       s.op->newline() << "#include \"runtime.h\"";
       s.op->newline() << "#include \"current.c\"";
       s.op->newline() << "#include \"stack.c\"";
       s.op->newline() << "#include <linux/string.h>";
       s.op->newline() << "#include <linux/timer.h>";
       s.op->newline() << "#endif";
+      s.op->newline() << "#include \"loc2c-runtime.h\" ";
 
       s.up->emit_common_header ();
 
@@ -2418,9 +2447,9 @@ translate_pass (systemtap_session& s)
       s.op->newline() << "#else";
 
       s.op->newline();
-      // XXX
+      // XXX impedance mismatch
       s.op->newline() << "int probe_start () {";
-      s.op->newline(1) << "return systemtap_module_init ();";
+      s.op->newline(1) << "return systemtap_module_init () ? -1 : 0;";
       s.op->newline(-1) << "}";
       s.op->newline();
       s.op->newline() << "void probe_exit () {";