Re: [PATCH] Handle partially optimized out values similarly to unavailable values (Re: [patchv2] Fix crash on optimized-out entry data values)

Jan Kratochvil
2014-07-16T21:58:48Z

On Mon, 14 Jul 2014 20:11:41 +0200, Pedro Alves wrote:
[...]
> --- a/gdb/value.c
> +++ b/gdb/value.c
[...]
> @@ -339,6 +330,15 @@ struct value
> value to be available. This is filled in at value read time. The
> unavailable ranges are tracked in bits. */
> VEC(range_s) *unavailable;
> +
> + /* Likewise, but for optimized out contents (a chunk of the value of
> + a variable that does not actually exist in the program). If LVAL
> + is lval_register, this is a register ($pc, $sp, etc., never a
> + program variable) that has not been saved in the frame. Not
> + saved registers and optimized-out program variables values are
> + treated pretty much the same, except not-saved registers have a
> + different string representation and related error strings. */
> + VEC(range_s) *optimized_out;

I miss here explicit description if a bit of the range can appear on both
'unavailable' and 'optimized_out'. IMO it should not.

> };
>
> int
[...]
> +/* Helper function for value_contents_eq. The only difference is that
> + this function is bit rather than byte based.
> +
> + Compare LENGTH bits of VAL1's contents starting at OFFSET1 bits
> + with LENGTH bits of VAL2's contents starting at OFFSET2 bits.
> + Return true if the available bits match. */
> +
> +static int
> +value_contents_bits_eq (const struct value *val1, int offset1,
> + const struct value *val2, int offset2,
> + int length)
> +{
> + /* Each array element corresponds to a ranges source (unavailable,
> + optimized out). '1' is for VAL1, '2' for VAL2. */
> + struct ranges_and_idx rp1[2], rp2[2];
> +
> + /* See function description in value.h. */
> + gdb_assert (!val1->lazy && !val2->lazy);
> +
> + /* We shouldn't be trying to compare past the end of the values. */
> + gdb_assert (offset1 + length
> + <= TYPE_LENGTH (val1->enclosing_type) * TARGET_CHAR_BIT);
> + gdb_assert (offset2 + length
> + <= TYPE_LENGTH (val2->enclosing_type) * TARGET_CHAR_BIT);
> +
> + memset (&rp1, 0, sizeof (rp1));
> + memset (&rp2, 0, sizeof (rp2));
> + rp1[0].ranges = val1->unavailable;
> + rp2[0].ranges = val2->unavailable;
> + rp1[1].ranges = val1->optimized_out;
> + rp2[1].ranges = val2->optimized_out;
> +
> + while (length > 0)
> + {
> + ULONGEST l, h;
> + int i;
> +
> + for (i = 0; i < 2; i++)
> + {
> + ULONGEST l_tmp, h_tmp;
> +
> + /* The contents only match equal if the invalid/unavailable
> + contents ranges match as well. */
> + if (!find_first_range_overlap_and_match (&rp1[i], &rp2[i],
> + offset1, offset2, length,
> + &l_tmp, &h_tmp))
> + return 0;
> +
> + /* We're interested in the lowest/first range found. */
> + if (i == 0 || l_tmp < l)

Here should be:
if (i == 0 || l_tmp < l || (l_tmp == l && h_tmp < h))

It could skip [0]'s part which is non-equal otherwise.

> + {
> + l = l_tmp;
> + h = h_tmp;
> + }
> + }
> +
> + /* Compare the available/valid contents. */
> if (memcmp_with_bit_offsets (val1->contents, offset1,
> - val2->contents, offset2, l1) != 0)
> + val2->contents, offset2, l) != 0)
> return 0;
>
> - length -= h1;
> - offset1 += h1;
> - offset2 += h1;
> + length -= h;
> + offset1 += h;
> + offset2 += h;
> }
>
> return 1;
> }
[...]
> @@ -1566,7 +1671,6 @@ value_copy (struct value *arg)
> VALUE_FRAME_ID (val) = VALUE_FRAME_ID (arg);
> VALUE_REGNUM (val) = VALUE_REGNUM (arg);
> val->lazy = arg->lazy;
> - val->optimized_out = arg->optimized_out;

Why 'optimized_out' no longer needs to be copied?

> val->embedded_offset = value_embedded_offset (arg);
> val->pointed_to_offset = arg->pointed_to_offset;
> val->modifiable = arg->modifiable;
[...]
> @@ -3662,10 +3753,11 @@ value_fetch_lazy (struct value *val)
> if (value_lazy (parent))
> value_fetch_lazy (parent);
>
> - if (!value_bits_valid (parent,
> - TARGET_CHAR_BIT * offset + value_bitpos (val),
> - value_bitsize (val)))
> - set_value_optimized_out (val, 1);
> + if (value_bits_any_optimized_out (parent,
> + TARGET_CHAR_BIT * offset + value_bitpos (val),
> + value_bitsize (val)))
> + mark_value_bytes_optimized_out (val, value_embedded_offset (val),
> + TYPE_LENGTH (type));

That was required before but it could handle partially optimized out
bitfields; but maybe it is not useful much.

> else if (!unpack_value_bits_as_long (value_type (val),
> value_contents_for_printing (parent),
> offset,
[...]
> --- a/gdb/value.h
> +++ b/gdb/value.h
> @@ -33,6 +33,43 @@ struct language_defn;
> struct value_print_options;
> struct xmethod_worker;
>
> +/* Values can be partially 'optimized out' and/or 'unavailable'.
> + These are distinct states and have different string representations
> + and related error strings.
> +
> + 'unavailable' has a specific meaning in this context. It means the
> + value exists in the program (at the machine level), but GDB has no
> + means to get to it. Such a value is normally printed as. Examples

"printed as." - is the sentence finished?

> + of how to end up with an unavailable value would be:
> +
> + - We're inspecting a traceframe, and the memory or registers the
> + debug information says the value lives on haven't been collected.
> +
> + - We're inspecting a core dump, the memory or registers the debug
> + information says the value lives aren't present in the dump
> + (that is, we have a partial/trimmed core dump, or we don't fully
> + understand/handle the core dump's format).
> +
> + - We're doing live debugging, but the debug API has no means to
> + get at where the value lives in the machine, like e.g., ptrace
> + not having access to some register or register set.
> +
> + - Any other similar scenario.
> +
> + OTOH, "optimized out" is about what the compiler decided to generate
> + (or not generate). A chunk of a value that was optimized out does
> + not actually exist in the program. There's no way to get at it
> + short of compiling the program differently. A register that has not
> + been saved in a frame is likewise considered optimized out, except
> + not-saved registers have a different string representation and
> + related error strings. E.g., we'll print them as <not-saved>

s/<not-saved>/<unavailable>/?

> + instead of <optimized out>.
> +
> + When comparing value contents, optimized out chunks, unavailable
> + chunks, and valid contents data are all all considered different.
> + See value_contents_eq for more info.
> +*/
> +
> /* The structure which defines the type of a value. It should never
> be possible for a program lval value to survive over a call to the
> inferior (i.e. to be put into the history list or an internal
[...]
> @@ -473,35 +508,53 @@ extern void mark_value_bits_unavailable (struct value *value,
> its enclosing type chunk, you'd do:
>
> int len = TYPE_LENGTH (check_typedef (value_enclosing_type (val)));
> - value_available_contents (val, 0, val, 0, len);
> + value_contents_eq (val, 0, val, 0, len);
> +
> + Returns true iff the set of available/valid contents match.
> +
> + Optimized out contents compare equal with optimized out contents,
> + and different with any non-optimized-out byte.

English: s/different/differ/

>
> - Returns true iff the set of available contents match. Unavailable
> - contents compare equal with unavailable contents, and different
> - with any available byte. For example, if 'x's represent an
> - unavailable byte, and 'V' and 'Z' represent different available
> - bytes, in a value with length 16:
> + Unavailable contents compare equal with unavailable contents, and
> + different with any non-unavailable byte.

English: s/different/differ/

>
> - offset: 0 4 8 12 16
> - contents: xxxxVVVVxxxxVVZZ
> + For example, if 'x's represent an unavailable byte, and 'V' and 'Z'
> + represent different available/valid bytes, in a value with length
> + 16:
> +
> + offset: 0 4 8 12 16
> + contents: xxxxVVVVxxxxVVZZ
>
> then:
>
> - value_available_contents_eq(val, 0, val, 8, 6) => 1
> - value_available_contents_eq(val, 0, val, 4, 4) => 1
> - value_available_contents_eq(val, 0, val, 8, 8) => 0
> - value_available_contents_eq(val, 4, val, 12, 2) => 1
> - value_available_contents_eq(val, 4, val, 12, 4) => 0
> - value_available_contents_eq(val, 3, val, 4, 4) => 0
> -
> - We only know whether a value chunk is available if we've tried to
> - read it. As this routine is used by printing routines, which may
> - be printing values in the value history, long after the inferior is
> - gone, it works with const values. Therefore, this routine must not
> - be called with lazy values. */
> -
> -extern int value_available_contents_eq (const struct value *val1, int offset1,
> - const struct value *val2, int offset2,
> - int length);
> + value_contents_eq(val, 0, val, 8, 6) => 1
> + value_contents_eq(val, 0, val, 4, 4) => 0
> + value_contents_eq(val, 0, val, 8, 8) => 0
> + value_contents_eq(val, 4, val, 12, 2) => 1
> + value_contents_eq(val, 4, val, 12, 4) => 0
> + value_contents_eq(val, 3, val, 4, 4) => 0
> +
> + If 'x's represent an unavailable byte, 'o' represents an optimized
> + out byte, in a value with length 8:
> +
> + offset: 0 4 8
> + contents: xxxxoooo
> +
> + then:
> +
> + value_contents_eq(val, 0, val, 2, 2) => 1
> + value_contents_eq(val, 4, val, 6, 2) => 1
> + value_contents_eq(val, 0, val, 4, 4) => 0
> +
> + We only know whether a value chunk is unavailable or optimized out
> + if we've tried to read it. As this routine is used by printing
> + routines, which may be printing values in the value history, long
> + after the inferior is gone, it works with const values. Therefore,
> + this routine must not be called with lazy values. */
[...]

This patch is apparently not suitable for gdb-7.8 which is I guess often
crashing on -O2 -g entry values so there could be some rather minimal crash
avoiding patch instead.

Thanks,
Jan