[binutils-gdb] (AArch64) wrong value returned by "finish" for HFA

[Joel Brobecker]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d4718d5c9f9b5c41d88254429c61010c63139130

commit d4718d5c9f9b5c41d88254429c61010c63139130
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Thu Nov 8 10:27:31 2018 -0500

    (AArch64) wrong value returned by "finish" for HFA
    
    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.
    
    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

Diff:
---
 gdb/ChangeLog      | 5 +++++
 gdb/aarch64-tdep.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6c18be9..be1b3d8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2018-11-08  Joel Brobecker  <brobecker@adacore.com>
+
+	* aarch64-tdep.c (aapcs_is_vfp_call_or_return_candidate_1):
+	return the correct count for potential HFAs.
+
 2018-11-08  Jan Beulich  <jbeulich@suse.com>
 
 	* i387-tdep.c (i387_supply_xsave): Split handling of
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;
 	  }
       }