On Fri, Nov 22, 2013 at 12:53:57AM +0100, Petr Machata wrote: > Besides nits this > includes the following fairly large changes: > > - added hello_aarch64.ko.bz2 and enabled it in run-strip-reloc.sh. > > - added funcretval_test.c with source (not compiled, just for > reference, as it is too large and unwieldy to stick into a testfile > comment). This yields funcretval_test_aarch64.bz2, which is used in > a new test suite run-funcretval.sh.
Thanks a lot for these new testcase. Everything looks fine, but could you take a look at my comment for regtype (). With that fixed or explained it looks good to go in. > diff --git a/backends/ChangeLog b/backends/ChangeLog > index 3c57f8c..e7d1268 100644 > --- a/backends/ChangeLog > +++ b/backends/ChangeLog > @@ -1,3 +1,17 @@ > +2013-11-22 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. > + * aarch64_corenote.c, aarch64_init.c: New files. > + * aarch64_regs.c, aarch64_reloc.def: Likewise. > + * aarch64_retval.c, aarch64_symbol.c: Likewise. > + * libebl_CPU.h (dwarf_peel_type): New function. > + (dwarf_peeled_die_type): Likewise. OK. > diff --git a/backends/aarch64_regs.c b/backends/aarch64_regs.c > + 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; I would have expected if (s < 0 || (unsigned) s >= namelen) to signal a snprintf error. > + return s + 1; > + } So if there is an error (given name array is too short?) then s = -1 + 1 = 0. But that looks like an unused register number. > diff --git a/backends/aarch64_retval.c b/backends/aarch64_retval.c > +static int > +dwarf_bytesize_aux (Dwarf_Die *die, Dwarf_Word *sizep) > +{ > + int bits; > + if (((bits = 8 * dwarf_bytesize (die)) < 0 > + && (bits = dwarf_bitsize (die)) < 0) > + || bits % 8 != 0) > + return -1; > + > + *sizep = bits / 8; > + return 0; > +} Cute. > +static int > +member_is_fp (Dwarf_Die *membdie, Dwarf_Word *sizep, Dwarf_Word *countp) > +{ > + Dwarf_Die typedie; > + int tag = dwarf_peeled_die_type (membdie, &typedie); > + switch (tag) > + { > + case DW_TAG_base_type:; > + Dwarf_Word encoding; > + Dwarf_Attribute attr_mem; > + if (dwarf_attr_integrate (&typedie, DW_AT_encoding, &attr_mem) == NULL > + || dwarf_formudata (&attr_mem, &encoding) != 0) > + return -1; Nitpick. A block is nicer in this case than the :; case thingy. IMHO. > diff --git a/libebl/ChangeLog b/libebl/ChangeLog > +2013-11-22 Petr Machata <[email protected]> > + > + * eblopenbackend.c (machines): Add entry for AArch64. OK. > diff --git a/src/ChangeLog b/src/ChangeLog > +2013-11-22 Petr Machata <[email protected]> > + > + * elflint.c (valid_e_machine): Add EM_AARCH64. OK. > diff --git a/tests/ChangeLog b/tests/ChangeLog > +2013-11-22 Petr Machata <[email protected]> > + > + * testfile_aarch64_core.bz2, hello_aarch64.ko.bz2: New files. > + * funcretval_test.c, funcretval_test_aarch64.bz2: Likewise. > + * Makefile.am (EXTRA_DIST): Add these. > + (TESTS): Add run-funcretval.sh. > + * run-allregs.sh: Use testfile_aarch64_core.bz2 for a regs_test. > + * run-readelf-mixed-corenote.sh: ... and for a readelf -n test. > + * run-strip-reloc.sh: Add a test on hello_aarch64.ko.bz2. > + * run-funcretval.sh: New file. OK. It would be nice if funcretval printed the ops in a more human readable form. varlocs.c has some code for that. But that is for another time. Thanks, Mark
