Re: [patch] Arm EABI support

[Richard Earnshaw <rearnsha@gcc.gnu.org>]

On Mon, 2004-12-13 at 15:11, Paul Brook wrote:
> The attached patch adds gdb support for the new Arm EABI. This involves a new 
> calling convention, slightly different elf header flags, and different 
> default floating point format.
> 
> Tested with cross to arm-none-elf and arm-none-eabi.
> Ok?
> 
> 2004-12-13  Paul Brook  <paul@codesourcery.com>
> 
>  * arm-tdep.c (arm_type_align): New function.
>  (arm_push_dummy_call): Use it.  Ensure proper argument alignment.
>  (arm_extract_struct_value_address): Remove.
>  (arm_use_struct_convention): Rename...
>  (arm_return_in_memory): ... To this.  Return nonzero for all small
>  aggregates under the EABI.
>  (arm_return_value): New function.
>  (arm_elf_osabi_sniffer): Add EF_ARM_EABI_VER4:.
>  (arm_gdbarch_init): Set different fpu and abi defaults for EABI
>  objects.  Use set_gdbarch_return_value instead of obsolete functions.
>  (arm_init_abi_eabi_v4): New function.
>  (_initialize_arm_tdep): Register GDB_OSABI_ARM_EABI_V4.
>  * arm-tdep.h (enum arm_abi_variant): Add.
>  (struct gdbarch_tdep): Add abi field.
>  * defs.h (enum gdb_osabi): Add GDB_OSABI_ARM_EABI_V4.
>  * osabi.c (gdb_osabi_name): Add "ARM EABI v4".
>  * testsuite/long_long.exp: Exclude eabi targets from arm FPA float
>  format test.

OK, apart from the following issues:

+/* Return the alignment (in bytes) of the given type.  */
+
+static int
+arm_type_align (struct type *t)

Is this EABI-specific?  If so, it should be part of the function call
name.  If not, then I think this bit of code:

+      if (gdbarch_tdep (gdbarch)->abi == ARM_ABI_APCS_GNU)
+       {
+         /* The old APCS ABI does not require doubleword alignment.  */
+         align = INT_REGISTER_SIZE;
+       }
+      else
+       {
+         align = arm_type_align (arg_type);

Should be reworked so that we always call it.

+    case TYPE_CODE_UNDEF:
+    case TYPE_CODE_FUNC:
+    case TYPE_CODE_VOID:
+    case TYPE_CODE_STRING:
+    case TYPE_CODE_ERROR:
+    case TYPE_CODE_MEMBER:
+    case TYPE_CODE_METHOD:
+    case TYPE_CODE_TEMPLATE:
+    case TYPE_CODE_TEMPLATE_ARG:
+    case TYPE_CODE_NAMESPACE:
+    case TYPE_CODE_TYPEDEF:
+    default:
+      /* Should never happen, so make something up.  */

Why explicitly mention those values & default?  Are they the same case
or not?  My suspicion is that the default should raise an assertion (new
type?)

+   ...
+    case TYPE_CODE_REF:
+    case TYPE_CODE_CHAR:
+    case TYPE_CODE_BOOL:
+      return TYPE_LENGTH (t);

Wouldn't it be more sensible to limit this with an abi-specific
max_align value (or 8 if this is for the EABI only).

R.