RE: [PATCHv3 1/3] gdb: Remove a VEC from gdbsupport/btrace-common.h

["Metzger, Markus T" <markus.t.metzger@intel.com>]

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 testsuite.

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 != 0)
> > >      {
> > > -      btrace_block_s *block;
> > >        CORE_ADDR pc;
> > >
> > >        blk -= 1;
> > >
> > > -      block = VEC_index (btrace_block_s, btrace->blocks, blk);
> > > -      pc = block->begin;
> > > +      const btrace_block &block = btrace->blocks->at (blk);
> > > +      pc = block.begin;
> >
> > We could also turn BTRACE->BLOCKS into a const & outside the loop.
> 
> I was worried about the case where we get here and btrace->blocks == nullptr.  I
> don't know if such a path is possible, I had a little through the code and 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 == nullptr.  BLK is initialized to
BTRACE->BLOCKS->size () (or zero with your latest version) so we'd skip this if we
ever had BTRACE->BLOCKS == nullptr.


> > > @@ -1581,11 +1580,9 @@ btrace_add_pc (struct thread_info *tp)
> > >    pc = regcache_read_pc (regcache);
> > >
> > >    btrace.format = BTRACE_FORMAT_BTS;
> > > -  btrace.variant.bts.blocks = NULL;
> > > +  btrace.variant.bts.blocks = new std::vector <btrace_block>;
> >
> > A stack-allocated vector should do.  Looks like we leaked the
> > one-entry vector before.
> 
> Nope the btrace object is stack local, and its destructor deletes the vector, 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 complexity.

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