From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15623 invoked by alias); 26 Mar 2015 18:53:56 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 15498 invoked by uid 89); 26 Mar 2015 18:53:55 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.8 required=5.0 tests=AWL,BAYES_00,LIKELY_SPAM_BODY,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 26 Mar 2015 18:53:54 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (Postfix) with ESMTPS id 4DA0B8EB3D; Thu, 26 Mar 2015 18:53:53 +0000 (UTC) Received: from localhost (unused-10-15-17-126.yyz.redhat.com [10.15.17.126]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t2QIrq1w018611 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA256 bits=256 verify=NO); Thu, 26 Mar 2015 14:53:53 -0400 From: Sergio Durigan Junior To: Joel Brobecker Cc: "Jose E. Marchesi" , gdb-patches@sourceware.org Subject: Re: [PATCH V4 5/9] New probe type: DTrace USDT probes. References: <1422874968-382-1-git-send-email-jose.marchesi@oracle.com> <1422874968-382-6-git-send-email-jose.marchesi@oracle.com> <87r3tp722i.fsf@redhat.com> <20150325191418.GA32233@adacore.com> <87bnjfraq1.fsf@oracle.com> <20150326175028.GA13867@adacore.com> <20150326184350.GB13867@adacore.com> X-URL: http://blog.sergiodj.net Date: Thu, 26 Mar 2015 18:53:00 -0000 In-Reply-To: <20150326184350.GB13867@adacore.com> (Joel Brobecker's message of "Thu, 26 Mar 2015 11:43:50 -0700") Message-ID: <87zj6zy4hr.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2015-03/txt/msg00866.txt.bz2 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 > 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/