[PATCH] Fix redundant implicit probe points in listing mode.

[PATCH] Fix redundant implicit probe points in listing mode.

[Wenji Huang]

Hi,

There are redundant probe points those will be outputted in listing 
mode. Such as:
[wjhuang@systemtap]$ stap -l signal.d*
signal.do_action
begin(-1)
end idx0:long

This will just happen when functions calling included in definition of 
probe alias. There are initialization works done on implicitly 
begin(-1). And some operations on global variables are also processed on 
implicit end one. Those could enlarge the retrieved s.probes.

There is workaround to solve it.

[PATCH 1/1] Fix redundant implicit probe points in listing mode.

---
  elaborate.cxx |    2 ++
  main.cxx      |    3 ++-
  2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/elaborate.cxx b/elaborate.cxx
index 44b6e24..7014928 100644
--- a/elaborate.cxx
+++ b/elaborate.cxx
@@ -1148,6 +1148,8 @@ semantic_pass_symbols (systemtap_session& s)
  void add_global_var_display (systemtap_session& s)
  {
    varuse_collecting_visitor vut;
+
+  if (s.listing_mode)  return;
    for (unsigned i=0; i<s.probes.size(); i++)
      {
        s.probes[i]->body->visit (& vut);
diff --git a/main.cxx b/main.cxx
index 83afb91..74fba86 100644
--- a/main.cxx
+++ b/main.cxx
@@ -173,7 +173,8 @@ printscript(systemtap_session& s, ostream& o)
                second->locations[0]->print(tmps); // XXX: [0] is less 
arbitrary here, but still ...
              }
            string pp = tmps.str();
-
+          if (!pp.compare("begin(-1)")) continue;
+
            // Now duplicate-eliminate.  An alias may have expanded to
            // several actual derived probe points, but we only want to
            // print the alias head name once.

Regards,
Wenji

Re: [PATCH] Fix redundant implicit probe points in listing mode.

[Frank Ch. Eigler]

Wenji Huang <wenji.huang@oracle.com> writes:

> There are redundant probe points those will be outputted in listing
> mode. Such as:
> [wjhuang@systemtap]$ stap -l signal.d*
> signal.do_action
> begin(-1)
> end idx0:long

Right.

> @@ -1148,6 +1148,8 @@ semantic_pass_symbols (systemtap_session& s)
>  void add_global_var_display (systemtap_session& s)
>  {
>    varuse_collecting_visitor vut;
> +
> +  if (s.listing_mode)  return;

That's not too bad, though perhaps a session flag for
global_var_display per se could be used instead.  (It'd default to
"on"; not have any command line option to disable it directly yet; but
"-l"/'-L" would clear it.)


> @@ -173,7 +173,8 @@ printscript(systemtap_session& s, ostream& o)
>                second->locations[0]->print(tmps); // XXX: [0] is less
> arbitrary here, but still ...
>              }
>            string pp = tmps.str();
> -
> +          if (!pp.compare("begin(-1)")) continue;
> +

This one can't go in.  Instead, the tapset that includes that
begin(-1) probe could be changed to do the initialization in a
function (with a private initted-already? flag) rather than the begin
probe.  Future listings based on PR 3498 should make that workaround
unnecessary.

- FChE

Re: [PATCH] Fix redundant implicit probe points in listing mode.

[Wenji Huang]

Frank Ch. Eigler wrote:
[...]
>> @@ -1148,6 +1148,8 @@ semantic_pass_symbols (systemtap_session& s)
>>  void add_global_var_display (systemtap_session& s)
>>  {
>>    varuse_collecting_visitor vut;
>> +
>> +  if (s.listing_mode)  return;
> 
> That's not too bad, though perhaps a session flag for
> global_var_display per se could be used instead.  (It'd default to
> "on"; not have any command line option to disable it directly yet; but
> "-l"/'-L" would clear it.)
> 
> 
>> @@ -173,7 +173,8 @@ printscript(systemtap_session& s, ostream& o)
>>                second->locations[0]->print(tmps); // XXX: [0] is less
>> arbitrary here, but still ...
>>              }
>>            string pp = tmps.str();
>> -
>> +          if (!pp.compare("begin(-1)")) continue;
>> +
> 
> This one can't go in.  Instead, the tapset that includes that
> begin(-1) probe could be changed to do the initialization in a
> function (with a private initted-already? flag) rather than the begin
> probe.  Future listings based on PR 3498 should make that workaround
> unnecessary.
Thanks for your comments.
The begin(-1) probe is only included in tapset/signal.stp. So only
stap -l signal.* will list begin(-1) probe.

Updated the signal tapset and change begin(-1) to local function to 
avoid the redundant one.

---
  elaborate.cxx     |    2 ++
  tapset/signal.stp |    7 +------
  2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/elaborate.cxx b/elaborate.cxx
index 3dfc718..15ced84 100644
--- a/elaborate.cxx
+++ b/elaborate.cxx
@@ -1148,6 +1148,8 @@ semantic_pass_symbols (systemtap_session& s)
  void add_global_var_display (systemtap_session& s)
  {
    varuse_collecting_visitor vut;
+
+  if (s.listing_mode) return; //avoid end probe in listing_mode
    for (unsigned i=0; i<s.probes.size(); i++)
      {
        s.probes[i]->body->visit (& vut);
diff --git a/tapset/signal.stp b/tapset/signal.stp
index 1128e6f..d08c0c1 100644
--- a/tapset/signal.stp
+++ b/tapset/signal.stp
@@ -609,12 +609,6 @@ function sa_handler_str(handler) {
   * Signals start from 1 not 0.
   */
  function signal_str(num) {
-       return __sig[num]
-}
-
-global __sig[64]
-
-probe begin(-1) {
         __sig[1] = "HUP"
         __sig[2] = "INT"
         __sig[3] = "QUIT"
@@ -679,4 +673,5 @@ probe begin(-1) {
         __sig[62] = "RTMIN+30"
         __sig[63] = "RTMIN+31"
         __sig[64] = "RTMIN+32"
+       return __sig[num]
  }
-- 


Regards,
Wenji