BZ 6954: Automagic printing for ++ operations

BZ 6954: Automagic printing for ++ operations

[Rajan Arora]

[-- Attachment #1: Type: text/plain, Size: 396 bytes --]

Hello everyone,

  Attached is a patch for the BZ 6954 bug - automagic printing for ++ 
operations on global variables. A few tests in the testsuite seem to 
have broken after applying this patch but I suspect that is mostly 
because of the automagic printing in place and I am working on them. 
Meanwhile, please have a look and feel free to reply with any 
suggestions/feedback.

Cheers
-Rajan

[-- Attachment #2: 6954-a.patch --]
[-- Type: text/x-patch, Size: 7002 bytes --]

diff --git a/ChangeLog b/ChangeLog
index ac29d2b..7eede1d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,11 +1,25 @@
-2008-10-20	Elliott Baron  <ebaron@redhat.com>
+2008-10-22  Rajan Arora  <rarora@redhat.com>
+
+	* staptree.h (struct varuse_collecting_visitor): New members
+	read_spl, modify_seen, print_seen.
+	(varuse_collecting_visitor::varuse_collecting_visitor): 
+	Initialize new members.
+	* staptree.cxx (varuse_collecting_visitor::visit_print_format):
+	Update flag.
+	(varuse_collecting_visitor::visit_assignment): Update flag.
+	(varuse_collecting_visitor::visit_symbol): Update read_spl.
+	(varuse_collecting_visitor::visit_pre_crement): Update flag.
+	(varuse_collecting_visitor::visit_post_crement): Update flag.
+	* elaborate.cxx (add_global_var_display): Check read_spl.
+
+2008-10-20  Elliott Baron  <ebaron@redhat.com>
 
 	PR6851
 	* elaborate.cxx (typeresolution_info::visit_print_format): add case
 	for conv_char.
 	* staptree.cxx (print_format::components_to_string): add case for
 	conv_char.
-	(print_format::string_to_components): add parsing for "c"	conversion.
+	(print_format::string_to_components): add parsing for "c" conversion.
 	* staptree.h (enum conversion_type): add conv_char member.
 
 2008-10-17  Frank Ch. Eigler  <fche@elastic.org>
diff --git a/elaborate.cxx b/elaborate.cxx
index e4251f2..0ed4596 100644
--- a/elaborate.cxx
+++ b/elaborate.cxx
@@ -1161,8 +1161,9 @@ void add_global_var_display (systemtap_session& s)
   for (unsigned g=0; g < s.globals.size(); g++)
     {
       vardecl* l = s.globals[g];
-      if (vut.read.find (l) != vut.read.end()
-          || vut.written.find (l) == vut.written.end())
+      if ((vut.read.find (l) != vut.read.end()
+	   && vut.read_spl.find (l) == vut.read_spl.end())
+	  || vut.written.find (l) == vut.written.end())
 	continue;
 
       print_format* pf = new print_format;
diff --git a/staptree.cxx b/staptree.cxx
index 38166c5..236584c 100644
--- a/staptree.cxx
+++ b/staptree.cxx
@@ -1692,7 +1692,9 @@ varuse_collecting_visitor::visit_print_format (print_format* e)
   if (e->print_to_stream)
     embedded_seen = true; // a proxy for "has unknown side-effects"
 
+  print_seen = true; //set a flag for ::visit_symbol
   functioncall_traversing_visitor::visit_print_format (e);
+  print_seen = false;
 }
 
 
@@ -1710,8 +1712,11 @@ varuse_collecting_visitor::visit_assignment (assignment *e)
     {
       expression* last_lrvalue = current_lrvalue;
       current_lrvalue = e->left; // leave a mark for ::visit_symbol
+      if (e->op.find ('=') != string::npos)
+	modify_seen = true; //set a flag for ::visit_symbol
       functioncall_traversing_visitor::visit_assignment (e);
       current_lrvalue = last_lrvalue;
+      modify_seen = false;
     }
 }
 
@@ -1733,6 +1738,8 @@ varuse_collecting_visitor::visit_symbol (symbol *e)
   if (current_lvalue == e || current_lrvalue == e)
     {
       written.insert (e->referent);
+      if (modify_seen && !print_seen)
+	read_spl.insert (e->referent);
       // clog << "write ";
     }
   if (current_lvalue != e || current_lrvalue == e)
@@ -1790,8 +1797,10 @@ varuse_collecting_visitor::visit_pre_crement (pre_crement *e)
 {
   expression* last_lrvalue = current_lrvalue;
   current_lrvalue = e->operand; // leave a mark for ::visit_symbol
+  modify_seen = true; //set a flag for ::visit_symbol
   functioncall_traversing_visitor::visit_pre_crement (e);
   current_lrvalue = last_lrvalue;
+  modify_seen = false;
 }
 
 void
@@ -1799,8 +1808,10 @@ varuse_collecting_visitor::visit_post_crement (post_crement *e)
 {
   expression* last_lrvalue = current_lrvalue;
   current_lrvalue = e->operand; // leave a mark for ::visit_symbol
+  modify_seen = true; //set a flag for ::visit_symbol
   functioncall_traversing_visitor::visit_post_crement (e);
   current_lrvalue = last_lrvalue;
+  modify_seen = false;
 }
 
 void
diff --git a/staptree.h b/staptree.h
index 40be8e0..82d790d 100644
--- a/staptree.h
+++ b/staptree.h
@@ -742,11 +742,16 @@ struct varuse_collecting_visitor: public functioncall_traversing_visitor
 {
   std::set<vardecl*> read;
   std::set<vardecl*> written;
+  std::set<vardecl*> read_spl;  //Modified but not printed symbols
   bool embedded_seen;
+  bool modify_seen;
+  bool print_seen;
   expression* current_lvalue;
   expression* current_lrvalue;
   varuse_collecting_visitor():
     embedded_seen (false),
+    modify_seen (false),
+    print_seen (false),
     current_lvalue(0),
     current_lrvalue(0) {}
   void visit_embeddedcode (embeddedcode *s);
diff --git a/testsuite/ChangeLog b/testsuite/ChangeLog
index 2ec0fb0..34c5821 100644
--- a/testsuite/ChangeLog
+++ b/testsuite/ChangeLog
@@ -1,4 +1,11 @@
-2008-10-20	Elliott Baron  <ebaron@redhat.com>
+2008-10-22  Rajan Arora  <rarora@redhat.com>
+
+	* systemtap.base/global_end.stp: Added pre_crement,
+	post_crement and assignment modify operations.
+	* systemtap.base/global_end.exp: Allow for new automatic
+	printing.
+
+2008-10-20  Elliott Baron  <ebaron@redhat.com>
 
 	PR6851
 	* systemtap.printf/char1.exp: New test.
diff --git a/testsuite/systemtap.base/global_end.exp b/testsuite/systemtap.base/global_end.exp
index b6b9fd3..90b83f3 100644
--- a/testsuite/systemtap.base/global_end.exp
+++ b/testsuite/systemtap.base/global_end.exp
@@ -9,16 +9,17 @@ set ok 0
 expect {
     -timeout 180
     -re {one,0x1.*one,0x2.*two,0x1.*two,0x2} { incr ok; exp_continue }
-    -re {alpha."two".2.=0x4} { incr ok; exp_continue }
+    -re {alpha."two".2.=0x5} { incr ok; exp_continue }
     -re {alpha."two".1.=0x3} { incr ok; exp_continue }
-    -re {alpha."one".2.=0x2} { incr ok; exp_continue }
-    -re {alpha."one".1.=0x1} { incr ok; exp_continue }
+    -re {alpha."one".2.=0x4} { incr ok; exp_continue }
+    -re {alpha."one".1.=0x2} { incr ok; exp_continue }
     -re {gamma="abcdefghijklmnopqrstuvwxyz"} { incr ok; exp_continue }
     -re {iota."two".="twelve"} { incr ok; exp_continue }
     -re {iota."one".="eleven"} { incr ok; exp_continue }
     -re {epsilon."one",1. @count=0x4 @min=0x1 @max=0x4 @sum=0xa @avg=0x2} { incr ok; exp_continue }
     -re {epsilon."two",2. @count=0x4 @min=0xa @max=0x28 @sum=0x64 @avg=0x19} { incr ok; exp_continue }
     -re {phi @count=0x4 @min=0x1 @max=0x4 @sum=0xa @avg=0x2} { incr ok; exp_continue }
+    -re {psi=0x5} { incr ok; exp_continue }
     timeout { fail "$test (timeout)" }
     eof { }
 }
diff --git a/testsuite/systemtap.base/global_end.stp b/testsuite/systemtap.base/global_end.stp
index 876eac8..4b81ae3 100644
--- a/testsuite/systemtap.base/global_end.stp
+++ b/testsuite/systemtap.base/global_end.stp
@@ -1,4 +1,4 @@
-global alpha, beta, gamma, iota, epsilon, phi
+global alpha, beta, gamma, iota, epsilon, phi, psi
 
 probe begin {
  gamma = "abcdefghijklmnopqrstuvwxyz"
@@ -7,9 +7,13 @@ probe begin {
  iota["two"] = "twelve"
 
  alpha["one",1] = 1
+ alpha["one",1] ++
  alpha["one",2] = 2
+ alpha["one",2] *= 2
  alpha["two",1] = 3
  alpha["two",2] = 4
+ 
+ psi = ++alpha["two",2]
 
  beta["one",1] = 1
  beta["one",2] = 2

Re: BZ 6954: Automagic printing for ++ operations

[Rajan Arora]

[-- Attachment #1: Type: text/plain, Size: 514 bytes --]


Here is a second go at the patch incorporating a few suggested improvements.

Rajan Arora wrote:
> Hello everyone,
>
>  Attached is a patch for the BZ 6954 bug - automagic printing for ++ 
> operations on global variables. A few tests in the testsuite seem to 
> have broken after applying this patch but I suspect that is mostly 
> because of the automagic printing in place and I am working on them. 
> Meanwhile, please have a look and feel free to reply with any 
> suggestions/feedback.
>
> Cheers
> -Rajan


[-- Attachment #2: 6954-b.patch --]
[-- Type: text/x-patch, Size: 7084 bytes --]

diff --git a/ChangeLog b/ChangeLog
index 4db3af2..0140c4e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2008-10-27  Rajan Arora  <rarora@redhat.com>
+
+	* staptree.h (struct varuse_collecting_visitor): New members
+	read_spl, modify_seen, read_seen.
+	(varuse_collecting_visitor::varuse_collecting_visitor): 
+	Initialize new members.
+	* staptree.cxx (varuse_collecting_visitor::visit_print_format):
+	Update flag.
+	(varuse_collecting_visitor::visit_assignment): Update flag.
+	(varuse_collecting_visitor::visit_symbol): Update read_spl.
+	(varuse_collecting_visitor::visit_pre_crement): Update flag.
+	(varuse_collecting_visitor::visit_post_crement): Update flag.
+	(varuse_collecting_visitor::visit_arrayindex): Update flag.
+	* elaborate.cxx (add_global_var_display): Check read_spl.
+
 2008-10-27  Josh Stone  <joshua.i.stone@intel.com>
 
 	* translate.cxx (c_unparser::visit_print_format): Fix the argument passed
diff --git a/elaborate.cxx b/elaborate.cxx
index 7cbac31..96c47c4 100644
--- a/elaborate.cxx
+++ b/elaborate.cxx
@@ -1163,8 +1163,9 @@ void add_global_var_display (systemtap_session& s)
   for (unsigned g=0; g < s.globals.size(); g++)
     {
       vardecl* l = s.globals[g];
-      if (vut.read.find (l) != vut.read.end()
-          || vut.written.find (l) == vut.written.end())
+      if ((vut.read.find (l) != vut.read.end()
+	   && vut.read_spl.find (l) == vut.read_spl.end())
+	  || vut.written.find (l) == vut.written.end())
 	continue;
 
       // Don't generate synthetic end probes for unread globals 
diff --git a/staptree.cxx b/staptree.cxx
index 38166c5..efc3269 100644
--- a/staptree.cxx
+++ b/staptree.cxx
@@ -1692,7 +1692,9 @@ varuse_collecting_visitor::visit_print_format (print_format* e)
   if (e->print_to_stream)
     embedded_seen = true; // a proxy for "has unknown side-effects"
 
+  read_seen = true; //set a flag for ::visit_symbol
   functioncall_traversing_visitor::visit_print_format (e);
+  read_seen = false;
 }
 
 
@@ -1710,8 +1712,14 @@ varuse_collecting_visitor::visit_assignment (assignment *e)
     {
       expression* last_lrvalue = current_lrvalue;
       current_lrvalue = e->left; // leave a mark for ::visit_symbol
+      if (e->op.find ('=') != string::npos && e->op.size() > 1)
+	modify_seen = true; //set a flag for ::visit_symbol
+      else
+	read_seen = true;
       functioncall_traversing_visitor::visit_assignment (e);
       current_lrvalue = last_lrvalue;
+      modify_seen = false;
+      read_seen = false;
     }
 }
 
@@ -1733,6 +1741,8 @@ varuse_collecting_visitor::visit_symbol (symbol *e)
   if (current_lvalue == e || current_lrvalue == e)
     {
       written.insert (e->referent);
+      if (modify_seen && !read_seen)
+	read_spl.insert (e->referent);
       // clog << "write ";
     }
   if (current_lvalue != e || current_lrvalue == e)
@@ -1777,7 +1787,9 @@ varuse_collecting_visitor::visit_arrayindex (arrayindex *e)
     {
       if (current_lrvalue == e) current_lrvalue = hist->stat;
       if (current_lvalue == e) current_lvalue = hist->stat;
+      read_seen = true;
       functioncall_traversing_visitor::visit_arrayindex (e);
+      read_seen = false;
     }
 
   current_lrvalue = last_lrvalue;
@@ -1790,8 +1802,10 @@ varuse_collecting_visitor::visit_pre_crement (pre_crement *e)
 {
   expression* last_lrvalue = current_lrvalue;
   current_lrvalue = e->operand; // leave a mark for ::visit_symbol
+  modify_seen = true; //set a flag for ::visit_symbol
   functioncall_traversing_visitor::visit_pre_crement (e);
   current_lrvalue = last_lrvalue;
+  modify_seen = false;
 }
 
 void
@@ -1799,8 +1813,10 @@ varuse_collecting_visitor::visit_post_crement (post_crement *e)
 {
   expression* last_lrvalue = current_lrvalue;
   current_lrvalue = e->operand; // leave a mark for ::visit_symbol
+  modify_seen = true; //set a flag for ::visit_symbol
   functioncall_traversing_visitor::visit_post_crement (e);
   current_lrvalue = last_lrvalue;
+  modify_seen = false;
 }
 
 void
diff --git a/staptree.h b/staptree.h
index 40be8e0..24512b4 100644
--- a/staptree.h
+++ b/staptree.h
@@ -742,11 +742,16 @@ struct varuse_collecting_visitor: public functioncall_traversing_visitor
 {
   std::set<vardecl*> read;
   std::set<vardecl*> written;
+  std::set<vardecl*> read_spl;  //Modified but not printed symbols
   bool embedded_seen;
+  bool modify_seen;
+  bool read_seen;
   expression* current_lvalue;
   expression* current_lrvalue;
   varuse_collecting_visitor():
     embedded_seen (false),
+    modify_seen (false),
+    read_seen (false),
     current_lvalue(0),
     current_lrvalue(0) {}
   void visit_embeddedcode (embeddedcode *s);
diff --git a/testsuite/ChangeLog b/testsuite/ChangeLog
index ec4cc40..271e1b1 100644
--- a/testsuite/ChangeLog
+++ b/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2008-10-27  Rajan Arora  <rarora@redhat.com>
+
+	* systemtap.base/global_end.stp: Added pre_crement,
+	post_crement and assignment modify operations.
+	* systemtap.base/global_end.exp: Allow for new automagic
+	printing.
+
 2008-10-27  Josh Stone  <joshua.i.stone@intel.com>
 
 	* systemtap.printf/char1.stp: Expose i686 failure
diff --git a/testsuite/systemtap.base/global_end.exp b/testsuite/systemtap.base/global_end.exp
index b811bff..e8c7fbf 100644
--- a/testsuite/systemtap.base/global_end.exp
+++ b/testsuite/systemtap.base/global_end.exp
@@ -9,16 +9,17 @@ set ok 0
 expect {
     -timeout 180
     -re {one,0x1.*one,0x2.*two,0x1.*two,0x2} { incr ok; exp_continue }
-    -re {alpha."two".2.=0x4} { incr ok; exp_continue }
+    -re {alpha."two".2.=0x5} { incr ok; exp_continue }
     -re {alpha."two".1.=0x3} { incr ok; exp_continue }
-    -re {alpha."one".2.=0x2} { incr ok; exp_continue }
-    -re {alpha."one".1.=0x1} { incr ok; exp_continue }
+    -re {alpha."one".2.=0x4} { incr ok; exp_continue }
+    -re {alpha."one".1.=0x2} { incr ok; exp_continue }
     -re {gamma="abcdefghijklmnopqrstuvwxyz"} { incr ok; exp_continue }
     -re {iota."two".="twelve"} { incr ok; exp_continue }
     -re {iota."one".="eleven"} { incr ok; exp_continue }
     -re {epsilon."one",1. @count=0x4 @min=0x1 @max=0x4 @sum=0xa @avg=0x2} { incr ok; exp_continue }
     -re {epsilon."two",2. @count=0x4 @min=0xa @max=0x28 @sum=0x64 @avg=0x19} { incr ok; exp_continue }
     -re {phi @count=0x4 @min=0x1 @max=0x4 @sum=0xa @avg=0x2} { incr ok; exp_continue }
+    -re {psi=0x5} { incr ok; exp_continue }
     timeout { fail "$test (timeout)" }
     eof { }
 }
diff --git a/testsuite/systemtap.base/global_end.stp b/testsuite/systemtap.base/global_end.stp
index 876eac8..4b81ae3 100644
--- a/testsuite/systemtap.base/global_end.stp
+++ b/testsuite/systemtap.base/global_end.stp
@@ -1,4 +1,4 @@
-global alpha, beta, gamma, iota, epsilon, phi
+global alpha, beta, gamma, iota, epsilon, phi, psi
 
 probe begin {
  gamma = "abcdefghijklmnopqrstuvwxyz"
@@ -7,9 +7,13 @@ probe begin {
  iota["two"] = "twelve"
 
  alpha["one",1] = 1
+ alpha["one",1] ++
  alpha["one",2] = 2
+ alpha["one",2] *= 2
  alpha["two",1] = 3
  alpha["two",2] = 4
+ 
+ psi = ++alpha["two",2]
 
  beta["one",1] = 1
  beta["one",2] = 2