Hi Petr, This all looks good. Just some nitpicks below. Feel free to ignore, I don't actually expect you to refactor other backends and common methods. Just noted you had some nice helpers that might be helpful for the next backend porter.
On Thu, 2013-11-14 at 01:05 +0100, Petr Machata wrote: > diff --git a/backends/ChangeLog b/backends/ChangeLog > +2013-11-14 Petr Machata <[email protected]> > + > + * Makefile.am (modules): Add aarch64. > + (libebl_pic): Add libebl_aarch64_pic.a. > + (aarch64_SRCS): New variable. > + (libebl_aarch64_pic_a_SOURCES): Likewise. > + (am_libebl_aarch64_pic_a_OBJECTS): Likewise. > + (aarch64_regs_no_Wformat): Likewise. OK, but why do you need no_Wformat? > + * aarch64_corenote.c, aarch64_init.c: New files. > + * aarch64_regs.c, aarch64_reloc.def: Likewise. > + * aarch64_retval.c, aarch64_symbol.c: Likewise. > +++ b/backends/aarch64_corenote.c > @@ -0,0 +1,162 @@ > +/* AArch64 specific core note handling. > +++ b/backends/aarch64_init.c > @@ -0,0 +1,61 @@ > +/* Initialization of AArch64 specific backend library. > diff --git a/backends/aarch64_regs.c b/backends/aarch64_regs.c > +/* Register names and numbers for AArch64 DWARF. > + ssize_t > + regtype (const char *setname, int type, const char *fmt, int arg) > + { > + *setnamep = setname; > + *typep = type; > + int s = snprintf (name, namelen, fmt, arg); > + if (s > 0 && (unsigned) s >= namelen) > + s = -1; > + return s; > + } > + > + ssize_t > + reserved (void) > + { > + return regtype (NULL, DW_ATE_void, "", 0); > + } I would just have done *setnamep = NULL; return 0; But I guess that will be how the compiler eventually optimizes it. > diff --git a/backends/aarch64_reloc.def b/backends/aarch64_reloc.def > @@ -0,0 +1,157 @@ > +/* List the relocation types for AArch64. -*- C -*- > diff --git a/backends/aarch64_retval.c b/backends/aarch64_retval.c > +/* Function return value location for Linux/AArch64 ABI. > +static int > +peel_type (Dwarf_Die **typediep, Dwarf_Attribute **attrp) > +{ > + int tag = DWARF_TAG_OR_RETURN (*typediep); > + /* Follow typedefs and qualifiers to get to the actual type. */ > + while (tag == DW_TAG_typedef > + || tag == DW_TAG_const_type || tag == DW_TAG_volatile_type > + || tag == DW_TAG_restrict_type || tag == DW_TAG_mutable_type) > + { > + *attrp = dwarf_attr_integrate (*typediep, DW_AT_type, *attrp); > + *typediep = dwarf_formref_die (*attrp, *typediep); > + tag = DWARF_TAG_OR_RETURN (*typediep); > + } > + > + return tag; > +} That might be handy in libebl_CPU.h so other backends can reuse it. > +static int > +get_die_type (Dwarf_Die *die, Dwarf_Die *result) > +{ > + Dwarf_Attribute attr_mem; > + Dwarf_Attribute *attr = dwarf_attr_integrate (die, DW_AT_type, &attr_mem); > + if (attr == NULL) > + /* The function has no return value, like a `void' function in C. */ > + return 0; > + > + if (dwarf_formref_die (attr, result) == NULL) > + return -1; > + > + return peel_type (&result, &attr); > +} Likewise. > +static int > +skip_until (Dwarf_Die *child, int tag) > +{ > + int i; > + while (DWARF_TAG_OR_RETURN (child) != tag) > + if ((i = dwarf_siblingof (child, child)) != 0) > + /* If there are no members, then this is not a HFA. Errors > + are propagated. */ > + return i; > + return 0; > +} Generic function, but specific to hfa_type. Maybe define there? > +static int hfa_type (Dwarf_Die *ftypedie, int tag, > + Dwarf_Word *sizep, Dwarf_Word *countp); hfa is "homogeneous floating-point aggregate"? > +/* Return 0 if MEMBDIE refers to a member with a floating-point or HFA > + type, or 1 if it's not. Return -1 for errors. The meaning of the > + remaining arguments is as documented at hfa_type. */ > +static int > +member_is_fp (Dwarf_Die *membdie, Dwarf_Word *sizep, Dwarf_Word *countp) > +{ > + Dwarf_Die type_die; > + int tag = get_die_type (membdie, &type_die); > + switch (tag) > + { > + case DW_TAG_base_type:; > + Dwarf_Word encoding; > + Dwarf_Attribute attr_mem; > + if (dwarf_attr_integrate (&type_die, DW_AT_encoding, &attr_mem) == NULL > + || dwarf_formudata (&attr_mem, &encoding) != 0) > + return -1; > + > + switch (encoding) > + { > + case DW_ATE_complex_float: > + *countp = 2; > + break; > + > + case DW_ATE_float: > + *countp = 1; > + break; > + > + default: > + return 1; > + } > + > + if (dwarf_formudata (dwarf_attr_integrate (&type_die, DW_AT_byte_size, > + &attr_mem), sizep) != 0) > + return -1; I guess we will never actually see DW_AT_bit_size. > + *sizep /= *countp; > + return 0; > + > + case DW_TAG_structure_type: > + case DW_TAG_union_type: > + case DW_TAG_array_type: > + return hfa_type (&type_die, tag, sizep, countp); > + } > + > + return 1; > +} > + > +/* HFA (Homogeneous Floating-point Aggregate) is an aggregate type > + whose members are all of the same floating-point type, which is > + then base type of this HFA. Instead of being floating-point types > + directly, members can instead themselves be HFA. Such HFA fields > + are handled as if their type were HFA base type. > + > + This function returns 0 if TYPEDIE is HFA, 1 if it is not, or -1 if > + there were errors. In the former case, *SIZEP contains byte size > + of the base type (e.g. 8 for IEEE double). *COUNT is set to the > + number of leaf members of the HFA. */ > +static int > +hfa_type (Dwarf_Die *ftypedie, int tag, Dwarf_Word *sizep, Dwarf_Word > *countp) > +{ > + assert (tag == DW_TAG_structure_type || tag == DW_TAG_class_type > + || tag == DW_TAG_union_type || tag == DW_TAG_array_type); > + > + int i; > + if (tag == DW_TAG_array_type) > + { > + Dwarf_Word tot_size; > + if (dwarf_aggregate_size (ftypedie, &tot_size) < 0) > + return -1; > + > + /* For vector types, we don't care about the underlying > + type, but only about the vector type itself. */ > + bool vec; > + Dwarf_Attribute attr_mem; > + if (dwarf_formflag (dwarf_attr_integrate (ftypedie, DW_AT_GNU_vector, > + &attr_mem), &vec) == 0 > + && vec) > + { > + *sizep = tot_size; > + *countp = 1; > + > + return 0; > + } > + > + if ((i = member_is_fp (ftypedie, sizep, countp)) == 0) > + { > + *countp = tot_size / *sizep; > + return 0; > + } > + > + return 1; You already saw this should return i. I would have missed it. > + } > + > + /* Find first DW_TAG_member and determine its type. */ > + Dwarf_Die member; > + if ((i = dwarf_child (ftypedie, &member) != 0)) > + return i; > + > + if ((i = skip_until (&member, DW_TAG_member)) != 0) > + return i; > + > + *countp = 0; > + if ((i = member_is_fp (&member, sizep, countp)) != 0) > + return i; > + > + while ((i = dwarf_siblingof (&member, &member)) == 0 > + && (i = skip_until (&member, DW_TAG_member)) == 0) > + { > + Dwarf_Word size, count; > + if ((i = member_is_fp (&member, &size, &count)) != 0) > + return i; > + > + if (*sizep != size) > + return 1; > + > + *countp += count; > + } > + > + return i < 0 ? i : 0; > +} Wow, would be nice to have some testcases for these. Your suggested comment would be nice. The rest looks OK. > diff --git a/backends/aarch64_symbol.c b/backends/aarch64_symbol.c > +/* AArch64 specific symbolic name handling. > [...] > +/* Check for the simple reloc types. */ > +Elf_Type > +aarch64_reloc_simple_type (Ebl *ebl __attribute__ ((unused)), int type) > +{ > + switch (type) > + { > + case R_AARCH64_ABS64: > + return ELF_T_XWORD; > + case R_AARCH64_ABS32: > + return ELF_T_WORD; > + case R_AARCH64_ABS16: > + return ELF_T_HALF; > + > + default: > + return ELF_T_NUM; > + } > +} Looks OK. BTW. tests/run-strip-reloc.sh might be nice to add for aarch64. > diff --git a/libebl/ChangeLog b/libebl/ChangeLog > index 57f9f46..0ecd16f 100644 > --- a/libebl/ChangeLog > +++ b/libebl/ChangeLog > @@ -1,3 +1,7 @@ > +2013-11-14 Petr Machata <[email protected]> > + > + * eblopenbackend.c (machines): Add entry for AArch64. OK. > +2013-11-14 Petr Machata <[email protected]> > + > + * elflint.c (valid_e_machine): Add EM_AARCH64. OK. > +2013-11-14 Petr Machata <[email protected]> > + > + * testfile_aarch64_core.bz2: New file. > + * Makefile.am (EXTRA_DIST): Add it. OK. > + * run-allregs.sh: Use it to add a regs_test. > + * run-readelf-mixed-corenote.sh: Use it to add a readelf -n test. > diff --git a/tests/run-allregs.sh b/tests/run-allregs.sh > index 885a1d1..e4a40ce 100755 > --- a/tests/run-allregs.sh > +++ b/tests/run-allregs.sh > @@ -1,5 +1,5 @@ > #! /bin/sh > -# Copyright (C) 2005, 2006, 2007, 2012 Red Hat, Inc. > +# Copyright (C) 2005, 2006, 2007, 2012, 2013 Red Hat, Inc. > # This file is part of elfutils. > # > # This file is free software; you can redistribute it and/or modify > @@ -2724,4 +2724,74 @@ VFP registers: > 287: d31 (d31), float 64 bits > EOF > > +regs_test testfile_aarch64_core <<\EOF > +integer registers: Add a comment, see run-readelf-mixed-corenote.sh how to regenerate this core file. > diff --git a/tests/run-readelf-mixed-corenote.sh > b/tests/run-readelf-mixed-corenote.sh > index 9a43809..c176e28 100755 > --- a/tests/run-readelf-mixed-corenote.sh > +++ b/tests/run-readelf-mixed-corenote.sh > @@ -285,4 +285,144 @@ Note segment of 1476 bytes at offset 0x430: > 3e001ba000-3e001bc000 001ba000 8192 /usr/lib64/libc-2.17.so > EOF > > +# To reproduce this core dump, do this on an aarch64 machine: > +# $ gcc -x c <(echo 'int main () { return *(int *)0x12345678; }') > +# $ ./a.out > +testfiles testfile_aarch64_core > +testrun_compare ${abs_top_builddir}/src/readelf -n testfile_aarch64_core > <<\EOF OK.
