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

[Pedro Alves <palves at redhat dot com>]

On 07/09/2014 11:33 AM, Jan Kratochvil wrote:
> Hi,
> 
> former post of this fix was for:
> 	[patch+7.8] Fix crash on optimized-out entry data values
> 	https://sourceware.org/ml/gdb-patches/2014-06/msg00797.html
> =
> 	https://bugzilla.redhat.com/show_bug.cgi?id=1111910
> 	this can happen for real world -O2 -g executables:
> 	#9  0x0000003b6e0998b2 in wxEntry (argc=@0x7fffffffd86c: 1,
> 	    argc@entry=@0x7fffffffd86c: <optimized out>,
> 					^^^^^^^^^^^^^^^
> 	    argv=<optimized out>) at src/common/init.cpp:460
> 	GDB did crash in such case.
> 
> But the fix was wrong/regressing as shown here:
> 	https://bugzilla.redhat.com/show_bug.cgi?id=1117192
> 	https://bugzilla.redhat.com/attachment.cgi?id=916298 (at the bottom)
> 
> Here is a new fix, also with a new testcase reproducing crash of the wrong fix
> above.
> 
> No regressions on {x86_64,x86_64-m32,i686}-fedorarawhide-linux-gnu.
> 
> 
> Jan
> 
> 
> optimfix2.patch
> 
> 
> gdb/
> 2014-07-09  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* value.c (struct value): Extend the comment for fields optimized_out
> 	and unavailable.
> 	(value_available_contents_bits_eq): Handle OPTIMIZED_OUT values with
> 	empty UNAVAILABLE as special cases.
> 
> gdb/testsuite/
> 2014-07-09  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* gdb.arch/amd64-entry-value-paramref.S: New file.
> 	* gdb.arch/amd64-entry-value-paramref.cc: New file.
> 	* gdb.arch/amd64-entry-value-paramref.exp: New file.
> 	* gdb.arch/amd64-optimout-repeat.S: New file.
> 	* gdb.arch/amd64-optimout-repeat.c: New file.
> 	* gdb.arch/amd64-optimout-repeat.exp: New file.
> 
> diff --git a/gdb/value.c b/gdb/value.c
> index 557056f..4b7495e 100644
> --- 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?  Does this manage to somehow pass the tests
under gdb.trace/ (against --target_board=native-gdbserver) ?

> 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));

-- 
Pedro Alves