elftoolchain-developers

[Elftoolchain-developers] Remaining FreeBSD ELF Tool Chain changes in the queue

[Ed M. <em...@fr...>]

I have the changes listed below remaining in my queue to merging into
upstream ELF Tool Chain. After each commit headline I've included the
FreeBSD code review (if it has one) or the FreeBSD commit; in some
cases there are followup improvements to include in the change.

libelf: reload section headers after update with ELF_C_WRITE
https://reviews.freebsd.org/D10486

libelf: Use a red-black tree to manage the section list
https://reviews.freebsd.org/D20443

libelf: fix .note conversion for non-native byte order ELF files
https://reviews.freebsd.org/rS273443

libelf: remove 4-byte .note alignment for PowerPC
https://reviews.freebsd.org/rS273442

libdwarf: prepend DW_AT_comp_dir to relative line number dir entries
https://reviews.freebsd.org/D19705

elfcopy: copy raw (untranslated) contents to binary output
https://reviews.freebsd.org/rS327489

elfcopy: fix little-endian MIPS64 objects
https://reviews.freebsd.org/D15734
https://reviews.freebsd.org/rS344855
https://reviews.freebsd.org/rS339083
https://reviews.freebsd.org/rS339473

readelf: speed up readelf -wo
https://reviews.freebsd.org/D18843
https://reviews.freebsd.org/rS346327

readelf: decode flag bits in DT_FLAGS/DT_FLAGS_1
https://reviews.freebsd.org/D18784
https://reviews.freebsd.org/rS343593
https://reviews.freebsd.org/rS343665

readelf: decode FreeBSD note types
https://reviews.freebsd.org/rS343669

readelf: Add support for RISC-V specific e_flags
https://reviews.freebsd.org/rS349482

readelf: display NT_GNU_PROPERTY_TYPE_0 note name
https://reviews.freebsd.org/rS337569

Re: [Elftoolchain-developers] Remaining FreeBSD ELF Tool Chain changes in the queue

[Ed M. <em...@fr...>]

On Fri, 28 Jun 2019 at 19:05, Ed Maste <em...@fr...> wrote:
>
> elfcopy: fix little-endian MIPS64 objects
> https://reviews.freebsd.org/D15734
> https://reviews.freebsd.org/rS344855
> https://reviews.freebsd.org/rS339083
> https://reviews.freebsd.org/rS339473

This is (part of) ELF Tool Chain ticket #559, although I notice now
that the FreeBSD commit did not address the /* TODO: Handle MIPS64
REL{,A} sections (ticket #559). */ in libelf_convert.m4, and in this
commit the conversion is done in gelf_rel.c/gelf_rela.c, not as a
translator.

> readelf: decode flag bits in DT_FLAGS/DT_FLAGS_1
> https://reviews.freebsd.org/D18784
> https://reviews.freebsd.org/rS343593
> https://reviews.freebsd.org/rS343665

Committed to ELF Tool Chain as r3765.

> readelf: decode FreeBSD note types
> https://reviews.freebsd.org/rS343669

Committed to ELF Tool Chain as r3766.

Re: [Elftoolchain-developers] Remaining FreeBSD ELF Tool Chain changes in the queue

[Ed M. <em...@fr...>]

> libelf: fix .note conversion for non-native byte order ELF files
> https://reviews.freebsd.org/rS273443

In fact this was an incorrect change; I already merged the corrected
version in r3158.

Re: [Elftoolchain-developers] Remaining FreeBSD ELF Tool Chain changes in the queue

[Joseph K. <jk...@us...>]

Ed,

First, thank you for stepping up to shepherd these
changes from FreeBSD into the Elftoolchain sources.

I can confirm that I was able to build and
successfully run the test suites at Elftoolchain/r3769
on NetBSD 8/i386 and Ubuntu 18.04.

The Elftoolchain tree currently fails to compile
on NetBSD 7.1.2/i386 and OpenBSD 6.3/amd64,
but these failures seem unrelated to the current
changes. I haven't checked the other platforms,
namely DragonFlyBSD, Minix, other combinations of
BSD OS versions and architectures yet.

A few comments below:

> libelf: reload section headers after update with ELF_C_WRITE
> https://reviews.freebsd.org/D10486

I would like to understand what the actual problem is
before folding in this patch. A small test program
(or a pointer to existing source that needs this
change to work correctly) would be helpful.

Specifically, I would like confirm that a program
that uses the ELF(3) API correctly would need this
patch. It may be that our documentation about
the state of an Elf descriptor after a call to
elf_update(3) may need to be improved.

> libelf: Use a red-black tree to manage the section list
> https://reviews.freebsd.org/D20443

Two questions here:

1. Per the FreeBSD Phabricator entry the motivation
   for this change was to speed up elf_getscn(3).
   If that is the case, how many sections does the
   ELF object need to have before the use of a RB tree
   vs a simpler data structure become relevant? I'm
   curious if performance measurements were done?

2. The other thing to check would be whether our
   target platforms have <sys/tree.h> implementations
   suitable for use from userland.

Regards,
Joseph Koshy

Re: [Elftoolchain-developers] Remaining FreeBSD ELF Tool Chain changes in the queue

[Mark J. <ma...@fr...>]

On Sat, Jun 29, 2019 at 10:51:57PM +0100, Joseph Koshy via Elftoolchain-developers wrote:
> > libelf: Use a red-black tree to manage the section list
> > https://reviews.freebsd.org/D20443
> 
> Two questions here:

Hi Joseph,

> 1. Per the FreeBSD Phabricator entry the motivation
>    for this change was to speed up elf_getscn(3).
>    If that is the case, how many sections does the
>    ELF object need to have before the use of a RB tree
>    vs a simpler data structure become relevant? I'm
>    curious if performance measurements were done?

The context for that change is FreeBSD PR 234949
(https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=234949).
It comes from a series of changes which reduces elftoolchain's strip
runtime on an archive of Haskell libraries compiled with
-ffunction-sections.

One of the test files is libHSCabal-2.4.0.1_p.a, which contains 220
objects.  Among them, the minimum number of sections is 26, the
maximum is 46125, and the median and average are 964 and 2085,
respectively.

Reverting just this change (use a red-black tree) increases the runtime
of strip from 4s to 80s for this file.

> 2. The other thing to check would be whether our
>    target platforms have <sys/tree.h> implementations
>    suitable for use from userland.

I know that *BSD provides tree.h and libbsd has a tree.h as well; that
seemed sufficient to me, but maybe I'm missing something?  It seemed
reasonable to use tree.h given that elftoolchain already makes extensive
use of queue.h.

Re: [Elftoolchain-developers] Remaining FreeBSD ELF Tool Chain changes in the queue

[Joseph K. <jk...@us...>]

Mark,

m.j> The context for that change is FreeBSD PR 234949
m.j> (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=234949).
m.j> It comes from a series of changes which reduces
m.j> elftoolchain's strip runtime on an archive of
m.j> Haskell libraries compiled with -ffunction-sections.

Thanks for providing the context; nice work on that PR.

m.j> I know that *BSD provides tree.h and libbsd has a
m.j> tree.h as well; that seemed sufficient to me, but
m.j> maybe I'm missing something?  It seemed reasonable
m.j> to use tree.h given that elftoolchain already makes
m.j> extensive use of queue.h.

We've seen incompatibilities between *BSD's
<sys/queue.h> implementations; for example, OpenBSD
6.3's <sys/queue.h> lacked the STAILQ_* macros, and so
we had to provide a local copy in "common/_elftc.h".

I hope <sys/tree.h> is more portable than
<sys/queue.h>, but if not we could add local
definitions to "common/_elftc.h" till the time the
differences are reconciled upstream.

Thanks again for tracking down this performance
issue.  I have created a ticket (on myself)
(https://sourceforge.net/p/elftoolchain/tickets/574/)
to fold in this change.

Regards,
Joseph Koshy

Re: [Elftoolchain-developers] Remaining FreeBSD ELF Tool Chain changes in the queue

[Mark J. <ma...@fr...>]

On Sun, Jun 30, 2019 at 02:35:16PM +0100, Joseph Koshy wrote:
> Mark,
> 
> m.j> The context for that change is FreeBSD PR 234949
> m.j> (https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=234949).
> m.j> It comes from a series of changes which reduces
> m.j> elftoolchain's strip runtime on an archive of
> m.j> Haskell libraries compiled with -ffunction-sections.
> 
> Thanks for providing the context; nice work on that PR.
> 
> m.j> I know that *BSD provides tree.h and libbsd has a
> m.j> tree.h as well; that seemed sufficient to me, but
> m.j> maybe I'm missing something?  It seemed reasonable
> m.j> to use tree.h given that elftoolchain already makes
> m.j> extensive use of queue.h.
> 
> We've seen incompatibilities between *BSD's
> <sys/queue.h> implementations; for example, OpenBSD
> 6.3's <sys/queue.h> lacked the STAILQ_* macros, and so
> we had to provide a local copy in "common/_elftc.h".
> 
> I hope <sys/tree.h> is more portable than
> <sys/queue.h>, but if not we could add local
> definitions to "common/_elftc.h" till the time the
> differences are reconciled upstream.

I'm fairly confident that all of the BSDs have roughly the same RB_*
macros in tree.h.  I can try compiling elftoolchain on each of them, but
you may be better set up to test that than me.

> Thanks again for tracking down this performance
> issue.  I have created a ticket (on myself)
> (https://sourceforge.net/p/elftoolchain/tickets/574/)
> to fold in this change.

Thanks.  The full set of changes needed to get parity with GNU strip is
in the PR; they are all now committed to FreeBSD.