From patchwork Fri Aug 21 23:37:32 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sergio Durigan Junior X-Patchwork-Id: 8389 Received: (qmail 103425 invoked by alias); 21 Aug 2015 23:37:50 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 103399 invoked by uid 89); 21 Aug 2015 23:37:49 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.6 required=5.0 tests=AWL, BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD, SPF_HELO_PASS 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; Fri, 21 Aug 2015 23:37:48 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id F110F15774A for ; Fri, 21 Aug 2015 23:37:46 +0000 (UTC) Received: from psique.yyz.redhat.com (unused-10-15-17-51.yyz.redhat.com [10.15.17.51]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t7LNbk19004671; Fri, 21 Aug 2015 19:37:46 -0400 From: Sergio Durigan Junior To: GDB Patches Cc: Sergio Durigan Junior Subject: [PATCH 1/2] Improve error reporting when handling SystemTap SDT probes Date: Fri, 21 Aug 2015 19:37:32 -0400 Message-Id: <1440200253-28603-2-git-send-email-sergiodj@redhat.com> In-Reply-To: <1440200253-28603-1-git-send-email-sergiodj@redhat.com> References: <1440200253-28603-1-git-send-email-sergiodj@redhat.com> X-IsSubscribed: yes This patch improves the error reporting when handling SystemTap SDT probes. "Handling", in this case, mostly means "parsing". On gdb/probe.h, only trivial changes on functions' comments in order to explicitly mention that some of them can throw exceptions. This is just to make the API a bit more clear. On gdb/stap-probe.c, I have s/internal_error/error/ on two functions that are responsible for parsing specific bits of the probes' arguments: stap_get_opcode and stap_get_expected_argument_type. It is not correct to call internal_error on such situations because it is not really GDB's fault if the probes have malformed arguments. I also improved the error reported on stap_get_expected_argument_type by also including the probe name on it. Aside from that, and perhaps most importantly, I added a check on stap_get_arg to make sure that we don't try to extract an argument from a probe that has no arguments. This check issues an internal_error, because it really means that GDB is doing something it shouldn't. Although it can be considered almost trivial, and despite the fact that I am the maintainer for this part of the code, I am posting this patch for review. I will wait a few days, and if nobody has anything to say, I will go ahead and push it. gdb/ChangeLog: 2015-08-21 Sergio Durigan Junior * probe.h (struct probe_ops) : Mention in the comment that the function can throw an exception. (get_probe_argument_count): Likewise. (evaluate_probe_argument): Likewise. * stap-probe.c (stap_get_opcode): Call error instead of internal_error. (stap_get_expected_argument_type): Likewise. Add argument 'probe'. Improve error message by mentioning the probe's name. (stap_parse_probe_arguments): Adjust call to stap_get_expected_argument_type. (stap_get_arg): Add comment. Assert that 'probe->args_parsed' is not zero. Call internal_error if GDB requests an argument but the probe has no arguments. --- gdb/probe.h | 20 ++++++++++++++------ gdb/stap-probe.c | 29 ++++++++++++++++++++++------- 2 files changed, 36 insertions(+), 13 deletions(-) diff --git a/gdb/probe.h b/gdb/probe.h index c058a38..317aaab 100644 --- a/gdb/probe.h +++ b/gdb/probe.h @@ -72,7 +72,8 @@ struct probe_ops CORE_ADDR (*get_probe_address) (struct probe *probe, struct objfile *objfile); - /* Return the number of arguments of PROBE. */ + /* Return the number of arguments of PROBE. This function can + throw an exception. */ unsigned (*get_probe_argument_count) (struct probe *probe, struct frame_info *frame); @@ -84,7 +85,8 @@ struct probe_ops int (*can_evaluate_probe_arguments) (struct probe *probe); /* Evaluate the Nth argument from the PROBE, returning a value - corresponding to it. The argument number is represented N. */ + corresponding to it. The argument number is represented N. + This function can throw an exception. */ struct value *(*evaluate_probe_argument) (struct probe *probe, unsigned n, @@ -143,13 +145,15 @@ struct probe_ops /* Enable a probe. The semantics of "enabling" a probe depend on the specific backend and the field can be NULL in case enabling - probes is not supported. */ + probes is not supported. This function can throw an + exception. */ void (*enable_probe) (struct probe *probe); /* Disable a probe. The semantics of "disabling" a probe depend on the specific backend and the field can be NULL in case - disabling probes is not supported. */ + disabling probes is not supported. This function can throw an + exception. */ void (*disable_probe) (struct probe *probe); }; @@ -266,7 +270,9 @@ extern struct cmd_list_element **info_probes_cmdlist_get (void); extern CORE_ADDR get_probe_address (struct probe *probe, struct objfile *objfile); -/* Return the argument count of the specified probe. */ +/* Return the argument count of the specified probe. + + This function can throw an exception. */ extern unsigned get_probe_argument_count (struct probe *probe, struct frame_info *frame); @@ -278,7 +284,9 @@ extern unsigned get_probe_argument_count (struct probe *probe, extern int can_evaluate_probe_arguments (struct probe *probe); /* Evaluate argument N of the specified probe. N must be between 0 - inclusive and get_probe_argument_count exclusive. */ + inclusive and get_probe_argument_count exclusive. + + This function can throw an exception. */ extern struct value *evaluate_probe_argument (struct probe *probe, unsigned n, diff --git a/gdb/stap-probe.c b/gdb/stap-probe.c index 9ee9767..2462acc 100644 --- a/gdb/stap-probe.c +++ b/gdb/stap-probe.c @@ -313,9 +313,8 @@ stap_get_opcode (const char **s) break; default: - internal_error (__FILE__, __LINE__, - _("Invalid opcode in expression `%s' for SystemTap" - "probe"), *s); + error (_("Invalid opcode in expression `%s' for SystemTap" + "probe"), *s); } return op; @@ -326,7 +325,8 @@ stap_get_opcode (const char **s) static struct type * stap_get_expected_argument_type (struct gdbarch *gdbarch, - enum stap_arg_bitness b) + enum stap_arg_bitness b, + const struct stap_probe *probe) { switch (b) { @@ -361,8 +361,8 @@ stap_get_expected_argument_type (struct gdbarch *gdbarch, return builtin_type (gdbarch)->builtin_uint64; default: - internal_error (__FILE__, __LINE__, - _("Undefined bitness for probe.")); + error (_("Undefined bitness for probe '%s'."), + probe->p.name); break; } } @@ -1172,7 +1172,8 @@ stap_parse_probe_arguments (struct stap_probe *probe, struct gdbarch *gdbarch) else arg.bitness = STAP_ARG_BITNESS_UNDEFINED; - arg.atype = stap_get_expected_argument_type (gdbarch, arg.bitness); + arg.atype = stap_get_expected_argument_type (gdbarch, arg.bitness, + probe); expr = stap_parse_argument (&cur, arg.atype, gdbarch); @@ -1278,12 +1279,26 @@ stap_is_operator (const char *op) return ret; } +/* Return argument N of probe PROBE. + + If the probe's arguments have not been parsed yet, parse them. If + there are no arguments, throw an exception (error). Otherwise, + return the requested argument. */ + static struct stap_probe_arg * stap_get_arg (struct stap_probe *probe, unsigned n, struct gdbarch *gdbarch) { if (!probe->args_parsed) stap_parse_probe_arguments (probe, gdbarch); + gdb_assert (probe->args_parsed); + if (probe->args_u.vec == NULL) + internal_error (__FILE__, __LINE__, + _("Probe '%s' apparently does not have arguments, but \n" + "GDB is requesting its argument number %u anyway. " + "This should not happen. Please report this bug."), + probe->p.name, n); + return VEC_index (stap_probe_arg_s, probe->args_u.vec, n); }