On Thu, Aug 02, 2012 at 01:43:10AM +0200, Petr Machata wrote:
> these two patches implement support for 64-bit symbol table in .a
> archives. As of recently, binutils' ar produces such archives on s390x
> (regardless of actual archive size). This appears to be the same as
> "/", except all fields are 64-bit.
Just a quick partial review, with some pedantic comments. Have to read
up on ar archives. But it looks correct to me. Are the magic entries
("/SYM64/") described somewhere in a specification document or is this
GNU specific?
> From 37118707e4974c6bc95d2e3d7fbf59ac1e1912c3 Mon Sep 17 00:00:00 2001
> From: Petr Machata <[email protected]>
> Date: Wed, 1 Aug 2012 21:37:52 +0200
> Subject: [PATCH 1/2] Implement support for archives with 64-bit symbol table
> diff --git a/libelf/ChangeLog b/libelf/ChangeLog
> index 8c9ff8b..18ada85 100644
> --- a/libelf/ChangeLog
> +++ b/libelf/ChangeLog
> @@ -1,3 +1,11 @@
> +2012-08-01 Petr Machata <[email protected]>
> +
> + * elf_getarsym (read_number_entries): New function.
> + (elf_getarsym): Handle 64-bit symbol table, stored in special
> + entry named "/SYM64/".
> + * elf_begin.c (__libelf_next_arhdr_wrlock): Don't reject archive
> + because it contains 64-bit symbol table.
> +
> [...]
> --- a/libelf/elf_getarsym.c
> +++ b/libelf/elf_getarsym.c
> @@ -1,5 +1,5 @@
> /* Return symbol table of archive.
> - Copyright (C) 1998, 1999, 2000, 2002, 2005 Red Hat, Inc.
> + Copyright (C) 1998-2012 Red Hat, Inc.
This should be 1998-2000, 2002, 2005, 2012. Only the first is a full
range. Similarly in some other places. (Yes, I did ask a lawyer.)
> - /* Now test whether this is the index. It is denoted by the
> - name being "/ ".
> + bool index64_p;
> + /* Now test whether this is the index. If the name is "/", this
> + is 32-bit index, if it's "/SYM64/", it's 64-bit index.
> +
> XXX This is not entirely true. There are some more forms.
> Which of them shall we handle? */
Is this still true? Which other forms are there?
> + /* Check whether 64-bit offset fits into 32-bit
> + size_t. */
> + if (sizeof (arsym[cnt].as_off) < 8
> + && arsym[cnt].as_off != tmp)
> + {
> + if (elf->map_address == NULL)
> + {
> + free (elf->state.ar.ar_sym);
> + elf->state.ar.ar_sym = NULL;
> + }
> +
> + __libelf_seterrno (ELF_E_RANGE);
> + goto out;
> + }
Urgh. How messy, all these public size_t arguments and fields :{
But this is the best we can do on a 32-bit arch. nice.
> From ca57cfba6db5d7122c1bcccd3985ea4d09493df6 Mon Sep 17 00:00:00 2001
> From: Petr Machata <[email protected]>
> Date: Wed, 1 Aug 2012 21:41:36 +0200
> Subject: [PATCH 2/2] Test case for handling archives with 64-bit symbol table
> [...]
> diff --git a/tests/archive64.a.bz2 b/tests/archive64.a.bz2
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..a8dc081ceb0d17fd75de3d516b549e766f23af2c
> [...]
> diff --git a/tests/test-archive64.sh b/tests/test-archive64.sh
> new file mode 100755
> index 0000000..f9d1cc6
> --- /dev/null
> +++ b/tests/test-archive64.sh
It would be more consistent to cal this run-test-archive64.sh.
> +
> +testfiles archive64.a
Please document here how archive64.a was created.
Thanks,
Mark
_______________________________________________
elfutils-devel mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/elfutils-devel
