2005-08-01 Frank Ch. Eigler <fche@elastic.org>

	* elaborate.cxx (derive_probes, semantic_pass_symbols): Improve
	error message specificity.
	* translate.cxx (emit_module_init): Compact partial registration
	recovery code.
	(emit_module_exit): Invert deregistration sequence.
	* testsuite/buildok/four.stp: Some module() test case.

5 files changed, 68 insertions, 55 deletions

diff --git a/ChangeLog b/ChangeLog
index fc014c0c..0d0c110c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
 2005-08-01  Frank Ch. Eigler  <fche@elastic.org>
 
+	* elaborate.cxx (derive_probes, semantic_pass_symbols): Improve
+	error message specificity.
+	* translate.cxx (emit_module_init): Compact partial registration
+	recovery code.
+	(emit_module_exit): Invert deregistration sequence.
+	* testsuite/buildok/four.stp: Some module() test case.
+	
+2005-08-01  Frank Ch. Eigler  <fche@elastic.org>
+
 	* elaborate.cxx (derive_probes): Print error if results empty.
 	* tapsets.cxx (dwflpp_assert): Handle positive RCs, which likely
 	came from errno.
diff --git a/elaborate.cxx b/elaborate.cxx
index 90de32c7..54d102ec 100644
--- a/elaborate.cxx
+++ b/elaborate.cxx
@@ -359,27 +359,35 @@ symresolution_info::derive_probes (match_node * root,
       map<string, literal *> param_map;
       vector<probe *> re_expand;
 
-      derived_probe_builder * builder = 
-	root->find_builder(loc->components, 0, param_vec);
-
-      if (!builder)
-	throw semantic_error ("no match for probe point family");
-
-      param_vec_to_map(param_vec, param_map);
-
-      unsigned num_atbegin = dps.size();
-      builder->build(session, p, loc, param_map, re_expand, dps);
-
-      // Recursively expand any further-expanding results
-      if (!re_expand.empty())
-	{
-	  for (unsigned j = 0; j < re_expand.size(); ++j)
-	    derive_probes(root, re_expand[j], dps);
-	}
-
-      unsigned num_atend = dps.size();
-      if (num_atbegin == num_atend) // nothing new derived!
-        throw semantic_error ("no match for probe point");
+      try
+        {
+          derived_probe_builder * builder = 
+            root->find_builder(loc->components, 0, param_vec);
+          
+          if (!builder)
+            throw semantic_error ("no match for probe point family");
+          
+          param_vec_to_map(param_vec, param_map);
+          
+          unsigned num_atbegin = dps.size();
+          builder->build(session, p, loc, param_map, re_expand, dps);
+          
+          // Recursively expand any further-expanding results
+          if (!re_expand.empty())
+            {
+              for (unsigned j = 0; j < re_expand.size(); ++j)
+                derive_probes(root, re_expand[j], dps);
+            }
+          
+          unsigned num_atend = dps.size();
+          if (num_atbegin == num_atend) // nothing new derived!
+            throw semantic_error ("no match for probe point");
+        }
+      catch (const semantic_error& e)
+        {
+          session.print_error (e);
+          cerr << "         while: resolving probe point " << *loc << endl;
+        }
     }
 }
 
@@ -612,20 +620,9 @@ semantic_pass_symbols (systemtap_session& s)
           probe* p = dome->probes [i];
           vector<derived_probe*> dps;
 
-          try
-            {
-              // much magic happens here: probe alias expansion,
-              // provider identification
-              sym.derive_probes (s.pattern_root, p, dps);
-            }
-          catch (const semantic_error& e)
-            {
-              cerr << "while resolving probe point list:" << endl;
-              for (unsigned k=0; k<p->locations.size(); k++)
-                cerr << "  " << *p->locations[k]  << endl;
-              s.print_error (e);
-              // dps.erase (dps.begin(), dps.end());
-            }
+          // much magic happens here: probe alias expansion,
+          // provider identification
+          sym.derive_probes (s.pattern_root, p, dps);
 
           for (unsigned j=0; j<dps.size(); j++)
             {
@@ -645,7 +642,7 @@ semantic_pass_symbols (systemtap_session& s)
             }
         }
     }
-
+  
   return s.num_errors; // all those print_error calls
 }
 
diff --git a/tapsets.cxx b/tapsets.cxx
index a51ce4e4..25be7166 100644
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -1275,6 +1275,7 @@ dwarf_derived_probe::register_patterns(match_node * root)
   // .process("foo")
 
   register_function_and_statement_variants(root->bind(TOK_KERNEL), dw);
+  // XXX: may need to disable these for 2005-08 release
   register_function_and_statement_variants(root->bind_str(TOK_MODULE), dw);
   register_function_and_statement_variants(root->bind_str(TOK_PROCESS), dw);
 }
@@ -1329,7 +1330,6 @@ dwarf_derived_probe::emit_registrations (translator_output* o, unsigned probenum
 void
 dwarf_derived_probe::emit_deregistrations (translator_output* o, unsigned probenum)
 {
-  o->newline();
   o->newline() << "unregister_kprobe (& "
 	       << probe_entry_struct_kprobe_name(probenum)
 	       << ");";
diff --git a/testsuite/buildok/four.stp b/testsuite/buildok/four.stp
new file mode 100755
index 00000000..8ceff188
--- /dev/null
+++ b/testsuite/buildok/four.stp
@@ -0,0 +1,6 @@
+#! stap -p4
+
+probe module("ext3").function("*")
+{
+	log ("ext3 fn")
+}
diff --git a/translate.cxx b/translate.cxx
index 78a75d91..85dfc925 100644
--- a/translate.cxx
+++ b/translate.cxx
@@ -633,15 +633,8 @@ c_unparser::emit_module_init ()
       // We need to deregister any already probes set up - this is
       // essential for kprobes.
       if (i > 0)
-        // NB: This may be an END probe.  It may refuse to run
-        // if the session_state was ERRORed
-        for (unsigned j=i; j>0; j--)
-          {
-            o->newline() << "/* deregister " << j-1 << " */";
-            session->probes[j-1]->emit_deregistrations (o, j-1);
-          }
-      // XXX: ignore rc
-      o->newline() << "goto out;";
+        o->newline() << "goto unregister_" << (i-1) << ";";
+
       o->newline(-1) << "}";
     }
 
@@ -652,10 +645,22 @@ c_unparser::emit_module_init ()
   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->indent(-1);
+  o->newline(-1) << "goto out;";
+
+  // recovery code for partially successful registration (rc != 0)
+  o->newline();
+  for (int i=session->probes.size()-2; i >= 0; i--) // NB: -2
+    {
+      o->newline(-1) << "unregister_" << i << ":";
+      o->indent(1);
+      session->probes[i]->emit_deregistrations (o, i);
+      // NB: This may be an END probe.  It will refuse to run
+      // if the session_state was ERRORed.
+    }  
+  o->newline();
 
-  o->newline() << "out:";
-  o->newline() << "return rc;";
+  o->newline(-1) << "out:";
+  o->newline(1) << "return rc;";
   o->newline(-1) << "}" << endl;
 }
 
@@ -688,15 +693,11 @@ c_unparser::emit_module_exit ()
   // o->newline(-1) << "if (holdon) msleep (5);";
   o->newline(-1) << "} while (holdon);";
   o->newline(-1);
-
   // XXX: might like to have an escape hatch, in case some probe is
   // genuinely stuck
 
-  for (unsigned i=0; i<session->probes.size(); i++)
-    {
-      o->newline() << "/* deregister " << i << " */";
-      session->probes[i]->emit_deregistrations (o, i);
-    }
+  for (int i=session->probes.size()-1; i>=0; i--)
+    session->probes[i]->emit_deregistrations (o, i);
 
   for (unsigned i=0; i<session->globals.size(); i++)
     {