Re: [patchv2] Fix crash on optimized-out entry data values

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Wed, 09 Jul 2014 13:52:00 +0200, Pedro Alves wrote:
> On 07/09/2014 11:33 AM, Jan Kratochvil wrote:
> > --- a/gdb/value.c
> > +++ b/gdb/value.c
> > @@ -198,12 +198,13 @@ struct value
> >    unsigned int lazy : 1;
> >  
> >    /* If nonzero, this is the value of a variable that does not
> > -     actually exist in the program.  If nonzero, and LVAL is
> > +     actually fully exist in the program.  If nonzero, and LVAL is
> >       lval_register, this is a register ($pc, $sp, etc., never a
> >       program variable) that has not been saved in the frame.  All
> >       optimized-out values are treated pretty much the same, except
> >       registers have a different string representation and related
> > -     error strings.  */
> > +     error strings.  It is true also for only partially optimized
> > +     out variables - see the 'unavailable' field below.  */
> >    unsigned int optimized_out : 1;
> >  
> >    /* If value is a variable, is it initialized or not.  */
> > @@ -334,7 +335,10 @@ struct value
> >       valid if lazy is nonzero.  */
> >    gdb_byte *contents;
> >  
> > -  /* Unavailable ranges in CONTENTS.  We mark unavailable ranges,
> > +  /* If OPTIMIZED_OUT is false then UNAVAILABLE must be VEC_empty
> > +     (not necessarily NULL).  
> 
> Hmm, why?  We can collect only part of a non-optimized out value.
> What am I missing?

I miss some documentation how these availability fields interact together.
value_available_contents_bits_eq is written according to the struct value
comments but they do not correspond to the real world struct value data.

'unavailable' suggests that optimized out variable should have it set to the
full range of the variable.  But struct value with OPTIMIZED_OUT field set
still has UNAVAILABLE empty. As the variable is optimized out the CONTENTS is
also NULL - this causes the crash reproduced by
'gdb.arch/amd64-entry-value-paramref.exp'.

There are also check_validity and check_any_valid methods but how they are
related to the 'unavailable' data field is also not documented.

This patch was my second attempt to fix/document the availability stuff, it
would be useful to suggest how it is designed.


> Does this manage to somehow pass the tests
> under gdb.trace/ (against --target_board=native-gdbserver) ?

You are right, this patch regresses during gdbserver mode.


> > If OPTIMIZED_OUT is true then VEC_empty
> > +     UNAVAILABLE means the whole value range.  Otherwise it specifies
> > +     unavailable ranges in CONTENTS.  We mark unavailable ranges,
> >       rather than available, since the common and default case is for a
> >       value to be available.  This is filled in at value read time.  The
> >       unavailable ranges are tracked in bits.  */
> > @@ -701,6 +705,15 @@ value_available_contents_bits_eq (const struct value *val1, int offset1,
> >    /* See function description in value.h.  */
> >    gdb_assert (!val1->lazy && !val2->lazy);
> >  
> > +  gdb_assert (val1->optimized_out || VEC_empty (range_s, val1->unavailable));
> > +  gdb_assert (val2->optimized_out || VEC_empty (range_s, val2->unavailable));


Thanks,
Jan