From patchwork Fri Jul 27 21:03:15 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Franco de Carvalho X-Patchwork-Id: 28660 Received: (qmail 117069 invoked by alias); 27 Jul 2018 21:03:47 -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 116989 invoked by uid 89); 27 Jul 2018 21:03:47 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0b-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.158.5) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 27 Jul 2018 21:03:45 +0000 Received: from pps.filterd (m0098419.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.22/8.16.0.22) with SMTP id w6RKsMqY024487 for ; Fri, 27 Jul 2018 17:03:43 -0400 Received: from e11.ny.us.ibm.com (e11.ny.us.ibm.com [129.33.205.201]) by mx0b-001b2d01.pphosted.com with ESMTP id 2kg6d0jytn-1 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 27 Jul 2018 17:03:43 -0400 Received: from localhost by e11.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 27 Jul 2018 17:03:42 -0400 Received: from b01cxnp22033.gho.pok.ibm.com (9.57.198.23) by e11.ny.us.ibm.com (146.89.104.198) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; (version=TLSv1/SSLv3 cipher=AES256-GCM-SHA384 bits=256/256) Fri, 27 Jul 2018 17:03:40 -0400 Received: from b01ledav005.gho.pok.ibm.com (b01ledav005.gho.pok.ibm.com [9.57.199.110]) by b01cxnp22033.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id w6RL3cbl9109948 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Fri, 27 Jul 2018 21:03:38 GMT Received: from b01ledav005.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 9F80EAE063; Fri, 27 Jul 2018 17:02:50 -0400 (EDT) Received: from b01ledav005.gho.pok.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 4F3FFAE05C; Fri, 27 Jul 2018 17:02:50 -0400 (EDT) Received: from pedro.localdomain (unknown [9.85.199.57]) by b01ledav005.gho.pok.ibm.com (Postfix) with ESMTP; Fri, 27 Jul 2018 17:02:50 -0400 (EDT) Received: by pedro.localdomain (Postfix, from userid 1000) id B0E283C03CE; Fri, 27 Jul 2018 18:03:35 -0300 (-03) From: Pedro Franco de Carvalho To: gdb-patches@sourceware.org Cc: uweigand@de.ibm.com Subject: [PATCH v2 3/6] Use get_remote_packet_size in download_tracepoint Date: Fri, 27 Jul 2018 18:03:15 -0300 In-Reply-To: <20180727210318.2960-1-pedromfc@linux.ibm.com> References: <20180727210318.2960-1-pedromfc@linux.ibm.com> x-cbid: 18072721-2213-0000-0000-000002D0E30F X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00009440; HX=3.00000241; KW=3.00000007; PH=3.00000004; SC=3.00000266; SDB=6.01067028; UDB=6.00548259; IPR=6.00844894; MB=3.00022359; MTD=3.00000008; XFM=3.00000015; UTC=2018-07-27 21:03:41 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 18072721-2214-0000-0000-00005AFCBF95 Message-Id: <20180727210318.2960-4-pedromfc@linux.ibm.com> This patch changes the remote target to use the remote packet size to build QTDP packets, and to check if there is enough room for the packet. I changed the function to raise an error if the packet is too small, instead of aborting gdb (through xsnprintf). It isn't clear if gdb will be in a consistent state with respect to the stub after this, since it's possible that some packets will be sent but not others, and there could be an incomplete tracepoint on the stub. The char array used to build the packets is changed to a gdb::char_vector and sized with the result from get_remote_packet_size. When checking if the buffer is large enough to hold the tracepoint condition agent expression, the length of the expression is multiplied by two, since it is encoded with two hex digits per expression byte. For simplicity, I assume that the result won't overflow, which can happen for very long condition expressions. gdb/ChangeLog: YYYY-MM-DD Pedro Franco de Carvalho * remote.c (remote_target::download_tracepoint): Remove BUF_SIZE. Replace array buf with gdb::char_vector buf, of size get_remote_packet_size (). Replace references to buf and BUF_SIZE to buf.data () and buf.size (). Replace strcpy, strcat and pack_hex_byte with snprintf. Raise errors if the buffer is too small. --- gdb/remote.c | 135 ++++++++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 101 insertions(+), 34 deletions(-) diff --git a/gdb/remote.c b/gdb/remote.c index e3180923ee..5c8b6861fc 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -12832,26 +12832,35 @@ remote_target::remote_download_command_source (int num, ULONGEST addr, void remote_target::download_tracepoint (struct bp_location *loc) { -#define BUF_SIZE 2048 - CORE_ADDR tpaddr; char addrbuf[40]; - char buf[BUF_SIZE]; std::vector tdp_actions; std::vector stepping_actions; char *pkt; struct breakpoint *b = loc->owner; struct tracepoint *t = (struct tracepoint *) b; struct remote_state *rs = get_remote_state (); + int ret; + char *err_msg = _("Tracepoint packet too large for target."); + size_t size_left; + + /* We use a buffer other than rs->buf because we'll build strings + across multiple statements, and other statements in between could + modify rs->buf. */ + gdb::char_vector buf (get_remote_packet_size ()); encode_actions_rsp (loc, &tdp_actions, &stepping_actions); tpaddr = loc->address; sprintf_vma (addrbuf, tpaddr); - xsnprintf (buf, BUF_SIZE, "QTDP:%x:%s:%c:%lx:%x", b->number, - addrbuf, /* address */ - (b->enable_state == bp_enabled ? 'E' : 'D'), - t->step_count, t->pass_count); + ret = snprintf (buf.data (), buf.size (), "QTDP:%x:%s:%c:%lx:%x", + b->number, addrbuf, /* address */ + (b->enable_state == bp_enabled ? 'E' : 'D'), + t->step_count, t->pass_count); + + if (ret < 0 || ret >= buf.size ()) + error (err_msg); + /* Fast tracepoints are mostly handled by the target, but we can tell the target how big of an instruction block should be moved around. */ @@ -12863,8 +12872,15 @@ remote_target::download_tracepoint (struct bp_location *loc) { if (gdbarch_fast_tracepoint_valid_at (loc->gdbarch, tpaddr, NULL)) - xsnprintf (buf + strlen (buf), BUF_SIZE - strlen (buf), ":F%x", - gdb_insn_length (loc->gdbarch, tpaddr)); + { + size_left = buf.size () - strlen (buf.data ()); + ret = snprintf (buf.data () + strlen (buf.data ()), + size_left, ":F%x", + gdb_insn_length (loc->gdbarch, tpaddr)); + + if (ret < 0 || ret >= size_left) + error (err_msg); + } else /* If it passed validation at definition but fails now, something is very wrong. */ @@ -12888,7 +12904,15 @@ remote_target::download_tracepoint (struct bp_location *loc) struct static_tracepoint_marker marker; if (target_static_tracepoint_marker_at (tpaddr, &marker)) - strcat (buf, ":S"); + { + size_left = buf.size () - strlen (buf.data ()); + ret = snprintf (buf.data () + strlen (buf.data ()), + size_left, ":F%x", + gdb_insn_length (loc->gdbarch, tpaddr)); + + if (ret < 0 || ret >= size_left) + error (err_msg); + } else error (_("Static tracepoint not valid during download")); } @@ -12906,10 +12930,26 @@ remote_target::download_tracepoint (struct bp_location *loc) capabilities at definition time. */ if (remote_supports_cond_tracepoints ()) { - agent_expr_up aexpr = gen_eval_for_expr (tpaddr, loc->cond.get ()); - xsnprintf (buf + strlen (buf), BUF_SIZE - strlen (buf), ":X%x,", - aexpr->len); - pkt = buf + strlen (buf); + agent_expr_up aexpr = gen_eval_for_expr (tpaddr, + loc->cond.get ()); + + size_left = buf.size () - strlen (buf.data ()); + + ret = snprintf (buf.data () + strlen (buf.data ()), + size_left, ":X%x,", aexpr->len); + + if (ret < 0 || ret >= size_left) + error (err_msg); + + size_left = buf.size () - strlen (buf.data ()); + + /* Two bytes to encode each aexpr byte, plus the terminating + null byte. */ + if (aexpr->len * 2 + 1 > size_left) + error (err_msg); + + pkt = buf.data () + strlen (buf.data ()); + for (int ndx = 0; ndx < aexpr->len; ++ndx) pkt = pack_hex_byte (pkt, aexpr->buf[ndx]); *pkt = '\0'; @@ -12920,8 +12960,17 @@ remote_target::download_tracepoint (struct bp_location *loc) } if (b->commands || *default_collect) - strcat (buf, "-"); - putpkt (buf); + { + size_left = buf.size () - strlen (buf.data ()); + + ret = snprintf (buf.data () + strlen (buf.data ()), + size_left, "-"); + + if (ret < 0 || ret >= size_left) + error (err_msg); + } + + putpkt (buf.data ()); remote_get_noisy_reply (); if (strcmp (rs->buf, "OK")) error (_("Target does not support tracepoints.")); @@ -12935,11 +12984,15 @@ remote_target::download_tracepoint (struct bp_location *loc) bool has_more = ((action_it + 1) != tdp_actions.end () || !stepping_actions.empty ()); - xsnprintf (buf, BUF_SIZE, "QTDP:-%x:%s:%s%c", - b->number, addrbuf, /* address */ - action_it->c_str (), - has_more ? '-' : 0); - putpkt (buf); + ret = snprintf (buf.data (), buf.size (), "QTDP:-%x:%s:%s%c", + b->number, addrbuf, /* address */ + action_it->c_str (), + has_more ? '-' : 0); + + if (ret < 0 || ret >= buf.size ()) + error (err_msg); + + putpkt (buf.data ()); remote_get_noisy_reply (); if (strcmp (rs->buf, "OK")) error (_("Error on target while setting tracepoints.")); @@ -12953,12 +13006,16 @@ remote_target::download_tracepoint (struct bp_location *loc) bool is_first = action_it == stepping_actions.begin (); bool has_more = (action_it + 1) != stepping_actions.end (); - xsnprintf (buf, BUF_SIZE, "QTDP:-%x:%s:%s%s%s", - b->number, addrbuf, /* address */ - is_first ? "S" : "", - action_it->c_str (), - has_more ? "-" : ""); - putpkt (buf); + ret = snprintf (buf.data (), buf.size (), "QTDP:-%x:%s:%s%s%s", + b->number, addrbuf, /* address */ + is_first ? "S" : "", + action_it->c_str (), + has_more ? "-" : ""); + + if (ret < 0 || ret >= buf.size ()) + error (err_msg); + + putpkt (buf.data ()); remote_get_noisy_reply (); if (strcmp (rs->buf, "OK")) error (_("Error on target while setting tracepoints.")); @@ -12968,22 +13025,32 @@ remote_target::download_tracepoint (struct bp_location *loc) { if (b->location != NULL) { - strcpy (buf, "QTDPsrc:"); + ret = snprintf (buf.data (), buf.size (), "QTDPsrc:"); + + if (ret < 0 || ret >= buf.size ()) + error (err_msg); + encode_source_string (b->number, loc->address, "at", event_location_to_string (b->location.get ()), - buf + strlen (buf), 2048 - strlen (buf)); - putpkt (buf); + buf.data () + strlen (buf.data ()), + buf.size () - strlen (buf.data ())); + putpkt (buf.data ()); remote_get_noisy_reply (); if (strcmp (rs->buf, "OK")) warning (_("Target does not support source download.")); } if (b->cond_string) { - strcpy (buf, "QTDPsrc:"); + ret = snprintf (buf.data (), buf.size (), "QTDPsrc:"); + + if (ret < 0 || ret >= buf.size ()) + error (err_msg); + encode_source_string (b->number, loc->address, - "cond", b->cond_string, buf + strlen (buf), - 2048 - strlen (buf)); - putpkt (buf); + "cond", b->cond_string, + buf.data () + strlen (buf.data ()), + buf.size () - strlen (buf.data ())); + putpkt (buf.data ()); remote_get_noisy_reply (); if (strcmp (rs->buf, "OK")) warning (_("Target does not support source download."));