Re: On upgrading elftoolchain in src/

[christos%astron.com@localhost (Christos Zoulas)]

In article <588382087.79098.1616533944261%ox91.mailbox.org@localhost>,
 <jkoshy%netbsd.org@localhost> wrote:
>Greetings,
>
>I have been looking at upgrading "external/bsd/elftoolchain" in src/,
>and have a question about our sys/*.h headers.
>
>:: <elfdefinitions.h>
>
>In 2010 I had moved the C structure definitions and constants describing
>the ELF format into a standalone file, $ELFTC/common/elfdefinitions.h.
>
>I did this because in the BSD OSes ELF structure definitions and constants
>are placed in differently named headers, with there also being some
>variation in architecture support between the OS projects. Moving all
>structures and constants related to the ELF file format into a common
>header allowed Elftoolchain code to compile on multiple target OSes.
>
>Patch www.netbsd.org/~jkoshy/install-elfdefinitions-2021-03-21.txt will
>install Elftoolchain's <elfdefinitions.h> into ${DESTDIR}/usr/include.
>
>The issue is that some (but not all) of the symbols in <elfdefinitions.h>
>are also present in the system header <$NBSRC/sys/sys/exec_elf.h>.
>Consequently, a source file will not be able to #include both files
>at once.
>
><$NBSRC/sys/sys/exec_elf.h> is how source code in NetBSD currently
>accesses ELF file format related types and constants (Elf32_*, EM_*, DF_*,
>etc). But this file also has symbols used by the kernel for its own needs
>e.g. the symbols needed to implement execve(2) (the AT_* constants),
>to implement core dumps, prototypes for kernel functions and so on.
>So <$NBSRC/sys/exec_elf.h> today has a mix of kernel-only definitions,
>definitions shared between the userspace C runtime and the kernel,
>and ELF file format related symbols.
>
>There are a small number of files in $NBSRC userspace that #include
><sys/exec_elf.h>, but most of these seem to doing so to gain access to
>the ELF file format related types and constants.  Such files could as
>well use a file-format-only header like <elfdefinitions.h>.
>
>A few files under "external/cddl/" #include both <libelf.h> and
><sys/exec_elf.h>, but these seem to be doing so indirectly via
><external/cddl/sys/sys/elf.h>.
>
>So this is the plan of action that I am considering:
>
>1. Install <elfdefinitions.h> into /usr/include, per the patch
>   linked to earlier. (Would /usr/include/sys be a better location?).
>
>2. Change $NBSRC userspace code that only needs the file format related
>   types and constants to use the newly installed <elfdefinitions.h>,
>   instead of <sys/exec_elf.h>.
>
>3. Then perhaps change <sys/exec_elf.h> to use <elfdefinitions.h> for
>   the file format related symbols, so that we would have a single 'source
>   of truth' for the ELF file format related information, and so that
>   the <sys/exec_elf.h> header can then focus on kernel functionality.
>
>Do you foresee any issues with implementing this plan?

- Is there any need today to install elfdefinitions.h in /usr/include?
  What will use it that does not work today (elftoolchain already uses
  its own copy)?
- The elf stuff that is used by the kernel needs to live in sys/ so
  the kernel will never be able to use elfdefinitions.h (in case in
  the long term we wanted to delete exec_elf.h to avoid duplicated
  definitions).
- It is nice and convenient to define the constants as macros together
  with string explanations but readability suffers. If that was a private
  header, that would be a good solution but for system headers brevity
  and clarity trumps convenience IMHO (and most things will not need
  the string explanations which can be accessed using library functions).
- the elf headers (both the existing one and elfdefinitions.h are too
  long and could be functionally split).
- and yes it sucks not to have a portable elf header across OS's so if
  yours becomes a standard that would be a welcome change... Talking from
  experience (/usr/src/external/bsd/file/dist/src/readelf.h)

Best,

christos