From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1930 invoked by alias); 26 Sep 2019 13:07:44 -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 1922 invoked by uid 89); 26 Sep 2019 13:07:43 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-5.0 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_1 autolearn=ham version=3.3.1 spammy= X-HELO: mga01.intel.com Received: from mga01.intel.com (HELO mga01.intel.com) (192.55.52.88) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 26 Sep 2019 13:07:42 +0000 Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 Sep 2019 06:07:40 -0700 Received: from irsmsx153.ger.corp.intel.com ([163.33.192.75]) by orsmga006.jf.intel.com with ESMTP; 26 Sep 2019 06:07:39 -0700 Received: from irsmsx104.ger.corp.intel.com ([169.254.5.103]) by IRSMSX153.ger.corp.intel.com ([169.254.9.123]) with mapi id 14.03.0439.000; Thu, 26 Sep 2019 14:07:38 +0100 From: "Metzger, Markus T" To: Andrew Burgess CC: gdb-patches , Simon Marchi , Tom Tromey Subject: RE: [PATCHv3 1/3] gdb: Remove a VEC from gdbsupport/btrace-common.h Date: Thu, 26 Sep 2019 13:07:00 -0000 Message-ID: References: <87lfucf2qs.fsf@tromey.com> <2edcf1984fc1dbe09ceaac8d79f80ca2051e1062.1569455609.git.andrew.burgess@embecosm.com> <20190926113219.GS4962@embecosm.com> In-Reply-To: <20190926113219.GS4962@embecosm.com> Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2019-09/txt/msg00516.txt.bz2 Hello Andrew, > Any additional testing is always welcome, though from Simon's feedback I = believe > that I should be testing this OK - I'm running on a post-Broadwell Intel = CPU, and I > am linking GDB against libipt, and see no regressions over the entire tes= tsuite. That should suffice. I wasn't sure if you had access to a suitable system. > > > @@ -1068,13 +1069,12 @@ btrace_compute_ftrace_bts (struct > > > thread_info *tp, > > > > > > while (blk !=3D 0) > > > { > > > - btrace_block_s *block; > > > CORE_ADDR pc; > > > > > > blk -=3D 1; > > > > > > - block =3D VEC_index (btrace_block_s, btrace->blocks, blk); > > > - pc =3D block->begin; > > > + const btrace_block &block =3D btrace->blocks->at (blk); > > > + pc =3D block.begin; > > > > We could also turn BTRACE->BLOCKS into a const & outside the loop. >=20 > I was worried about the case where we get here and btrace->blocks =3D=3D = nullptr. I > don't know if such a path is possible, I had a little through the code an= d my > conclusion was that the answer is not obvious. So ideally I'd leave the = code as close > to the original as possible to avoid introducing bugs. We wouldn't get into this loop if BTRACE->BLOCKS =3D=3D nullptr. BLK is in= itialized to BTRACE->BLOCKS->size () (or zero with your latest version) so we'd skip thi= s if we ever had BTRACE->BLOCKS =3D=3D nullptr. > > > @@ -1581,11 +1580,9 @@ btrace_add_pc (struct thread_info *tp) > > > pc =3D regcache_read_pc (regcache); > > > > > > btrace.format =3D BTRACE_FORMAT_BTS; > > > - btrace.variant.bts.blocks =3D NULL; > > > + btrace.variant.bts.blocks =3D new std::vector ; > > > > A stack-allocated vector should do. Looks like we leaked the > > one-entry vector before. >=20 > Nope the btrace object is stack local, and its destructor deletes the vec= tor, so its > easier if this just stays being new'd for now. Thanks. We could clear it again but it's probably not worth the added comp= lexity. Regards, Markus. Intel Deutschland GmbH Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany Tel: +49 89 99 8853-0, www.intel.de Managing Directors: Christin Eisenschmid, Gary Kershaw Chairperson of the Supervisory Board: Nicole Lau Registered Office: Munich Commercial Register: Amtsgericht Muenchen HRB 186928