This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH V4 5/9] New probe type: DTrace USDT probes.
- From: Sergio Durigan Junior <sergiodj at redhat dot com>
- To: Joel Brobecker <brobecker at adacore dot com>
- Cc: "Jose E. Marchesi" <jose dot marchesi at oracle dot com>, gdb-patches at sourceware dot org
- Date: Thu, 26 Mar 2015 14:53:52 -0400
- Subject: Re: [PATCH V4 5/9] New probe type: DTrace USDT probes.
- Authentication-results: sourceware.org; auth=none
- References: <1422874968-382-1-git-send-email-jose dot marchesi at oracle dot com> <1422874968-382-6-git-send-email-jose dot marchesi at oracle dot com> <87r3tp722i dot fsf at redhat dot com> <20150325191418 dot GA32233 at adacore dot com> <87bnjfraq1 dot fsf at oracle dot com> <20150326175028 dot GA13867 at adacore dot com> <20150326184350 dot GB13867 at adacore dot com>
On Thursday, March 26 2015, Joel Brobecker wrote:
> [Sergio - a change in dtrace-probe.c needs your approval, I think.
> Also, if you have a chance to double-check the rest of the patch,
> that'd be a much appreciated second pair of eyes!]
Thanks, Joel. I saw the messages yesterday but did not get a chance to
review until today.
>> > Well, the TRY..CATCH in your prototype would catch any error that may be
>> > thrown in parse_expression, and the `set_language' must take care of
>> > changing the language, so I would say that is enough...
>>
>> OK - I will send an updated patch that makes things a little cleaner.
>> I didn't know whether it was OK to default to type long makes much
>> sense when the probe says the parameter should be using type "mutex_t".
I remember we had the same problem (i.e., not parsing using language_c)
on SystemTap SDT probes as well. Coincidentally or not, you triggered
the problem twice!
> Here it is.
>
> gdb/ChangeLog:
>
> * dtrace-probe.c (dtrace_process_dof_probe): Contain any
> exception raised while parsing the probe arguments.
> Force parsing to be done using the C language parser.
> * expression.h (parse_expression_with_language): Declare.
> * parse.c (parse_expression_with_language): New function.
>
> Tested on sparc-solaris.
>
> --
> Joel
>
> From ba8991270e994cd96f92316932f65f96e47bf329 Mon Sep 17 00:00:00 2001
> From: Joel Brobecker <brobecker@adacore.com>
> Date: Thu, 26 Mar 2015 19:14:03 +0100
> Subject: [PATCH] dtrace-probe: Handle error while parsing probe argument.
>
> The debugger on Solaris has been broken since the introduction of
> DTrace probe support:
>
> (gdb) start
> Temporary breakpoint 1 at 0x80593bc: file simple_main.adb, line 4.
> Starting program: /[...]/simple_main
> [Thread debugging using libthread_db enabled]
> No definition of "mutex_t" in current context.
>
> The problem occurs while trying to parse a probe's arguument,
"argument"
> and the exception propagates all the way to the top. This patch
> fixes the issue by containing the exception and falling back on
> using the "long" builtin type if the argument's type could not
> be determined.
>
> Also, the parsing should be done using the C language parser.
>
> gdb/ChangeLog:
>
> * dtrace-probe.c (dtrace_process_dof_probe): Contain any
> exception raised while parsing the probe arguments.
> Force parsing to be done using the C language parser.
> * expression.h (parse_expression_with_language): Declare.
> * parse.c (parse_expression_with_language): New function.
> ---
> gdb/dtrace-probe.c | 14 ++++++++++++--
> gdb/expression.h | 3 +++
> gdb/parse.c | 22 ++++++++++++++++++++++
> 3 files changed, 37 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/dtrace-probe.c b/gdb/dtrace-probe.c
> index 491d853..ff7ce7d 100644
> --- a/gdb/dtrace-probe.c
> +++ b/gdb/dtrace-probe.c
> @@ -427,8 +427,18 @@ dtrace_process_dof_probe (struct objfile *objfile,
> this does not work then we set the type to `long
> int'. */
> arg.type = builtin_type (gdbarch)->builtin_long;
> - expr = parse_expression (arg.type_str);
> - if (expr->elts[0].opcode == OP_TYPE)
> +
> + TRY
> + {
> + expr = parse_expression_with_language (arg.type_str, language_c);
> + }
> + CATCH (ex, RETURN_MASK_ERROR)
> + {
> + expr = NULL;
> + }
> + END_CATCH
> +
> + if (expr != NULL && expr->elts[0].opcode == OP_TYPE)
> arg.type = expr->elts[1].type;
>
> VEC_safe_push (dtrace_probe_arg_s, ret->args, &arg);
This part looks OK.
> diff --git a/gdb/expression.h b/gdb/expression.h
> index f5cd0e5..d08ce05 100644
> --- a/gdb/expression.h
> +++ b/gdb/expression.h
> @@ -97,6 +97,9 @@ struct expression
>
> extern struct expression *parse_expression (const char *);
>
> +extern struct expression *parse_expression_with_language (const char *string,
> + enum language lang);
> +
> extern struct expression *parse_expression_in_context (const char *, int);
>
> extern struct type *parse_field_expression (char *, char **);
> diff --git a/gdb/parse.c b/gdb/parse.c
> index 2ffe52e..bc95cf1 100644
> --- a/gdb/parse.c
> +++ b/gdb/parse.c
> @@ -1268,6 +1268,28 @@ parse_expression (const char *string)
> return exp;
> }
>
> +/* Same as parse_expression, but using the given language (LANG)
> + to parse the expression. */
> +
> +struct expression *
> +parse_expression_with_language (const char *string, enum language lang)
> +{
> + struct cleanup *old_chain = NULL;
> + struct expression *expr;
> +
> + if (current_language->la_language != language_c)
> + {
> + old_chain = make_cleanup_restore_current_language ();
> + set_language (language_c);
> + }
Here I think you meant to use "lang" instead of "language_c", right?
> +
> + expr = parse_expression (string);
> +
> + if (old_chain != NULL)
> + do_cleanups (old_chain);
> + return expr;
> +}
> +
> /* As for parse_expression, except that if VOID_CONTEXT_P, then
> no value is expected from the expression. */
>
> --
> 1.7.10.4
>
OK with the fix above.
Thanks,
--
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/