Re: [PATCH [2/2] Convert the unavailable vector to be bit, not byte, based.

Pedro Alves
2013-12-05T16:18:52Z

On 12/04/2013 03:26 PM, Andrew Burgess wrote:

> * gdb/dwarf2loc.c (read_pieced_value): Mark bits, not bytes
> unavailable, use correct bit length.

No "gdb/" prefix.

> (unpack_value_bits_as_long_1): Check availability in bits, bot
> bytes.

Typo: "bot".

> * gdb/value.h (int value_bits_available): Declare new function.

Stale "int".

> (mark_value_bits_unavailable): Declare new function.

> *val1, int offset1,
> return 0;
>
> /* Compare the _available_ contents. */
> - if (memcmp (val1->contents + offset1,
> - val2->contents + offset2,
> - l1) != 0)
> + if (memcmp (val1->contents + (offset1 / TARGET_CHAR_BIT),
> + val2->contents + (offset2 / TARGET_CHAR_BIT),
> + (l1 / TARGET_CHAR_BIT)) != 0)
> return 0;

As memcmp compares bytes, isn't this potentially comparing bits
at the beginning and end of the values' buffers, when it
should not? That is, it looks like the
'offset1 % TARGET_CHAR_BIT != 0' and
'(offset1 + l1) % TARGET_CHAR_BIT' cases should be considered here?

> + gdb_test "tfind 0" "Found trace frame 0, tracepoint .*"
> + gdb_test "p x" "\\\$${decimal} = {a = 0 '\\\\000', b = <unavailable>, c = 0 '\\\\000', d = 0 '\\\\000', e = 0 '\\\\000', f = 0 '\\\\000', g = 0 '\\\\000', h = 0 '\\\\000', i = 0 '\\\\000', j = 0 '\\\\000'}"
> + gdb_test "p y" "\\\$${decimal} = {a = 0 '\\\\000', b = 0 '\\\\000', c = 0 '\\\\000', d = 0 '\\\\000', e = <unavailable>, f = 0 '\\\\000', g = 0 '\\\\000', h = 0 '\\\\000', i = 0 '\\\\000', j = 0 '\\\\000'}"
> + gdb_test "p z" "\\\$${decimal} = {a = 0 '\\\\000', b = 0 '\\\\000', c = 0 '\\\\000', d = 0 '\\\\000', e = 0 '\\\\000', f = 0 '\\\\000', g = 0 '\\\\000', h = 0 '\\\\000', i = <unavailable>, j = 0 '\\\\000'}"

(I think you could have used "p/d" or "p/x" to simplify the output.)

> +
> + gdb_test "tfind none" "No longer looking at any trace frame.*"

> +void
> +end ()

end (void)

There are more instances.

> + struct s v = { 0, 1, 2};

Missing space before }.

> + struct t w = { 5, 0, 1, 0, 1, 0, 1, 0, 1, 7 };

Otherwise looks good to me.

--
Pedro Alves