[PATCH v2 0/5] linux: Avoid va_list for generic syscall wrappers if possible

[PATCH v2 0/5] linux: Avoid va_list for generic syscall wrappers if possible

[Xi Ruoyao]

Currently GCC generates highly sub-optimal code on architectures where
the calling convention prefers registers for arugment passing.  This is
GCC PR100955.  While it's technically a missed-optimization in GCC, it
seems not trivial to fix (I've not seen any compiler which can optimize
this properly yet).

As the generic Linux syscall wrappers often uses a fixed number of
arguments, and "overreading" the variable arguments is usually safe
(fcntl etc. is already doing this), we can pretend these wrappers were
declared with named arguments and make the compiler do right thing.

Add a macro __ASSUME_SYSCALL_NAMED_WORKS to control this, which should
be defined if we can safely replace "..." with several named arguments.
Use an internal function prototype with named arguments if it's defined,
for fcntl64, fcntl_nocancel, ioctl, mremap, open64, open64_nocancel,
openat64, openat64_nocancel, prctl, ptrace, and generic syscall()
wrapper.  I've not changed open* without "64" because I don't have a
test platform.  shm_ctl is also not changed because it contains
aggregate variable arugment which is more tricky than integers or
pointers.

Define this macro for LoongArch, x86-64, and AArch64.  This should be
also suitable for some other architectures (I think it will be fine on
RISC-V) but again I don't have a test platform.

This is the first time I make such a large change in Glibc so it's
likely I've done something wrong.  Please correct me :).

Xi Ruoyao (5):
  linux: Add __ASSUME_SYSCALL_NAMED_WORKS to allow avoiding va_list for
    generic syscall
  linux: [__ASSUME_SYSCALL_NAMED_WORKS] Avoid using va_list for various
    syscall wrappers
  LoongArch: Define __ASSUME_SYSCALL_NAMED_WORKS for Linux
  x86_64: Define __ASSUME_SYSCALL_NAMED_WORKS for Linux
  aarch64: Define __ASSUME_SYSCALL_NAMED_WORKS for Linux

 include/fcntl.h                               | 32 ++++++++-----
 .../unix/sysv/linux/aarch64/kernel-features.h |  9 ++++
 sysdeps/unix/sysv/linux/fcntl64.c             | 40 +++++++++++++----
 sysdeps/unix/sysv/linux/fcntl_nocancel.c      | 30 ++++++++++---
 sysdeps/unix/sysv/linux/ioctl.c               | 38 ++++++++++++----
 .../sysv/linux/loongarch/kernel-features.h    | 29 ++++++++++++
 sysdeps/unix/sysv/linux/mremap.c              | 36 +++++++++++++--
 sysdeps/unix/sysv/linux/not-cancel.h          |  8 ++--
 sysdeps/unix/sysv/linux/open64.c              | 35 ++++++++++++---
 sysdeps/unix/sysv/linux/open64_nocancel.c     | 29 ++++++++++--
 sysdeps/unix/sysv/linux/openat64.c            | 30 +++++++++++--
 sysdeps/unix/sysv/linux/openat64_nocancel.c   | 29 ++++++++++--
 sysdeps/unix/sysv/linux/prctl.c               | 23 +++++++++-
 sysdeps/unix/sysv/linux/ptrace.c              | 45 ++++++++++++++-----
 sysdeps/unix/sysv/linux/syscall.c             | 35 +++++++++++----
 .../unix/sysv/linux/x86_64/kernel-features.h  |  9 ++++
 16 files changed, 382 insertions(+), 75 deletions(-)
 create mode 100644 sysdeps/unix/sysv/linux/loongarch/kernel-features.h

-- 
2.40.0

[PATCH v2 1/5] linux: Add __ASSUME_SYSCALL_NAMED_WORKS to allow avoiding va_list for generic syscall

[Xi Ruoyao]

Currently GCC generates highly sub-optimal code on architectures where
the calling convention prefers registers for argument passing.  This is
GCC PR100955.  While it's technically a missed-optimization in GCC, it
seems not trivial to fix (I've not seen any compiler which can optimize
this properly yet).

As the generic Linux syscall actually uses a fixed number of arguments,
we can avoid va_list if possible and make the compiler do right thing.

Add a macro __ASSUME_SYSCALL_NAMED_WORKS which should be defined if the
calling convention treats (x named arguments + y variable arguments)
exactly same as (x + y) named arguments, while each argument is either
an integer of which the width is less than or equal to "long" or a
pointer; and each argument can be fetched from the same register or the
same offset from the stack pointer no matter how many (maybe zero)
arguments are passed after it.
---
 sysdeps/unix/sysv/linux/syscall.c | 35 ++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/syscall.c b/sysdeps/unix/sysv/linux/syscall.c
index a5a2843b73..ed5ad5afd5 100644
--- a/sysdeps/unix/sysv/linux/syscall.c
+++ b/sysdeps/unix/sysv/linux/syscall.c
@@ -16,9 +16,33 @@
    License along with the GNU C Library.  If not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <stdarg.h>
 #include <sysdep.h>
 
+#ifndef __ASSUME_SYSCALL_NAMED_WORKS
+#include <stdarg.h>
+#endif
+
+static inline long int
+__syscall (long int number, long int a0, long int a1, long int a2, long int a3,
+	   long int a4, long int a5)
+{
+  long int r = INTERNAL_SYSCALL_NCS_CALL (number, a0, a1, a2, a3, a4, a5);
+  if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (r)))
+    {
+      __set_errno (-r);
+      return -1;
+    }
+  return r;
+}
+
+#ifdef __ASSUME_SYSCALL_NAMED_WORKS
+long int
+syscall (long int number, long int a0, long int a1, long int a2, long int a3,
+	 long int a4, long int a5)
+{
+  return __syscall (number, a0, a1, a2, a3, a4, a5);
+}
+#else
 long int
 syscall (long int number, ...)
 {
@@ -33,11 +57,6 @@ syscall (long int number, ...)
   long int a5 = va_arg (args, long int);
   va_end (args);
 
-  long int r = INTERNAL_SYSCALL_NCS_CALL (number, a0, a1, a2, a3, a4, a5);
-  if (__glibc_unlikely (INTERNAL_SYSCALL_ERROR_P (r)))
-    {
-      __set_errno (-r);
-      return -1;
-    }
-  return r;
+  return __syscall (number, a0, a1, a2, a3, a4, a5);
 }
+#endif
-- 
2.40.0

[PATCH v2 2/5] linux: [__ASSUME_SYSCALL_NAMED_WORKS] Avoid using va_list for various syscall wrappers

[Xi Ruoyao]

This patch changes fcntl64, fcntl_nocancel, ioctl, mremap, open64,
open64_nocancel, openat64, openat64_nocancel, prctl, and ptrace not to
use va_list if the named arguments is passed in the same way as variable
arguments in the calling convention.  This removes the stack store
operation emitted by the compiler for va_start macro.
---
 include/fcntl.h                             | 32 ++++++++++-----
 sysdeps/unix/sysv/linux/fcntl64.c           | 40 ++++++++++++++----
 sysdeps/unix/sysv/linux/fcntl_nocancel.c    | 30 +++++++++++---
 sysdeps/unix/sysv/linux/ioctl.c             | 38 +++++++++++++----
 sysdeps/unix/sysv/linux/mremap.c            | 36 +++++++++++++++--
 sysdeps/unix/sysv/linux/not-cancel.h        |  8 ++--
 sysdeps/unix/sysv/linux/open64.c            | 35 +++++++++++++---
 sysdeps/unix/sysv/linux/open64_nocancel.c   | 29 +++++++++++--
 sysdeps/unix/sysv/linux/openat64.c          | 30 ++++++++++++--
 sysdeps/unix/sysv/linux/openat64_nocancel.c | 29 +++++++++++--
 sysdeps/unix/sysv/linux/prctl.c             | 23 ++++++++++-
 sysdeps/unix/sysv/linux/ptrace.c            | 45 +++++++++++++++------
 12 files changed, 308 insertions(+), 67 deletions(-)

diff --git a/include/fcntl.h b/include/fcntl.h
index be435047bc..9fa0475138 100644
--- a/include/fcntl.h
+++ b/include/fcntl.h
@@ -2,31 +2,43 @@
 #include <io/fcntl.h>
 
 #ifndef _ISOMAC
+
+#ifdef FCNTL_INTERNAL_NAMED_ARGS
+#define MODE mode_t mode
+#define FCNTL_ARG void *arg
+#else
+#define MODE ...
+#define FCNTL_ARG ...
+#endif
+
 /* Now define the internal interfaces.  */
-extern int __open64 (const char *__file, int __oflag, ...);
+extern int __open64 (const char *__file, int __oflag, MODE);
 libc_hidden_proto (__open64)
-extern int __libc_open64 (const char *file, int oflag, ...);
-extern int __libc_open (const char *file, int oflag, ...);
+extern int __libc_open64 (const char *file, int oflag, MODE);
+extern int __libc_open (const char *file, int oflag, MODE);
 libc_hidden_proto (__libc_open)
-extern int __libc_fcntl (int fd, int cmd, ...);
+extern int __libc_fcntl (int fd, int cmd, FCNTL_ARG);
 libc_hidden_proto (__libc_fcntl)
 extern int __fcntl64_nocancel_adjusted (int fd, int cmd, void *arg)
    attribute_hidden;
-extern int __libc_fcntl64 (int fd, int cmd, ...);
+extern int __libc_fcntl64 (int fd, int cmd, FCNTL_ARG);
 libc_hidden_proto (__libc_fcntl64)
-extern int __open (const char *__file, int __oflag, ...);
+extern int __open (const char *__file, int __oflag, MODE);
 libc_hidden_proto (__open)
-extern int __fcntl (int __fd, int __cmd, ...);
+extern int __fcntl (int __fd, int __cmd, FCNTL_ARG);
 libc_hidden_proto (__fcntl)
-extern int __fcntl64 (int __fd, int __cmd, ...) attribute_hidden;
+extern int __fcntl64 (int __fd, int __cmd, FCNTL_ARG) attribute_hidden;
 libc_hidden_proto (__fcntl64)
-extern int __openat (int __fd, const char *__file, int __oflag, ...)
+extern int __openat (int __fd, const char *__file, int __oflag, MODE)
   __nonnull ((2));
 libc_hidden_proto (__openat)
-extern int __openat64 (int __fd, const char *__file, int __oflag, ...)
+extern int __openat64 (int __fd, const char *__file, int __oflag, MODE)
   __nonnull ((2));
 libc_hidden_proto (__openat64)
 
+#undef MODE
+#undef FCNTL_ARG
+
 extern int __open_2 (const char *__path, int __oflag);
 extern int __open64_2 (const char *__path, int __oflag);
 extern int __openat_2 (int __fd, const char *__path, int __oflag);
diff --git a/sysdeps/unix/sysv/linux/fcntl64.c b/sysdeps/unix/sysv/linux/fcntl64.c
index 509cf0a6e2..d896d47ffb 100644
--- a/sysdeps/unix/sysv/linux/fcntl64.c
+++ b/sysdeps/unix/sysv/linux/fcntl64.c
@@ -16,14 +16,23 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
+#include <sysdep-cancel.h>
+
+#ifdef __ASSUME_SYSCALL_NAMED_WORKS
+#define FCNTL_INTERNAL_NAMED_ARGS	1
+#define fcntl64 __no_decl_fcntl64
+#else
+#include <stdarg.h>
+#endif
+
 #define fcntl __no_decl_fcntl
 #define __fcntl __no_decl___fcntl
 #include <fcntl.h>
 #undef fcntl
 #undef __fcntl
-#include <stdarg.h>
+#undef fcntl64
+
 #include <errno.h>
-#include <sysdep-cancel.h>
 
 #ifndef __NR_fcntl64
 # define __NR_fcntl64 __NR_fcntl
@@ -33,6 +42,24 @@
 # define FCNTL_ADJUST_CMD(__cmd) __cmd
 #endif
 
+static inline int
+do_fcntl64 (int fd, int cmd, void *arg)
+{
+  cmd = FCNTL_ADJUST_CMD (cmd);
+
+  if (cmd == F_SETLKW || cmd == F_SETLKW64 || cmd == F_OFD_SETLKW)
+    return SYSCALL_CANCEL (fcntl64, fd, cmd, arg);
+
+  return __fcntl64_nocancel_adjusted (fd, cmd, arg);
+}
+
+#if __ASSUME_SYSCALL_NAMED_WORKS
+int
+__libc_fcntl64 (int fd, int cmd, void *arg)
+{
+  return do_fcntl64 (fd, cmd, arg);
+}
+#else
 int
 __libc_fcntl64 (int fd, int cmd, ...)
 {
@@ -43,13 +70,10 @@ __libc_fcntl64 (int fd, int cmd, ...)
   arg = va_arg (ap, void *);
   va_end (ap);
 
-  cmd = FCNTL_ADJUST_CMD (cmd);
-
-  if (cmd == F_SETLKW || cmd == F_SETLKW64 || cmd == F_OFD_SETLKW)
-    return SYSCALL_CANCEL (fcntl64, fd, cmd, arg);
-
-  return __fcntl64_nocancel_adjusted (fd, cmd, arg);
+  return do_fcntl64 (fd, cmd, arg);
 }
+#endif
+
 libc_hidden_def (__libc_fcntl64)
 weak_alias (__libc_fcntl64, __fcntl64)
 libc_hidden_weak (__fcntl64)
diff --git a/sysdeps/unix/sysv/linux/fcntl_nocancel.c b/sysdeps/unix/sysv/linux/fcntl_nocancel.c
index 005d324fda..b755973ce2 100644
--- a/sysdeps/unix/sysv/linux/fcntl_nocancel.c
+++ b/sysdeps/unix/sysv/linux/fcntl_nocancel.c
@@ -16,10 +16,17 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <fcntl.h>
+#include <sysdep-cancel.h>
+
+#ifdef __ASSUME_SYSCALL_NAMED_WORKS
+#define FCNTL_INTERNAL_NAMED_ARGS	1
+#else
 #include <stdarg.h>
+#endif
+
 #include <errno.h>
-#include <sysdep-cancel.h>
+
+#include <fcntl.h>
 #include <not-cancel.h>
 
 #ifndef __NR_fcntl64
@@ -30,6 +37,19 @@
 # define FCNTL_ADJUST_CMD(__cmd) __cmd
 #endif
 
+static inline int
+do_fcntl64_nocancel (int fd, int cmd, void *arg)
+{
+  return __fcntl64_nocancel_adjusted (fd, FCNTL_ADJUST_CMD (cmd), arg);
+}
+
+#ifdef __ASSUME_SYSCALL_NAMED_WORKS
+int
+__fcntl64_nocancel (int fd, int cmd, void *arg)
+{
+  return do_fcntl64_nocancel (fd, cmd, arg);
+}
+#else
 int
 __fcntl64_nocancel (int fd, int cmd, ...)
 {
@@ -40,10 +60,10 @@ __fcntl64_nocancel (int fd, int cmd, ...)
   arg = va_arg (ap, void *);
   va_end (ap);
 
-  cmd = FCNTL_ADJUST_CMD (cmd);
-
-  return __fcntl64_nocancel_adjusted (fd, cmd, arg);
+  return do_fcntl64_nocancel (fd, cmd, arg);
 }
+#endif
+
 hidden_def (__fcntl64_nocancel)
 
 int
diff --git a/sysdeps/unix/sysv/linux/ioctl.c b/sysdeps/unix/sysv/linux/ioctl.c
index e059942801..aca5ad5daa 100644
--- a/sysdeps/unix/sysv/linux/ioctl.c
+++ b/sysdeps/unix/sysv/linux/ioctl.c
@@ -16,19 +16,18 @@
    License along with the GNU C Library.  If not, see
    <https://www.gnu.org/licenses/>.  */
 
+#include <sysdep.h>
+
+#ifndef __ASSUME_SYSCALL_NAMED_WORKS
 #include <stdarg.h>
 #include <sys/ioctl.h>
-#include <sysdep.h>
+#endif
+
 #include <internal-ioctl.h>
 
-int
-__ioctl (int fd, unsigned long int request, ...)
+static inline int
+do_ioctl (int fd, unsigned long int request, void *arg)
 {
-  va_list args;
-  va_start (args, request);
-  void *arg = va_arg (args, void *);
-  va_end (args);
-
   int r;
   if (!__ioctl_arch (&r, fd, request, arg))
     {
@@ -41,6 +40,29 @@ __ioctl (int fd, unsigned long int request, ...)
     }
   return r;
 }
+
+#ifdef __ASSUME_SYSCALL_NAMED_WORKS
+__typeof (do_ioctl) __ioctl;
+libc_hidden_proto(__ioctl)
+
+int
+__ioctl (int fd, unsigned long int request, void *arg)
+{
+  return do_ioctl (fd, request, arg);
+}
+#else
+int
+__ioctl (int fd, unsigned long int request, ...)
+{
+  va_list args;
+  va_start (args, request);
+  void *arg = va_arg (args, void *);
+  va_end (args);
+
+  return do_ioctl (fd, request, arg);
+}
+#endif
+
 libc_hidden_def (__ioctl)
 weak_alias (__ioctl, ioctl)
 
diff --git a/sysdeps/unix/sysv/linux/mremap.c b/sysdeps/unix/sysv/linux/mremap.c
index 0ad5da86a2..26db22ffa5 100644
--- a/sysdeps/unix/sysv/linux/mremap.c
+++ b/sysdeps/unix/sysv/linux/mremap.c
@@ -16,11 +16,40 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <sys/mman.h>
 #include <sysdep.h>
+
+#ifdef __ASSUME_SYSCALL_NAMED_WORKS
+#define mremap __no_decl_mremap
+#define __mremap __no_decl___mremap
+#else
 #include <stdarg.h>
+#endif
+
+#include <sys/mman.h>
+
+#undef mremap
+#undef __mremap
+
 #include <stddef.h>
 
+static inline void *
+do_mremap (void *addr, size_t old_len, size_t new_len, int flags,
+	   void *new_addr)
+{
+  return (void *) INLINE_SYSCALL_CALL (mremap, addr, old_len, new_len, flags,
+				       new_addr);
+}
+
+#ifdef __ASSUME_SYSCALL_NAMED_WORKS
+__typeof (do_mremap) __mremap;
+libc_hidden_proto (__mremap)
+
+void *
+__mremap (void *addr, size_t old_len, size_t new_len, int flags, void *new_addr)
+{
+  return do_mremap (addr, old_len, new_len, flags, new_addr);
+}
+#else
 void *
 __mremap (void *addr, size_t old_len, size_t new_len, int flags, ...)
 {
@@ -34,8 +63,9 @@ __mremap (void *addr, size_t old_len, size_t new_len, int flags, ...)
       va_end (va);
     }
 
-  return (void *) INLINE_SYSCALL_CALL (mremap, addr, old_len, new_len, flags,
-				       new_addr);
+  return do_mremap (addr, old_len, new_len, flags, new_addr);
 }
+#endif
+
 libc_hidden_def (__mremap)
 weak_alias (__mremap, mremap)
diff --git a/sysdeps/unix/sysv/linux/not-cancel.h b/sysdeps/unix/sysv/linux/not-cancel.h
index ea2615b38a..1a95e1b008 100644
--- a/sysdeps/unix/sysv/linux/not-cancel.h
+++ b/sysdeps/unix/sysv/linux/not-cancel.h
@@ -29,16 +29,16 @@
 #include <time.h>
 
 /* Non cancellable open syscall.  */
-__typeof (open) __open_nocancel;
+__typeof (__open) __open_nocancel;
 
 /* Non cancellable open syscall (LFS version).  */
-__typeof (open64) __open64_nocancel;
+__typeof (__open64) __open64_nocancel;
 
 /* Non cancellable openat syscall.  */
-__typeof (openat) __openat_nocancel;
+__typeof (__openat) __openat_nocancel;
 
 /* Non cacellable openat syscall (LFS version).  */
-__typeof (openat64) __openat64_nocancel;
+__typeof (__openat64) __openat64_nocancel;
 
 /* Non cancellable read syscall.  */
 __typeof (__read) __read_nocancel;
diff --git a/sysdeps/unix/sysv/linux/open64.c b/sysdeps/unix/sysv/linux/open64.c
index 9f33733ebc..af3fdb09cd 100644
--- a/sysdeps/unix/sysv/linux/open64.c
+++ b/sysdeps/unix/sysv/linux/open64.c
@@ -18,17 +18,42 @@
 
 #include <sys/types.h>
 #include <sys/stat.h>
-#include <fcntl.h>
-#include <stdarg.h>
 #include <sysdep-cancel.h>
 #include <shlib-compat.h>
 
+#ifdef __ASSUME_SYSCALL_NAMED_WORKS
+#define FCNTL_INTERNAL_NAMED_ARGS	1
+#define open64 __no_decl_open64
+#define open __no_decl_open
+#else
+#include <stdarg.h>
+#endif
+
+#include <fcntl.h>
+
+#undef open64
+#undef open
+
+static inline int
+do_open64 (const char *file, int oflag, mode_t mode)
+{
+  return SYSCALL_CANCEL (openat, AT_FDCWD, file, oflag | O_LARGEFILE,
+			 mode);
+}
+
 /* Open FILE with access OFLAG.  If O_CREAT or O_TMPFILE is in OFLAG,
    a third argument is the file protection.  */
+#ifdef __ASSUME_SYSCALL_NAMED_WORKS
+int
+__libc_open64 (const char *file, int oflag, mode_t mode)
+{
+  return do_open64 (file, oflag, mode);
+}
+#else
 int
 __libc_open64 (const char *file, int oflag, ...)
 {
-  int mode = 0;
+  mode_t mode = 0;
 
   if (__OPEN_NEEDS_MODE (oflag))
     {
@@ -38,9 +63,9 @@ __libc_open64 (const char *file, int oflag, ...)
       va_end (arg);
     }
 
-  return SYSCALL_CANCEL (openat, AT_FDCWD, file, oflag | O_LARGEFILE,
-			 mode);
+  return do_open64 (file, oflag, mode);
 }
+#endif
 
 strong_alias (__libc_open64, __open64)
 libc_hidden_weak (__open64)
diff --git a/sysdeps/unix/sysv/linux/open64_nocancel.c b/sysdeps/unix/sysv/linux/open64_nocancel.c
index 5035521386..f64c308d40 100644
--- a/sysdeps/unix/sysv/linux/open64_nocancel.c
+++ b/sysdeps/unix/sysv/linux/open64_nocancel.c
@@ -16,17 +16,38 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
+#include <sysdep.h>
+
+#ifdef __ASSUME_SYSCALL_NAMED_WORKS
+#define FCNTL_INTERNAL_NAMED_ARGS	1
+#else
+#include <stdarg.h>
+#endif
+
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
-#include <stdarg.h>
 
 #include <not-cancel.h>
 
+static inline int
+do_open64_nocancel (const char *file, int oflag, mode_t mode)
+{
+  return INLINE_SYSCALL_CALL (openat, AT_FDCWD, file, oflag | O_LARGEFILE,
+			      mode);
+}
+
+#ifdef __ASSUME_SYSCALL_NAMED_WORKS
+int
+__open64_nocancel (const char *file, int oflag, mode_t mode)
+{
+  return do_open64_nocancel (file, oflag, mode);
+}
+#else
 int
 __open64_nocancel (const char *file, int oflag, ...)
 {
-  int mode = 0;
+  mode_t mode = 0;
 
   if (__OPEN_NEEDS_MODE (oflag))
     {
@@ -36,9 +57,9 @@ __open64_nocancel (const char *file, int oflag, ...)
       va_end (arg);
     }
 
-  return INLINE_SYSCALL_CALL (openat, AT_FDCWD, file, oflag | O_LARGEFILE,
-			      mode);
+  return do_open64_nocancel (file, oflag, mode);
 }
+#endif
 
 hidden_def (__open64_nocancel)
 
diff --git a/sysdeps/unix/sysv/linux/openat64.c b/sysdeps/unix/sysv/linux/openat64.c
index 105eb8ab08..d5718eb261 100644
--- a/sysdeps/unix/sysv/linux/openat64.c
+++ b/sysdeps/unix/sysv/linux/openat64.c
@@ -16,14 +16,37 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <fcntl.h>
+#include <sysdep-cancel.h>
+
+#ifdef __ASSUME_SYSCALL_NAMED_WORKS
+#define FCNTL_INTERNAL_NAMED_ARGS	1
+#define openat64 __no_decl_openat64
+#define openat __no_decl_openat
+#else
 #include <stdarg.h>
+#endif
 
-#include <sysdep-cancel.h>
+#include <fcntl.h>
+
+#undef openat64
+#undef openat
+
+static inline int
+do_openat64 (int fd, const char *file, int oflag, mode_t mode)
+{
+  return SYSCALL_CANCEL (openat, fd, file, oflag | O_LARGEFILE, mode);
+}
 
 /* Open FILE with access OFLAG.  Interpret relative paths relative to
    the directory associated with FD.  If OFLAG includes O_CREAT or
    O_TMPFILE, a fourth argument is the file protection.  */
+#ifdef __ASSUME_SYSCALL_NAMED_WORKS
+int
+__libc_openat64 (int fd, const char *file, int oflag, mode_t mode)
+{
+  return do_openat64 (fd, file, oflag, mode);
+}
+#else
 int
 __libc_openat64 (int fd, const char *file, int oflag, ...)
 {
@@ -36,8 +59,9 @@ __libc_openat64 (int fd, const char *file, int oflag, ...)
       va_end (arg);
     }
 
-  return SYSCALL_CANCEL (openat, fd, file, oflag | O_LARGEFILE, mode);
+  return do_openat64 (fd, file, oflag, mode);
 }
+#endif
 
 strong_alias (__libc_openat64, __openat64)
 libc_hidden_weak (__openat64)
diff --git a/sysdeps/unix/sysv/linux/openat64_nocancel.c b/sysdeps/unix/sysv/linux/openat64_nocancel.c
index b9105805d2..55ef645179 100644
--- a/sysdeps/unix/sysv/linux/openat64_nocancel.c
+++ b/sysdeps/unix/sysv/linux/openat64_nocancel.c
@@ -16,12 +16,32 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <fcntl.h>
+#include <sysdep-cancel.h>
+
+#ifdef __ASSUME_SYSCALL_NAMED_WORKS
+#define FCNTL_INTERNAL_NAMED_ARGS	1
+#else
 #include <stdarg.h>
+#endif
+
+#include <fcntl.h>
 
-#include <sysdep-cancel.h>
 #include <not-cancel.h>
 
+static inline int
+do_openat64_nocancel (int fd, const char *file, int oflag, mode_t mode)
+{
+  return INLINE_SYSCALL_CALL (openat, fd, file, oflag | O_LARGEFILE,
+			      mode);
+}
+
+#ifdef __ASSUME_SYSCALL_NAMED_WORKS
+int
+__openat64_nocancel (int fd, const char *file, int oflag, mode_t mode)
+{
+  return do_openat64_nocancel (fd, file, oflag, mode);
+}
+#else
 int
 __openat64_nocancel (int fd, const char *file, int oflag, ...)
 {
@@ -34,9 +54,10 @@ __openat64_nocancel (int fd, const char *file, int oflag, ...)
       va_end (arg);
     }
 
-  return INLINE_SYSCALL_CALL (openat, fd, file, oflag | O_LARGEFILE,
-			      mode);
+  return do_openat64_nocancel (fd, file, oflag, mode);
 }
+#endif
+
 hidden_def (__openat64_nocancel)
 
 #ifdef __OFF_T_MATCHES_OFF64_T
diff --git a/sysdeps/unix/sysv/linux/prctl.c b/sysdeps/unix/sysv/linux/prctl.c
index 68b97a4388..2c6e889f18 100644
--- a/sysdeps/unix/sysv/linux/prctl.c
+++ b/sysdeps/unix/sysv/linux/prctl.c
@@ -17,14 +17,34 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <sysdep.h>
+
+#ifndef __ASSUME_SYSCALL_NAMED_WORKS
 #include <stdarg.h>
 #include <sys/prctl.h>
+#endif
+
+static inline int
+do_prctl (int option, unsigned long int arg2, unsigned long int arg3,
+	  unsigned long int arg4, unsigned long int arg5)
+{
+  return INLINE_SYSCALL_CALL (prctl, option, arg2, arg3, arg4, arg5);
+}
 
 /* Unconditionally read all potential arguments.  This may pass
    garbage values to the kernel, but avoids the need for teaching
    glibc the argument counts of individual options (including ones
    that are added to the kernel in the future).  */
+#ifdef __ASSUME_SYSCALL_NAMED_WORKS
+__typeof (do_prctl) __prctl;
+libc_hidden_proto (__prctl)
 
+int
+__prctl (int option, unsigned long int arg2, unsigned long int arg3,
+	 unsigned long int arg4, unsigned long int arg5)
+{
+  return do_prctl (option, arg2, arg3, arg4, arg5);
+}
+#else
 int
 __prctl (int option, ...)
 {
@@ -35,8 +55,9 @@ __prctl (int option, ...)
   unsigned long int arg4 = va_arg (arg, unsigned long int);
   unsigned long int arg5 = va_arg (arg, unsigned long int);
   va_end (arg);
-  return INLINE_SYSCALL_CALL (prctl, option, arg2, arg3, arg4, arg5);
+  return do_prctl (option, arg2, arg3, arg4, arg5);
 }
+#endif
 
 libc_hidden_def (__prctl)
 weak_alias (__prctl, prctl)
diff --git a/sysdeps/unix/sysv/linux/ptrace.c b/sysdeps/unix/sysv/linux/ptrace.c
index 239344fe7a..e1a1282505 100644
--- a/sysdeps/unix/sysv/linux/ptrace.c
+++ b/sysdeps/unix/sysv/linux/ptrace.c
@@ -17,7 +17,6 @@
 
 #include <errno.h>
 #include <sys/types.h>
-#include <sys/ptrace.h>
 #include <sys/user.h>
 #include <stdarg.h>
 #include <signal.h>
@@ -25,19 +24,17 @@
 #include <sysdep.h>
 #include <sys/syscall.h>
 
-long int
-ptrace (enum __ptrace_request request, ...)
+#ifdef __ASSUME_SYSCALL_NAMED_WORKS
+#define ptrace __no_decl_ptrace
+#endif
+
+#include <sys/ptrace.h>
+#undef ptrace
+
+static inline long int
+do_ptrace (enum __ptrace_request request, pid_t pid, void *addr, void *data)
 {
   long int res, ret;
-  va_list ap;
-  pid_t pid;
-  void *addr, *data;
-
-  va_start (ap, request);
-  pid = va_arg (ap, pid_t);
-  addr = va_arg (ap, void *);
-  data = va_arg (ap, void *);
-  va_end (ap);
 
   if (request > 0 && request < 4)
     data = &ret;
@@ -51,3 +48,27 @@ ptrace (enum __ptrace_request request, ...)
 
   return res;
 }
+
+#ifdef __ASSUME_SYSCALL_NAMED_WORKS
+long int
+ptrace (enum __ptrace_request request, pid_t pid, void *addr, void *data)
+{
+  return do_ptrace (request, pid, addr, data);
+}
+#else
+long int
+ptrace (enum __ptrace_request request, ...)
+{
+  va_list ap;
+  pid_t pid;
+  void *addr, *data;
+
+  va_start (ap, request);
+  pid = va_arg (ap, pid_t);
+  addr = va_arg (ap, void *);
+  data = va_arg (ap, void *);
+  va_end (ap);
+
+  return do_ptrace (request, pid, addr, data);
+}
+#endif
-- 
2.40.0

[PATCH v2 3/5] LoongArch: Define __ASSUME_SYSCALL_NAMED_WORKS for Linux

[Xi Ruoyao]

LoongArch calling convention treats the non-floating-point variable
arguments same as named ones, and when each argument is an integer
not wider than long or a pointer, the ith argument is in register $ai
(0 <= i < 8) or the stack slot at ($sp + sizeof(long) * (i - 8))
(i >= 8) no matter how many arguments are passed. So we can define
__ASSUME_SYSCALL_NAMED_WORKS to avoid unnecessary stack stores in the
syscall wrappers caused by va_start.
---
 .../sysv/linux/loongarch/kernel-features.h    | 29 +++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 sysdeps/unix/sysv/linux/loongarch/kernel-features.h

diff --git a/sysdeps/unix/sysv/linux/loongarch/kernel-features.h b/sysdeps/unix/sysv/linux/loongarch/kernel-features.h
new file mode 100644
index 0000000000..f862d0623f
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/loongarch/kernel-features.h
@@ -0,0 +1,29 @@
+/* Set flags signalling availability of kernel features based on given
+   kernel version number.
+   Copyright (C) 2023 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library.  If not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include_next <kernel-features.h>
+
+/* Define this if the calling convention for passing x named arguments and y
+   variable arguments is same as passing (x + y) named arguments, while each
+   argument is either an integer of which the width is less than or equal to
+   "long", or a pointer; and an argument can be fetched from the same register
+   or the same offset from the stack pointer no matter how many (maybe zero)
+   arguments are passed after it.  It avoids useless stack stores caused by
+   usage of va_start.  */
+#define __ASSUME_SYSCALL_NAMED_WORKS	1
-- 
2.40.0

[PATCH v2 4/5] x86_64: Define __ASSUME_SYSCALL_NAMED_WORKS for Linux

[Xi Ruoyao]

x86_64 calling convention treats the variable arguments same as named
ones (it sets %al to the number of floating-point variable arguments but
it's simply ignored because our syscall wrappers don't take
floating-point arguments), and when each argument is an integer
not wider than 8 bytes, the ith argument is in registers %rdi, %rsi,
%rdx, %rcx, %r8, and %r9 (0 <= i < 6), or the stack slot at
(%rsp + 8 * (i - 6)) (i >= 8) no matter how many arguments are passed.
So we can define __ASSUME_SYSCALL_NAMED_WORKS to avoid unnecessary stack
stores in the syscall wrappers caused by va_start.
---
 sysdeps/unix/sysv/linux/x86_64/kernel-features.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/sysdeps/unix/sysv/linux/x86_64/kernel-features.h b/sysdeps/unix/sysv/linux/x86_64/kernel-features.h
index 68322ff476..7783a0eebc 100644
--- a/sysdeps/unix/sysv/linux/x86_64/kernel-features.h
+++ b/sysdeps/unix/sysv/linux/x86_64/kernel-features.h
@@ -23,4 +23,13 @@
 # define __ASSUME_WORDSIZE64_ILP32	1
 #endif
 
+/* Define this if the calling convention for passing x named arguments and y
+   variable arguments is same as passing (x + y) named arguments, while each
+   argument is either an integer of which the width is less than or equal to
+   "long", or a pointer; and an argument can be fetched from the same register
+   or the same offset from the stack pointer no matter how many (maybe zero)
+   arguments are passed after it.  It avoids useless stack stores caused by
+   usage of va_start.  */
+#define __ASSUME_SYSCALL_NAMED_WORKS	1
+
 #include_next <kernel-features.h>
-- 
2.40.0

[PATCH v2 5/5] aarch64: Define __ASSUME_SYSCALL_NAMED_WORKS for Linux

[Xi Ruoyao]

AAPCS treats the variable arguments same as named ones, and when each
argument is an integer not wider than 8 bytes, the ith argument is in
register xi (0 <= i < 8), or the stack slot at (sp + 8 * (i - 8))
(i >= 8) no matter how many arguments are passed.  So we can define
__ASSUME_SYSCALL_NAMED_WORKS to avoid unnecessary stack stores in the
syscall wrappers caused by va_start.
---
 sysdeps/unix/sysv/linux/aarch64/kernel-features.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/sysdeps/unix/sysv/linux/aarch64/kernel-features.h b/sysdeps/unix/sysv/linux/aarch64/kernel-features.h
index 3546f6de96..c39ff39da2 100644
--- a/sysdeps/unix/sysv/linux/aarch64/kernel-features.h
+++ b/sysdeps/unix/sysv/linux/aarch64/kernel-features.h
@@ -21,3 +21,12 @@
 
 #undef __ASSUME_CLONE_DEFAULT
 #define __ASSUME_CLONE_BACKWARDS 1
+
+/* Define this if the calling convention for passing x named arguments and y
+   variable arguments is same as passing (x + y) named arguments, while each
+   argument is either an integer of which the width is less than or equal to
+   "long", or a pointer; and an argument can be fetched from the same register
+   or the same offset from the stack pointer no matter how many (maybe zero)
+   arguments are passed after it.  It avoids useless stack stores caused by
+   usage of va_start.  */
+#define __ASSUME_SYSCALL_NAMED_WORKS	1
-- 
2.40.0

Re: [PATCH v2 5/5] aarch64: Define __ASSUME_SYSCALL_NAMED_WORKS for Linux

[Adhemerval Zanella Netto]

On 25/03/23 11:08, Xi Ruoyao wrote:
> AAPCS treats the variable arguments same as named ones, and when each
> argument is an integer not wider than 8 bytes, the ith argument is in
> register xi (0 <= i < 8), or the stack slot at (sp + 8 * (i - 8))
> (i >= 8) no matter how many arguments are passed.  So we can define
> __ASSUME_SYSCALL_NAMED_WORKS to avoid unnecessary stack stores in the
> syscall wrappers caused by va_start.

This triggered a build failure on i686 patchwork buildbot [1].

[1] https://www.delorie.com/trybots/32bit/18431/

> ---
>  sysdeps/unix/sysv/linux/aarch64/kernel-features.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/sysdeps/unix/sysv/linux/aarch64/kernel-features.h b/sysdeps/unix/sysv/linux/aarch64/kernel-features.h
> index 3546f6de96..c39ff39da2 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/kernel-features.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/kernel-features.h
> @@ -21,3 +21,12 @@
>  
>  #undef __ASSUME_CLONE_DEFAULT
>  #define __ASSUME_CLONE_BACKWARDS 1
> +
> +/* Define this if the calling convention for passing x named arguments and y
> +   variable arguments is same as passing (x + y) named arguments, while each
> +   argument is either an integer of which the width is less than or equal to
> +   "long", or a pointer; and an argument can be fetched from the same register
> +   or the same offset from the stack pointer no matter how many (maybe zero)
> +   arguments are passed after it.  It avoids useless stack stores caused by
> +   usage of va_start.  */
> +#define __ASSUME_SYSCALL_NAMED_WORKS	1

Re: [PATCH v2 5/5] aarch64: Define __ASSUME_SYSCALL_NAMED_WORKS for Linux

[Xi Ruoyao]

On Mon, 2023-03-27 at 10:06 -0300, Adhemerval Zanella Netto wrote:
> 
> 
> On 25/03/23 11:08, Xi Ruoyao wrote:
> > AAPCS treats the variable arguments same as named ones, and when
> > each
> > argument is an integer not wider than 8 bytes, the ith argument is
> > in
> > register xi (0 <= i < 8), or the stack slot at (sp + 8 * (i - 8))
> > (i >= 8) no matter how many arguments are passed.  So we can define
> > __ASSUME_SYSCALL_NAMED_WORKS to avoid unnecessary stack stores in
> > the
> > syscall wrappers caused by va_start.
> 
> This triggered a build failure on i686 patchwork buildbot [1].

Ouch, misused #if as #ifdef.

Will fix and submit v3.

> 
> [1] https://www.delorie.com/trybots/32bit/18431/
> 
> > ---
> >  sysdeps/unix/sysv/linux/aarch64/kernel-features.h | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/sysdeps/unix/sysv/linux/aarch64/kernel-features.h
> > b/sysdeps/unix/sysv/linux/aarch64/kernel-features.h
> > index 3546f6de96..c39ff39da2 100644
> > --- a/sysdeps/unix/sysv/linux/aarch64/kernel-features.h
> > +++ b/sysdeps/unix/sysv/linux/aarch64/kernel-features.h
> > @@ -21,3 +21,12 @@
> >  
> >  #undef __ASSUME_CLONE_DEFAULT
> >  #define __ASSUME_CLONE_BACKWARDS 1
> > +
> > +/* Define this if the calling convention for passing x named
> > arguments and y
> > +   variable arguments is same as passing (x + y) named arguments,
> > while each
> > +   argument is either an integer of which the width is less than or
> > equal to
> > +   "long", or a pointer; and an argument can be fetched from the
> > same register
> > +   or the same offset from the stack pointer no matter how many
> > (maybe zero)
> > +   arguments are passed after it.  It avoids useless stack stores
> > caused by
> > +   usage of va_start.  */
> > +#define __ASSUME_SYSCALL_NAMED_WORKS   1

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH v2 0/5] linux: Avoid va_list for generic syscall wrappers if possible

[Carlos O'Donell]

On 3/25/23 10:08, Xi Ruoyao via Libc-alpha wrote:
> Currently GCC generates highly sub-optimal code on architectures where
> the calling convention prefers registers for arugment passing.  This is
> GCC PR100955.  While it's technically a missed-optimization in GCC, it
> seems not trivial to fix (I've not seen any compiler which can optimize
> this properly yet).

I'm glad to see we have a gcc PR open for this.

We should be working to improve the compiler instead of working around the issue in
glibc.

> As the generic Linux syscall wrappers often uses a fixed number of
> arguments, and "overreading" the variable arguments is usually safe
> (fcntl etc. is already doing this), we can pretend these wrappers were
> declared with named arguments and make the compiler do right thing.



> Add a macro __ASSUME_SYSCALL_NAMED_WORKS to control this, which should
> be defined if we can safely replace "..." with several named arguments.
> Use an internal function prototype with named arguments if it's defined,
> for fcntl64, fcntl_nocancel, ioctl, mremap, open64, open64_nocancel,
> openat64, openat64_nocancel, prctl, ptrace, and generic syscall()
> wrapper.  I've not changed open* without "64" because I don't have a
> test platform.  shm_ctl is also not changed because it contains
> aggregate variable arugment which is more tricky than integers or
> pointers.


 
> Define this macro for LoongArch, x86-64, and AArch64.  This should be
> also suitable for some other architectures (I think it will be fine on
> RISC-V) but again I don't have a test platform.
> 
> This is the first time I make such a large change in Glibc so it's
> likely I've done something wrong.  Please correct me :).

There are few things that I see are wrong, and I'll list them out here at the top-level:

(1) Error prone macro usage.

#ifdef foo
/* Some Stuff.  */
#else
/* Other stuff. */
#endif

(a) Always define foo, either 0 or 1.
(b) Always use #if foo / #else

(2) Public declarations must match internal declarations.

The biggest problem I see here is that the public declaration of the functions
you are changing are all variadic, and so my strong opinion is that we should not
change the internal definition of these functions to be non-variadic.

I expect there are going to be knock-on effects with developer tooling that
expects the implementation not to over-read e.g. valgrind looking at reading
of registers that have undefined values.

---

In summary, I think this is a compiler problem, and that working around this in glibc
is going to result in:

- Odd corner case ABI issues between public declarations of variadic functions and
  internal non-variadic definitions.

- Poorer testing of #else code that uses variadic arguments, as the public interface
  requires.

I don't support going in this direction.

Is there an alternative that could generate better code that doesn't go this way?

> Xi Ruoyao (5):
>   linux: Add __ASSUME_SYSCALL_NAMED_WORKS to allow avoiding va_list for
>     generic syscall
>   linux: [__ASSUME_SYSCALL_NAMED_WORKS] Avoid using va_list for various
>     syscall wrappers
>   LoongArch: Define __ASSUME_SYSCALL_NAMED_WORKS for Linux
>   x86_64: Define __ASSUME_SYSCALL_NAMED_WORKS for Linux
>   aarch64: Define __ASSUME_SYSCALL_NAMED_WORKS for Linux
> 
>  include/fcntl.h                               | 32 ++++++++-----
>  .../unix/sysv/linux/aarch64/kernel-features.h |  9 ++++
>  sysdeps/unix/sysv/linux/fcntl64.c             | 40 +++++++++++++----
>  sysdeps/unix/sysv/linux/fcntl_nocancel.c      | 30 ++++++++++---
>  sysdeps/unix/sysv/linux/ioctl.c               | 38 ++++++++++++----
>  .../sysv/linux/loongarch/kernel-features.h    | 29 ++++++++++++
>  sysdeps/unix/sysv/linux/mremap.c              | 36 +++++++++++++--
>  sysdeps/unix/sysv/linux/not-cancel.h          |  8 ++--
>  sysdeps/unix/sysv/linux/open64.c              | 35 ++++++++++++---
>  sysdeps/unix/sysv/linux/open64_nocancel.c     | 29 ++++++++++--
>  sysdeps/unix/sysv/linux/openat64.c            | 30 +++++++++++--
>  sysdeps/unix/sysv/linux/openat64_nocancel.c   | 29 ++++++++++--
>  sysdeps/unix/sysv/linux/prctl.c               | 23 +++++++++-
>  sysdeps/unix/sysv/linux/ptrace.c              | 45 ++++++++++++++-----
>  sysdeps/unix/sysv/linux/syscall.c             | 35 +++++++++++----
>  .../unix/sysv/linux/x86_64/kernel-features.h  |  9 ++++
>  16 files changed, 382 insertions(+), 75 deletions(-)
>  create mode 100644 sysdeps/unix/sysv/linux/loongarch/kernel-features.h
> 

-- 
Cheers,
Carlos.

Re: [PATCH v2 0/5] linux: Avoid va_list for generic syscall wrappers if possible

[Xi Ruoyao]

On Mon, 2023-03-27 at 10:04 -0400, Carlos O'Donell wrote:

> In summary, I think this is a compiler problem

Definitely true.

> and that working around this in glibc
> is going to result in:
> 
> - Odd corner case ABI issues between public declarations of variadic functions and
>   internal non-variadic definitions.
> 
> - Poorer testing of #else code that uses variadic arguments, as the public interface
>   requires.
> 
> I don't support going in this direction.

Valid reasons.  Abandon this series then.

But I hope these could be raised earlier (in the discussion about
LoongArch syscall.S) so I wouldn't write all the code :).

> Is there an alternative that could generate better code that doesn't go this way?

For LoongArch I can improve GCC to save only the GARs containing the
arguments really used in va_arg (i.e. one GAR for things like open() or
fcntl() instead of all 8 GARs), but I guess the patch will be delayed
into GCC 14.

Generally I've not got an idea about how to make GCC avoid saving GARs
unnecessarily with va_arg.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH v2 0/5] linux: Avoid va_list for generic syscall wrappers if possible

[Xi Ruoyao]

On Mon, 2023-03-27 at 10:04 -0400, Carlos O'Donell wrote:

> In summary, I think this is a compiler problem

Definitely true.

> and that working around this in glibc
> is going to result in:
> 
> - Odd corner case ABI issues between public declarations of variadic
> functions and
>   internal non-variadic definitions.
> 
> - Poorer testing of #else code that uses variadic arguments, as the
> public interface
>   requires.
> 
> I don't support going in this direction.

Valid reasons.  Abandon this series then.

But I hope these could be raised earlier (in the discussion about
LoongArch syscall.S) so I wouldn't write all the code :).

> Is there an alternative that could generate better code that doesn't
> go this way?

For LoongArch I can improve GCC to save only the GARs containing the
arguments really used in va_arg (i.e. one GAR for things like open() or
fcntl() instead of all 8 GARs), but I guess the patch will be delayed
into GCC 14.

Generally I've not got an idea about how to make GCC avoid saving GARs
unnecessarily with va_arg.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH v2 0/5] linux: Avoid va_list for generic syscall wrappers if possible

[Xi Ruoyao]

Sorry, mail duplicated because of some network issue.

On Mon, 2023-03-27 at 22:45 +0800, Xi Ruoyao wrote:
> On Mon, 2023-03-27 at 10:04 -0400, Carlos O'Donell wrote:
> 
> > In summary, I think this is a compiler problem
> 
> Definitely true.

/* snip */

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH v2 0/5] linux: Avoid va_list for generic syscall wrappers if possible

[caiyinyu]

在 2023/3/27 下午10:45, Xi Ruoyao 写道:
> On Mon, 2023-03-27 at 10:04 -0400, Carlos O'Donell wrote:
>
>> In summary, I think this is a compiler problem
> Definitely true.
>
>> and that working around this in glibc
>> is going to result in:
>>
>> - Odd corner case ABI issues between public declarations of variadic
>> functions and
>>    internal non-variadic definitions.
>>
>> - Poorer testing of #else code that uses variadic arguments, as the
>> public interface
>>    requires.
>>
>> I don't support going in this direction.
> Valid reasons.  Abandon this series then.
>
> But I hope these could be raised earlier (in the discussion about
> LoongArch syscall.S) so I wouldn't write all the code :).
>
>> Is there an alternative that could generate better code that doesn't
>> go this way?
> For LoongArch I can improve GCC to save only the GARs containing the
> arguments really used in va_arg (i.e. one GAR for things like open() or
> fcntl() instead of all 8 GARs), but I guess the patch will be delayed
> into GCC 14.
>
> Generally I've not got an idea about how to make GCC avoid saving GARs
> unnecessarily with va_arg.

So I believe that the assembly implementation of syscalls is still 
necessary, especially for users who are using GCC<=13.

patch:

https://sourceware.org/pipermail/libc-alpha/2023-March/146588.html

>

Re: [PATCH v2 0/5] linux: Avoid va_list for generic syscall wrappers if possible

[Xi Ruoyao]

On Tue, 2023-04-04 at 09:25 +0800, caiyinyu wrote:
> 
> 在 2023/3/27 下午10:45, Xi Ruoyao 写道:
> > On Mon, 2023-03-27 at 10:04 -0400, Carlos O'Donell wrote:
> > 
> > > In summary, I think this is a compiler problem
> > Definitely true.
> > 
> > > and that working around this in glibc
> > > is going to result in:
> > > 
> > > - Odd corner case ABI issues between public declarations of variadic
> > > functions and
> > >    internal non-variadic definitions.
> > > 
> > > - Poorer testing of #else code that uses variadic arguments, as the
> > > public interface
> > >    requires.
> > > 
> > > I don't support going in this direction.
> > Valid reasons.  Abandon this series then.
> > 
> > But I hope these could be raised earlier (in the discussion about
> > LoongArch syscall.S) so I wouldn't write all the code :).
> > 
> > > Is there an alternative that could generate better code that doesn't
> > > go this way?
> > For LoongArch I can improve GCC to save only the GARs containing the
> > arguments really used in va_arg (i.e. one GAR for things like open() or
> > fcntl() instead of all 8 GARs), but I guess the patch will be delayed
> > into GCC 14.
> > 
> > Generally I've not got an idea about how to make GCC avoid saving GARs
> > unnecessarily with va_arg.
> 
> So I believe that the assembly implementation of syscalls is still 
> necessary, especially for users who are using GCC<=13.

Maybe we can use a custom C implementation (like RISC-V) as well.  But
strictly speaking the RISC-V syscall.c is invoking undefined behavior
(like my proposal) so I agree with assembly here.

> patch:
> 
> https://sourceware.org/pipermail/libc-alpha/2023-March/146588.html
> 

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

[PATCH v2 0/5] linux: Avoid va_list for generic syscall wrappers if possible

[Wilco Dijkstra]

Hi,

> So I believe that the assembly implementation of syscalls is still 
> necessary, especially for users who are using GCC<=13.

As far as I can tell, all internal syscalls are inlined so they never use
syscall() and thus don't have this issue. Does that not happen on
LoongArch? If not, you might need something to enable the inlining.

It's not clear to me how often syscall() is used by user code instead of
direct calls to read() etc. So is it really necessary to consider optimizing
syscall()? Are there microbenchmarks that use it?

Also does sysdeps/unix/sysv/linux/riscv/syscall.c work for you?

So I think we can avoid unnecessary assembly code.

Cheers,
Wilco

Re: [PATCH v2 0/5] linux: Avoid va_list for generic syscall wrappers if possible

[Wilco Dijkstra]

Hi,

> Maybe we can use a custom C implementation (like RISC-V) as well.  But
> strictly speaking the RISC-V syscall.c is invoking undefined behavior
> (like my proposal) so I agree with assembly here.

How does assembly code not invoke the exact same undefined behaviour?
In both cases you rely on variable arguments being passed in the same
way as integer args - this is true in many ABIs. The goal is to avoid introducing
unnecessary assembly and use generic implementations where possible.

So if that C version works as well as assembler on LoongArch and assuming
that bare syscall() performance is essential, then why not use it?

Cheers,
Wilco

Re: [PATCH v2 0/5] linux: Avoid va_list for generic syscall wrappers if possible

[Adhemerval Zanella Netto]

On 05/04/23 10:19, Wilco Dijkstra via Libc-alpha wrote:
> Hi,
> 
>> Maybe we can use a custom C implementation (like RISC-V) as well.  But
>> strictly speaking the RISC-V syscall.c is invoking undefined behavior
>> (like my proposal) so I agree with assembly here.
> 
> How does assembly code not invoke the exact same undefined behaviour?
> In both cases you rely on variable arguments being passed in the same
> way as integer args - this is true in many ABIs. The goal is to avoid introducing
> unnecessary assembly and use generic implementations where possible.
> 
> So if that C version works as well as assembler on LoongArch and assuming
> that bare syscall() performance is essential, then why not use it?

After this discussion, it seems that RISCV implementation is essentially
broken and it would be better to either provide a arch-specific assembly
implementation and/or use the generic C one and fix the code generation
issue.

Re: [PATCH v2 0/5] linux: Avoid va_list for generic syscall wrappers if possible

[caiyinyu]

在 2023/4/5 下午9:19, Wilco Dijkstra 写道:
> Hi,
>
>> Maybe we can use a custom C implementation (like RISC-V) as well.  But
>> strictly speaking the RISC-V syscall.c is invoking undefined behavior
>> (like my proposal) so I agree with assembly here.
> How does assembly code not invoke the exact same undefined behaviour?
> In both cases you rely on variable arguments being passed in the same
> way as integer args - this is true in many ABIs. The goal is to avoid introducing
> unnecessary assembly and use generic implementations where possible.
>
> So if that C version works as well as assembler on LoongArch and assuming
> that bare syscall() performance is essential, then why not use it?

Thanks.

The patch is dropped and we will go with generic code.




>
> Cheers,
> Wilco