From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 38513 invoked by alias); 25 Jun 2018 20:51:46 -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 38482 invoked by uid 89); 25 Jun 2018 20:51:43 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-3.6 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_LOW autolearn=no 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; Mon, 25 Jun 2018 20:51:41 +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 w5PKnfjS086157 for ; Mon, 25 Jun 2018 16:51:40 -0400 Received: from e32.co.us.ibm.com (e32.co.us.ibm.com [32.97.110.150]) by mx0b-001b2d01.pphosted.com with ESMTP id 2ju7jm0ewe-1 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NOT) for ; Mon, 25 Jun 2018 16:51:39 -0400 Received: from localhost by e32.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 25 Jun 2018 14:51:39 -0600 Received: from b03cxnp07029.gho.boulder.ibm.com (9.17.130.16) by e32.co.us.ibm.com (192.168.1.132) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; (version=TLSv1/SSLv3 cipher=AES256-GCM-SHA384 bits=256/256) Mon, 25 Jun 2018 14:51:36 -0600 Received: from b03ledav005.gho.boulder.ibm.com (b03ledav005.gho.boulder.ibm.com [9.17.130.236]) by b03cxnp07029.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id w5PKpYb89568698 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Mon, 25 Jun 2018 13:51:34 -0700 Received: from b03ledav005.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 09CF2BE05A; Mon, 25 Jun 2018 14:51:34 -0600 (MDT) Received: from b03ledav005.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id C611EBE04F; Mon, 25 Jun 2018 14:51:33 -0600 (MDT) Received: from pedro.localdomain (unknown [9.18.235.82]) by b03ledav005.gho.boulder.ibm.com (Postfix) with ESMTP; Mon, 25 Jun 2018 14:51:33 -0600 (MDT) Received: by pedro.localdomain (Postfix, from userid 1000) id 3FC8F3C0426; Mon, 25 Jun 2018 17:51:31 -0300 (-03) From: Pedro Franco de Carvalho To: Ulrich Weigand Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 3/4] Use get_remote_packet_size in download_tracepoint In-Reply-To: <20180625103720.2F6DAD801CC@oc3748833570.ibm.com> References: <20180625103720.2F6DAD801CC@oc3748833570.ibm.com> Date: Mon, 25 Jun 2018 20:51:00 -0000 MIME-Version: 1.0 Content-Type: text/plain x-cbid: 18062520-0004-0000-0000-0000145A042F X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00009254; HX=3.00000241; KW=3.00000007; PH=3.00000004; SC=3.00000266; SDB=6.01052263; UDB=6.00539425; IPR=6.00830188; MB=3.00021853; MTD=3.00000008; XFM=3.00000015; UTC=2018-06-25 20:51:37 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 18062520-0005-0000-0000-000087DDDBC9 Message-Id: <878t72mvxo.fsf@linux.vnet.ibm.com> X-IsSubscribed: yes X-SW-Source: 2018-06/txt/msg00587.txt.bz2 Ulrich Weigand writes: > You know from the beginning that the agent expression will take > (2 * aexpr->len) bytes, so it should be OK to only check this > once, ahead of time. In fact, sending a partial agent expression > seems to be worse than sending none, so if the agent expression > is too long, I think it should be just omitted (and the user > warned). I don't think a partial agent expression would be sent in this case, since this is before the first putpkt is called in the function. But I can still issue the warning and ignore the condition expression instead of failing on the assertion. Otherwise I can check the size once and call a gdb_assert if its too small, like the rest of the function. Which is better? Would something like one of the two alternative below be ok for checking the size only once? The second one looks complicated, but my goal was to avoid overflows in 2 * aexpr->len, since that length ultimately comes from the condition expression the user supplies. I am also assuming throughout this function that size_t and gdb::char_vector::size_type are compatible (since buf.size () returns the latter and xsnprintf takes a size_t). Is this ok? Thanks! 1: 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); - for (int ndx = 0; ndx < aexpr->len; ++ndx) - pkt = pack_hex_byte (pkt, aexpr->buf[ndx]); - *pkt = '\0'; + + int cond_str_size = snprintf (NULL, 0, ":X%x,", aexpr->len); + gdb_assert (cond_str_size >= 0); + + cond_str_size += aexpr->len * 2; + + if (cond_str_size < buf.size () - strlen (buf.data ())) + { + sprintf (buf.data () + strlen (buf.data ()), + ":X%x,", aexpr->len); + + pkt = buf.data () + strlen (buf.data ()); + + for (int ndx = 0; ndx < aexpr->len; ++ndx) + pkt = pack_hex_byte (pkt, aexpr->buf[ndx]); + *pkt = '\0'; + } + else + warning (_("Condition expression too long, " + "ignoring tp %d cond"), b->number); } else warning (_("Target does not support conditional tracepoints, " 2: 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); - for (int ndx = 0; ndx < aexpr->len; ++ndx) - pkt = pack_hex_byte (pkt, aexpr->buf[ndx]); - *pkt = '\0'; + + int cond_str_size = snprintf (NULL, 0, ":X%x,", aexpr->len); + gdb_assert (cond_str_size >= 0); + + size_t size_left = buf.size () - strlen (buf.data ()); + + bool size_ok = (cond_str_size < size_left); + + if (size_ok) + { + size_left -= cond_str_size; + + size_ok = (size_left/2 > aexpr->len); + + if (size_ok) + { + sprintf (buf.data () + strlen (buf.data ()), + ":X%x,", aexpr->len); + + pkt = buf.data () + strlen (buf.data ()); + + for (int ndx = 0; ndx < aexpr->len; ++ndx) + pkt = pack_hex_byte (pkt, aexpr->buf[ndx]); + *pkt = '\0'; + } + } + + if (!size_ok) + warning (_("Condition expression too long, " + "ignoring tp %d cond"), b->number); }