From 6f52fdf40465ee8f5421c66ceb21f37856bf6e5e Mon Sep 17 00:00:00 2001 From: Sergio Durigan Junior Date: Sun, 3 Jan 2021 02:42:52 -0500 Subject: [PATCH] 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 Tom Tromey * 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 * gdb.arch/amd64-stap-expressions.S: New file. * gdb.arch/amd64-stap-expressions.exp: New file. --- gdb/ChangeLog | 13 ++++ gdb/stap-probe.c | 31 +++++++-- gdb/testsuite/ChangeLog | 5 ++ .../gdb.arch/amd64-stap-expressions.S | 43 ++++++++++++ .../gdb.arch/amd64-stap-expressions.exp | 68 +++++++++++++++++++ 5 files changed, 155 insertions(+), 5 deletions(-) create mode 100644 gdb/testsuite/gdb.arch/amd64-stap-expressions.S create mode 100644 gdb/testsuite/gdb.arch/amd64-stap-expressions.exp diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 8a9507223a..b63d7a6d6e 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,16 @@ +2021-01-20 Sergio Durigan Junior + Tom Tromey + + * 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 * top.h (switch_thru_all_uis): Use DISABLE_COPY_AND_ASSIGN. diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c index c2ddd047f6..224dd5714f 100644 --- 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 f6f0274976..a0211565d2 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2021-01-20 Sergio Durigan Junior + + * gdb.arch/amd64-stap-expressions.S: New file. + * gdb.arch/amd64-stap-expressions.exp: New file. + 2021-01-19 Tom de Vries * 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 index 0000000000..76a47aa9b5 --- /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 . */ + +#include + + .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 index 0000000000..5e3cb60a9e --- /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 . + +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 + } +} -- 2.34.1