2008-02-13 Dave Brolley <brolley@redhat.com>

        PR5609
        * staptree.h (probe::collect_derivation_chain): Now takes vector<probe*>.
        (probe::get_alias): New virtual method.
        * elaborate.h (derived_probe::collect_derivation_chain): Now takes vector<probe*>.
        * staptree.cxx (probe::collect_derivation_chain): Now takes vector<probe*>. Don't
        cast 'this' to (derived_probe*).
        * elaborate.cxx (derived_probe::collect_derivation_chain): Now takes vector<probe*>.
        (alias_derived_probe::get_alias): New virtual method.
        (alias_derived_probe::alias): New member.
        (alias_expansion_builder::build): Call checkForRecursiveExpansion and emit a
        diagnostic if recursion is detected. Pass alias to constructor of
        alias_derived_probe.
        (alias_expansion_builder::checkForRecursiveExpansion): New method.
        * coveragedb.cxx: Pass vector<probe*> on all calls to collect_derivation_chain.

9 files changed, 82 insertions, 11 deletions

diff --git a/ChangeLog b/ChangeLog
index 4e306158..0695df39 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,20 @@
+2008-02-13  Dave Brolley  <brolley@redhat.com>
+
+	PR5609
+	* staptree.h (probe::collect_derivation_chain): Now takes vector<probe*>.
+	(probe::get_alias): New virtual method.
+	* elaborate.h (derived_probe::collect_derivation_chain): Now takes vector<probe*>.
+	* staptree.cxx (probe::collect_derivation_chain): Now takes vector<probe*>. Don't
+	cast 'this' to (derived_probe*).
+	* elaborate.cxx (derived_probe::collect_derivation_chain): Now takes vector<probe*>.
+	(alias_derived_probe::get_alias): New virtual method.
+	(alias_derived_probe::alias): New member.
+	(alias_expansion_builder::build): Call checkForRecursiveExpansion and emit a
+	diagnostic if recursion is detected. Pass alias to constructor of
+	alias_derived_probe.
+	(alias_expansion_builder::checkForRecursiveExpansion): New method.
+	* coveragedb.cxx: Pass vector<probe*> on all calls to collect_derivation_chain.
+
 2008-02-12  Frank Ch. Eigler  <fche@elastic.org>
 
 	PR 4393
diff --git a/coveragedb.cxx b/coveragedb.cxx
index 6def56e7..8258b359 100644
--- a/coveragedb.cxx
+++ b/coveragedb.cxx
@@ -28,7 +28,7 @@ void print_coverage_info(systemtap_session &s)
   clog << "---- used probes-----" << endl;
   for (unsigned i=0; i<s.probes.size(); i++) {
     // walk through the chain of probes
-    vector<derived_probe*> used_probe_list;
+    vector<probe*> used_probe_list;
     s.probes[i]->collect_derivation_chain(used_probe_list);
     for (unsigned j=0; j<used_probe_list.size(); ++j) {
       for (unsigned k=0; k< used_probe_list[j]->locations.size(); ++k)
@@ -51,7 +51,7 @@ void print_coverage_info(systemtap_session &s)
   clog << "---- unused probes----- " << endl;
   for (unsigned i=0; i<s.unused_probes.size(); i++) {
     // walk through the chain of probes
-    vector<derived_probe*> unused_probe_list;
+    vector<probe*> unused_probe_list;
     s.unused_probes[i]->collect_derivation_chain(unused_probe_list);
     for (unsigned j=0; j<unused_probe_list.size(); ++j) {
       for (unsigned k=0; k< unused_probe_list[j]->locations.size(); ++k)
@@ -197,7 +197,7 @@ sql_update_used_probes(sqlite3 *db, systemtap_session &s)
   // update database used probes
   for (unsigned i=0; i<s.probes.size(); i++) {
     // walk through the chain of probes
-    vector<derived_probe*> used_probe_list;
+    vector<probe*> used_probe_list;
     s.probes[i]->collect_derivation_chain(used_probe_list);
     for (unsigned j=0; j<used_probe_list.size(); ++j) {
 	    for (unsigned k=0; k< used_probe_list[j]->locations.size(); ++k){
@@ -240,7 +240,7 @@ sql_update_unused_probes(sqlite3 *db, systemtap_session &s)
   // update database unused probes
   for (unsigned i=0; i<s.unused_probes.size(); i++) {
     // walk through the chain of probes
-    vector<derived_probe*> unused_probe_list;
+    vector<probe*> unused_probe_list;
     s.unused_probes[i]->collect_derivation_chain(unused_probe_list);
     for (unsigned j=0; j<unused_probe_list.size(); ++j) {
 	    for (unsigned k=0; k< unused_probe_list[j]->locations.size(); ++k) {
diff --git a/elaborate.cxx b/elaborate.cxx
index 445c7ff0..bc0d1489 100644
--- a/elaborate.cxx
+++ b/elaborate.cxx
@@ -112,7 +112,7 @@ derived_probe::printsig_nested (ostream& o) const
 
 
 void
-derived_probe::collect_derivation_chain (std::vector<derived_probe*> &probes_list)
+derived_probe::collect_derivation_chain (std::vector<probe*> &probes_list)
 {
   probes_list.push_back(this);
   base->collect_derivation_chain(probes_list);
@@ -432,7 +432,8 @@ match_node::build_no_more (systemtap_session& s)
 
 struct alias_derived_probe: public derived_probe
 {
-  alias_derived_probe (probe* base, probe_point *l): derived_probe (base, l) {}
+  alias_derived_probe (probe* base, probe_point *l, const probe_alias *a):
+    derived_probe (base, l), alias(a) {}
 
   void upchuck () { throw semantic_error ("inappropriate", this->tok); }
 
@@ -441,6 +442,11 @@ struct alias_derived_probe: public derived_probe
   // systemtap_session.probes
 
   void join_group (systemtap_session&) { upchuck (); }
+
+  virtual const probe_alias *get_alias () const { return alias; }
+
+private:
+  const probe_alias *alias; // Used to check for recursion
 };
 
 
@@ -460,11 +466,20 @@ alias_expansion_builder
 		     std::map<std::string, literal *> const &,
 		     vector<derived_probe *> & finished_results)
   {
+    // Don't build the alias expansion if infinite recursion is detected.
+    if (checkForRecursiveExpansion (use)) {
+      stringstream msg;
+      msg << "Recursive loop in alias expansion of " << *location  << " at " << location->tok->location;
+      // semantic_errors thrown here are ignored.
+      sess.print_error (semantic_error (msg.str()));
+      return;
+    }
+
     // We're going to build a new probe and wrap it up in an
     // alias_expansion_probe so that the expansion loop recognizes it as
     // such and re-expands its expansion.
     
-    alias_derived_probe * n = new alias_derived_probe (use, location /* soon overwritten */);
+    alias_derived_probe * n = new alias_derived_probe (use, location /* soon overwritten */, this->alias);
     n->body = new block();
 
     // The new probe gets the location list of the alias (with incoming condition joined)
@@ -508,6 +523,26 @@ alias_expansion_builder
     
     derive_probes (sess, n, finished_results, location->optional);
   }
+
+  bool checkForRecursiveExpansion (probe *use)
+  {
+    // Collect the derivation chain of this probe.
+    vector<probe*>derivations;
+    use->collect_derivation_chain (derivations);
+
+    // Check all probe points in the alias expansion against the currently-being-expanded probe point
+    // of each of the probes in the derivation chain, looking for a match. This
+    // indicates infinite recursion.
+    // The first element of the derivation chain will be the derived_probe representing 'use', so
+    // start the search with the second element.
+    assert (derivations.size() > 0);
+    assert (derivations[0] == use);
+    for (unsigned d = 1; d < derivations.size(); ++d) {
+      if (use->get_alias() == derivations[d]->get_alias())
+	return true; // recursion detected
+    }
+    return false;
+  }
 };
 
 
diff --git a/elaborate.h b/elaborate.h
index b478b6d8..607f8689 100644
--- a/elaborate.h
+++ b/elaborate.h
@@ -118,7 +118,7 @@ struct derived_probe: public probe
   virtual probe_point* sole_location () const;
   virtual void printsig (std::ostream &o) const;
   void printsig_nested (std::ostream &o) const;
-  virtual void collect_derivation_chain (std::vector<derived_probe*> &probes_list);
+  virtual void collect_derivation_chain (std::vector<probe*> &probes_list);
 
   virtual void emit_probe_context_vars (translator_output*) {}
   // From within unparser::emit_common_header, add any extra variables
diff --git a/staptree.cxx b/staptree.cxx
index 61a931f8..36ef04f5 100644
--- a/staptree.cxx
+++ b/staptree.cxx
@@ -903,9 +903,9 @@ void probe::printsig (ostream& o) const
 
 
 void
-probe::collect_derivation_chain (std::vector<derived_probe*> &probes_list)
+probe::collect_derivation_chain (std::vector<probe*> &probes_list)
 {
-  probes_list.push_back((derived_probe*)this);
+  probes_list.push_back(this);
 }
 
 
diff --git a/staptree.h b/staptree.h
index de5b3726..8235a535 100644
--- a/staptree.h
+++ b/staptree.h
@@ -595,7 +595,8 @@ struct probe
   probe ();
   void print (std::ostream& o) const;
   virtual void printsig (std::ostream &o) const;
-  virtual void collect_derivation_chain (std::vector<derived_probe*> &probes_list);
+  virtual void collect_derivation_chain (std::vector<probe*> &probes_list);
+  virtual const probe_alias *get_alias () const { return 0; }
   virtual probe* basest () { return this; }
   virtual ~probe() {}
   bool privileged;
diff --git a/testsuite/semko/fortyfour.stp b/testsuite/semko/fortyfour.stp
new file mode 100755
index 00000000..8f3caeeb
--- /dev/null
+++ b/testsuite/semko/fortyfour.stp
@@ -0,0 +1,5 @@
+#! stap -p2
+
+probe foo = bar {}
+probe bar = foo {}
+probe foo {}
diff --git a/testsuite/semko/fortythree.stp b/testsuite/semko/fortythree.stp
new file mode 100755
index 00000000..cbf52a91
--- /dev/null
+++ b/testsuite/semko/fortythree.stp
@@ -0,0 +1,4 @@
+#! stap -p2
+
+probe foo.bar = foo.* { }
+probe foo.* { }
diff --git a/testsuite/semok/thirty.stp b/testsuite/semok/thirty.stp
new file mode 100755
index 00000000..27612ed0
--- /dev/null
+++ b/testsuite/semok/thirty.stp
@@ -0,0 +1,9 @@
+#! stap -p2
+
+probe foo.a.one = foo.b, foo.c {} // not recursive
+probe foo.a.two = foo.c, foo.b {} // not recursive
+probe foo.b = foo.c {}
+probe foo.c = begin {}
+probe foo.a.one {}
+probe foo.a.two {}
+