Explicitly use language_c when evaluating a SDT probe argument

Message ID 1412886259-11383-1-git-send-email-sergiodj@redhat.com
State New, archived
Headers

Commit Message

Sergio Durigan Junior Oct. 9, 2014, 8:24 p.m. UTC
  Joel contacted me offlist with a question about an warning that one of
his customers was seeing.  The message came apparently came from the
new linker-debugger interface, which uses SDT probes internally.  The
warning said:

    (gdb) run
    [...]
    warning: Probes-based dynamic linker interface failed.
    Reverting to original interface.

    Argument to arithmetic operation not a number or boolean.

This should not have happened in the environment the customer was
using (RHEL-6.x), so I found it strange.  Another thing caught my
attention: the last message, saying "Argument to arithmetic operation
not a number or boolean.".

Joel kindly investigated the issue further, and found the answer for
this.  To quote him:

	(gdb) set lang c
	(gdb) p 48+$ebp
	$4 = (void *) 0xffffd0f8

    So far so good. But...

	(gdb) set lang ada
	(gdb) p 48+$ebp
	Argument to arithmetic operation not a number or boolean.

    Ooops! Interestingly, if you revert the order of the operands...

	(gdb) p $ebp+48
	$5 = (access void) 0xffffd0f8

So the problem is doing pointer arithmetics when the language is set
to Ada.

I remembered that, during the parsing and the evaluation of SDT probe
arguments, the code sets the language as current_language, because, at
that time, I thought it was not necessary to worry about the language
given that the code implements its own parser.  I was wrong.  So here
is a patch to fix that, by setting the language as C, which should
guarantee that the maths are done in the right way (TM).

Joel kindly tested this for me, and it worked.  I also ran a full
regression test here on my Fedora 20 x86_64, and everything is fine.

I will push this patch in a few days if there are no comments.

gdb/ChangeLog:
2014-10-09  Sergio Durigan Junior  <sergiodj@redhat.com>

	* stap-probe.c (stap_parse_argument): Initialize expout explicitly
	using language_c, instead of current_language.
---
 gdb/stap-probe.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
  

Comments

Pedro Alves Oct. 10, 2014, 12:38 p.m. UTC | #1
On 10/09/2014 09:24 PM, Sergio Durigan Junior wrote:

> I will push this patch in a few days if there are no comments.

No comments on the code itself, but I am wondering if we can add
a test.  :-)  As per your example, it wouldn't even have to be
written in Ada, rather we'd just need to force it with "set lang ada".

Thanks,
Pedro Alves
  
Sergio Durigan Junior Oct. 11, 2014, 6:40 p.m. UTC | #2
On Friday, October 10 2014, Pedro Alves wrote:

> No comments on the code itself, but I am wondering if we can add
> a test.  :-)  As per your example, it wouldn't even have to be
> written in Ada, rather we'd just need to force it with "set lang ada".

Good idea, indeed :-).  I submitted v2 now with a testcase attached.

Thanks!
  

Patch

diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c
index a51bf8b..3902997 100644
--- a/gdb/stap-probe.c
+++ b/gdb/stap-probe.c
@@ -1050,9 +1050,9 @@  stap_parse_argument (const char **arg, struct type *atype,
   struct cleanup *back_to;
 
   /* We need to initialize the expression buffer, in order to begin
-     our parsing efforts.  The language here does not matter, since we
-     are using our own parser.  */
-  initialize_expout (&p.pstate, 10, current_language, gdbarch);
+     our parsing efforts.  We use language_c here because we may need
+     to do pointer arithmetics.  */
+  initialize_expout (&p.pstate, 10, language_def (language_c), gdbarch);
   back_to = make_cleanup (free_current_contents, &p.pstate.expout);
 
   p.saved_arg = *arg;