Re: [RFA] Fix DW_OP_GNU_regval_type with FP registers

[Pedro Alves <palves@redhat.com>]

On 11/14/2013 01:02 PM, Joel Brobecker wrote:
> Hello,
> 
> Consider the following code, compiled at -O2 on ppc-linux:
> 
>     procedure Increment (Val : in out Float; Msg : String);
> 
> The implementation does not really matter in this case). In our example,
> this function is being called from a function with Param_1 set to 99.0.
> Trying to break inside that function, and running until reaching that
> breakpoint yields:
> 
>     (gdb) b increment
>     Breakpoint 1 at 0x100014b4: file callee.adb, line 6.
>     (gdb) run
>     Starting program: /[...]/foo
> 
>     Breakpoint 1, callee.increment (val=99.0, val@entry=0.0, msg=...)
>         at callee.adb:6
>     6             if Val > 200.0 then
> 
> The @entry value for parameter "val" is incorrect, it should be 99.0.
> 
> The associated call-site parameter DIE looks like this:
> 
>         .uleb128 0xc     # (DIE (0x115) DW_TAG_GNU_call_site_parameter)
>         .byte   0x2      # DW_AT_location
>         .byte   0x90     # DW_OP_regx
>         .uleb128 0x21
>         .byte   0x3      # DW_AT_GNU_call_site_value
>         .byte   0xf5     # DW_OP_GNU_regval_type
>         .uleb128 0x3f
>         .uleb128 0x25
> 
> The DW_AT_GNU_call_site_value uses a DW_OP_GNU_regval_type
> operation, referencing register 0x3f=63, which is $f31,
> an 8-byte floating register. In that register, the value is
> stored using the usual 8-byte float format:
> 
>     (gdb) info float
>     f31            99.0 (raw 0x4058c00000000000)
> 
> The current code evaluating DW_OP_GNU_regval_type operations
> currently is (dwarf2expr.c:execute_stack_op):
> 
>             result = (ctx->funcs->read_reg) (ctx->baton, reg);
>             result_val = value_from_ulongest (address_type, result);
>             result_val = value_from_contents (type,
>                                               value_contents_all (result_val));
> 
> What the ctx->funcs->read_reg function does is read the contents
> of the register as if it contained an address. The rest of the code
> continues that assumption, thinking it's OK to then use that to
> create an address/ulongest struct value, which we then re-type
> to the type specified by DW_OP_GNU_regval_type.
> 
> We're getting 0.0 above because the read_reg implementations
> end up treating the contents of the FP register as an integral,
> reading only 4 out of the 8 bytes. 
>
> Being a big-endian target,
> we read the high-order ones, which gives us zero.
> 
> This patch fixes the problem by introducing a new callback to
> read the contents of a register as a given type, and then adjust
> the handling of DW_OP_GNU_regval_type to use that new callback.

I definitely agree with this.  The patch looked fine to me.

(Eliminating read_reg might be tricky, but I think it'd be nice
if it were be renamed to something that implies "address", like
read_addr_reg or some such.)

-- 
Pedro Alves