Message ID | C7AECD2C-570A-4934-A153-FFA8B66E8185@arm.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 4861 invoked by alias); 12 Jun 2017 13:59:55 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 4391 invoked by uid 89); 12 Jun 2017 13:59:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.0 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, MIME_BASE64_BLANKS, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: EUR03-AM5-obe.outbound.protection.outlook.com Received: from mail-eopbgr30051.outbound.protection.outlook.com (HELO EUR03-AM5-obe.outbound.protection.outlook.com) (40.107.3.51) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 12 Jun 2017 13:59:52 +0000 Received: from AM3PR08MB0101.eurprd08.prod.outlook.com (10.160.211.19) by AM3PR08MB0101.eurprd08.prod.outlook.com (10.160.211.19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1157.12; Mon, 12 Jun 2017 13:59:54 +0000 Received: from AM3PR08MB0101.eurprd08.prod.outlook.com ([fe80::f0a8:fd0f:69e1:e280]) by AM3PR08MB0101.eurprd08.prod.outlook.com ([fe80::f0a8:fd0f:69e1:e280%17]) with mapi id 15.01.1157.017; Mon, 12 Jun 2017 13:59:54 +0000 From: Alan Hayward <Alan.Hayward@arm.com> To: Yao Qi <qiyaoltc@gmail.com> CC: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>, nd <nd@arm.com> Subject: Re: [PATCH] Remove an instance of MAX_REGISTER_SIZE from record-full.c Date: Mon, 12 Jun 2017 13:59:54 +0000 Message-ID: <C7AECD2C-570A-4934-A153-FFA8B66E8185@arm.com> References: <69EEE46A-D88C-4B4F-86A2-E35F6DAFD90A@arm.com> <86d1a9a2k9.fsf@gmail.com> In-Reply-To: <86d1a9a2k9.fsf@gmail.com> authentication-results: gmail.com; dkim=none (message not signed) header.d=none; gmail.com; dmarc=none action=none header.from=arm.com; x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; AM3PR08MB0101; 7:RHs2fILL/n9DSkT6BggXIMlw06vZc4Xs/nNReRG+a3BMVa43IkVVaoPL1X/A2vYXKNTZIu1HXpV1EVKxuocq61n6SPmXFgqn5FBHDCMhbuUw4uRocd54AN/dUi1rJLvKz4URYBRHQ7unLuvz9BjgC3kxM6PZzdgd2NGQOC4kIe3z8GdO5xkEHh5CP5MQZzPcz0w9Zadx3ZnzyTLZr1bML2dfaAeqW3SSAoNIrIrY2O5y6q3LYJrY4OE99ZWqBQ0rChBeGUI/cdmVjPT9eOk26PR5MOcjFMnGa3Dv/XZYmhyjvak6sqTXqr80+WIrwUP/A8AZq+yphZu0U5B1ho8XbA== x-ms-traffictypediagnostic: AM3PR08MB0101: x-ms-office365-filtering-correlation-id: b31f183f-8a0f-46e0-fe5d-08d4b19b4d75 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(22001)(2017030254075)(48565401081)(201703131423075)(201703031133081); SRVR:AM3PR08MB0101; nodisclaimer: True x-microsoft-antispam-prvs: <AM3PR08MB0101F8993DA90AB0CD95959997CD0@AM3PR08MB0101.eurprd08.prod.outlook.com> x-exchange-antispam-report-test: UriScan:(180628864354917); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(601004)(2401047)(5005006)(8121501046)(10201501046)(3002001)(100000703101)(100105400095)(93006095)(93001095)(6055026)(6041248)(20161123555025)(20161123562025)(20161123558100)(20161123564025)(20161123560025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(6072148)(100000704101)(100105200095)(100000705101)(100105500095); SRVR:AM3PR08MB0101; BCL:0; PCL:0; RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095); SRVR:AM3PR08MB0101; x-forefront-prvs: 03361FCC43 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(6009001)(39840400002)(39850400002)(39410400002)(39400400002)(39450400003)(39860400002)(24454002)(377424004)(3846002)(6486002)(229853002)(8936002)(6512007)(5660300001)(8676002)(81166006)(39060400002)(6436002)(5250100002)(72206003)(2906002)(305945005)(7736002)(2900100001)(86362001)(6506006)(6116002)(2950100002)(53936002)(4326008)(478600001)(6916009)(102836003)(575784001)(82746002)(99286003)(36756003)(54906002)(189998001)(83716003)(1411001)(50986999)(54356999)(76176999)(25786009)(53546009)(6246003)(38730400002)(110136004)(33656002)(3280700002)(14454004)(3660700001)(66066001); DIR:OUT; SFP:1101; SCL:1; SRVR:AM3PR08MB0101; H:AM3PR08MB0101.eurprd08.prod.outlook.com; FPR:; SPF:None; MLV:ovrnspm; PTR:InfoNoRecords; LANG:en; spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="utf-8" Content-ID: <E4FE09DE6669D5429E99061A42CF8E02@eurprd08.prod.outlook.com> Content-Transfer-Encoding: base64 MIME-Version: 1.0 X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-originalarrivaltime: 12 Jun 2017 13:59:54.1833 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM3PR08MB0101 |
Commit Message
Alan Hayward
June 12, 2017, 1:59 p.m. UTC
> On 12 Jun 2017, at 12:05, Yao Qi <qiyaoltc@gmail.com> wrote: > > Alan Hayward <Alan.Hayward@arm.com> writes: > >> - regcache_cooked_read (regcache, entry->u.reg.num, reg); >> - regcache_cooked_write (regcache, entry->u.reg.num, >> - record_full_get_loc (entry)); >> - memcpy (record_full_get_loc (entry), reg, entry->u.reg.len); > > The original code is about swapping contents of register REG in regcache > and record_full_get_loc (entry), and the length is known entry->u.reg.len. > Yes. >> + value = regcache->cooked_read_value (entry->u.reg.num); >> + gdb_assert (value != NULL); >> + regcache->cooked_write (entry->u.reg.num, record_full_get_loc (entry)); >> + memcpy (record_full_get_loc (entry), value_contents_all (value), >> + entry->u.reg.len); >> + release_value (value); >> + value_free (value); > > It is a overkill to use value to swap these two buffers, IMO. How about > xmalloc "reg" instead? > Given that the code doesn’t use any of the error checking, then agreed. Tested on a --enable-targets=all build with board files unix and native-gdbserver. Ok to commit? Alan. 2017-06-12 Alan Hayward <alan.hayward@arm.com> * gdb/record-full.c (record_full_exec_insn): Allocate buffer.
Comments
PING > On 12 Jun 2017, at 14:59, Alan Hayward <Alan.Hayward@arm.com> wrote: > > >> On 12 Jun 2017, at 12:05, Yao Qi <qiyaoltc@gmail.com> wrote: >> >> Alan Hayward <Alan.Hayward@arm.com> writes: >> >>> - regcache_cooked_read (regcache, entry->u.reg.num, reg); >>> - regcache_cooked_write (regcache, entry->u.reg.num, >>> - record_full_get_loc (entry)); >>> - memcpy (record_full_get_loc (entry), reg, entry->u.reg.len); >> >> The original code is about swapping contents of register REG in regcache >> and record_full_get_loc (entry), and the length is known entry->u.reg.len. >> > > Yes. > >>> + value = regcache->cooked_read_value (entry->u.reg.num); >>> + gdb_assert (value != NULL); >>> + regcache->cooked_write (entry->u.reg.num, record_full_get_loc (entry)); >>> + memcpy (record_full_get_loc (entry), value_contents_all (value), >>> + entry->u.reg.len); >>> + release_value (value); >>> + value_free (value); >> >> It is a overkill to use value to swap these two buffers, IMO. How about >> xmalloc "reg" instead? >> > > Given that the code doesn’t use any of the error checking, then agreed. > > > Tested on a --enable-targets=all build with board files unix and > native-gdbserver. > > > Ok to commit? > Alan. > > 2017-06-12 Alan Hayward <alan.hayward@arm.com> > > * gdb/record-full.c (record_full_exec_insn): Allocate buffer. > > > diff --git a/gdb/record-full.c b/gdb/record-full.c > index 31ff558d2a633cff71d3e6082e42f5d6fb88bcf1..4f73e2a5ad0d4a2407b31a1d391e813147e15798 100644 > --- a/gdb/record-full.c > +++ b/gdb/record-full.c > @@ -698,7 +698,7 @@ record_full_exec_insn (struct regcache *regcache, > { > case record_full_reg: /* reg */ > { > - gdb_byte reg[MAX_REGISTER_SIZE]; > + gdb_byte *reg = (gdb_byte *) xmalloc (entry->u.reg.len); > > if (record_debug > 1) > fprintf_unfiltered (gdb_stdlog, > @@ -711,6 +711,7 @@ record_full_exec_insn (struct regcache *regcache, > regcache_cooked_write (regcache, entry->u.reg.num, > record_full_get_loc (entry)); > memcpy (record_full_get_loc (entry), reg, entry->u.reg.len); > + xfree (reg); > } > break; > > >
Alan Hayward <Alan.Hayward@arm.com> writes: > 2017-06-12 Alan Hayward <alan.hayward@arm.com> > > * gdb/record-full.c (record_full_exec_insn): Allocate buffer. Patch is good to me.
On 06/12/2017 02:59 PM, Alan Hayward wrote: > diff --git a/gdb/record-full.c b/gdb/record-full.c > index 31ff558d2a633cff71d3e6082e42f5d6fb88bcf1..4f73e2a5ad0d4a2407b31a1d391e813147e15798 100644 > --- a/gdb/record-full.c > +++ b/gdb/record-full.c > @@ -698,7 +698,7 @@ record_full_exec_insn (struct regcache *regcache, > { > case record_full_reg: /* reg */ > { > - gdb_byte reg[MAX_REGISTER_SIZE]; > + gdb_byte *reg = (gdb_byte *) xmalloc (entry->u.reg.len); > > if (record_debug > 1) > fprintf_unfiltered (gdb_stdlog, > @@ -711,6 +711,7 @@ record_full_exec_insn (struct regcache *regcache, > regcache_cooked_write (regcache, entry->u.reg.num, > record_full_get_loc (entry)); > memcpy (record_full_get_loc (entry), reg, entry->u.reg.len); > + xfree (reg); You can use gdb::byte_vector reg (entry->u.reg.len); to avoid the explicit xfree here. (and a potential leak if regcache_* throws). Thanks, Pedro Alves
diff --git a/gdb/record-full.c b/gdb/record-full.c index 31ff558d2a633cff71d3e6082e42f5d6fb88bcf1..4f73e2a5ad0d4a2407b31a1d391e813147e15798 100644 --- a/gdb/record-full.c +++ b/gdb/record-full.c @@ -698,7 +698,7 @@ record_full_exec_insn (struct regcache *regcache, { case record_full_reg: /* reg */ { - gdb_byte reg[MAX_REGISTER_SIZE]; + gdb_byte *reg = (gdb_byte *) xmalloc (entry->u.reg.len); if (record_debug > 1) fprintf_unfiltered (gdb_stdlog, @@ -711,6 +711,7 @@ record_full_exec_insn (struct regcache *regcache, regcache_cooked_write (regcache, entry->u.reg.num, record_full_get_loc (entry)); memcpy (record_full_get_loc (entry), reg, entry->u.reg.len); + xfree (reg); } break;