[PATCH v2 2/3] [gdb] Add gdbarch_dwarf2_reg_piece_offset hook

[Andreas Arnez]

Hi Tom,

Thanks for your proposal, and sorry for my late response.

On Wed, Dec 18 2024, Tom de Vries wrote:

> In rw_pieced_value, when reading/writing part of a register, DW_OP_piece and
> DW_OP_bit_piece are handled the same, but the standard tells us:
> - DW_OP_piece: if the piece is located in a register, but does not occupy the
>   entire register, the placement of the piece within that register is defined
>   by the ABI.
> - DW_OP_bit_piece: if the location is a register, the offset is from the least
>   significant bit end of the register.

Do you know any technical reason to distinguish the two cases?  To my
knowledge, producers such as GCC don't distinguish them, neither did
consumers such as GDB so far.  At least not intentionally.  Last time I
checked there was an "optimization" in GCC that converted a bit piece to
a byte piece if the offset was zero and the number of bits was a
multiple of 8.  AFAIK, your patch would introduce that distinction to
the toolchain for the first time.

About the rationale for leaving the positioning of register pieces to
the ABI: the main reason I can think of is that the same register can
have different sizes on different installments of an architecture.
Think of a case where integer registers are expanded from 32 to 64 bits;
the same applies to floating-point registers and certainly vector
registers.  Which bits are shared between the different versions of a
register is architecture-specific.  For GPRs it's the least significant
bits on all architectures I know of, but for FPRs and VRs it's not that
obvious.  On s390x the FPRs were expanded in such a way that 32-bit and
64-bit floating-point values overlap at the "leftmost" bytes, not the
"least significant".  And for consistency, the first 16 VRs expand the
FPRs in the same way.

Now, if bit pieces wouldn't support ABI-specific handling, they couldn't
be used consistently from one installment of the architecture to the
next, except if the "LSB" rule applies.  We already have this issue with
s390x, where the FPRs have the same DWARF register numbers as their
containing VRs, making it impossible to cut out bit pieces from FPRs
according to the DWARF standard.  Fortunately (?), compilers are
struggling to emit useful DWARF variable location information and rarely
(if ever) emit bit pieces for FPRs.  But I don't like establishing a
road block for compilers to become smarter.

My conclusion is that the DWARF standard's inconsistency, even if
intentional (which I don't know) is certainly not useful, nobody follows
it anyhow, and thus we should ignore it.  So if you define a hook, I
suggest to do it for bit pieces and treat byte pieces as a special case.
This already seems to be common practice, and we shouldn't diverge from
that without good reason.

> Add a new hook gdbarch_dwarf2_reg_piece_offset that allows us to define the
> ABI-specific behaviour for DW_OP_piece.

I wonder if the new hook can cover all uses of the existing hook
value_from_register(), in which case we should get rid of the old one.
If not, we should document why both hooks are needed.

[...]

> diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h
> index 9fda85f860f..5934da032cb 100644
> --- a/gdb/gdbarch-gen.h
> +++ b/gdb/gdbarch-gen.h
> @@ -430,6 +430,14 @@ typedef struct value * (gdbarch_value_from_register_ftype) (struct gdbarch *gdba
>  extern struct value * gdbarch_value_from_register (struct gdbarch *gdbarch, struct type *type, int regnum, const frame_info_ptr &this_frame);
>  extern void set_gdbarch_value_from_register (struct gdbarch *gdbarch, gdbarch_value_from_register_ftype *value_from_register);
>  
> +/* For a DW_OP_piece located in a register, but not occupying the
> +   entire register, return the placement of the piece within that
> +   register as defined by the ABI. */
> +

Perhaps the comment should also explain what the returned bit offset
means.  Something like "according to the architecture's bit ordering
when stored in memory" or so.

-- 
Andreas