Fix a few stap parser issues and add a new test for probe expressions

The creation of this patch was motivated by Tom's "Change handling of
'!' operator in stap probes" patch.

While reviewing his patch, I stumbled upon a few issues with the stap
expression parser.  They are:

- As it turns out, even with Tom's patch applied the parser doesn't
  properly handle the '!' operator.  The underlying issue was the fact
  that stap_parse_argument_conditionally also needed to be patched in
  order to recognize '!' as an operator that is part of a single
  operand, and parse it accordingly.

- While writing the testcase I'm proposing on this patch, I found that
  parenthesized sub-expressions were not being parsed correctly when
  there was another term after them.  For example:

    1 - (2 + 3) + 4

  In this case, the parser was considering "1" to be the left-side of
  the expression, and "(2 + 3) + 4" to be the right-side.  The patch
  fixes the parser by making it identify whether a parenthesized
  sub-expression has just been parsed, and act accordingly.

I've tested this on my Debian testing amd64, and everything seems OK.

gdb/ChangeLog:
2021-01-20  Sergio Durigan Junior  <sergiodj@sergiodj.net>
	    Tom Tromey <tom@tromey.com>

	* stap-probe.c (stap_parse_single_operand): Handle '!'
	operator.
	(stap_parse_argument_conditionally): Likewise.
	Skip spaces after processing open-parenthesis sub-expression.
	(stap_parse_argument_1): Skip spaces after call to
	stap_parse_argument_conditionally.
	Handle case when right-side expression is a parenthesized
	sub-expression.
	Skip spaces after call to stap_parse_argument_1.

gdb/testsuite/ChangeLog:
2021-01-20  Sergio Durigan Junior  <sergiodj@sergiodj.net>

	* gdb.arch/amd64-stap-expressions.S: New file.
	* gdb.arch/amd64-stap-expressions.exp: New file.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 8a9507223a518d956c296b442fc12f2ff348b24e..b63d7a6d6edbca4f9100d3946b885ea9356488f3 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,16 @@
+2021-01-20  Sergio Durigan Junior  <sergiodj@sergiodj.net>
+           Tom Tromey <tom@tromey.com>
+
+       * stap-probe.c (stap_parse_single_operand): Handle '!'
+       operator.
+       (stap_parse_argument_conditionally): Likewise.
+       Skip spaces after processing open-parenthesis sub-expression.
+       (stap_parse_argument_1): Skip spaces after call to
+       stap_parse_argument_conditionally.
+       Handle case when right-side expression is a parenthesized
+       sub-expression.
+       Skip spaces after call to stap_parse_argument_1.
+
 2021-01-19  Lancelot SIX  <lsix@lancelotsix.com>
 
        * top.h (switch_thru_all_uis): Use DISABLE_COPY_AND_ASSIGN.

diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c
index c2ddd047f6fc9849adb217c3e4de0c3a0c4d9805..224dd5714fac05f5077661ae567c18ecb2aca647 100644 (file)
--- a/gdb/stap-probe.c
+++ b/gdb/stap-probe.c
@@ -870,7 +870,7 @@ stap_parse_single_operand (struct stap_parse_info *p)
       return;
     }
 
-  if (*p->arg == '-' || *p->arg == '~' || *p->arg == '+')
+  if (*p->arg == '-' || *p->arg == '~' || *p->arg == '+' || *p->arg == '!')
     {
       char c = *p->arg;
       /* We use this variable to do a lookahead.  */
@@ -924,6 +924,8 @@ stap_parse_single_operand (struct stap_parse_info *p)
            write_exp_elt_opcode (&p->pstate, UNOP_NEG);
          else if (c == '~')
            write_exp_elt_opcode (&p->pstate, UNOP_COMPLEMENT);
+         else if (c == '!')
+           write_exp_elt_opcode (&p->pstate, UNOP_LOGICAL_NOT);
        }
     }
   else if (isdigit (*p->arg))
@@ -1012,7 +1014,7 @@ stap_parse_argument_conditionally (struct stap_parse_info *p)
 {
   gdb_assert (gdbarch_stap_is_single_operand_p (p->gdbarch));
 
-  if (*p->arg == '-' || *p->arg == '~' || *p->arg == '+' /* Unary.  */
+  if (*p->arg == '-' || *p->arg == '~' || *p->arg == '+' || *p->arg == '!'
       || isdigit (*p->arg)
       || gdbarch_stap_is_single_operand (p->gdbarch, p->arg))
     stap_parse_single_operand (p);
@@ -1027,11 +1029,12 @@ stap_parse_argument_conditionally (struct stap_parse_info *p)
 
       stap_parse_argument_1 (p, 0, STAP_OPERAND_PREC_NONE);
 
-      --p->inside_paren_p;
+      p->arg = skip_spaces (p->arg);
       if (*p->arg != ')')
-       error (_("Missign close-paren on expression `%s'."),
+       error (_("Missign close-parenthesis on expression `%s'."),
               p->saved_arg);
 
+      --p->inside_paren_p;
       ++p->arg;
       if (p->inside_paren_p)
        p->arg = skip_spaces (p->arg);
@@ -1067,6 +1070,9 @@ stap_parse_argument_1 (struct stap_parse_info *p, bool has_lhs,
       stap_parse_argument_conditionally (p);
     }
 
+  if (p->inside_paren_p)
+    p->arg = skip_spaces (p->arg);
+
   /* Start to parse the right-side, and to "join" left and right sides
      depending on the operation specified.
 
@@ -1104,8 +1110,21 @@ stap_parse_argument_1 (struct stap_parse_info *p, bool has_lhs,
       if (p->inside_paren_p)
        p->arg = skip_spaces (p->arg);
 
-      /* Parse the right-side of the expression.  */
+      /* Parse the right-side of the expression.
+
+        We save whether the right-side is a parenthesized
+        subexpression because, if it is, we will have to finish
+        processing this part of the expression before continuing.  */
+      bool paren_subexp = *p->arg == '(';
+
       stap_parse_argument_conditionally (p);
+      if (p->inside_paren_p)
+       p->arg = skip_spaces (p->arg);
+      if (paren_subexp)
+       {
+         write_exp_elt_opcode (&p->pstate, opcode);
+         continue;
+       }
 
       /* While we still have operators, try to parse another
         right-side, but using the current right-side as a left-side.  */
@@ -1130,6 +1149,8 @@ stap_parse_argument_1 (struct stap_parse_info *p, bool has_lhs,
          /* Parse the right-side of the expression, but since we already
             have a left-side at this point, set `has_lhs' to 1.  */
          stap_parse_argument_1 (p, 1, lookahead_prec);
+         if (p->inside_paren_p)
+           p->arg = skip_spaces (p->arg);
        }
 
       write_exp_elt_opcode (&p->pstate, opcode);

diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index f6f027497621f18fe2b7d0b89c08d1ee415fb3b5..a0211565d2e91c387bac1ff05a1f40a222d762ba 100644 (file)
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2021-01-20  Sergio Durigan Junior  <sergiodj@sergiodj.net>
+
+       * gdb.arch/amd64-stap-expressions.S: New file.
+       * gdb.arch/amd64-stap-expressions.exp: New file.
+
 2021-01-19  Tom de Vries  <tdevries@suse.de>
 
        * gdb.base/step-over-syscall.exp: Detect and handle sysenter/int

diff --git a/gdb/testsuite/gdb.arch/amd64-stap-expressions.S b/gdb/testsuite/gdb.arch/amd64-stap-expressions.S
new file mode 100644 (file)
index 0000000..76a47aa
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-stap-expressions.S
@@ -0,0 +1,43 @@
+/* Copyright (C) 2021 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <sys/sdt.h>
+
+       .file   "amd64-stap-expressions.S"
+       .text
+       .globl  main
+main:
+       /* We use a nop here because we don't want the first probe to
+          be placed at the same location as the main label.  */
+       nop
+
+       /* Single operands.  */
+       STAP_PROBE1(probe, log_neg, 8@!($0+$1))
+       STAP_PROBE1(probe, minus, -8@-($3+$4))
+       STAP_PROBE1(probe, bit_neg, -8@~$22)
+
+       /* Arithmetic expressions.  */
+       STAP_PROBE1(probe, plus1, 8@$3+($10-$8)-$1)
+       STAP_PROBE1(probe, plus2, 8@$100-(  ($8+$10) -$50)+$3)
+       STAP_PROBE1(probe, plus3, 8@$100-(($8+$10)-$50)+((($8 - $9) + $40) - $4)+$4)
+
+       /* Bitwise expressions.  */
+       STAP_PROBE1(probe, and, 8@$128&$128)
+       STAP_PROBE1(probe, or, 8@$8|$4)
+
+       xor     %rax,%rax
+       ret

diff --git a/gdb/testsuite/gdb.arch/amd64-stap-expressions.exp b/gdb/testsuite/gdb.arch/amd64-stap-expressions.exp
new file mode 100644 (file)
index 0000000..5e3cb60
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-stap-expressions.exp
@@ -0,0 +1,68 @@
+# Copyright 2021 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile ".S"
+
+if { ![istarget "x86_64-*-*"] || ![is_lp64_target] } {
+    verbose "Skipping $testfile.exp"
+    return
+}
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile] } {
+    return -1
+}
+
+# Helper procedure to go to probe NAME
+
+proc goto_probe { name } {
+    global decimal hex
+
+    gdb_test "break -pstap $name" "Breakpoint $decimal at $hex"
+    gdb_test "continue" "Breakpoint $decimal, main \\(\\) at .*\r\n.*STAP_PROBE1.*${name},.*\\)"
+}
+
+# Helper procedure to test the probe's argument
+
+proc test_probe_value { value } {
+    gdb_test "print \$_probe_argc" "= 1"
+    gdb_test "print \$_probe_arg0" "= $value"
+}
+
+if { ![runto_main] } {
+    return -1
+}
+
+# Name and expected value for each probe.
+set probe_names_and_values {
+    { "log_neg" "0" }
+    { "minus" "-7" }
+    { "bit_neg" "-23" }
+
+    { "plus1" "4" }
+    { "plus2" "135" }
+    { "plus3" "171" }
+
+    { "and" "128" }
+    { "or" "12" }
+}
+
+foreach probe_info $probe_names_and_values {
+    set name [lindex $probe_info 0]
+    set value [lindex $probe_info 1]
+    with_test_prefix $name {
+       goto_probe $name
+       test_probe_value $value
+    }
+}