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

Pedro Alves
2014-07-09T11:52:06Z

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