[PATCH v3] Parentheses after unary '&' with a target-symbol expression is now accepted

["Yichun Zhang (agentzh)" <yichun at openresty dot com>]

From: Yichun Zhang <yichun@openresty.com>

The translator did not allow stap expressions like `&(@var("foo"))`.

Added quite some tests to cover various use cases of the unary '&'
address-taking operator.

Updated NEWS to document this change.
---
 NEWS                                   |   3 +
 parse.cxx                              |  13 ++-
 testsuite/systemtap.base/addr_op.exp   | 145 +++++++++++++++++++++++++++++++++
 testsuite/systemtap.base/addr_op_1.c   |  10 +++
 testsuite/systemtap.base/addr_op_1.stp |   6 ++
 testsuite/systemtap.base/addr_op_2.stp |   8 ++
 testsuite/systemtap.base/addr_op_3.stp |   6 ++
 testsuite/systemtap.base/addr_op_4.c   |   5 ++
 testsuite/systemtap.base/addr_op_4.stp |   7 ++
 testsuite/systemtap.base/addr_op_5.stp |   6 ++
 10 files changed, 207 insertions(+), 2 deletions(-)
 create mode 100644 testsuite/systemtap.base/addr_op.exp
 create mode 100644 testsuite/systemtap.base/addr_op_1.c
 create mode 100644 testsuite/systemtap.base/addr_op_1.stp
 create mode 100644 testsuite/systemtap.base/addr_op_2.stp
 create mode 100644 testsuite/systemtap.base/addr_op_3.stp
 create mode 100644 testsuite/systemtap.base/addr_op_4.c
 create mode 100644 testsuite/systemtap.base/addr_op_4.stp
 create mode 100644 testsuite/systemtap.base/addr_op_5.stp

diff --git a/NEWS b/NEWS
index c58f19dbd..26b03f62b 100644
--- a/NEWS
+++ b/NEWS
@@ -48,6 +48,9 @@
   following semicolon (';') or a closing curly bracket ('}') as a
   terminator for such bare return statements.
 
+- Parentheses after unary '&' with a target-symbol expression is
+  now accepted in the script language.
+
 * What's new in version 3.3, 2018-06-08
 
 - A new "stap --example FOO.stp" mode searches the example scripts
diff --git a/parse.cxx b/parse.cxx
index a7b755f35..0574f87c2 100644
--- a/parse.cxx
+++ b/parse.cxx
@@ -3702,8 +3702,17 @@ parser::parse_dwarf_value ()
     // '&' on old version only allowed specific target_symbol types
     throw PARSE_ERROR (_("expected @cast, @var or $var"));
   else
-    // Otherwise just get a plain value of any sort.
-    expr = parse_value ();
+    {
+      // Otherwise just get a plain value of any sort.
+      expr = parse_value ();
+      if (addressof)
+        {
+          tsym = dynamic_cast<target_symbol*> (expr);
+          if (tsym && tsym->addressof)
+            throw PARSE_ERROR (_("cannot take address more than once"),
+                               addrtok);
+        }
+    }
 
   // If we had '&' or see any target suffixes, that forces a target_symbol.
   // For compatibility, we only do this starting with 2.6.
diff --git a/testsuite/systemtap.base/addr_op.exp b/testsuite/systemtap.base/addr_op.exp
new file mode 100644
index 000000000..211377041
--- /dev/null
+++ b/testsuite/systemtap.base/addr_op.exp
@@ -0,0 +1,145 @@
+set test "addr_op"
+set testpath "$srcdir/$subdir"
+
+if {! [installtest_p]} { untested "$test"; return }
+if {! [uretprobes_p]} { untested "$test"; return }
+
+# --- TEST 1 ---
+
+set subtest1 "TEST 1: 2 excessive parens after unary '&' - with components"
+
+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 exit_code [run_cmd_2way "stap --runtime=$runtime -c ./a.out '$srcdir/$subdir/${test}_1.stp'" \
+                                    out stderr]
+        set exp_out "1\n"
+        is "${test_name}: stdout" $out $exp_out
+        is "${test_name}: exit code" $exit_code 0
+        if {$stderr ne ""} {
+            send_log "stderr:\n$stderr"
+        }
+    }
+}
+
+# --- TEST 2 ---
+
+set subtest2 "TEST 2: one excessive paren after unary '&' - with components"
+
+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 exit_code [run_cmd_2way "stap --runtime=$runtime -c ./a.out '$srcdir/$subdir/${test}_2.stp'" \
+                                    out stderr]
+        set exp_out "1
+1
+0
+"
+        is "${test_name}: stdout" $out $exp_out
+        is "${test_name}: exit code" $exit_code 0
+        if {$stderr ne ""} {
+            send_log "stderr:\n$stderr"
+        }
+    }
+}
+
+# --- TEST 3 ---
+
+set subtest3 "TEST 3: 2 excessive parens after unary '&' - without components"
+
+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: $subtest3: unable to compile ${test}_1.c"
+} else {
+    foreach runtime [get_runtime_list] {
+        if {$runtime eq ""} {
+            set runtime "kernel"
+        }
+        set test_name "$test: $subtest3 ($runtime)"
+        set exit_code [run_cmd_2way "stap --runtime=$runtime -c ./a.out '$srcdir/$subdir/${test}_3.stp'" \
+                                    out stderr]
+        set exp_out "1\n"
+        is "${test_name}: stdout" $out $exp_out
+        is "${test_name}: exit code" $exit_code 0
+        if {$stderr ne ""} {
+            send_log "stderr:\n$stderr"
+        }
+    }
+}
+
+# --- TEST 4 ---
+
+set subtest4 "TEST 4: taking address of int globals"
+
+set res [target_compile ${testpath}/${test}_4.c ./a.out executable \
+    "additional_flags=-O additional_flags=-g"]
+if {$res ne ""} {
+    verbose "target_compile failed: $res" 2
+    fail "$test: $subtest4: unable to compile ${test}_4.c"
+} else {
+    foreach runtime [get_runtime_list] {
+        if {$runtime eq ""} {
+            set runtime "kernel"
+        }
+        set test_name "$test: $subtest4 ($runtime)"
+        set exit_code [run_cmd_2way "stap --runtime=$runtime -c ./a.out '$srcdir/$subdir/${test}_4.stp'" \
+                                    out stderr]
+        set exp_out "1
+-32
+"
+        is "${test_name}: stdout" $out $exp_out
+        is "${test_name}: exit code" $exit_code 0
+        if {$stderr ne ""} {
+            send_log "stderr:\n$stderr"
+        }
+    }
+}
+
+# --- TEST 5 ---
+
+set subtest5 "TEST 5: duplicate unary '&'"
+
+set res [target_compile ${testpath}/${test}_4.c ./a.out executable \
+    "additional_flags=-O additional_flags=-g"]
+if {$res ne ""} {
+    verbose "target_compile failed: $res" 2
+    fail "$test: $subtest5: unable to compile ${test}_4.c"
+} else {
+    foreach runtime [get_runtime_list] {
+        if {$runtime eq ""} {
+            set runtime "kernel"
+        }
+        set test_name "$test: $subtest5 ($runtime)"
+        set exit_code [run_cmd_2way "stap --runtime=$runtime -c ./a.out '$srcdir/$subdir/${test}_5.stp'" \
+                                    out stderr]
+        set exp_out ""
+        is "${test_name}: stdout" $out $exp_out
+        isnt "${test_name}: exit code" $exit_code 0
+        set stderr_pat "\\Aparse error: cannot take address more than once
+         at: operator '&' at .*?\\.stp:2:9
+     source:     q = &\\(\\(&\\@var\\(\"a\"\\)\\)\\);
+                     \\^
+
+1 parse error\\.
+"
+        like "${test_name}: stderr" $stderr $stderr_pat "-linestop"
+    }
+}
diff --git a/testsuite/systemtap.base/addr_op_1.c b/testsuite/systemtap.base/addr_op_1.c
new file mode 100644
index 000000000..c9ff20e1f
--- /dev/null
+++ b/testsuite/systemtap.base/addr_op_1.c
@@ -0,0 +1,10 @@
+struct foo {
+    int a;
+    int b[3];
+};
+
+struct foo v;
+
+int main(void) {
+    return 0;
+}
diff --git a/testsuite/systemtap.base/addr_op_1.stp b/testsuite/systemtap.base/addr_op_1.stp
new file mode 100644
index 000000000..2e8f88639
--- /dev/null
+++ b/testsuite/systemtap.base/addr_op_1.stp
@@ -0,0 +1,6 @@
+probe process.function("main") {
+    p = &@var("v")->b[0];
+    q = &((@var("v")->b[0]));
+    printf("%d\n", p == q);
+    exit();
+}
diff --git a/testsuite/systemtap.base/addr_op_2.stp b/testsuite/systemtap.base/addr_op_2.stp
new file mode 100644
index 000000000..0e7c14269
--- /dev/null
+++ b/testsuite/systemtap.base/addr_op_2.stp
@@ -0,0 +1,8 @@
+probe process.function("main") {
+    p = &@var("v")->b[0];
+    q = &(@var("v")->b[0]);
+    printf("%d\n", p == q);
+    printf("%d\n", q == @var("v")->b);
+    printf("%d\n", q == &((@var("v")->b[1])));
+    exit();
+}
diff --git a/testsuite/systemtap.base/addr_op_3.stp b/testsuite/systemtap.base/addr_op_3.stp
new file mode 100644
index 000000000..55ca2ff10
--- /dev/null
+++ b/testsuite/systemtap.base/addr_op_3.stp
@@ -0,0 +1,6 @@
+probe process.function("main") {
+    p = &@var("v");
+    q = &((@var("v")));
+    printf("%d\n", p == q);
+    exit();
+}
diff --git a/testsuite/systemtap.base/addr_op_4.c b/testsuite/systemtap.base/addr_op_4.c
new file mode 100644
index 000000000..62261ef53
--- /dev/null
+++ b/testsuite/systemtap.base/addr_op_4.c
@@ -0,0 +1,5 @@
+int a = -32;
+
+int main(void) {
+    return 0;
+}
diff --git a/testsuite/systemtap.base/addr_op_4.stp b/testsuite/systemtap.base/addr_op_4.stp
new file mode 100644
index 000000000..d4cb86d13
--- /dev/null
+++ b/testsuite/systemtap.base/addr_op_4.stp
@@ -0,0 +1,7 @@
+probe process.function("main") {
+    p = &@var("a");
+    q = &((@var("a")));
+    printf("%d\n", p == q);
+    printf("%d\n", user_int(q));
+    exit();
+}
diff --git a/testsuite/systemtap.base/addr_op_5.stp b/testsuite/systemtap.base/addr_op_5.stp
new file mode 100644
index 000000000..843b301df
--- /dev/null
+++ b/testsuite/systemtap.base/addr_op_5.stp
@@ -0,0 +1,6 @@
+probe process.function("main") {
+    q = &((&@var("a")));
+    printf("%d\n", p == q);
+    printf("%d\n", user_int(q));
+    exit();
+}
-- 
2.11.0.295.gd7dffce