This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: collecting optimized out variables regression (Re: RFA: rewrite dwarf->ax translator


On Thursday 01 July 2010 16:24:58, Tom Tromey wrote:
> >>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:
> 
> >> -  if (!data || size == 0)
> >> -    {
> >> -      value->optimized_out = 1;
> >> -      return;
> >> -    }
> 
> Pedro> Apparently, this bit was lost with the rewrite.
> 
> Oops, sorry about that.
> 
> Pedro> This patch fixes it for me.  Is this the correct approach, or
> Pedro> maybe there was intent to handle this differently?
> 
> It looks good to me.

Thanks, I've applied it.

On Thursday 01 July 2010 16:33:50, Jan Kratochvil wrote:
> On Thu, 01 Jul 2010 14:42:59 +0200, Pedro Alves wrote:
> > +  if (dlbaton->data == NULL || dlbaton->size == 0)
> 
> If SIZE != 0 then DATA must be != NULL otherwise GDB would crash elsewhere.

Maybe in the case, I'm not sure.  In the loclist case, it can be
seen that SIZE is undefined when DATA is NULL (see the internal error
example I posted before), so checking on SIZE isn't sufficient then.
I was prefering having both cases handled with the same condition,
and I used the same condition we were using before the rewrite.

> dwarf2_evaluate_loc_desc is already using this easier to read conditional:
>   if (size == 0)
> which means:
>   if (dlbaton->size == 0)

... while loclist_read_variable itself checks for data == NULL.  That
didn't seem that consistent and easier to read.

(Feel free to clean this up.  I'm disappearing for two weeks soon,
and I really should be focusing on other stuff.  I've applied it
as is, mainly to avoid branching 7.2 with the regression.)

Thanks,
-- 
Pedro Alves


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]