From 6f52fdf40465ee8f5421c66ceb21f37856bf6e5e Mon Sep 17 00:00:00 2001 From: Sergio Durigan Junior Date: Sun, 3 Jan 2021 02:42:52 -0500 Subject: 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/stap-probe.c | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) (limited to 'gdb/stap-probe.c') diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c index c2ddd04..224dd57 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); -- cgit v1.1