gdb: use value::embedded_offset in check_pieced_synthetic_pointer
This commit builds on the previous two commits. You should go and
read them both for context.
Looking at the users of value::bits_synthetic_pointer, all callers but
two are now passing either the offset of a field within the value, or
hard-coded zero as they want to ask about the entire value.
The two exceptions are in coerce_pieced_ref (in dwarf2/expr.c) and in
value_addr (in valops.c) where we still pass value::embedded_offset.
The problem is that some of the other callers, where we currently pass
just a field offset, might also have a non-zero embedded offset,
specifically, the call in cp_print_value_fields is in this category.
In cp_print_value_fields we are printing fields from a value which is
potentially a base class contained within an instance of a derived
class, this will be represented by a non-zero embedded_offset, which
is currently not taken into account.
Additionally, the two callers that currently pass the
value::embedded_offset don't scale the embedded offset from bytes to
bits.
The failure to scale from bytes to bits is interesting, I originally
tried to create some tests that exposed this, but constantly failed.
It turns out, that in both these locations, in all code paths that I
could find, the value::embedded_offset will always be zero. For
example in coerce_pieced_ref, a reference cannot be a base class, and
so we never expect to see a non-zero embedded_offset in this case.
Similarly, in value_addr, the use of value::bits_synthetic_pointer is
within a block that only applies to reference types, which means the
embedded_offset will be zero.
Now within check_pieced_synthetic_pointer we already add the
value::offset into the bit_offset which is passed in. Remember, after
the previous two commits, the incoming bit_offset is (almost) always
the offset of a field within the value. The almost here is the two
cases mentioned above.
I propose that within check_pieced_synthetic_pointer we should take
into account both value::offset and value::embedded_offset. Then the
two cases that currently pass value::embedded_offset can be changed to
just pass zero.
With this done, and the previous two commits, we now have a consistent
model. value::bits_synthetic_pointer expects the (bit) offset of a
field within the value to check. In many cases this will be zero
meaning we want to check from the start of the value, but in some
cases it can be non-zero.
Then within value::bits_synthetic_pointer implementations, like
check_pieced_synthetic_pointer, we will take the value::offset and
value::embedded_offset into account, remembering to scale them from
bytes to bits.
I have also changed indirect_pieced_value to also take the
embedded_offset into account. I have done this for consistency rather
than necessity. I believe that the embedded_offset will always be
zero within indirect_pieced_value. The indirect_pieced_value function
is only called from value_ind (in valops.c), and only operates on
TYPE_CODE_PTR types (checked for in indirect_pieced_value). As a
TYPE_CODE_PTR cannot be the base class for a derived type, then we
don't expect to ever see a TYPE_CODE_PTR value with a non-zero
embedded_offset. But, having indirect_pieced_value take the embedded
offset into account is simple enough, and future proofs the code.
In both check_pieced_synthetic_pointer and indirect_pieced_value I
have changed uses of '8' to 'TARGET_CHAR_BIT', I was already touching
some of these lines, and I think TARGET_CHAR_BIT is clearer, but one
line in indirect_pieced_value was just updated to use TARGET_CHAR_BIT,
this is done for consistency.
The gdb.dwarf2/multi-piece-inherited-bitfield.exp test fails without
this patch, this exposes the case where the embedded_offset is
non-zero and we were previously failing to take this into account.
The gdb.dwarf2/multi-piece-primitive-field.exp test was something I
wrote while trying to exercise the coerce_pieced_ref code path some
more. It is an inheritance based version of the existing test, I was
wondering if this would result in a value with a non-zero
embedded_offset, but due to how the fields are extracted from the
aggregate prior to calling coerce_pieced_ref the embedded_offset is
always zero in this function.
Approved-By: Tom Tromey <tom@tromey.com>
5 files changed