[PATCH] Make opeartor @var() no longer assume @entry() in return probes.

[PATCH] Make opeartor @var() no longer assume @entry() in return probes.

[Yichun Zhang (agentzh)]

The old behavior would yield stale values when the function being probed
changes the global variables being read via @var() in the return probe
handler.

Added tests to cover this fix (and the old behavior for compatibility).
---
 NEWS                                          |  2 +
 tapsets.cxx                                   |  1 +
 testsuite/systemtap.base/ret-uprobe-var.exp   | 58 +++++++++++++++++++++++++++
 testsuite/systemtap.base/ret-uprobe-var_1.c   |  6 +++
 testsuite/systemtap.base/ret-uprobe-var_1.stp |  3 ++
 5 files changed, 70 insertions(+)
 create mode 100644 testsuite/systemtap.base/ret-uprobe-var.exp
 create mode 100644 testsuite/systemtap.base/ret-uprobe-var_1.c
 create mode 100644 testsuite/systemtap.base/ret-uprobe-var_1.stp

diff --git a/NEWS b/NEWS
index a456bf7aa..0191b61e2 100644
--- a/NEWS
+++ b/NEWS
@@ -16,6 +16,8 @@
          printf("%s", user_string(buf_uaddr))
      }
 
+- Operator @var() no longer assumes @entry() in return probes.
+  THe old behavior can be restored by the '--compatible 4.0' option.
 
 * What's new in version 4.0, 2018-10-13
 
diff --git a/tapsets.cxx b/tapsets.cxx
index 25bb35d19..8704202b7 100644
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -4463,6 +4463,7 @@ dwarf_var_expanding_visitor::visit_target_symbol (target_symbol *e)
       // parameters from a return probe.  PR 1382.
       if (q.has_return
           && !defined_being_checked
+          && (strverscmp(sess.compatible.c_str(), "4.1") < 0 || e->name != "@var")
           && e->name != "$return" // not the special return-value variable handled below
           && e->name != "$$return") // nor the other special variable handled below
         {
diff --git a/testsuite/systemtap.base/ret-uprobe-var.exp b/testsuite/systemtap.base/ret-uprobe-var.exp
new file mode 100644
index 000000000..500dd09e4
--- /dev/null
+++ b/testsuite/systemtap.base/ret-uprobe-var.exp
@@ -0,0 +1,58 @@
+set test "ret-uprobe-var"
+set testpath "$srcdir/$subdir"
+
+if {! [installtest_p]} { untested $test; return }
+if {! [uretprobes_p]} { untested $test; return }
+
+# --- TEST 1 ---
+
+set subtest1 "TEST 1: @var in return probes should not be stale (4.1+)"
+
+set res [target_compile ${testpath}/${test}_1.c ./a.out executable \
+    "additional_flags=-O additional_flags=-g"]
+if {$res ne ""} {
+    verbose "target_compile failed: $res" 2
+    fail "$test: $subtest1: unable to compile ${test}_1.c"
+} else {
+    foreach runtime [get_runtime_list] {
+        if {$runtime eq ""} {
+            set runtime "kernel"
+        }
+        set test_name "$test: $subtest1 ($runtime)"
+        set cmd "stap --runtime=$runtime -c ./a.out '$srcdir/$subdir/${test}_1.stp'"
+        set exit_code [run_cmd_2way $cmd out stderr]
+        set exp_out "a = 4\n"
+        is "${test_name}: stdout" $out $exp_out
+        set exp_stderr ""
+        is "${test_name}: stderr" $stderr $exp_stderr
+        is "${test_name}: exit code" $exit_code 0
+    }
+}
+
+# --- TEST 2 ---
+
+set subtest2 "TEST 2: @var in return probes assumed @entry in release 4.0 or older"
+
+set res [target_compile ${testpath}/${test}_1.c ./a.out executable \
+    "additional_flags=-O additional_flags=-g"]
+if {$res ne ""} {
+    verbose "target_compile failed: $res" 2
+    fail "$test: $subtest2: unable to compile ${test}_1.c"
+} else {
+    foreach runtime [get_runtime_list] {
+        if {$runtime eq ""} {
+            set runtime "kernel"
+        }
+        set test_name "$test: $subtest2 ($runtime)"
+        set cmd "stap --compatible 4.0 --runtime=$runtime -c ./a.out '$srcdir/$subdir/${test}_1.stp'"
+        set exit_code [run_cmd_2way $cmd out stderr]
+        set exp_out "a = 3\n"
+        is "${test_name}: stdout" $out $exp_out
+        set stderr_pat "\\AWARNING: confusing usage, consider \\@entry\\(\\@var\\(\"a\", \"\[^\"\]+\"\\)\\) in \\.return probe: operator '\\@var' at .+?\\.stp:2:24
+ source:     printf\\(\"a = %d\\\\n\", \\@var\\(\"a\"\\)\\);
+                                \\^
+\\Z"
+        like "${test_name}: stderr" $stderr $stderr_pat "-linestop"
+        is "${test_name}: exit code" $exit_code 0
+    }
+}
diff --git a/testsuite/systemtap.base/ret-uprobe-var_1.c b/testsuite/systemtap.base/ret-uprobe-var_1.c
new file mode 100644
index 000000000..d989d57ba
--- /dev/null
+++ b/testsuite/systemtap.base/ret-uprobe-var_1.c
@@ -0,0 +1,6 @@
+int a = 3;
+
+int main(void) {
+    a++;
+    return 0;
+}
diff --git a/testsuite/systemtap.base/ret-uprobe-var_1.stp b/testsuite/systemtap.base/ret-uprobe-var_1.stp
new file mode 100644
index 000000000..d4397ef30
--- /dev/null
+++ b/testsuite/systemtap.base/ret-uprobe-var_1.stp
@@ -0,0 +1,3 @@
+probe process.function("main").return {
+    printf("a = %d\n", @var("a"));
+}
-- 
2.11.0.295.gd7dffce

Re: [PATCH] Make opeartor @var() no longer assume @entry() in return probes.

[Yichun Zhang]

Hello!

On Wed, Nov 21, 2018 at 11:57 PM Yichun Zhang (agentzh)
<yichun@openresty.com> wrote:
>
> The old behavior would yield stale values when the function being probed
> changes the global variables being read via @var() in the return probe
> handler.
>
> Added tests to cover this fix (and the old behavior for compatibility).

Just committed with the greenlight from fche.

Regards,
Yichun