This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] (AArch64) wrong value returned by "finish" for HFA
- From: Alan Hayward <Alan dot Hayward at arm dot com>
- To: Joel Brobecker <brobecker at adacore dot com>
- Cc: GDB Patches <gdb-patches at sourceware dot org>, nd <nd at arm dot com>
- Date: Thu, 8 Nov 2018 10:18:45 +0000
- Subject: Re: [RFA] (AArch64) wrong value returned by "finish" for HFA
- References: <1541632206-143680-1-git-send-email-brobecker@adacore.com>
> On 7 Nov 2018, at 23:10, Joel Brobecker <brobecker@adacore.com> wrote:
>
> Hello,
>
> Consider the gdb.ada/array_return.exp testcase, and in particular,
> consider the following code...
>
> type Small_Float_Vector is array (1 .. 2) of Float;
>
> function Create_Small_Float_Vector return Small_Float_Vector is
> begin
> return (others => 4.25);
> end Create_Small_Float_Vector;
>
> ... which declares a type which is an array with 2 floats in it
> (floats are 4 bytes on AArch64), trying to get GDB to print
> the return value from that function does not work:
>
> (gdb) fin
> Run till exit from #0 pck.create_small_float_vector () at /[...]/pck.adb:15
> 0x000000000000062c in p () at /[...]/p.adb:11
> 11 Vector := Create_Small_Float_Vector;
> Value returned is $1 = (4.25, 0.0)
> ^^^
> |||
>
> We expected the value shown to be:
>
> (gdb) fin
> Run till exit from #0 pck.create_small_float_vector () at /[...]/pck.adb:15
> 0x000000000000062c in p () at /[...]/p.adb:11
> 11 Vector := Create_Small_Float_Vector;
> Value returned is $1 = (4.25, 4.25)
>
> Because the return type is an HFA, it is returned via the first two
> SIMD registers. However, what happens is that the current implementation
> fails to realize that this is an HFA, and therefore fetches the return
> value from the wrong location. And the reason why it fails to realize
> this is because it thinks that our array has 8 elements (HFAs have
> a maximum of 4). Looking at aapcs_is_vfp_call_or_return_candidate_1,
> where this is determined, we can easily see why (looks like a thinko):
>
> | case TYPE_CODE_ARRAY:
> | [...]
> | struct type *target_type = TYPE_TARGET_TYPE (type);
> | int count = aapcs_is_vfp_call_or_return_candidate_1
> | (target_type, fundamental_type);
> |
> | if (count == -1)
> | return count;
> |
> !! -> | count *= TYPE_LENGTH (type);
> | return count;
>
> Here, we first determine the count for one element of our array,
> and so we should then be multiplying that count by the number
> of elements in our array (2 in our case). But instead, we multiply it
> by the total size (8). As a result, we do not classify the return
> type as an HFA, and thus pick the wrong location for fetching
> the return value.
>
Apologies, this was my goof. Fix looks good to me.
> gdb/ChangeLog:
>
> * aarch64-tdep.c (aapcs_is_vfp_call_or_return_candidate_1):
> return the correct count for potential HFAs.
>
> Tested on aarch64-linux, fixes:
>
> array_return.exp: value printed by finish of Create_Small_Float_Vector
I had to install gnat to test this (which is probably why I never spotted the
fail before). Ideally, this case should have been tested as part of
gdb.base/infcall-nested-structs.exp.
As a side note, I’ve also got a work in progress fix for this area of code
to fix gdb.dwarf2/dw2-cp-infcall-ref-static.exp which uses a recursive type
written in dwarf.
>
> OK to commit?
>
> Thank you,
> --
> Joel
>
> ---
> gdb/aarch64-tdep.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> index 0211930..51f1632 100644
> --- a/gdb/aarch64-tdep.c
> +++ b/gdb/aarch64-tdep.c
> @@ -1212,7 +1212,7 @@ aapcs_is_vfp_call_or_return_candidate_1 (struct type *type,
> if (count == -1)
> return count;
>
> - count *= TYPE_LENGTH (type);
> + count *= (TYPE_LENGTH (type) / TYPE_LENGTH (target_type));
> return count;
> }
> }
> --
> 2.1.4
>