Re: [PATCH] Linux: Add tables with system call numbers

[Zack Weinberg <zackw at panix dot com>]

On Wed, May 29, 2019 at 1:48 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Zack Weinberg:
>
> > On Tue, May 28, 2019 at 6:10 PM Florian Weimer <fweimer@redhat.com> wrote:
> > ...
> >> The goal is to use these architecture-specific tables to ensure
> >> that system call wrappers are available irrespective of the version
> >> of the installed kernel headers.
> >>
> >> The tables are formatted in the form of C header files so that they
> >> can be used directly in an #include directive, without external
> >> preprocessing.  (External preprocessing of a plain table file
> >> would introduce cross-subdirectory dependency issues.)  However,
> >> the intent is that they can still be treated as tables and processed
> >> by simple tools.
> >
> > Is there any reason why, instead of inventing a new name
> > "arch-syscall.h", these tables could not be checked in as
> > bits/syscall.h for each architecture, allowing us to eliminate the
> > process of generating bits/syscall.h at build time, and decoupling our
> > sys/syscall.h altogether from the installed kernel headers?
>
> I think I explained in a comment somewhere that it is desirable to still
> have the global list, so that the SYS_ names magically show up if a
> known system call is wired up for any architecture, without upgrade or
> rebuilding glibc.

I don't understand why a global list of known system calls is
incompatible with the checked-in architecture-specific lists of system
call numbers (in the form of a C header file) being the bits/syscall.h
for each architecture.  I'm just trying to avoid having two header
files with very nearly the same function.

> > my old-fashioned heart is inclined to
> > say that that's a win anyway, the __NR_ names were never part of *our*
> > official API, include <asm/unistd.h> yourself if you want them.
>
> The __NR_ names are widely used in the source tree.  We can replace
> them, but the second patch would be much, much larger.

That's probably not worth doing in this patch.  I'd kind of like to
see it happen, regardless, but I can't commit the time to do it
myself.

> >> +++ b/sysdeps/unix/sysv/linux/glibcsyscalls.py
> >> @@ -0,0 +1,141 @@
> >> +#!/usr/bin/python3
> >> +# Helpers for glibc system call list processing.
> >> +# Copyright (C) 2018-2019 Free Software Foundation, Inc.
> >> +# This file is part of the GNU C Library.
> >
> > This appears _not_ to be the script Joseph was asking for.  Could you
> > please make it clearer in the file what it does do?
>
> It's a set of helper functions, and in contrast to
> scripts/glibcextract.py, it is Linux-specific.  Not sure how to describe
> it better.

Helper functions for what, though?  I couldn't find uses in the patch,
possibly because the arch-syscall.h files are so long and repetitive
that I missed them.  (It might be helpful to post all of the code that
needs reviewing as a separate patch from the creation of the
arch-syscall.h files.)

> >> diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/arch-syscall.h b/sysdeps/unix/sysv/linux/x86_64/x32/arch-syscall.h
> >> new file mode 100644
> >> index 0000000000..ef4ec281ef
> >> --- /dev/null
> >> +++ b/sysdeps/unix/sysv/linux/x86_64/x32/arch-syscall.h
> >> @@ -0,0 +1,324 @@
> >> +#define __NR_accept 1073741867
> >
> > Would it be possible to make this come out as (0x40000000 | 43), and
> > similarly for the others?
>
> Can you clarify why you would want to do this?

Just to make clear to human readers that it's the same as __NR_accept
for x86_64/64, plus a flag bit.  (asm/unistd_x32.h does this.)

Is the issue that you anticipate uses of these files by programs other
than glibc's build process, and for which parsing a C integer constant
expression would be inconvenient?  Could you give examples, even if
they have to be hypothetical?

zw