PR20416: handle @entry(@perf("..."))

The dwarf_var_expanding_visitor::visit_entry_op function inherited
some old logic from expanding $var's in .function().return probes.
This logic is not approproate for the more formal way by which current
code synthesizes a .call probe to collect @entry(EXPR) values.  Now we
pass the EXPR tree to the .call probe unmodified, for resolution
there, rather than in a fake non-.return context here.  This makes
@entry(@perf("...")) expressions work.

diff --git a/tapsets.cxx b/tapsets.cxx
index 6d82069c6499d075bc763f875c40610646eecc13..c1ffb6b4412939f0ac300aa5950076816a0ab66f 100644 (file)
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -4402,10 +4402,11 @@ dwarf_var_expanding_visitor::visit_entry_op (entry_op *e)
   expression *repl = e;
   if (q.has_return)
     {
-      // expand the operand as if it weren't a return probe
-      q.has_return = false;
-      replace (e->operand);
-      q.has_return = true;
+      // NB: don't expand the operand here, as if it weren't a return
+      // probe.  The original operand expression is transcribed into
+      // the synthetic .call probe that gen_mapped_saved_return calls.
+      // If we were to expand it here, we may e.g. map @perf("...") to
+      // __perf_read_... prematurely & incorrectly.  PR20416
 
       // XXX it would be nice to use gen_kretprobe_saved_return when available,
       // but it requires knowing the types already, which is problematic for
@@ -5163,6 +5164,9 @@ dwarf_derived_probe::dwarf_derived_probe(interned_string funcname,
           q.has_return = false;
           q.has_call = true;
 
+          // NB: any moved @entry(EXPR) bits will be expanded during this
+          // nested *derived_probe ctor for the synthetic .call probe.
+          // PR20416
           if (q.has_process)
             {
               // Place handler probe at the same addr as where the vars were

diff --git a/testsuite/systemtap.base/perf.exp b/testsuite/systemtap.base/perf.exp
index 3e50a120fcb49cd4a9bb6142470a1fc2f0660141..87d537df9f78a9be428e9da2acc14ebaa58497f7 100644 (file)
--- a/testsuite/systemtap.base/perf.exp
+++ b/testsuite/systemtap.base/perf.exp
@@ -86,7 +86,7 @@ if {! [uprobes_p]} {
     }
     catch {close}; catch {wait}
 
-    spawn $stap_path $srcdir/$subdir/perf04.stp $exepath -c $exepath
+    spawn $stap_path -W $srcdir/$subdir/perf04.stp $exepath -c $exepath
 
     expect {
        -timeout 180

diff --git a/testsuite/systemtap.base/perf04.stp b/testsuite/systemtap.base/perf04.stp
index 1dca72e88978dc90fa6a940918d05d03ef070071..abbee362332813facb5b98640696411a41d4107e 100644 (file)
--- a/testsuite/systemtap.base/perf04.stp
+++ b/testsuite/systemtap.base/perf04.stp
@@ -2,10 +2,10 @@ global main
 
 probe perf.hw.instructions.counter("a") { }
 
-probe process(@1).statement("main@towers.c")
+probe process(@1).function("main").return
 {
     # counter "a" has no defined process so the default is ours
-    main <<< @perf("a")
+    main <<< @perf("a") - @entry(@perf("a"))
 }
 
 probe end