V2 [PATCH 0/2] x32: Properly pass long to syscall [BZ #25810]

V2 [PATCH 0/2] x32: Properly pass long to syscall [BZ #25810]

[H.J. Lu]

X32 has 32-bit long and pointer with 64-bit off_t.  Since x32 psABI
requires that pointers passed in registers must be zero-extended to
64bit, x32 can share many syscall interfaces with LP64.  When a LP64
syscall with long and unsigned long arguments is used for x32, these
arguments must be properly extended to 64-bit.  Otherwise if the upper
32 bits of the register have undefined value, such a syscall will be
rejected by kernel.

For syscalls implemented in assembly codes, 'U' is added to syscall
signature key letters for unsigned long.  SYSCALL_ULONG_ARG_1 and
SYSCALL_ULONG_ARG_2 are passed to syscall-template.S for the first
and the second unsigned long arguments if PSEUDOS_HAVE_4_ARGS is
defined.  They are used by x32 to zero-extend 32-bit arguments to
64 bits.

Tested on i386, x86-64 and x32 as well as with build-many-glibcs.py.

OK for master branch?

H.J. Lu (2):
  Add SYSCALL_ULONG_ARG_[12] to pass long to syscall [BZ #25810]
  Add a syscall test for [BZ #25810]

 misc/Makefile                               |   2 +-
 misc/tst-syscalls.c                         | 146 ++++++++++++++++++++
 sysdeps/unix/make-syscalls.sh               |  88 ++++++++++++
 sysdeps/unix/syscall-template.S             |  43 +++++-
 sysdeps/unix/syscalls.list                  |   6 +-
 sysdeps/unix/sysv/linux/syscalls.list       |  14 +-
 sysdeps/unix/sysv/linux/x86_64/sysdep.h     |  70 ++++++++--
 sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h |  20 ++-
 8 files changed, 360 insertions(+), 29 deletions(-)
 create mode 100644 misc/tst-syscalls.c

-- 
2.25.2

V2 [PATCH 1/2] Add SYSCALL_ULONG_ARG_[12] to pass long to syscall [BZ #25810]

[H.J. Lu]

X32 has 32-bit long and pointer with 64-bit off_t.  Since x32 psABI
requires that pointers passed in registers must be zero-extended to
64bit, x32 can share many syscall interfaces with LP64.  When a LP64
syscall with long and unsigned long arguments is used for x32, these
arguments must be properly extended to 64-bit.  Otherwise if the upper
32 bits of the register have undefined value, such a syscall will be
rejected by kernel.

For syscalls implemented in assembly codes, 'U' is added to syscall
signature key letters for unsigned long.  SYSCALL_ULONG_ARG_1 and
SYSCALL_ULONG_ARG_2 are passed to syscall-template.S for the first
and the second unsigned long arguments if PSEUDOS_HAVE_4_ARGS is
defined.  They are used by x32 to zero-extend 32-bit arguments to
64 bits.

Tested on i386, x86-64 and x32 as well as with build-many-glibcs.py.
---
 sysdeps/unix/make-syscalls.sh               | 88 +++++++++++++++++++++
 sysdeps/unix/syscall-template.S             | 43 +++++++++-
 sysdeps/unix/syscalls.list                  |  6 +-
 sysdeps/unix/sysv/linux/syscalls.list       | 14 ++--
 sysdeps/unix/sysv/linux/x86_64/sysdep.h     | 70 ++++++++++++----
 sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h | 20 ++++-
 6 files changed, 213 insertions(+), 28 deletions(-)

diff --git a/sysdeps/unix/make-syscalls.sh b/sysdeps/unix/make-syscalls.sh
index c07626677f..4a28badea6 100644
--- a/sysdeps/unix/make-syscalls.sh
+++ b/sysdeps/unix/make-syscalls.sh
@@ -30,6 +30,7 @@
 # P: optionally-NULL pointer to typed object (e.g., 3rd argument to sigaction)
 # s: non-NULL string (e.g., 1st arg to open)
 # S: optionally-NULL string (e.g., 1st arg to acct)
+# U: unsigned long
 # v: vararg scalar (e.g., optional 3rd arg to open)
 # V: byte-per-page vector (3rd arg to mincore)
 # W: wait status, optionally-NULL pointer to int (e.g., 2nd arg of wait4)
@@ -184,6 +185,91 @@ while read file srcfile caller syscall args strong weak; do
   ?:?????????) nargs=9;;
   esac
 
+  # Derive the unsigned long arguments from the argument signature
+  ulong_arg_1=0
+  ulong_arg_2=0
+  case $args in
+  ?:U*)
+    ulong_arg_1=1
+    case $args in
+    ?:UU*) ulong_arg_2=2;;
+    ?:U?U*) ulong_arg_2=3;;
+    ?:U??U*) ulong_arg_2=4;;
+    ?:U???U*) ulong_arg_2=5;;
+    ?:U????U*) ulong_arg_2=6;;
+    ?:U?????U*) ulong_arg_2=7;;
+    ?:U??????U*) ulong_arg_2=8;;
+    ?:U???????U) ulong_arg_2=9;;
+    esac
+    ;;
+  ?:?U*)
+    ulong_arg_1=2
+    case $args in
+    ?:?UU*) ulong_arg_2=3;;
+    ?:?U?U*) ulong_arg_2=4;;
+    ?:?U??U*) ulong_arg_2=5;;
+    ?:?U???U*) ulong_arg_2=6;;
+    ?:?U????U*) ulong_arg_2=7;;
+    ?:?U?????U*) ulong_arg_2=8;;
+    ?:?U??????U) ulong_arg_2=9;;
+    esac
+    ;;
+  ?:??U*)
+    ulong_arg_1=3
+    case $args in
+    ?:??UU*) ulong_arg_2=4;;
+    ?:??U?U*) ulong_arg_2=5;;
+    ?:??U??U*) ulong_arg_2=6;;
+    ?:??U???U*) ulong_arg_2=7;;
+    ?:??U????U*) ulong_arg_2=8;;
+    ?:??U?????U) ulong_arg_2=9;;
+    esac
+    ;;
+  ?:???U*)
+    ulong_arg_1=4
+    case $args in
+    ?:???UU*) ulong_arg_2=5;;
+    ?:???U?U*) ulong_arg_2=6;;
+    ?:???U??U*) ulong_arg_2=7;;
+    ?:???U???U*) ulong_arg_2=8;;
+    ?:???U????U) ulong_arg_2=9;;
+    esac
+    ;;
+  ?:????U*)
+    ulong_arg_1=5
+    case $args in
+    ?:????UU*) ulong_arg_2=6;;
+    ?:????U?U*) ulong_arg_2=7;;
+    ?:????U??U*) ulong_arg_2=8;;
+    ?:????U???U) ulong_arg_2=9;;
+    esac
+    ;;
+  ?:?????U*)
+    ulong_arg_1=6
+    case $args in
+    ?:?????UU*) ulong_arg_2=7;;
+    ?:?????U?U*) ulong_arg_2=8;;
+    ?:?????U??U) ulong_arg_2=9;;
+    esac
+    ;;
+  ?:??????U*)
+    ulong_arg_1=7
+    case $args in
+    ?:??????UU*) ulong_arg_2=8;;
+    ?:??????U?U) ulong_arg_2=9;;
+    esac
+    ;;
+  ?:???????U*)
+    ulong_arg_1=8
+    case $args in
+    ?:??????UU) ulong_arg_2=9;;
+    esac
+    ;;
+  ?:????????U)
+    ulong_arg_1=9
+    ;;
+  esac
+
   # Make sure only the first syscall rule is used, if multiple dirs
   # define the same syscall.
   echo ''
@@ -245,6 +331,8 @@ while read file srcfile caller syscall args strong weak; do
 	\$(make-target-directory)
 	(echo '#define SYSCALL_NAME $syscall'; \\
 	 echo '#define SYSCALL_NARGS $nargs'; \\
+	 echo '#define SYSCALL_ULONG_ARG_1 $ulong_arg_1'; \\
+	 echo '#define SYSCALL_ULONG_ARG_2 $ulong_arg_2'; \\
 	 echo '#define SYSCALL_SYMBOL $strong'; \\
 	 echo '#define SYSCALL_NOERRNO $noerrno'; \\
 	 echo '#define SYSCALL_ERRVAL $errval'; \\
diff --git a/sysdeps/unix/syscall-template.S b/sysdeps/unix/syscall-template.S
index cf6c7a58fb..0824a3c61e 100644
--- a/sysdeps/unix/syscall-template.S
+++ b/sysdeps/unix/syscall-template.S
@@ -25,6 +25,10 @@
    defining a few macros:
 	SYSCALL_NAME		syscall name
 	SYSCALL_NARGS		number of arguments this call takes
+	SYSCALL_ULONG_ARG_1	the first unsigned long argument this
+				call takes
+	SYSCALL_ULONG_ARG_2	the second unsigned long argument this
+				call takes
 	SYSCALL_SYMBOL		primary symbol name
 	SYSCALL_NOERRNO		1 to define a no-errno version (see below)
 	SYSCALL_ERRVAL		1 to define an error-value version (see below)
@@ -44,9 +48,27 @@
 /* This indirection is needed so that SYMBOL gets macro-expanded.  */
 #define syscall_hidden_def(SYMBOL)		hidden_def (SYMBOL)
 
-#define T_PSEUDO(SYMBOL, NAME, N)		PSEUDO (SYMBOL, NAME, N)
-#define T_PSEUDO_NOERRNO(SYMBOL, NAME, N)	PSEUDO_NOERRNO (SYMBOL, NAME, N)
-#define T_PSEUDO_ERRVAL(SYMBOL, NAME, N)	PSEUDO_ERRVAL (SYMBOL, NAME, N)
+/* If PSEUDOS_HAVE_4_ARGS is defined, PSEUDO macros have 4 arguments.  */
+#ifndef PSEUDOS_HAVE_4_ARGS
+# undef SYSCALL_ULONG_ARG_1
+# define SYSCALL_ULONG_ARG_1 0
+#endif
+
+#if SYSCALL_ULONG_ARG_1
+# define T_PSEUDO(SYMBOL, NAME, N, U1, U2) \
+  PSEUDO (SYMBOL, NAME, N, U1, U2)
+# define T_PSEUDO_NOERRNO(SYMBOL, NAME, N, U1, U2) \
+  PSEUDO_NOERRNO (SYMBOL, NAME, N, U1, U2)
+# define T_PSEUDO_ERRVAL(SYMBOL, NAME, N, U1, U2) \
+  PSEUDO_ERRVAL (SYMBOL, NAME, N, U1, U2)
+#else
+# define T_PSEUDO(SYMBOL, NAME, N) \
+  PSEUDO (SYMBOL, NAME, N)
+# define T_PSEUDO_NOERRNO(SYMBOL, NAME, N) \
+  PSEUDO_NOERRNO (SYMBOL, NAME, N)
+# define T_PSEUDO_ERRVAL(SYMBOL, NAME, N) \
+  PSEUDO_ERRVAL (SYMBOL, NAME, N)
+#endif
 #define T_PSEUDO_END(SYMBOL)			PSEUDO_END (SYMBOL)
 #define T_PSEUDO_END_NOERRNO(SYMBOL)		PSEUDO_END_NOERRNO (SYMBOL)
 #define T_PSEUDO_END_ERRVAL(SYMBOL)		PSEUDO_END_ERRVAL (SYMBOL)
@@ -56,7 +78,12 @@
 /* This kind of system call stub never returns an error.
    We return the return value register to the caller unexamined.  */
 
+# if SYSCALL_ULONG_ARG_1
+T_PSEUDO_NOERRNO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS,
+		  SYSCALL_ULONG_ARG_1, SYSCALL_ULONG_ARG_2)
+# else
 T_PSEUDO_NOERRNO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
+# endif
 	ret_NOERRNO
 T_PSEUDO_END_NOERRNO (SYSCALL_SYMBOL)
 
@@ -66,7 +93,12 @@ T_PSEUDO_END_NOERRNO (SYSCALL_SYMBOL)
    value, or zero for success.  We may massage the kernel's return value
    to meet that ABI, but we never set errno here.  */
 
+# if SYSCALL_ULONG_ARG_1
+T_PSEUDO_ERRVAL (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS,
+		 SYSCALL_ULONG_ARG_1, SYSCALL_ULONG_ARG_2)
+# else
 T_PSEUDO_ERRVAL (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
+# endif
 	ret_ERRVAL
 T_PSEUDO_END_ERRVAL (SYSCALL_SYMBOL)
 
@@ -75,7 +107,12 @@ T_PSEUDO_END_ERRVAL (SYSCALL_SYMBOL)
 /* This is a "normal" system call stub: if there is an error,
    it returns -1 and sets errno.  */
 
+# if SYSCALL_ULONG_ARG_1
+T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS,
+	  SYSCALL_ULONG_ARG_1, SYSCALL_ULONG_ARG_2)
+# else
 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
+# endif
 	ret
 T_PSEUDO_END (SYSCALL_SYMBOL)
 
diff --git a/sysdeps/unix/syscalls.list b/sysdeps/unix/syscalls.list
index 01c4a0e6b1..e63a8b9d23 100644
--- a/sysdeps/unix/syscalls.list
+++ b/sysdeps/unix/syscalls.list
@@ -37,11 +37,11 @@ kill		-	kill		i:ii	__kill		kill
 link		-	link		i:ss	__link		link
 listen		-	listen		i:ii	__listen	listen
 lseek		-	lseek		i:iii	__libc_lseek	__lseek lseek
-madvise		-	madvise		i:pii	__madvise	madvise
+madvise		-	madvise		i:pUi	__madvise	madvise
 mkdir		-	mkdir		i:si	__mkdir		mkdir
 mmap		-	mmap		b:aniiii __mmap		mmap
-mprotect	-	mprotect	i:aii	__mprotect	mprotect
-munmap		-	munmap		i:ai	__munmap	munmap
+mprotect	-	mprotect	i:aUi	__mprotect	mprotect
+munmap		-	munmap		i:aU	__munmap	munmap
 open		-	open		Ci:siv	__libc_open __open open
 profil		-	profil		i:piii	__profil	profil
 ptrace		-	ptrace		i:iiii	ptrace
diff --git a/sysdeps/unix/sysv/linux/syscalls.list b/sysdeps/unix/sysv/linux/syscalls.list
index e40f993495..4d4af84f9f 100644
--- a/sysdeps/unix/sysv/linux/syscalls.list
+++ b/sysdeps/unix/sysv/linux/syscalls.list
@@ -32,12 +32,12 @@ ioperm		-	ioperm		i:iii	ioperm
 iopl		-	iopl		i:i	iopl
 klogctl		EXTRA	syslog		i:isi	klogctl
 lchown		-	lchown		i:sii	__lchown	lchown
-mincore		-	mincore		i:anV	mincore
-mlock		-	mlock		i:bn	mlock
+mincore		-	mincore		i:aUV	mincore
+mlock		-	mlock		i:bU	mlock
 mlockall	-	mlockall	i:i	mlockall
-mount		EXTRA	mount		i:sssip	__mount	mount
-mremap		EXTRA	mremap		b:ainip	__mremap	mremap
-munlock		-	munlock		i:ai	munlock
+mount		EXTRA	mount		i:sssUp	__mount	mount
+mremap		EXTRA	mremap		b:aUUip	__mremap	mremap
+munlock		-	munlock		i:aU	munlock
 munlockall	-	munlockall	i:	munlockall
 nfsservctl	EXTRA	nfsservctl	i:ipp	__compat_nfsservctl	nfsservctl@GLIBC_2.0:GLIBC_2.28
 pipe		-	pipe		i:f	__pipe		pipe
@@ -46,7 +46,7 @@ pivot_root	EXTRA	pivot_root	i:ss	pivot_root
 prctl		EXTRA	prctl		i:iiiii	__prctl		prctl
 query_module	EXTRA	query_module	i:sipip	__compat_query_module	query_module@GLIBC_2.0:GLIBC_2.23
 quotactl	EXTRA	quotactl	i:isip	quotactl
-remap_file_pages -	remap_file_pages i:piiii	__remap_file_pages remap_file_pages
+remap_file_pages -	remap_file_pages i:pUiUi	__remap_file_pages remap_file_pages
 sched_getp	-	sched_getparam	i:ip	__sched_getparam	sched_getparam
 sched_gets	-	sched_getscheduler	i:i	__sched_getscheduler	sched_getscheduler
 sched_primax	-	sched_get_priority_max	i:i	__sched_get_priority_max	sched_get_priority_max
@@ -54,7 +54,7 @@ sched_primin	-	sched_get_priority_min	i:i	__sched_get_priority_min	sched_get_pri
 sched_setp	-	sched_setparam	i:ip	__sched_setparam	sched_setparam
 sched_sets	-	sched_setscheduler	i:iip	__sched_setscheduler	sched_setscheduler
 sched_yield	-	sched_yield	i:	__sched_yield	sched_yield
-sendfile	-	sendfile	i:iipi	sendfile
+sendfile	-	sendfile	i:iipU	sendfile
 sendfile64	-	sendfile64	i:iipi	sendfile64
 setfsgid	EXTRA	setfsgid	i:i	setfsgid
 setfsuid	EXTRA	setfsuid	i:i	setfsuid
diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
index 85f8f94820..3596273c94 100644
--- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h
+++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
@@ -57,13 +57,30 @@
 #  define SYSCALL_ERROR_LABEL syscall_error
 # endif
 
+/* PSEUDO macros have 4 arguments.  */
+# define PSEUDOS_HAVE_4_ARGS 1
+
+# ifndef SYSCALL_ULONG_ARG_1
+#  define SYSCALL_ULONG_ARG_1 0
+#  define SYSCALL_ULONG_ARG_2 0
+# endif
+
 # undef	PSEUDO
-# define PSEUDO(name, syscall_name, args)				      \
-  .text;								      \
-  ENTRY (name)								      \
-    DO_CALL (syscall_name, args);					      \
-    cmpq $-4095, %rax;							      \
+# if SYSCALL_ULONG_ARG_1
+#  define PSEUDO(name, syscall_name, args, ulong_arg_1, ulong_arg_2) \
+  .text;							      \
+  ENTRY (name)							      \
+    DO_CALL (syscall_name, args, ulong_arg_1, ulong_arg_2);	      \
+    cmpq $-4095, %rax;						      \
     jae SYSCALL_ERROR_LABEL
+# else
+#  define PSEUDO(name, syscall_name, args) \
+  .text;							      \
+  ENTRY (name)							      \
+    DO_CALL (syscall_name, args, 0, 0);				      \
+    cmpq $-4095, %rax;						      \
+    jae SYSCALL_ERROR_LABEL
+# endif
 
 # undef	PSEUDO_END
 # define PSEUDO_END(name)						      \
@@ -71,10 +88,17 @@
   END (name)
 
 # undef	PSEUDO_NOERRNO
-# define PSEUDO_NOERRNO(name, syscall_name, args) \
-  .text;								      \
-  ENTRY (name)								      \
-    DO_CALL (syscall_name, args)
+# if SYSCALL_ULONG_ARG_1
+#  define PSEUDO_NOERRNO(name, syscall_name, args, ulong_arg_1, ulong_arg_2) \
+  .text;							      \
+  ENTRY (name)							      \
+    DO_CALL (syscall_name, args, ulong_arg_1, ulong_arg_2)
+# else
+#  define PSEUDO_NOERRNO(name, syscall_name, args) \
+  .text;							      \
+  ENTRY (name)							      \
+    DO_CALL (syscall_name, args, 0, 0)
+# endif
 
 # undef	PSEUDO_END_NOERRNO
 # define PSEUDO_END_NOERRNO(name) \
@@ -83,11 +107,19 @@
 # define ret_NOERRNO ret
 
 # undef	PSEUDO_ERRVAL
-# define PSEUDO_ERRVAL(name, syscall_name, args) \
-  .text;								      \
-  ENTRY (name)								      \
-    DO_CALL (syscall_name, args);					      \
+# if SYSCALL_ULONG_ARG_1
+#  define PSEUDO_ERRVAL(name, syscall_name, args, ulong_arg_1, ulong_arg_2) \
+  .text;							\
+  ENTRY (name)							\
+    DO_CALL (syscall_name, args, ulong_arg_1, ulong_arg_2);	\
+    negq %rax
+# else
+#  define PSEUDO_ERRVAL(name, syscall_name, args) \
+  .text;							\
+  ENTRY (name)							\
+    DO_CALL (syscall_name, args, 0, 0);				\
     negq %rax
+# endif
 
 # undef	PSEUDO_END_ERRVAL
 # define PSEUDO_END_ERRVAL(name) \
@@ -159,8 +191,10 @@
     Syscalls of more than 6 arguments are not supported.  */
 
 # undef	DO_CALL
-# define DO_CALL(syscall_name, args)		\
+# define DO_CALL(syscall_name, args, ulong_arg_1, ulong_arg_2) \
     DOARGS_##args				\
+    ZERO_EXTEND_##ulong_arg_1			\
+    ZERO_EXTEND_##ulong_arg_2			\
     movl $SYS_ify (syscall_name), %eax;		\
     syscall;
 
@@ -172,6 +206,14 @@
 # define DOARGS_5 DOARGS_4
 # define DOARGS_6 DOARGS_5
 
+# define ZERO_EXTEND_0 /* nothing */
+# define ZERO_EXTEND_1 /* nothing */
+# define ZERO_EXTEND_2 /* nothing */
+# define ZERO_EXTEND_3 /* nothing */
+# define ZERO_EXTEND_4 /* nothing */
+# define ZERO_EXTEND_5 /* nothing */
+# define ZERO_EXTEND_6 /* nothing */
+
 #else	/* !__ASSEMBLER__ */
 
 /* Registers clobbered by syscall.  */
diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
index a37d520f86..b8a74ad2c2 100644
--- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
+++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
@@ -26,7 +26,25 @@
 #undef LO_HI_LONG
 #define LO_HI_LONG(val) (val)
 
-#ifndef __ASSEMBLER__
+#ifdef __ASSEMBLER__
+# undef ZERO_EXTEND_1
+# define ZERO_EXTEND_1 movl %edi, %edi;
+# undef ZERO_EXTEND_2
+# define ZERO_EXTEND_2 movl %esi, %esi;
+# undef ZERO_EXTEND_3
+# define ZERO_EXTEND_3 movl %edx, %edx;
+# if SYSCALL_ULONG_ARG_1 == 4 || SYSCALL_ULONG_ARG_2 == 4
+#  undef DOARGS_4
+#  define DOARGS_4 movl %ecx, %r10d;
+# else
+#  undef ZERO_EXTEND_4
+#  define ZERO_EXTEND_4 movl %r10d, %r10d;
+# endif
+# undef ZERO_EXTEND_5
+# define ZERO_EXTEND_5 movl %r8d, %r8d;
+# undef ZERO_EXTEND_6
+# define ZERO_EXTEND_6 movl %r9d, %r9d;
+#else /* !__ASSEMBLER__ */
 # undef ARGIFY
 /* Enforce zero-extension for pointers and array system call arguments.
    For integer types, extend to int64_t (the full register) using a
-- 
2.25.2

V2 [PATCH 2/2] Add a syscall test for [BZ #25810]

[H.J. Lu]

Add a test to pass 64-bit long arguments to syscall with undefined upper
32 bits on ILP32 systems.

Tested on i386, x86-64 and x32 as well as with build-many-glibcs.py.
---
 misc/Makefile       |   2 +-
 misc/tst-syscalls.c | 146 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 147 insertions(+), 1 deletion(-)
 create mode 100644 misc/tst-syscalls.c

diff --git a/misc/Makefile b/misc/Makefile
index b8fed5783d..67c5237f97 100644
--- a/misc/Makefile
+++ b/misc/Makefile
@@ -87,7 +87,7 @@ tests := tst-dirname tst-tsearch tst-fdset tst-mntent tst-hsearch \
 	 tst-preadvwritev tst-preadvwritev64 tst-makedev tst-empty \
 	 tst-preadvwritev2 tst-preadvwritev64v2 tst-warn-wide \
 	 tst-ldbl-warn tst-ldbl-error tst-dbl-efgcvt tst-ldbl-efgcvt \
-	 tst-mntent-autofs
+	 tst-mntent-autofs tst-syscalls
 
 # Tests which need libdl.
 ifeq (yes,$(build-shared))
diff --git a/misc/tst-syscalls.c b/misc/tst-syscalls.c
new file mode 100644
index 0000000000..d07f03633b
--- /dev/null
+++ b/misc/tst-syscalls.c
@@ -0,0 +1,146 @@
+/* Test for syscall interfaces.
+   Copyright (C) 2020 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 <stdio.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <sys/mman.h>
+#include <support/check.h>
+#include <support/xunistd.h>
+
+struct Array
+{
+  size_t length;
+  void *ptr;
+};
+
+static int error_count;
+
+__attribute__ ((noclone, noinline))
+struct Array
+allocate (size_t bytes)
+{
+  if (!bytes)
+    return __extension__ (struct Array) {0, 0};
+
+  void *p = mmap (0x0, bytes, PROT_READ | PROT_WRITE,
+		  MAP_PRIVATE | MAP_ANON, -1, 0);
+  if (p == MAP_FAILED)
+    return __extension__ (struct Array) {0, 0};
+
+  return __extension__ (struct Array) {bytes, p};
+}
+
+__attribute__ ((noclone, noinline))
+void
+deallocate (struct Array b)
+{
+  if (b.length && munmap (b.ptr, b.length))
+    {
+      printf ("munmap error: %m\n");
+      error_count++;
+    }
+}
+
+__attribute__ ((noclone, noinline))
+void *
+do_mmap (void *addr, size_t length)
+{
+  return mmap (addr, length, PROT_READ | PROT_WRITE,
+	       MAP_PRIVATE | MAP_ANON, -1, 0);
+}
+
+__attribute__ ((noclone, noinline))
+void *
+reallocate (struct Array b)
+{
+  if (b.length)
+    return do_mmap (b.ptr, b.length);
+  return NULL;
+}
+
+__attribute__ ((noclone, noinline))
+void
+protect (struct Array b)
+{
+  if (b.length)
+    {
+      if (mprotect (b.ptr, b.length,
+		    PROT_READ | PROT_WRITE | PROT_EXEC))
+	{
+	  printf ("mprotect error: %m\n");
+	  error_count++;
+	}
+    }
+}
+
+__attribute__ ((noclone, noinline))
+ssize_t
+do_read (int fd, void *ptr, struct Array b)
+{
+  if (b.length)
+    return read (fd, ptr, b.length);
+  return 0;
+}
+
+__attribute__ ((noclone, noinline))
+ssize_t
+do_write (int fd, void *ptr, struct Array b)
+{
+  if (b.length)
+    return write (fd, ptr, b.length);
+  return 0;
+}
+
+static int
+do_test (void)
+{
+  struct Array array;
+
+  array = allocate (1);
+  protect (array);
+  deallocate (array);
+  void *p = reallocate (array);
+  if (p == MAP_FAILED)
+    {
+      printf ("mmap error: %m\n");
+      error_count++;
+    }
+  array.ptr = p;
+  protect (array);
+  deallocate (array);
+
+  int fd = xopen ("/dev/null", O_RDWR, 0);
+  char buf[2];
+  array.ptr = buf;
+  if (do_read (fd, array.ptr, array) == -1)
+    {
+      printf ("read error: %m\n");
+      error_count++;
+    }
+  if (do_write (fd, array.ptr, array) == -1)
+    {
+      printf ("write error: %m\n");
+      error_count++;
+    }
+  xclose (fd);
+
+  return error_count ? EXIT_FAILURE : EXIT_SUCCESS;
+}
+
+#include <support/test-driver.c>
-- 
2.25.2

PING: V2 [PATCH 0/2] x32: Properly pass long to syscall [BZ #25810]

[H.J. Lu]

On Mon, Apr 13, 2020 at 10:51 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> X32 has 32-bit long and pointer with 64-bit off_t.  Since x32 psABI
> requires that pointers passed in registers must be zero-extended to
> 64bit, x32 can share many syscall interfaces with LP64.  When a LP64
> syscall with long and unsigned long arguments is used for x32, these
> arguments must be properly extended to 64-bit.  Otherwise if the upper
> 32 bits of the register have undefined value, such a syscall will be
> rejected by kernel.
>
> For syscalls implemented in assembly codes, 'U' is added to syscall
> signature key letters for unsigned long.  SYSCALL_ULONG_ARG_1 and
> SYSCALL_ULONG_ARG_2 are passed to syscall-template.S for the first
> and the second unsigned long arguments if PSEUDOS_HAVE_4_ARGS is
> defined.  They are used by x32 to zero-extend 32-bit arguments to
> 64 bits.
>
> Tested on i386, x86-64 and x32 as well as with build-many-glibcs.py.
>
> OK for master branch?
>
> H.J. Lu (2):
>   Add SYSCALL_ULONG_ARG_[12] to pass long to syscall [BZ #25810]
>   Add a syscall test for [BZ #25810]
>
>  misc/Makefile                               |   2 +-
>  misc/tst-syscalls.c                         | 146 ++++++++++++++++++++
>  sysdeps/unix/make-syscalls.sh               |  88 ++++++++++++
>  sysdeps/unix/syscall-template.S             |  43 +++++-
>  sysdeps/unix/syscalls.list                  |   6 +-
>  sysdeps/unix/sysv/linux/syscalls.list       |  14 +-
>  sysdeps/unix/sysv/linux/x86_64/sysdep.h     |  70 ++++++++--
>  sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h |  20 ++-
>  8 files changed, 360 insertions(+), 29 deletions(-)
>  create mode 100644 misc/tst-syscalls.c
>

PING:

https://sourceware.org/pipermail/libc-alpha/2020-April/112758.html

-- 
H.J.

PING^2: V2 [PATCH 0/2] x32: Properly pass long to syscall [BZ #25810]

[H.J. Lu]

On Thu, Apr 16, 2020 at 5:52 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Mon, Apr 13, 2020 at 10:51 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > X32 has 32-bit long and pointer with 64-bit off_t.  Since x32 psABI
> > requires that pointers passed in registers must be zero-extended to
> > 64bit, x32 can share many syscall interfaces with LP64.  When a LP64
> > syscall with long and unsigned long arguments is used for x32, these
> > arguments must be properly extended to 64-bit.  Otherwise if the upper
> > 32 bits of the register have undefined value, such a syscall will be
> > rejected by kernel.
> >
> > For syscalls implemented in assembly codes, 'U' is added to syscall
> > signature key letters for unsigned long.  SYSCALL_ULONG_ARG_1 and
> > SYSCALL_ULONG_ARG_2 are passed to syscall-template.S for the first
> > and the second unsigned long arguments if PSEUDOS_HAVE_4_ARGS is
> > defined.  They are used by x32 to zero-extend 32-bit arguments to
> > 64 bits.
> >
> > Tested on i386, x86-64 and x32 as well as with build-many-glibcs.py.
> >
> > OK for master branch?
> >
> > H.J. Lu (2):
> >   Add SYSCALL_ULONG_ARG_[12] to pass long to syscall [BZ #25810]
> >   Add a syscall test for [BZ #25810]
> >
> >  misc/Makefile                               |   2 +-
> >  misc/tst-syscalls.c                         | 146 ++++++++++++++++++++
> >  sysdeps/unix/make-syscalls.sh               |  88 ++++++++++++
> >  sysdeps/unix/syscall-template.S             |  43 +++++-
> >  sysdeps/unix/syscalls.list                  |   6 +-
> >  sysdeps/unix/sysv/linux/syscalls.list       |  14 +-
> >  sysdeps/unix/sysv/linux/x86_64/sysdep.h     |  70 ++++++++--
> >  sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h |  20 ++-
> >  8 files changed, 360 insertions(+), 29 deletions(-)
> >  create mode 100644 misc/tst-syscalls.c
> >
>
> PING:
>
> https://sourceware.org/pipermail/libc-alpha/2020-April/112758.html
>

PING.

-- 
H.J.

Re: V2 [PATCH 1/2] Add SYSCALL_ULONG_ARG_[12] to pass long to syscall [BZ #25810]

[Florian Weimer]

* H. J. Lu via Libc-alpha:

> diff --git a/sysdeps/unix/make-syscalls.sh b/sysdeps/unix/make-syscalls.sh
> index c07626677f..4a28badea6 100644
> --- a/sysdeps/unix/make-syscalls.sh
> +++ b/sysdeps/unix/make-syscalls.sh
> @@ -30,6 +30,7 @@
>  # P: optionally-NULL pointer to typed object (e.g., 3rd argument to sigaction)
>  # s: non-NULL string (e.g., 1st arg to open)
>  # S: optionally-NULL string (e.g., 1st arg to acct)
> +# U: unsigned long

Maybe this?

# U: unsigned long int (32-bit types are zero-extended to 64-bit types)

>  # v: vararg scalar (e.g., optional 3rd arg to open)
>  # V: byte-per-page vector (3rd arg to mincore)
>  # W: wait status, optionally-NULL pointer to int (e.g., 2nd arg of wait4)
> @@ -184,6 +185,91 @@ while read file srcfile caller syscall args strong weak; do
>    ?:?????????) nargs=9;;
>    esac
>  
> +  # Derive the unsigned long arguments from the argument signature

“unsigned long int”

> +  ulong_arg_1=0
> +  ulong_arg_2=0
> +  case $args in
> +  ?:U*)
> +    ulong_arg_1=1
> +    case $args in
> +    ?:UU*) ulong_arg_2=2;;
> +    ?:U?U*) ulong_arg_2=3;;
> +    ?:U??U*) ulong_arg_2=4;;
> +    ?:U???U*) ulong_arg_2=5;;
> +    ?:U????U*) ulong_arg_2=6;;
> +    ?:U?????U*) ulong_arg_2=7;;
> +    ?:U??????U*) ulong_arg_2=8;;
> +    ?:U???????U) ulong_arg_2=9;;
> +    esac
> +    ;;

> +  ?:????????U)
> +    ulong_arg_1=9
> +    ;;
> +  esac
> +

I must say that I find this really, really ugly.  We should rewrite
this in Python as soon as possible (in a separate patch).

You could try this instead:

$ echo U1U | grep -ob U
0:U
2:U

And maybe guard it with a case match, so that performance does not
suffer too much.

In any case, there should be an error check that covers the more
than-two-Us case.


>    # Make sure only the first syscall rule is used, if multiple dirs
>    # define the same syscall.
>    echo ''
> @@ -245,6 +331,8 @@ while read file srcfile caller syscall args strong weak; do
>  	\$(make-target-directory)
>  	(echo '#define SYSCALL_NAME $syscall'; \\
>  	 echo '#define SYSCALL_NARGS $nargs'; \\
> +	 echo '#define SYSCALL_ULONG_ARG_1 $ulong_arg_1'; \\
> +	 echo '#define SYSCALL_ULONG_ARG_2 $ulong_arg_2'; \\

Should this be conditional on whether $ulong_arg_1 and $ulong_arg_2
are empty?  I think that might be less confusing.

Otherwise, the comment at the beginning should mention the special
value zero.

>  	 echo '#define SYSCALL_SYMBOL $strong'; \\
>  	 echo '#define SYSCALL_NOERRNO $noerrno'; \\
>  	 echo '#define SYSCALL_ERRVAL $errval'; \\
> diff --git a/sysdeps/unix/syscall-template.S b/sysdeps/unix/syscall-template.S
> index cf6c7a58fb..0824a3c61e 100644
> --- a/sysdeps/unix/syscall-template.S
> +++ b/sysdeps/unix/syscall-template.S
> @@ -25,6 +25,10 @@
>     defining a few macros:
>  	SYSCALL_NAME		syscall name
>  	SYSCALL_NARGS		number of arguments this call takes
> +	SYSCALL_ULONG_ARG_1	the first unsigned long argument this
> +				call takes
> +	SYSCALL_ULONG_ARG_2	the second unsigned long argument this
> +				call takes

“unsigned long int”

>  	SYSCALL_SYMBOL		primary symbol name
>  	SYSCALL_NOERRNO		1 to define a no-errno version (see below)
>  	SYSCALL_ERRVAL		1 to define an error-value version (see below)
> @@ -44,9 +48,27 @@
>  /* This indirection is needed so that SYMBOL gets macro-expanded.  */
>  #define syscall_hidden_def(SYMBOL)		hidden_def (SYMBOL)
>  
> -#define T_PSEUDO(SYMBOL, NAME, N)		PSEUDO (SYMBOL, NAME, N)
> -#define T_PSEUDO_NOERRNO(SYMBOL, NAME, N)	PSEUDO_NOERRNO (SYMBOL, NAME, N)
> -#define T_PSEUDO_ERRVAL(SYMBOL, NAME, N)	PSEUDO_ERRVAL (SYMBOL, NAME, N)
> +/* If PSEUDOS_HAVE_4_ARGS is defined, PSEUDO macros have 4 arguments.  */
> +#ifndef PSEUDOS_HAVE_4_ARGS
> +# undef SYSCALL_ULONG_ARG_1
> +# define SYSCALL_ULONG_ARG_1 0
> +#endif

PSEUDOS_HAVE_4_ARGS should be PSEUDOS_HAVE_ULONG_INDICES or something
like that.  And a comment that briefly explains all the macro
arguments.  (Not sure if T_PSEUDO is documented somewhere else
already.)

> diff --git a/sysdeps/unix/syscalls.list b/sysdeps/unix/syscalls.list
> index 01c4a0e6b1..e63a8b9d23 100644
> --- a/sysdeps/unix/syscalls.list
> +++ b/sysdeps/unix/syscalls.list
> @@ -37,11 +37,11 @@ kill		-	kill		i:ii	__kill		kill
>  link		-	link		i:ss	__link		link
>  listen		-	listen		i:ii	__listen	listen
>  lseek		-	lseek		i:iii	__libc_lseek	__lseek lseek
> -madvise		-	madvise		i:pii	__madvise	madvise
> +madvise		-	madvise		i:pUi	__madvise	madvise
>  mkdir		-	mkdir		i:si	__mkdir		mkdir
>  mmap		-	mmap		b:aniiii __mmap		mmap
> -mprotect	-	mprotect	i:aii	__mprotect	mprotect
> -munmap		-	munmap		i:ai	__munmap	munmap
> +mprotect	-	mprotect	i:aUi	__mprotect	mprotect
> +munmap		-	munmap		i:aU	__munmap	munmap
>  open		-	open		Ci:siv	__libc_open __open open
>  profil		-	profil		i:piii	__profil	profil
>  ptrace		-	ptrace		i:iiii	ptrace

What about read, readlink, write, etc.?

> diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> index a37d520f86..b8a74ad2c2 100644
> --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> @@ -26,7 +26,25 @@
>  #undef LO_HI_LONG
>  #define LO_HI_LONG(val) (val)
>  
> -#ifndef __ASSEMBLER__
> +#ifdef __ASSEMBLER__
> +# undef ZERO_EXTEND_1
> +# define ZERO_EXTEND_1 movl %edi, %edi;
> +# undef ZERO_EXTEND_2
> +# define ZERO_EXTEND_2 movl %esi, %esi;
> +# undef ZERO_EXTEND_3
> +# define ZERO_EXTEND_3 movl %edx, %edx;
> +# if SYSCALL_ULONG_ARG_1 == 4 || SYSCALL_ULONG_ARG_2 == 4
> +#  undef DOARGS_4
> +#  define DOARGS_4 movl %ecx, %r10d;
> +# else
> +#  undef ZERO_EXTEND_4
> +#  define ZERO_EXTEND_4 movl %r10d, %r10d;
> +# endif
> +# undef ZERO_EXTEND_5
> +# define ZERO_EXTEND_5 movl %r8d, %r8d;
> +# undef ZERO_EXTEND_6
> +# define ZERO_EXTEND_6 movl %r9d, %r9d;
> +#else /* !__ASSEMBLER__ */
>  # undef ARGIFY
>  /* Enforce zero-extension for pointers and array system call arguments.
>     For integer types, extend to int64_t (the full register) using a

The comment should come before the newly added changes, I think.

Re: V2 [PATCH 1/2] Add SYSCALL_ULONG_ARG_[12] to pass long to syscall [BZ #25810]

[Adhemerval Zanella]

On 22/04/2020 07:42, Florian Weimer wrote:
> * H. J. Lu via Libc-alpha:
> 
>> diff --git a/sysdeps/unix/make-syscalls.sh b/sysdeps/unix/make-syscalls.sh
>> index c07626677f..4a28badea6 100644
>> --- a/sysdeps/unix/make-syscalls.sh
>> +++ b/sysdeps/unix/make-syscalls.sh
>> @@ -30,6 +30,7 @@
>>  # P: optionally-NULL pointer to typed object (e.g., 3rd argument to sigaction)
>>  # s: non-NULL string (e.g., 1st arg to open)
>>  # S: optionally-NULL string (e.g., 1st arg to acct)
>> +# U: unsigned long
> 
> Maybe this?
> 
> # U: unsigned long int (32-bit types are zero-extended to 64-bit types)
> 
>>  # v: vararg scalar (e.g., optional 3rd arg to open)
>>  # V: byte-per-page vector (3rd arg to mincore)
>>  # W: wait status, optionally-NULL pointer to int (e.g., 2nd arg of wait4)
>> @@ -184,6 +185,91 @@ while read file srcfile caller syscall args strong weak; do
>>    ?:?????????) nargs=9;;
>>    esac
>>  
>> +  # Derive the unsigned long arguments from the argument signature
> 
> “unsigned long int”
> 
>> +  ulong_arg_1=0
>> +  ulong_arg_2=0
>> +  case $args in
>> +  ?:U*)
>> +    ulong_arg_1=1
>> +    case $args in
>> +    ?:UU*) ulong_arg_2=2;;
>> +    ?:U?U*) ulong_arg_2=3;;
>> +    ?:U??U*) ulong_arg_2=4;;
>> +    ?:U???U*) ulong_arg_2=5;;
>> +    ?:U????U*) ulong_arg_2=6;;
>> +    ?:U?????U*) ulong_arg_2=7;;
>> +    ?:U??????U*) ulong_arg_2=8;;
>> +    ?:U???????U) ulong_arg_2=9;;
>> +    esac
>> +    ;;
> 
>> +  ?:????????U)
>> +    ulong_arg_1=9
>> +    ;;
>> +  esac
>> +
> 
> I must say that I find this really, really ugly.  We should rewrite
> this in Python as soon as possible (in a separate patch).

And I think we should make long term gold to just get rid of this
assembly macro and focus on automatic generation to a C code file
as well.

> 
> You could try this instead:
> 
> $ echo U1U | grep -ob U
> 0:U
> 2:U
> 
> And maybe guard it with a case match, so that performance does not
> suffer too much.
> 
> In any case, there should be an error check that covers the more
> than-two-Us case.
> 
> 
>>    # Make sure only the first syscall rule is used, if multiple dirs
>>    # define the same syscall.
>>    echo ''
>> @@ -245,6 +331,8 @@ while read file srcfile caller syscall args strong weak; do
>>  	\$(make-target-directory)
>>  	(echo '#define SYSCALL_NAME $syscall'; \\
>>  	 echo '#define SYSCALL_NARGS $nargs'; \\
>> +	 echo '#define SYSCALL_ULONG_ARG_1 $ulong_arg_1'; \\
>> +	 echo '#define SYSCALL_ULONG_ARG_2 $ulong_arg_2'; \\
> 
> Should this be conditional on whether $ulong_arg_1 and $ulong_arg_2
> are empty?  I think that might be less confusing.
> 
> Otherwise, the comment at the beginning should mention the special
> value zero.
> 
>>  	 echo '#define SYSCALL_SYMBOL $strong'; \\
>>  	 echo '#define SYSCALL_NOERRNO $noerrno'; \\
>>  	 echo '#define SYSCALL_ERRVAL $errval'; \\
>> diff --git a/sysdeps/unix/syscall-template.S b/sysdeps/unix/syscall-template.S
>> index cf6c7a58fb..0824a3c61e 100644
>> --- a/sysdeps/unix/syscall-template.S
>> +++ b/sysdeps/unix/syscall-template.S
>> @@ -25,6 +25,10 @@
>>     defining a few macros:
>>  	SYSCALL_NAME		syscall name
>>  	SYSCALL_NARGS		number of arguments this call takes
>> +	SYSCALL_ULONG_ARG_1	the first unsigned long argument this
>> +				call takes
>> +	SYSCALL_ULONG_ARG_2	the second unsigned long argument this
>> +				call takes
> 
> “unsigned long int”
> 
>>  	SYSCALL_SYMBOL		primary symbol name
>>  	SYSCALL_NOERRNO		1 to define a no-errno version (see below)
>>  	SYSCALL_ERRVAL		1 to define an error-value version (see below)
>> @@ -44,9 +48,27 @@
>>  /* This indirection is needed so that SYMBOL gets macro-expanded.  */
>>  #define syscall_hidden_def(SYMBOL)		hidden_def (SYMBOL)
>>  
>> -#define T_PSEUDO(SYMBOL, NAME, N)		PSEUDO (SYMBOL, NAME, N)
>> -#define T_PSEUDO_NOERRNO(SYMBOL, NAME, N)	PSEUDO_NOERRNO (SYMBOL, NAME, N)
>> -#define T_PSEUDO_ERRVAL(SYMBOL, NAME, N)	PSEUDO_ERRVAL (SYMBOL, NAME, N)
>> +/* If PSEUDOS_HAVE_4_ARGS is defined, PSEUDO macros have 4 arguments.  */
>> +#ifndef PSEUDOS_HAVE_4_ARGS
>> +# undef SYSCALL_ULONG_ARG_1
>> +# define SYSCALL_ULONG_ARG_1 0
>> +#endif
> 
> PSEUDOS_HAVE_4_ARGS should be PSEUDOS_HAVE_ULONG_INDICES or something
> like that.  And a comment that briefly explains all the macro
> arguments.  (Not sure if T_PSEUDO is documented somewhere else
> already.)
> 
>> diff --git a/sysdeps/unix/syscalls.list b/sysdeps/unix/syscalls.list
>> index 01c4a0e6b1..e63a8b9d23 100644
>> --- a/sysdeps/unix/syscalls.list
>> +++ b/sysdeps/unix/syscalls.list
>> @@ -37,11 +37,11 @@ kill		-	kill		i:ii	__kill		kill
>>  link		-	link		i:ss	__link		link
>>  listen		-	listen		i:ii	__listen	listen
>>  lseek		-	lseek		i:iii	__libc_lseek	__lseek lseek
>> -madvise		-	madvise		i:pii	__madvise	madvise
>> +madvise		-	madvise		i:pUi	__madvise	madvise
>>  mkdir		-	mkdir		i:si	__mkdir		mkdir
>>  mmap		-	mmap		b:aniiii __mmap		mmap
>> -mprotect	-	mprotect	i:aii	__mprotect	mprotect
>> -munmap		-	munmap		i:ai	__munmap	munmap
>> +mprotect	-	mprotect	i:aUi	__mprotect	mprotect
>> +munmap		-	munmap		i:aU	__munmap	munmap
>>  open		-	open		Ci:siv	__libc_open __open open
>>  profil		-	profil		i:piii	__profil	profil
>>  ptrace		-	ptrace		i:iiii	ptrace
> 
> What about read, readlink, write, etc.?
> 
>> diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
>> index a37d520f86..b8a74ad2c2 100644
>> --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
>> +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
>> @@ -26,7 +26,25 @@
>>  #undef LO_HI_LONG
>>  #define LO_HI_LONG(val) (val)
>>  
>> -#ifndef __ASSEMBLER__
>> +#ifdef __ASSEMBLER__
>> +# undef ZERO_EXTEND_1
>> +# define ZERO_EXTEND_1 movl %edi, %edi;
>> +# undef ZERO_EXTEND_2
>> +# define ZERO_EXTEND_2 movl %esi, %esi;
>> +# undef ZERO_EXTEND_3
>> +# define ZERO_EXTEND_3 movl %edx, %edx;
>> +# if SYSCALL_ULONG_ARG_1 == 4 || SYSCALL_ULONG_ARG_2 == 4
>> +#  undef DOARGS_4
>> +#  define DOARGS_4 movl %ecx, %r10d;
>> +# else
>> +#  undef ZERO_EXTEND_4
>> +#  define ZERO_EXTEND_4 movl %r10d, %r10d;
>> +# endif
>> +# undef ZERO_EXTEND_5
>> +# define ZERO_EXTEND_5 movl %r8d, %r8d;
>> +# undef ZERO_EXTEND_6
>> +# define ZERO_EXTEND_6 movl %r9d, %r9d;
>> +#else /* !__ASSEMBLER__ */
>>  # undef ARGIFY
>>  /* Enforce zero-extension for pointers and array system call arguments.
>>     For integer types, extend to int64_t (the full register) using a
> 
> The comment should come before the newly added changes, I think.
> 

Re: V2 [PATCH 2/2] Add a syscall test for [BZ #25810]

[Florian Weimer]

* H. J. Lu via Libc-alpha:

> Add a test to pass 64-bit long arguments to syscall with undefined upper
> 32 bits on ILP32 systems.

What does this test, exactly?  How does it ensure that the upper bits
are indeed non-zero, to exercise the zero-extension case?

Re: V2 [PATCH 2/2] Add a syscall test for [BZ #25810]

[H.J. Lu]

On Wed, Apr 22, 2020 at 5:25 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>
> * H. J. Lu via Libc-alpha:
>
> > Add a test to pass 64-bit long arguments to syscall with undefined upper
> > 32 bits on ILP32 systems.
>
> What does this test, exactly?  How does it ensure that the upper bits
> are indeed non-zero, to exercise the zero-extension case?

On x32,

struct Array
{
  size_t length;
  void *ptr;
};

can be passed in a single 64-bit integer register.  When a 32-bit
integer is passed in
a 64-bit integer, its upper 32 bits can contain undefined value:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94541

This testcase arranges syscalls in such a way that the upper 32 bits
of 64 big integer
register, which is used to pass unsigned long to kernel, contains the
"ptr" value passed in
function arguments.   If the upper 32 bits aren't cleared, syscall
will fail since long in kernel
is 64 bits, not 32 bits.

-- 
H.J.

Re: V2 [PATCH 2/2] Add a syscall test for [BZ #25810]

[Florian Weimer]

* H. J. Lu:

> On Wed, Apr 22, 2020 at 5:25 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>>
>> * H. J. Lu via Libc-alpha:
>>
>> > Add a test to pass 64-bit long arguments to syscall with undefined upper
>> > 32 bits on ILP32 systems.
>>
>> What does this test, exactly?  How does it ensure that the upper bits
>> are indeed non-zero, to exercise the zero-extension case?
>
> On x32,
>
> struct Array
> {
>   size_t length;
>   void *ptr;
> };
>
> can be passed in a single 64-bit integer register.  When a 32-bit
> integer is passed in
> a 64-bit integer, its upper 32 bits can contain undefined value:
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94541
>
> This testcase arranges syscalls in such a way that the upper 32 bits
> of 64 big integer
> register, which is used to pass unsigned long to kernel, contains the
> "ptr" value passed in
> function arguments.   If the upper 32 bits aren't cleared, syscall
> will fail since long in kernel
> is 64 bits, not 32 bits.

Would you please add this as a comment to the patch, and one-line
comments where the clobbers happen?

And say that the test is in this formulation likely x32-specific, but
that it is expected to work on other architectures as well.

Re: V2 [PATCH 2/2] Add a syscall test for [BZ #25810]

[H.J. Lu]

On Wed, Apr 22, 2020 at 5:47 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>
> * H. J. Lu:
>
> > On Wed, Apr 22, 2020 at 5:25 AM Florian Weimer <fw@deneb.enyo.de> wrote:
> >>
> >> * H. J. Lu via Libc-alpha:
> >>
> >> > Add a test to pass 64-bit long arguments to syscall with undefined upper
> >> > 32 bits on ILP32 systems.
> >>
> >> What does this test, exactly?  How does it ensure that the upper bits
> >> are indeed non-zero, to exercise the zero-extension case?
> >
> > On x32,
> >
> > struct Array
> > {
> >   size_t length;
> >   void *ptr;
> > };
> >
> > can be passed in a single 64-bit integer register.  When a 32-bit
> > integer is passed in
> > a 64-bit integer, its upper 32 bits can contain undefined value:
> >
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94541
> >
> > This testcase arranges syscalls in such a way that the upper 32 bits
> > of 64 big integer
> > register, which is used to pass unsigned long to kernel, contains the
> > "ptr" value passed in
> > function arguments.   If the upper 32 bits aren't cleared, syscall
> > will fail since long in kernel
> > is 64 bits, not 32 bits.
>
> Would you please add this as a comment to the patch, and one-line
> comments where the clobbers happen?

I will add

+/* On x32, this can be passed in a single 64-bit integer register.  */
 struct Array
 {
   size_t length;
@@ -50,6 +51,9 @@ __attribute__ ((noclone, noinline))
 void
 deallocate (struct Array b)
 {
+  /* On x32, the 64-bit integer register containing `b' may be copied
+     to another 64-bit integer register to pass the second argument to
+     munmap.  */
   if (b.length && munmap (b.ptr, b.length))
     {
       printf ("munmap error: %m\n");

> And say that the test is in this formulation likely x32-specific, but
> that it is expected to work on other architectures as well.



-- 
H.J.

Re: V2 [PATCH 2/2] Add a syscall test for [BZ #25810]

[Florian Weimer]

* H. J. Lu:

> On Wed, Apr 22, 2020 at 5:47 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>>
>> * H. J. Lu:
>>
>> > On Wed, Apr 22, 2020 at 5:25 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>> >>
>> >> * H. J. Lu via Libc-alpha:
>> >>
>> >> > Add a test to pass 64-bit long arguments to syscall with undefined upper
>> >> > 32 bits on ILP32 systems.
>> >>
>> >> What does this test, exactly?  How does it ensure that the upper bits
>> >> are indeed non-zero, to exercise the zero-extension case?
>> >
>> > On x32,
>> >
>> > struct Array
>> > {
>> >   size_t length;
>> >   void *ptr;
>> > };
>> >
>> > can be passed in a single 64-bit integer register.  When a 32-bit
>> > integer is passed in
>> > a 64-bit integer, its upper 32 bits can contain undefined value:
>> >
>> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94541
>> >
>> > This testcase arranges syscalls in such a way that the upper 32 bits
>> > of 64 big integer
>> > register, which is used to pass unsigned long to kernel, contains the
>> > "ptr" value passed in
>> > function arguments.   If the upper 32 bits aren't cleared, syscall
>> > will fail since long in kernel
>> > is 64 bits, not 32 bits.
>>
>> Would you please add this as a comment to the patch, and one-line
>> comments where the clobbers happen?
>
> I will add
>
> +/* On x32, this can be passed in a single 64-bit integer register.  */
>  struct Array
>  {
>    size_t length;
> @@ -50,6 +51,9 @@ __attribute__ ((noclone, noinline))
>  void
>  deallocate (struct Array b)
>  {
> +  /* On x32, the 64-bit integer register containing `b' may be copied
> +     to another 64-bit integer register to pass the second argument to
> +     munmap.  */
>    if (b.length && munmap (b.ptr, b.length))
>      {
>        printf ("munmap error: %m\n");

Looks good.

Please also add something like this at the top of the file, after the
copyright header.

/* This test verifies that the x32 system call handling zero-extends
   unsigned 32-bit arguments to the 64-bit argument registers for
   system calls (bug 25810).  The bug is specific to x32, but the test
   should pass on all architectures.  */

V3 [PATCH 2/2] Add a syscall test for [BZ #25810]

[H.J. Lu]

On Wed, Apr 22, 2020 at 03:46:28PM +0200, Florian Weimer wrote:
> * H. J. Lu:
> 
> > On Wed, Apr 22, 2020 at 5:47 AM Florian Weimer <fw@deneb.enyo.de> wrote:
> >>
> >> * H. J. Lu:
> >>
> >> > On Wed, Apr 22, 2020 at 5:25 AM Florian Weimer <fw@deneb.enyo.de> wrote:
> >> >>
> >> >> * H. J. Lu via Libc-alpha:
> >> >>
> >> >> > Add a test to pass 64-bit long arguments to syscall with undefined upper
> >> >> > 32 bits on ILP32 systems.
> >> >>
> >> >> What does this test, exactly?  How does it ensure that the upper bits
> >> >> are indeed non-zero, to exercise the zero-extension case?
> >> >
> >> > On x32,
> >> >
> >> > struct Array
> >> > {
> >> >   size_t length;
> >> >   void *ptr;
> >> > };
> >> >
> >> > can be passed in a single 64-bit integer register.  When a 32-bit
> >> > integer is passed in
> >> > a 64-bit integer, its upper 32 bits can contain undefined value:
> >> >
> >> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94541
> >> >
> >> > This testcase arranges syscalls in such a way that the upper 32 bits
> >> > of 64 big integer
> >> > register, which is used to pass unsigned long to kernel, contains the
> >> > "ptr" value passed in
> >> > function arguments.   If the upper 32 bits aren't cleared, syscall
> >> > will fail since long in kernel
> >> > is 64 bits, not 32 bits.
> >>
> >> Would you please add this as a comment to the patch, and one-line
> >> comments where the clobbers happen?
> >
> > I will add
> >
> > +/* On x32, this can be passed in a single 64-bit integer register.  */
> >  struct Array
> >  {
> >    size_t length;
> > @@ -50,6 +51,9 @@ __attribute__ ((noclone, noinline))
> >  void
> >  deallocate (struct Array b)
> >  {
> > +  /* On x32, the 64-bit integer register containing `b' may be copied
> > +     to another 64-bit integer register to pass the second argument to
> > +     munmap.  */
> >    if (b.length && munmap (b.ptr, b.length))
> >      {
> >        printf ("munmap error: %m\n");
> 
> Looks good.
> 
> Please also add something like this at the top of the file, after the
> copyright header.
> 
> /* This test verifies that the x32 system call handling zero-extends
>    unsigned 32-bit arguments to the 64-bit argument registers for
>    system calls (bug 25810).  The bug is specific to x32, but the test
>    should pass on all architectures.  */

Here is the updated patch.  OK for master?

Thanks.

H.J.
---
Add a test to pass 64-bit long arguments to syscall with undefined upper
32 bits on x32.

Tested on i386, x86-64 and x32 as well as with build-many-glibcs.py.
---
 misc/Makefile       |   2 +-
 misc/tst-syscalls.c | 167 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 168 insertions(+), 1 deletion(-)
 create mode 100644 misc/tst-syscalls.c

diff --git a/misc/Makefile b/misc/Makefile
index b8fed5783d..67c5237f97 100644
--- a/misc/Makefile
+++ b/misc/Makefile
@@ -87,7 +87,7 @@ tests := tst-dirname tst-tsearch tst-fdset tst-mntent tst-hsearch \
 	 tst-preadvwritev tst-preadvwritev64 tst-makedev tst-empty \
 	 tst-preadvwritev2 tst-preadvwritev64v2 tst-warn-wide \
 	 tst-ldbl-warn tst-ldbl-error tst-dbl-efgcvt tst-ldbl-efgcvt \
-	 tst-mntent-autofs
+	 tst-mntent-autofs tst-syscalls
 
 # Tests which need libdl.
 ifeq (yes,$(build-shared))
diff --git a/misc/tst-syscalls.c b/misc/tst-syscalls.c
new file mode 100644
index 0000000000..cfcd382320
--- /dev/null
+++ b/misc/tst-syscalls.c
@@ -0,0 +1,167 @@
+/* Test for syscall interfaces.
+   Copyright (C) 2020 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/>.  */
+
+/* This test verifies that the x32 system call handling zero-extends
+   unsigned 32-bit arguments to the 64-bit argument registers for
+   system calls (bug 25810).  The bug is specific to x32, but the test
+   should pass on all architectures.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <sys/mman.h>
+#include <support/check.h>
+#include <support/xunistd.h>
+
+/* On x32, this can be passed in a single 64-bit integer register.  */
+struct Array
+{
+  size_t length;
+  void *ptr;
+};
+
+static int error_count;
+
+__attribute__ ((noclone, noinline))
+struct Array
+allocate (size_t bytes)
+{
+  if (!bytes)
+    return __extension__ (struct Array) {0, 0};
+
+  void *p = mmap (0x0, bytes, PROT_READ | PROT_WRITE,
+		  MAP_PRIVATE | MAP_ANON, -1, 0);
+  if (p == MAP_FAILED)
+    return __extension__ (struct Array) {0, 0};
+
+  return __extension__ (struct Array) {bytes, p};
+}
+
+__attribute__ ((noclone, noinline))
+void
+deallocate (struct Array b)
+{
+  /* On x32, the 64-bit integer register containing `b' may be copied
+     to another 64-bit integer register to pass the second argument to
+     munmap.  */
+  if (b.length && munmap (b.ptr, b.length))
+    {
+      printf ("munmap error: %m\n");
+      error_count++;
+    }
+}
+
+__attribute__ ((noclone, noinline))
+void *
+do_mmap (void *addr, size_t length)
+{
+  return mmap (addr, length, PROT_READ | PROT_WRITE,
+	       MAP_PRIVATE | MAP_ANON, -1, 0);
+}
+
+__attribute__ ((noclone, noinline))
+void *
+reallocate (struct Array b)
+{
+  /* On x32, the 64-bit integer register containing `b' may be copied
+     to another 64-bit integer register to pass the second argument to
+     do_mmap.  */
+  if (b.length)
+    return do_mmap (b.ptr, b.length);
+  return NULL;
+}
+
+__attribute__ ((noclone, noinline))
+void
+protect (struct Array b)
+{
+  if (b.length)
+    {
+      /* On x32, the 64-bit integer register containing `b' may be copied
+	 to another 64-bit integer register to pass the second argument
+	 to mprotect.  */
+      if (mprotect (b.ptr, b.length,
+		    PROT_READ | PROT_WRITE | PROT_EXEC))
+	{
+	  printf ("mprotect error: %m\n");
+	  error_count++;
+	}
+    }
+}
+
+__attribute__ ((noclone, noinline))
+ssize_t
+do_read (int fd, void *ptr, struct Array b)
+{
+  /* On x32, the 64-bit integer register containing `b' may be copied
+     to another 64-bit integer register to pass the second argument to
+     read.  */
+  if (b.length)
+    return read (fd, ptr, b.length);
+  return 0;
+}
+
+__attribute__ ((noclone, noinline))
+ssize_t
+do_write (int fd, void *ptr, struct Array b)
+{
+  /* On x32, the 64-bit integer register containing `b' may be copied
+     to another 64-bit integer register to pass the second argument to
+     write.  */
+  if (b.length)
+    return write (fd, ptr, b.length);
+  return 0;
+}
+
+static int
+do_test (void)
+{
+  struct Array array;
+
+  array = allocate (1);
+  protect (array);
+  deallocate (array);
+  void *p = reallocate (array);
+  if (p == MAP_FAILED)
+    {
+      printf ("mmap error: %m\n");
+      error_count++;
+    }
+  array.ptr = p;
+  protect (array);
+  deallocate (array);
+
+  int fd = xopen ("/dev/null", O_RDWR, 0);
+  char buf[2];
+  array.ptr = buf;
+  if (do_read (fd, array.ptr, array) == -1)
+    {
+      printf ("read error: %m\n");
+      error_count++;
+    }
+  if (do_write (fd, array.ptr, array) == -1)
+    {
+      printf ("write error: %m\n");
+      error_count++;
+    }
+  xclose (fd);
+
+  return error_count ? EXIT_FAILURE : EXIT_SUCCESS;
+}
+
+#include <support/test-driver.c>
-- 
2.25.3

Re: V3 [PATCH 2/2] Add a syscall test for [BZ #25810]

[Florian Weimer]

* H. J. Lu:

>> Please also add something like this at the top of the file, after the
>> copyright header.
>> 
>> /* This test verifies that the x32 system call handling zero-extends
>>    unsigned 32-bit arguments to the 64-bit argument registers for
>>    system calls (bug 25810).  The bug is specific to x32, but the test
>>    should pass on all architectures.  */

I realize now that the phrasing is a bit awkward.  But at least the
bug number is there.

> Here is the updated patch.  OK for master?

Yes, fine with me.  Thanks for your patience.

V3 [PATCH 1/2] Add SYSCALL_ULONG_ARG_[12] to pass long to syscall [BZ #25810]

[H.J. Lu]

On Wed, Apr 22, 2020 at 12:42:37PM +0200, Florian Weimer wrote:
> * H. J. Lu via Libc-alpha:
> 
> > diff --git a/sysdeps/unix/make-syscalls.sh b/sysdeps/unix/make-syscalls.sh
> > index c07626677f..4a28badea6 100644
> > --- a/sysdeps/unix/make-syscalls.sh
> > +++ b/sysdeps/unix/make-syscalls.sh
> > @@ -30,6 +30,7 @@
> >  # P: optionally-NULL pointer to typed object (e.g., 3rd argument to sigaction)
> >  # s: non-NULL string (e.g., 1st arg to open)
> >  # S: optionally-NULL string (e.g., 1st arg to acct)
> > +# U: unsigned long
> 
> Maybe this?
> 
> # U: unsigned long int (32-bit types are zero-extended to 64-bit types)

Fixed.

> 
> >  # v: vararg scalar (e.g., optional 3rd arg to open)
> >  # V: byte-per-page vector (3rd arg to mincore)
> >  # W: wait status, optionally-NULL pointer to int (e.g., 2nd arg of wait4)
> > @@ -184,6 +185,91 @@ while read file srcfile caller syscall args strong weak; do
> >    ?:?????????) nargs=9;;
> >    esac
> >  
> > +  # Derive the unsigned long arguments from the argument signature
> 
> “unsigned long int”

Fixed.

> 
> > +  ulong_arg_1=0
> > +  ulong_arg_2=0
> > +  case $args in
> > +  ?:U*)
> > +    ulong_arg_1=1
> > +    case $args in
> > +    ?:UU*) ulong_arg_2=2;;
> > +    ?:U?U*) ulong_arg_2=3;;
> > +    ?:U??U*) ulong_arg_2=4;;
> > +    ?:U???U*) ulong_arg_2=5;;
> > +    ?:U????U*) ulong_arg_2=6;;
> > +    ?:U?????U*) ulong_arg_2=7;;
> > +    ?:U??????U*) ulong_arg_2=8;;
> > +    ?:U???????U) ulong_arg_2=9;;
> > +    esac
> > +    ;;
> 
> > +  ?:????????U)
> > +    ulong_arg_1=9
> > +    ;;
> > +  esac
> > +
> 
> I must say that I find this really, really ugly.  We should rewrite
> this in Python as soon as possible (in a separate patch).
> 
> You could try this instead:
> 
> $ echo U1U | grep -ob U
> 0:U
> 2:U

Fixed.

> 
> And maybe guard it with a case match, so that performance does not
> suffer too much.

I didn't notice any significant build timing change.

> 
> In any case, there should be an error check that covers the more
> than-two-Us case.
> 

Fixed.

> 
> >    # Make sure only the first syscall rule is used, if multiple dirs
> >    # define the same syscall.
> >    echo ''
> > @@ -245,6 +331,8 @@ while read file srcfile caller syscall args strong weak; do
> >  	\$(make-target-directory)
> >  	(echo '#define SYSCALL_NAME $syscall'; \\
> >  	 echo '#define SYSCALL_NARGS $nargs'; \\
> > +	 echo '#define SYSCALL_ULONG_ARG_1 $ulong_arg_1'; \\
> > +	 echo '#define SYSCALL_ULONG_ARG_2 $ulong_arg_2'; \\
> 
> Should this be conditional on whether $ulong_arg_1 and $ulong_arg_2
> are empty?  I think that might be less confusing.
> 
> Otherwise, the comment at the beginning should mention the special
> value zero.

I updated comments of SYSCALL_ULONG_ARG_1 and SYSCALL_ULONG_ARG_2 in
syscall-template.S.

> 
> >  	 echo '#define SYSCALL_SYMBOL $strong'; \\
> >  	 echo '#define SYSCALL_NOERRNO $noerrno'; \\
> >  	 echo '#define SYSCALL_ERRVAL $errval'; \\
> > diff --git a/sysdeps/unix/syscall-template.S b/sysdeps/unix/syscall-template.S
> > index cf6c7a58fb..0824a3c61e 100644
> > --- a/sysdeps/unix/syscall-template.S
> > +++ b/sysdeps/unix/syscall-template.S
> > @@ -25,6 +25,10 @@
> >     defining a few macros:
> >  	SYSCALL_NAME		syscall name
> >  	SYSCALL_NARGS		number of arguments this call takes
> > +	SYSCALL_ULONG_ARG_1	the first unsigned long argument this
> > +				call takes
> > +	SYSCALL_ULONG_ARG_2	the second unsigned long argument this
> > +				call takes
> 
> “unsigned long int”

Fixed.

> 
> >  	SYSCALL_SYMBOL		primary symbol name
> >  	SYSCALL_NOERRNO		1 to define a no-errno version (see below)
> >  	SYSCALL_ERRVAL		1 to define an error-value version (see below)
> > @@ -44,9 +48,27 @@
> >  /* This indirection is needed so that SYMBOL gets macro-expanded.  */
> >  #define syscall_hidden_def(SYMBOL)		hidden_def (SYMBOL)
> >  
> > -#define T_PSEUDO(SYMBOL, NAME, N)		PSEUDO (SYMBOL, NAME, N)
> > -#define T_PSEUDO_NOERRNO(SYMBOL, NAME, N)	PSEUDO_NOERRNO (SYMBOL, NAME, N)
> > -#define T_PSEUDO_ERRVAL(SYMBOL, NAME, N)	PSEUDO_ERRVAL (SYMBOL, NAME, N)
> > +/* If PSEUDOS_HAVE_4_ARGS is defined, PSEUDO macros have 4 arguments.  */
> > +#ifndef PSEUDOS_HAVE_4_ARGS
> > +# undef SYSCALL_ULONG_ARG_1
> > +# define SYSCALL_ULONG_ARG_1 0
> > +#endif
> 
> PSEUDOS_HAVE_4_ARGS should be PSEUDOS_HAVE_ULONG_INDICES or something
> like that.  And a comment that briefly explains all the macro
> arguments.  (Not sure if T_PSEUDO is documented somewhere else
> already.)

Fixed.

> 
> > diff --git a/sysdeps/unix/syscalls.list b/sysdeps/unix/syscalls.list
> > index 01c4a0e6b1..e63a8b9d23 100644
> > --- a/sysdeps/unix/syscalls.list
> > +++ b/sysdeps/unix/syscalls.list
> > @@ -37,11 +37,11 @@ kill		-	kill		i:ii	__kill		kill
> >  link		-	link		i:ss	__link		link
> >  listen		-	listen		i:ii	__listen	listen
> >  lseek		-	lseek		i:iii	__libc_lseek	__lseek lseek
> > -madvise		-	madvise		i:pii	__madvise	madvise
> > +madvise		-	madvise		i:pUi	__madvise	madvise
> >  mkdir		-	mkdir		i:si	__mkdir		mkdir
> >  mmap		-	mmap		b:aniiii __mmap		mmap
> > -mprotect	-	mprotect	i:aii	__mprotect	mprotect
> > -munmap		-	munmap		i:ai	__munmap	munmap
> > +mprotect	-	mprotect	i:aUi	__mprotect	mprotect
> > +munmap		-	munmap		i:aU	__munmap	munmap
> >  open		-	open		Ci:siv	__libc_open __open open
> >  profil		-	profil		i:piii	__profil	profil
> >  ptrace		-	ptrace		i:iiii	ptrace
> 
> What about read, readlink, write, etc.?

I updated readlink and readlinkat.  read/write are implemented as inlined
SYSCALLs.

> 
> > diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> > index a37d520f86..b8a74ad2c2 100644
> > --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> > +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> > @@ -26,7 +26,25 @@
> >  #undef LO_HI_LONG
> >  #define LO_HI_LONG(val) (val)
> >  
> > -#ifndef __ASSEMBLER__
> > +#ifdef __ASSEMBLER__
> > +# undef ZERO_EXTEND_1
> > +# define ZERO_EXTEND_1 movl %edi, %edi;
> > +# undef ZERO_EXTEND_2
> > +# define ZERO_EXTEND_2 movl %esi, %esi;
> > +# undef ZERO_EXTEND_3
> > +# define ZERO_EXTEND_3 movl %edx, %edx;
> > +# if SYSCALL_ULONG_ARG_1 == 4 || SYSCALL_ULONG_ARG_2 == 4
> > +#  undef DOARGS_4
> > +#  define DOARGS_4 movl %ecx, %r10d;
> > +# else
> > +#  undef ZERO_EXTEND_4
> > +#  define ZERO_EXTEND_4 movl %r10d, %r10d;
> > +# endif
> > +# undef ZERO_EXTEND_5
> > +# define ZERO_EXTEND_5 movl %r8d, %r8d;
> > +# undef ZERO_EXTEND_6
> > +# define ZERO_EXTEND_6 movl %r9d, %r9d;
> > +#else /* !__ASSEMBLER__ */
> >  # undef ARGIFY
> >  /* Enforce zero-extension for pointers and array system call arguments.
> >     For integer types, extend to int64_t (the full register) using a
> 
> The comment should come before the newly added changes, I think.

This comment is C specific.  I added

/* Zero-extend 32-bit unsigned long int arguments to 64 bits.  */

to the newly added ZERO_EXTEND_X micros.

Here is the updated patch.  OK for master?

Thanks.


H.J.
---
X32 has 32-bit long and pointer with 64-bit off_t.  Since x32 psABI
requires that pointers passed in registers must be zero-extended to
64bit, x32 can share many syscall interfaces with LP64.  When a LP64
syscall with long and unsigned long int arguments is used for x32, these
arguments must be properly extended to 64-bit.  Otherwise if the upper
32 bits of the register have undefined value, such a syscall will be
rejected by kernel.

For syscalls implemented in assembly codes, 'U' is added to syscall
signature key letters for unsigned long, which is zero-extended to
64-bit types.  SYSCALL_ULONG_ARG_1 and SYSCALL_ULONG_ARG_2 are passed
to syscall-template.S for the first and the second unsigned long int
arguments if PSEUDOS_HAVE_ULONG_INDICES is defined.  They are used by
x32 to zero-extend 32-bit arguments to 64 bits.

Tested on i386, x86-64 and x32 as well as with build-many-glibcs.py.
---
 sysdeps/unix/make-syscalls.sh               | 24 +++++++
 sysdeps/unix/syscall-template.S             | 49 +++++++++++++-
 sysdeps/unix/syscalls.list                  |  8 +--
 sysdeps/unix/sysv/linux/syscalls.list       | 16 ++---
 sysdeps/unix/sysv/linux/x86_64/sysdep.h     | 71 +++++++++++++++++----
 sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h | 21 +++++-
 6 files changed, 159 insertions(+), 30 deletions(-)

diff --git a/sysdeps/unix/make-syscalls.sh b/sysdeps/unix/make-syscalls.sh
index c07626677f..4f6c3490a2 100644
--- a/sysdeps/unix/make-syscalls.sh
+++ b/sysdeps/unix/make-syscalls.sh
@@ -30,6 +30,7 @@
 # P: optionally-NULL pointer to typed object (e.g., 3rd argument to sigaction)
 # s: non-NULL string (e.g., 1st arg to open)
 # S: optionally-NULL string (e.g., 1st arg to acct)
+# U: unsigned long int (32-bit types are zero-extended to 64-bit types)
 # v: vararg scalar (e.g., optional 3rd arg to open)
 # V: byte-per-page vector (3rd arg to mincore)
 # W: wait status, optionally-NULL pointer to int (e.g., 2nd arg of wait4)
@@ -184,6 +185,27 @@ while read file srcfile caller syscall args strong weak; do
   ?:?????????) nargs=9;;
   esac
 
+  # Derive the unsigned long int arguments from the argument signature
+  ulong_arg_1=0
+  ulong_arg_2=0
+  ulong_count=0
+  for U in $(echo $args | sed -e "s/.*:/:/" | grep -ob U)
+  do
+    ulong_count=$(expr $ulong_count + 1)
+    ulong_arg=$(echo $U | sed -e "s/:U//")
+    case $ulong_count in
+    1)
+      ulong_arg_1=$ulong_arg
+      ;;
+    2)
+      ulong_arg_2=$ulong_arg
+      ;;
+    *)
+      echo >&2 "$0: Too many unsigned long int arguments for syscall ($strong $weak)"
+      exit 2
+    esac
+  done
+
   # Make sure only the first syscall rule is used, if multiple dirs
   # define the same syscall.
   echo ''
@@ -245,6 +267,8 @@ while read file srcfile caller syscall args strong weak; do
 	\$(make-target-directory)
 	(echo '#define SYSCALL_NAME $syscall'; \\
 	 echo '#define SYSCALL_NARGS $nargs'; \\
+	 echo '#define SYSCALL_ULONG_ARG_1 $ulong_arg_1'; \\
+	 echo '#define SYSCALL_ULONG_ARG_2 $ulong_arg_2'; \\
 	 echo '#define SYSCALL_SYMBOL $strong'; \\
 	 echo '#define SYSCALL_NOERRNO $noerrno'; \\
 	 echo '#define SYSCALL_ERRVAL $errval'; \\
diff --git a/sysdeps/unix/syscall-template.S b/sysdeps/unix/syscall-template.S
index cf6c7a58fb..f807a8603f 100644
--- a/sysdeps/unix/syscall-template.S
+++ b/sysdeps/unix/syscall-template.S
@@ -25,6 +25,12 @@
    defining a few macros:
 	SYSCALL_NAME		syscall name
 	SYSCALL_NARGS		number of arguments this call takes
+	SYSCALL_ULONG_ARG_1	the first unsigned long int argument this
+				call takes.  0 means that there are no
+				unsigned long int arguments.
+	SYSCALL_ULONG_ARG_2	the second unsigned long int argument this
+				call takes.  0 means that there is at most
+				one unsigned long int argument.
 	SYSCALL_SYMBOL		primary symbol name
 	SYSCALL_NOERRNO		1 to define a no-errno version (see below)
 	SYSCALL_ERRVAL		1 to define an error-value version (see below)
@@ -44,9 +50,31 @@
 /* This indirection is needed so that SYMBOL gets macro-expanded.  */
 #define syscall_hidden_def(SYMBOL)		hidden_def (SYMBOL)
 
-#define T_PSEUDO(SYMBOL, NAME, N)		PSEUDO (SYMBOL, NAME, N)
-#define T_PSEUDO_NOERRNO(SYMBOL, NAME, N)	PSEUDO_NOERRNO (SYMBOL, NAME, N)
-#define T_PSEUDO_ERRVAL(SYMBOL, NAME, N)	PSEUDO_ERRVAL (SYMBOL, NAME, N)
+/* If PSEUDOS_HAVE_ULONG_INDICES is defined, PSEUDO and T_PSEUDO macros
+   have 2 extra arguments for unsigned long int arguments:
+     Extra argument 1: Position of the first unsigned long int argument.
+     Extra argument 2: Position of the second unsigned long int argument.
+ */
+#ifndef PSEUDOS_HAVE_ULONG_INDICES
+# undef SYSCALL_ULONG_ARG_1
+# define SYSCALL_ULONG_ARG_1 0
+#endif
+
+#if SYSCALL_ULONG_ARG_1
+# define T_PSEUDO(SYMBOL, NAME, N, U1, U2) \
+  PSEUDO (SYMBOL, NAME, N, U1, U2)
+# define T_PSEUDO_NOERRNO(SYMBOL, NAME, N, U1, U2) \
+  PSEUDO_NOERRNO (SYMBOL, NAME, N, U1, U2)
+# define T_PSEUDO_ERRVAL(SYMBOL, NAME, N, U1, U2) \
+  PSEUDO_ERRVAL (SYMBOL, NAME, N, U1, U2)
+#else
+# define T_PSEUDO(SYMBOL, NAME, N) \
+  PSEUDO (SYMBOL, NAME, N)
+# define T_PSEUDO_NOERRNO(SYMBOL, NAME, N) \
+  PSEUDO_NOERRNO (SYMBOL, NAME, N)
+# define T_PSEUDO_ERRVAL(SYMBOL, NAME, N) \
+  PSEUDO_ERRVAL (SYMBOL, NAME, N)
+#endif
 #define T_PSEUDO_END(SYMBOL)			PSEUDO_END (SYMBOL)
 #define T_PSEUDO_END_NOERRNO(SYMBOL)		PSEUDO_END_NOERRNO (SYMBOL)
 #define T_PSEUDO_END_ERRVAL(SYMBOL)		PSEUDO_END_ERRVAL (SYMBOL)
@@ -56,7 +84,12 @@
 /* This kind of system call stub never returns an error.
    We return the return value register to the caller unexamined.  */
 
+# if SYSCALL_ULONG_ARG_1
+T_PSEUDO_NOERRNO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS,
+		  SYSCALL_ULONG_ARG_1, SYSCALL_ULONG_ARG_2)
+# else
 T_PSEUDO_NOERRNO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
+# endif
 	ret_NOERRNO
 T_PSEUDO_END_NOERRNO (SYSCALL_SYMBOL)
 
@@ -66,7 +99,12 @@ T_PSEUDO_END_NOERRNO (SYSCALL_SYMBOL)
    value, or zero for success.  We may massage the kernel's return value
    to meet that ABI, but we never set errno here.  */
 
+# if SYSCALL_ULONG_ARG_1
+T_PSEUDO_ERRVAL (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS,
+		 SYSCALL_ULONG_ARG_1, SYSCALL_ULONG_ARG_2)
+# else
 T_PSEUDO_ERRVAL (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
+# endif
 	ret_ERRVAL
 T_PSEUDO_END_ERRVAL (SYSCALL_SYMBOL)
 
@@ -75,7 +113,12 @@ T_PSEUDO_END_ERRVAL (SYSCALL_SYMBOL)
 /* This is a "normal" system call stub: if there is an error,
    it returns -1 and sets errno.  */
 
+# if SYSCALL_ULONG_ARG_1
+T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS,
+	  SYSCALL_ULONG_ARG_1, SYSCALL_ULONG_ARG_2)
+# else
 T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
+# endif
 	ret
 T_PSEUDO_END (SYSCALL_SYMBOL)
 
diff --git a/sysdeps/unix/syscalls.list b/sysdeps/unix/syscalls.list
index 01c4a0e6b1..0cf290076d 100644
--- a/sysdeps/unix/syscalls.list
+++ b/sysdeps/unix/syscalls.list
@@ -37,16 +37,16 @@ kill		-	kill		i:ii	__kill		kill
 link		-	link		i:ss	__link		link
 listen		-	listen		i:ii	__listen	listen
 lseek		-	lseek		i:iii	__libc_lseek	__lseek lseek
-madvise		-	madvise		i:pii	__madvise	madvise
+madvise		-	madvise		i:pUi	__madvise	madvise
 mkdir		-	mkdir		i:si	__mkdir		mkdir
 mmap		-	mmap		b:aniiii __mmap		mmap
-mprotect	-	mprotect	i:aii	__mprotect	mprotect
-munmap		-	munmap		i:ai	__munmap	munmap
+mprotect	-	mprotect	i:aUi	__mprotect	mprotect
+munmap		-	munmap		i:aU	__munmap	munmap
 open		-	open		Ci:siv	__libc_open __open open
 profil		-	profil		i:piii	__profil	profil
 ptrace		-	ptrace		i:iiii	ptrace
 read		-	read		Ci:ibn	__libc_read	__read read
-readlink	-	readlink	i:spi	__readlink	readlink
+readlink	-	readlink	i:spU	__readlink	readlink
 readv		-	readv		Ci:ipi	__readv		readv
 reboot		-	reboot		i:i	reboot
 recv		-	recv		Ci:ibni	__libc_recv	recv
diff --git a/sysdeps/unix/sysv/linux/syscalls.list b/sysdeps/unix/sysv/linux/syscalls.list
index e40f993495..1b1010d4c8 100644
--- a/sysdeps/unix/sysv/linux/syscalls.list
+++ b/sysdeps/unix/sysv/linux/syscalls.list
@@ -32,12 +32,12 @@ ioperm		-	ioperm		i:iii	ioperm
 iopl		-	iopl		i:i	iopl
 klogctl		EXTRA	syslog		i:isi	klogctl
 lchown		-	lchown		i:sii	__lchown	lchown
-mincore		-	mincore		i:anV	mincore
-mlock		-	mlock		i:bn	mlock
+mincore		-	mincore		i:aUV	mincore
+mlock		-	mlock		i:bU	mlock
 mlockall	-	mlockall	i:i	mlockall
-mount		EXTRA	mount		i:sssip	__mount	mount
-mremap		EXTRA	mremap		b:ainip	__mremap	mremap
-munlock		-	munlock		i:ai	munlock
+mount		EXTRA	mount		i:sssUp	__mount	mount
+mremap		EXTRA	mremap		b:aUUip	__mremap	mremap
+munlock		-	munlock		i:aU	munlock
 munlockall	-	munlockall	i:	munlockall
 nfsservctl	EXTRA	nfsservctl	i:ipp	__compat_nfsservctl	nfsservctl@GLIBC_2.0:GLIBC_2.28
 pipe		-	pipe		i:f	__pipe		pipe
@@ -46,7 +46,7 @@ pivot_root	EXTRA	pivot_root	i:ss	pivot_root
 prctl		EXTRA	prctl		i:iiiii	__prctl		prctl
 query_module	EXTRA	query_module	i:sipip	__compat_query_module	query_module@GLIBC_2.0:GLIBC_2.23
 quotactl	EXTRA	quotactl	i:isip	quotactl
-remap_file_pages -	remap_file_pages i:piiii	__remap_file_pages remap_file_pages
+remap_file_pages -	remap_file_pages i:pUiUi	__remap_file_pages remap_file_pages
 sched_getp	-	sched_getparam	i:ip	__sched_getparam	sched_getparam
 sched_gets	-	sched_getscheduler	i:i	__sched_getscheduler	sched_getscheduler
 sched_primax	-	sched_get_priority_max	i:i	__sched_get_priority_max	sched_get_priority_max
@@ -54,7 +54,7 @@ sched_primin	-	sched_get_priority_min	i:i	__sched_get_priority_min	sched_get_pri
 sched_setp	-	sched_setparam	i:ip	__sched_setparam	sched_setparam
 sched_sets	-	sched_setscheduler	i:iip	__sched_setscheduler	sched_setscheduler
 sched_yield	-	sched_yield	i:	__sched_yield	sched_yield
-sendfile	-	sendfile	i:iipi	sendfile
+sendfile	-	sendfile	i:iipU	sendfile
 sendfile64	-	sendfile64	i:iipi	sendfile64
 setfsgid	EXTRA	setfsgid	i:i	setfsgid
 setfsuid	EXTRA	setfsuid	i:i	setfsuid
@@ -71,7 +71,7 @@ chown		-	chown		i:sii	__libc_chown	__chown chown
 fchownat	-	fchownat	i:isiii	fchownat
 linkat		-	linkat		i:isisi	linkat
 mkdirat		-	mkdirat		i:isi	mkdirat
-readlinkat	-	readlinkat	i:issi	readlinkat
+readlinkat	-	readlinkat	i:issU	readlinkat
 symlinkat	-	symlinkat	i:sis	symlinkat
 unlinkat	-	unlinkat	i:isi	unlinkat
 
diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
index 85f8f94820..bf36875477 100644
--- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h
+++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
@@ -57,13 +57,31 @@
 #  define SYSCALL_ERROR_LABEL syscall_error
 # endif
 
+/* PSEUDO and T_PSEUDO macros have 2 extra arguments for unsigned long
+   int arguments.  */
+# define PSEUDOS_HAVE_ULONG_INDICES 1
+
+# ifndef SYSCALL_ULONG_ARG_1
+#  define SYSCALL_ULONG_ARG_1 0
+#  define SYSCALL_ULONG_ARG_2 0
+# endif
+
 # undef	PSEUDO
-# define PSEUDO(name, syscall_name, args)				      \
-  .text;								      \
-  ENTRY (name)								      \
-    DO_CALL (syscall_name, args);					      \
-    cmpq $-4095, %rax;							      \
+# if SYSCALL_ULONG_ARG_1
+#  define PSEUDO(name, syscall_name, args, ulong_arg_1, ulong_arg_2) \
+  .text;							      \
+  ENTRY (name)							      \
+    DO_CALL (syscall_name, args, ulong_arg_1, ulong_arg_2);	      \
+    cmpq $-4095, %rax;						      \
     jae SYSCALL_ERROR_LABEL
+# else
+#  define PSEUDO(name, syscall_name, args) \
+  .text;							      \
+  ENTRY (name)							      \
+    DO_CALL (syscall_name, args, 0, 0);				      \
+    cmpq $-4095, %rax;						      \
+    jae SYSCALL_ERROR_LABEL
+# endif
 
 # undef	PSEUDO_END
 # define PSEUDO_END(name)						      \
@@ -71,10 +89,17 @@
   END (name)
 
 # undef	PSEUDO_NOERRNO
-# define PSEUDO_NOERRNO(name, syscall_name, args) \
-  .text;								      \
-  ENTRY (name)								      \
-    DO_CALL (syscall_name, args)
+# if SYSCALL_ULONG_ARG_1
+#  define PSEUDO_NOERRNO(name, syscall_name, args, ulong_arg_1, ulong_arg_2) \
+  .text;							      \
+  ENTRY (name)							      \
+    DO_CALL (syscall_name, args, ulong_arg_1, ulong_arg_2)
+# else
+#  define PSEUDO_NOERRNO(name, syscall_name, args) \
+  .text;							      \
+  ENTRY (name)							      \
+    DO_CALL (syscall_name, args, 0, 0)
+# endif
 
 # undef	PSEUDO_END_NOERRNO
 # define PSEUDO_END_NOERRNO(name) \
@@ -83,11 +108,19 @@
 # define ret_NOERRNO ret
 
 # undef	PSEUDO_ERRVAL
-# define PSEUDO_ERRVAL(name, syscall_name, args) \
-  .text;								      \
-  ENTRY (name)								      \
-    DO_CALL (syscall_name, args);					      \
+# if SYSCALL_ULONG_ARG_1
+#  define PSEUDO_ERRVAL(name, syscall_name, args, ulong_arg_1, ulong_arg_2) \
+  .text;							\
+  ENTRY (name)							\
+    DO_CALL (syscall_name, args, ulong_arg_1, ulong_arg_2);	\
+    negq %rax
+# else
+#  define PSEUDO_ERRVAL(name, syscall_name, args) \
+  .text;							\
+  ENTRY (name)							\
+    DO_CALL (syscall_name, args, 0, 0);				\
     negq %rax
+# endif
 
 # undef	PSEUDO_END_ERRVAL
 # define PSEUDO_END_ERRVAL(name) \
@@ -159,8 +192,10 @@
     Syscalls of more than 6 arguments are not supported.  */
 
 # undef	DO_CALL
-# define DO_CALL(syscall_name, args)		\
+# define DO_CALL(syscall_name, args, ulong_arg_1, ulong_arg_2) \
     DOARGS_##args				\
+    ZERO_EXTEND_##ulong_arg_1			\
+    ZERO_EXTEND_##ulong_arg_2			\
     movl $SYS_ify (syscall_name), %eax;		\
     syscall;
 
@@ -172,6 +207,14 @@
 # define DOARGS_5 DOARGS_4
 # define DOARGS_6 DOARGS_5
 
+# define ZERO_EXTEND_0 /* nothing */
+# define ZERO_EXTEND_1 /* nothing */
+# define ZERO_EXTEND_2 /* nothing */
+# define ZERO_EXTEND_3 /* nothing */
+# define ZERO_EXTEND_4 /* nothing */
+# define ZERO_EXTEND_5 /* nothing */
+# define ZERO_EXTEND_6 /* nothing */
+
 #else	/* !__ASSEMBLER__ */
 
 /* Registers clobbered by syscall.  */
diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
index a37d520f86..62e6f8fe11 100644
--- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
+++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
@@ -26,7 +26,26 @@
 #undef LO_HI_LONG
 #define LO_HI_LONG(val) (val)
 
-#ifndef __ASSEMBLER__
+#ifdef __ASSEMBLER__
+/* Zero-extend 32-bit unsigned long int arguments to 64 bits.  */
+# undef ZERO_EXTEND_1
+# define ZERO_EXTEND_1 movl %edi, %edi;
+# undef ZERO_EXTEND_2
+# define ZERO_EXTEND_2 movl %esi, %esi;
+# undef ZERO_EXTEND_3
+# define ZERO_EXTEND_3 movl %edx, %edx;
+# if SYSCALL_ULONG_ARG_1 == 4 || SYSCALL_ULONG_ARG_2 == 4
+#  undef DOARGS_4
+#  define DOARGS_4 movl %ecx, %r10d;
+# else
+#  undef ZERO_EXTEND_4
+#  define ZERO_EXTEND_4 movl %r10d, %r10d;
+# endif
+# undef ZERO_EXTEND_5
+# define ZERO_EXTEND_5 movl %r8d, %r8d;
+# undef ZERO_EXTEND_6
+# define ZERO_EXTEND_6 movl %r9d, %r9d;
+#else /* !__ASSEMBLER__ */
 # undef ARGIFY
 /* Enforce zero-extension for pointers and array system call arguments.
    For integer types, extend to int64_t (the full register) using a
-- 
2.25.3

Re: V2 [PATCH 1/2] Add SYSCALL_ULONG_ARG_[12] to pass long to syscall [BZ #25810]

[Joseph Myers]

On Wed, 22 Apr 2020, Adhemerval Zanella via Libc-alpha wrote:

> > I must say that I find this really, really ugly.  We should rewrite
> > this in Python as soon as possible (in a separate patch).
> 
> And I think we should make long term gold to just get rid of this
> assembly macro and focus on automatic generation to a C code file
> as well.

There are a couple of possible improvements in this area that might be in 
tension here:

* Simplifying how the syscall arguments are specified.  The reason there 
are so many different letters is not because of anything relevant to 
assembly code generation now, it's because of the old support for bounded 
pointers (removed from GCC in 2002, actual substantive uses in the glibc 
syscall generation code removed in 
<https://sourceware.org/legacy-ml/libc-alpha/2013-01/msg00919.html>).

* Generating debug info for the automatically generated syscall 
implementations.  For that, the C code should have actual meaningful 
argument types, not just a sequence of integers that has the right ABI 
(but may not even have the right number of arguments, in cases where a 
64-bit argument uses two letters for syscall arguments in the 32-bit 
syscalls.list files).

What this suggests to me is that automatically-generated C code should get 
actual argument types from the header files in some way, and in the cases 
where header file information isn't sufficient (including syscalls not 
declared in header files or with different arguments to the public C 
function, etc.), using some sort of internal header to give the types 
might be better than having complicated encodings in syscalls.list.  Maybe 
syscalls.list doesn't need much more information than the conventions for 
return value / error handling and the *number* of arguments, if types can 
be extracted from C headers.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: V2 [PATCH 1/2] Add SYSCALL_ULONG_ARG_[12] to pass long to syscall [BZ #25810]

[Adhemerval Zanella]

On 22/04/2020 19:01, Joseph Myers wrote:
> On Wed, 22 Apr 2020, Adhemerval Zanella via Libc-alpha wrote:
> 
>>> I must say that I find this really, really ugly.  We should rewrite
>>> this in Python as soon as possible (in a separate patch).
>>
>> And I think we should make long term gold to just get rid of this
>> assembly macro and focus on automatic generation to a C code file
>> as well.
> 
> There are a couple of possible improvements in this area that might be in 
> tension here:
> 
> * Simplifying how the syscall arguments are specified.  The reason there 
> are so many different letters is not because of anything relevant to 
> assembly code generation now, it's because of the old support for bounded 
> pointers (removed from GCC in 2002, actual substantive uses in the glibc 
> syscall generation code removed in 
> <https://sourceware.org/legacy-ml/libc-alpha/2013-01/msg00919.html>).
> 
> * Generating debug info for the automatically generated syscall 
> implementations.  For that, the C code should have actual meaningful 
> argument types, not just a sequence of integers that has the right ABI 
> (but may not even have the right number of arguments, in cases where a 
> 64-bit argument uses two letters for syscall arguments in the 32-bit 
> syscalls.list files).
> 
> What this suggests to me is that automatically-generated C code should get 
> actual argument types from the header files in some way, and in the cases 
> where header file information isn't sufficient (including syscalls not 
> declared in header files or with different arguments to the public C 
> function, etc.), using some sort of internal header to give the types 
> might be better than having complicated encodings in syscalls.list.  Maybe 
> syscalls.list doesn't need much more information than the conventions for 
> return value / error handling and the *number* of arguments, if types can 
> be extracted from C headers.

My initial approach would be to extend the syscall argument definition 
on syscall file description (syscalls.list) with all the required 
information to synthesize the C implementation.  It increases the 
verbose required in the argument description, but I think it would
simpler than try infer from the header itself (which would require to 
parse the it and the this information). 

Something like, using wordsize-64 fstatfs syscalls.list entry:

  fstatfs fstatfs [sys/statfs.h] [const char *, struct statfs64 *] [fstatfs64:w, __fstatfs64:w]

which would translate to:

  #define fstatfs64 __redirect_fstatfs64
  #define __fstatfs64 __redirect___fstatfs64
  #include <sys/statfs.h>
  #undef fstatfs64
  #undef __fstatfs64

  int
  __fstatfs (int arg1, struct statfs *arg2)
  { 
   return INLINE_SYSCALL_CALL (fstatfs, arg1, arg2);
  }
  weak_alias (__fstatfs, fstatfs)
  weak_alias (__fstatfs, __fstatfs64)
  weak_alias (__fstatfs, fstatfs64)

The alias would require some pre-defined logic to handle the LFS
alias for __OFF_T_MATCHES_OFF64_T (and I am not sure if this
example would be better handled in a C implementation in fact),
to handle internal hidden alias, and version.

PING: V3 [PATCH 1/2] Add SYSCALL_ULONG_ARG_[12] to pass long to syscall [BZ #25810]

[H.J. Lu]

On Wed, Apr 22, 2020 at 9:09 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Wed, Apr 22, 2020 at 12:42:37PM +0200, Florian Weimer wrote:
> > * H. J. Lu via Libc-alpha:
> >
> > > diff --git a/sysdeps/unix/make-syscalls.sh b/sysdeps/unix/make-syscalls.sh
> > > index c07626677f..4a28badea6 100644
> > > --- a/sysdeps/unix/make-syscalls.sh
> > > +++ b/sysdeps/unix/make-syscalls.sh
> > > @@ -30,6 +30,7 @@
> > >  # P: optionally-NULL pointer to typed object (e.g., 3rd argument to sigaction)
> > >  # s: non-NULL string (e.g., 1st arg to open)
> > >  # S: optionally-NULL string (e.g., 1st arg to acct)
> > > +# U: unsigned long
> >
> > Maybe this?
> >
> > # U: unsigned long int (32-bit types are zero-extended to 64-bit types)
>
> Fixed.
>
> >
> > >  # v: vararg scalar (e.g., optional 3rd arg to open)
> > >  # V: byte-per-page vector (3rd arg to mincore)
> > >  # W: wait status, optionally-NULL pointer to int (e.g., 2nd arg of wait4)
> > > @@ -184,6 +185,91 @@ while read file srcfile caller syscall args strong weak; do
> > >    ?:?????????) nargs=9;;
> > >    esac
> > >
> > > +  # Derive the unsigned long arguments from the argument signature
> >
> > “unsigned long int”
>
> Fixed.
>
> >
> > > +  ulong_arg_1=0
> > > +  ulong_arg_2=0
> > > +  case $args in
> > > +  ?:U*)
> > > +    ulong_arg_1=1
> > > +    case $args in
> > > +    ?:UU*) ulong_arg_2=2;;
> > > +    ?:U?U*) ulong_arg_2=3;;
> > > +    ?:U??U*) ulong_arg_2=4;;
> > > +    ?:U???U*) ulong_arg_2=5;;
> > > +    ?:U????U*) ulong_arg_2=6;;
> > > +    ?:U?????U*) ulong_arg_2=7;;
> > > +    ?:U??????U*) ulong_arg_2=8;;
> > > +    ?:U???????U) ulong_arg_2=9;;
> > > +    esac
> > > +    ;;
> >
> > > +  ?:????????U)
> > > +    ulong_arg_1=9
> > > +    ;;
> > > +  esac
> > > +
> >
> > I must say that I find this really, really ugly.  We should rewrite
> > this in Python as soon as possible (in a separate patch).
> >
> > You could try this instead:
> >
> > $ echo U1U | grep -ob U
> > 0:U
> > 2:U
>
> Fixed.
>
> >
> > And maybe guard it with a case match, so that performance does not
> > suffer too much.
>
> I didn't notice any significant build timing change.
>
> >
> > In any case, there should be an error check that covers the more
> > than-two-Us case.
> >
>
> Fixed.
>
> >
> > >    # Make sure only the first syscall rule is used, if multiple dirs
> > >    # define the same syscall.
> > >    echo ''
> > > @@ -245,6 +331,8 @@ while read file srcfile caller syscall args strong weak; do
> > >     \$(make-target-directory)
> > >     (echo '#define SYSCALL_NAME $syscall'; \\
> > >      echo '#define SYSCALL_NARGS $nargs'; \\
> > > +    echo '#define SYSCALL_ULONG_ARG_1 $ulong_arg_1'; \\
> > > +    echo '#define SYSCALL_ULONG_ARG_2 $ulong_arg_2'; \\
> >
> > Should this be conditional on whether $ulong_arg_1 and $ulong_arg_2
> > are empty?  I think that might be less confusing.
> >
> > Otherwise, the comment at the beginning should mention the special
> > value zero.
>
> I updated comments of SYSCALL_ULONG_ARG_1 and SYSCALL_ULONG_ARG_2 in
> syscall-template.S.
>
> >
> > >      echo '#define SYSCALL_SYMBOL $strong'; \\
> > >      echo '#define SYSCALL_NOERRNO $noerrno'; \\
> > >      echo '#define SYSCALL_ERRVAL $errval'; \\
> > > diff --git a/sysdeps/unix/syscall-template.S b/sysdeps/unix/syscall-template.S
> > > index cf6c7a58fb..0824a3c61e 100644
> > > --- a/sysdeps/unix/syscall-template.S
> > > +++ b/sysdeps/unix/syscall-template.S
> > > @@ -25,6 +25,10 @@
> > >     defining a few macros:
> > >     SYSCALL_NAME            syscall name
> > >     SYSCALL_NARGS           number of arguments this call takes
> > > +   SYSCALL_ULONG_ARG_1     the first unsigned long argument this
> > > +                           call takes
> > > +   SYSCALL_ULONG_ARG_2     the second unsigned long argument this
> > > +                           call takes
> >
> > “unsigned long int”
>
> Fixed.
>
> >
> > >     SYSCALL_SYMBOL          primary symbol name
> > >     SYSCALL_NOERRNO         1 to define a no-errno version (see below)
> > >     SYSCALL_ERRVAL          1 to define an error-value version (see below)
> > > @@ -44,9 +48,27 @@
> > >  /* This indirection is needed so that SYMBOL gets macro-expanded.  */
> > >  #define syscall_hidden_def(SYMBOL)         hidden_def (SYMBOL)
> > >
> > > -#define T_PSEUDO(SYMBOL, NAME, N)          PSEUDO (SYMBOL, NAME, N)
> > > -#define T_PSEUDO_NOERRNO(SYMBOL, NAME, N)  PSEUDO_NOERRNO (SYMBOL, NAME, N)
> > > -#define T_PSEUDO_ERRVAL(SYMBOL, NAME, N)   PSEUDO_ERRVAL (SYMBOL, NAME, N)
> > > +/* If PSEUDOS_HAVE_4_ARGS is defined, PSEUDO macros have 4 arguments.  */
> > > +#ifndef PSEUDOS_HAVE_4_ARGS
> > > +# undef SYSCALL_ULONG_ARG_1
> > > +# define SYSCALL_ULONG_ARG_1 0
> > > +#endif
> >
> > PSEUDOS_HAVE_4_ARGS should be PSEUDOS_HAVE_ULONG_INDICES or something
> > like that.  And a comment that briefly explains all the macro
> > arguments.  (Not sure if T_PSEUDO is documented somewhere else
> > already.)
>
> Fixed.
>
> >
> > > diff --git a/sysdeps/unix/syscalls.list b/sysdeps/unix/syscalls.list
> > > index 01c4a0e6b1..e63a8b9d23 100644
> > > --- a/sysdeps/unix/syscalls.list
> > > +++ b/sysdeps/unix/syscalls.list
> > > @@ -37,11 +37,11 @@ kill            -       kill            i:ii    __kill          kill
> > >  link               -       link            i:ss    __link          link
> > >  listen             -       listen          i:ii    __listen        listen
> > >  lseek              -       lseek           i:iii   __libc_lseek    __lseek lseek
> > > -madvise            -       madvise         i:pii   __madvise       madvise
> > > +madvise            -       madvise         i:pUi   __madvise       madvise
> > >  mkdir              -       mkdir           i:si    __mkdir         mkdir
> > >  mmap               -       mmap            b:aniiii __mmap         mmap
> > > -mprotect   -       mprotect        i:aii   __mprotect      mprotect
> > > -munmap             -       munmap          i:ai    __munmap        munmap
> > > +mprotect   -       mprotect        i:aUi   __mprotect      mprotect
> > > +munmap             -       munmap          i:aU    __munmap        munmap
> > >  open               -       open            Ci:siv  __libc_open __open open
> > >  profil             -       profil          i:piii  __profil        profil
> > >  ptrace             -       ptrace          i:iiii  ptrace
> >
> > What about read, readlink, write, etc.?
>
> I updated readlink and readlinkat.  read/write are implemented as inlined
> SYSCALLs.
>
> >
> > > diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> > > index a37d520f86..b8a74ad2c2 100644
> > > --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> > > +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> > > @@ -26,7 +26,25 @@
> > >  #undef LO_HI_LONG
> > >  #define LO_HI_LONG(val) (val)
> > >
> > > -#ifndef __ASSEMBLER__
> > > +#ifdef __ASSEMBLER__
> > > +# undef ZERO_EXTEND_1
> > > +# define ZERO_EXTEND_1 movl %edi, %edi;
> > > +# undef ZERO_EXTEND_2
> > > +# define ZERO_EXTEND_2 movl %esi, %esi;
> > > +# undef ZERO_EXTEND_3
> > > +# define ZERO_EXTEND_3 movl %edx, %edx;
> > > +# if SYSCALL_ULONG_ARG_1 == 4 || SYSCALL_ULONG_ARG_2 == 4
> > > +#  undef DOARGS_4
> > > +#  define DOARGS_4 movl %ecx, %r10d;
> > > +# else
> > > +#  undef ZERO_EXTEND_4
> > > +#  define ZERO_EXTEND_4 movl %r10d, %r10d;
> > > +# endif
> > > +# undef ZERO_EXTEND_5
> > > +# define ZERO_EXTEND_5 movl %r8d, %r8d;
> > > +# undef ZERO_EXTEND_6
> > > +# define ZERO_EXTEND_6 movl %r9d, %r9d;
> > > +#else /* !__ASSEMBLER__ */
> > >  # undef ARGIFY
> > >  /* Enforce zero-extension for pointers and array system call arguments.
> > >     For integer types, extend to int64_t (the full register) using a
> >
> > The comment should come before the newly added changes, I think.
>
> This comment is C specific.  I added
>
> /* Zero-extend 32-bit unsigned long int arguments to 64 bits.  */
>
> to the newly added ZERO_EXTEND_X micros.
>
> Here is the updated patch.  OK for master?
>
> Thanks.
>
>
> H.J.
> ---
> X32 has 32-bit long and pointer with 64-bit off_t.  Since x32 psABI
> requires that pointers passed in registers must be zero-extended to
> 64bit, x32 can share many syscall interfaces with LP64.  When a LP64
> syscall with long and unsigned long int arguments is used for x32, these
> arguments must be properly extended to 64-bit.  Otherwise if the upper
> 32 bits of the register have undefined value, such a syscall will be
> rejected by kernel.
>
> For syscalls implemented in assembly codes, 'U' is added to syscall
> signature key letters for unsigned long, which is zero-extended to
> 64-bit types.  SYSCALL_ULONG_ARG_1 and SYSCALL_ULONG_ARG_2 are passed
> to syscall-template.S for the first and the second unsigned long int
> arguments if PSEUDOS_HAVE_ULONG_INDICES is defined.  They are used by
> x32 to zero-extend 32-bit arguments to 64 bits.
>
> Tested on i386, x86-64 and x32 as well as with build-many-glibcs.py.
> ---
>  sysdeps/unix/make-syscalls.sh               | 24 +++++++
>  sysdeps/unix/syscall-template.S             | 49 +++++++++++++-
>  sysdeps/unix/syscalls.list                  |  8 +--
>  sysdeps/unix/sysv/linux/syscalls.list       | 16 ++---
>  sysdeps/unix/sysv/linux/x86_64/sysdep.h     | 71 +++++++++++++++++----
>  sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h | 21 +++++-
>  6 files changed, 159 insertions(+), 30 deletions(-)

PING:

https://sourceware.org/pipermail/libc-alpha/2020-April/113036.html

I want to keep changes to make-syscalls.sh and syscall-template.S to
minimum so that they can be backported.  Any improvement to syscall
generation should be done after this bug fix.

Thanks.

H.J.

PING^2: V3 [PATCH 1/2] Add SYSCALL_ULONG_ARG_[12] to pass long to syscall [BZ #25810]

[H.J. Lu]

On Sat, Apr 25, 2020 at 6:56 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Wed, Apr 22, 2020 at 9:09 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Wed, Apr 22, 2020 at 12:42:37PM +0200, Florian Weimer wrote:
> > > * H. J. Lu via Libc-alpha:
> > >
> > > > diff --git a/sysdeps/unix/make-syscalls.sh b/sysdeps/unix/make-syscalls.sh
> > > > index c07626677f..4a28badea6 100644
> > > > --- a/sysdeps/unix/make-syscalls.sh
> > > > +++ b/sysdeps/unix/make-syscalls.sh
> > > > @@ -30,6 +30,7 @@
> > > >  # P: optionally-NULL pointer to typed object (e.g., 3rd argument to sigaction)
> > > >  # s: non-NULL string (e.g., 1st arg to open)
> > > >  # S: optionally-NULL string (e.g., 1st arg to acct)
> > > > +# U: unsigned long
> > >
> > > Maybe this?
> > >
> > > # U: unsigned long int (32-bit types are zero-extended to 64-bit types)
> >
> > Fixed.
> >
> > >
> > > >  # v: vararg scalar (e.g., optional 3rd arg to open)
> > > >  # V: byte-per-page vector (3rd arg to mincore)
> > > >  # W: wait status, optionally-NULL pointer to int (e.g., 2nd arg of wait4)
> > > > @@ -184,6 +185,91 @@ while read file srcfile caller syscall args strong weak; do
> > > >    ?:?????????) nargs=9;;
> > > >    esac
> > > >
> > > > +  # Derive the unsigned long arguments from the argument signature
> > >
> > > “unsigned long int”
> >
> > Fixed.
> >
> > >
> > > > +  ulong_arg_1=0
> > > > +  ulong_arg_2=0
> > > > +  case $args in
> > > > +  ?:U*)
> > > > +    ulong_arg_1=1
> > > > +    case $args in
> > > > +    ?:UU*) ulong_arg_2=2;;
> > > > +    ?:U?U*) ulong_arg_2=3;;
> > > > +    ?:U??U*) ulong_arg_2=4;;
> > > > +    ?:U???U*) ulong_arg_2=5;;
> > > > +    ?:U????U*) ulong_arg_2=6;;
> > > > +    ?:U?????U*) ulong_arg_2=7;;
> > > > +    ?:U??????U*) ulong_arg_2=8;;
> > > > +    ?:U???????U) ulong_arg_2=9;;
> > > > +    esac
> > > > +    ;;
> > >
> > > > +  ?:????????U)
> > > > +    ulong_arg_1=9
> > > > +    ;;
> > > > +  esac
> > > > +
> > >
> > > I must say that I find this really, really ugly.  We should rewrite
> > > this in Python as soon as possible (in a separate patch).
> > >
> > > You could try this instead:
> > >
> > > $ echo U1U | grep -ob U
> > > 0:U
> > > 2:U
> >
> > Fixed.
> >
> > >
> > > And maybe guard it with a case match, so that performance does not
> > > suffer too much.
> >
> > I didn't notice any significant build timing change.
> >
> > >
> > > In any case, there should be an error check that covers the more
> > > than-two-Us case.
> > >
> >
> > Fixed.
> >
> > >
> > > >    # Make sure only the first syscall rule is used, if multiple dirs
> > > >    # define the same syscall.
> > > >    echo ''
> > > > @@ -245,6 +331,8 @@ while read file srcfile caller syscall args strong weak; do
> > > >     \$(make-target-directory)
> > > >     (echo '#define SYSCALL_NAME $syscall'; \\
> > > >      echo '#define SYSCALL_NARGS $nargs'; \\
> > > > +    echo '#define SYSCALL_ULONG_ARG_1 $ulong_arg_1'; \\
> > > > +    echo '#define SYSCALL_ULONG_ARG_2 $ulong_arg_2'; \\
> > >
> > > Should this be conditional on whether $ulong_arg_1 and $ulong_arg_2
> > > are empty?  I think that might be less confusing.
> > >
> > > Otherwise, the comment at the beginning should mention the special
> > > value zero.
> >
> > I updated comments of SYSCALL_ULONG_ARG_1 and SYSCALL_ULONG_ARG_2 in
> > syscall-template.S.
> >
> > >
> > > >      echo '#define SYSCALL_SYMBOL $strong'; \\
> > > >      echo '#define SYSCALL_NOERRNO $noerrno'; \\
> > > >      echo '#define SYSCALL_ERRVAL $errval'; \\
> > > > diff --git a/sysdeps/unix/syscall-template.S b/sysdeps/unix/syscall-template.S
> > > > index cf6c7a58fb..0824a3c61e 100644
> > > > --- a/sysdeps/unix/syscall-template.S
> > > > +++ b/sysdeps/unix/syscall-template.S
> > > > @@ -25,6 +25,10 @@
> > > >     defining a few macros:
> > > >     SYSCALL_NAME            syscall name
> > > >     SYSCALL_NARGS           number of arguments this call takes
> > > > +   SYSCALL_ULONG_ARG_1     the first unsigned long argument this
> > > > +                           call takes
> > > > +   SYSCALL_ULONG_ARG_2     the second unsigned long argument this
> > > > +                           call takes
> > >
> > > “unsigned long int”
> >
> > Fixed.
> >
> > >
> > > >     SYSCALL_SYMBOL          primary symbol name
> > > >     SYSCALL_NOERRNO         1 to define a no-errno version (see below)
> > > >     SYSCALL_ERRVAL          1 to define an error-value version (see below)
> > > > @@ -44,9 +48,27 @@
> > > >  /* This indirection is needed so that SYMBOL gets macro-expanded.  */
> > > >  #define syscall_hidden_def(SYMBOL)         hidden_def (SYMBOL)
> > > >
> > > > -#define T_PSEUDO(SYMBOL, NAME, N)          PSEUDO (SYMBOL, NAME, N)
> > > > -#define T_PSEUDO_NOERRNO(SYMBOL, NAME, N)  PSEUDO_NOERRNO (SYMBOL, NAME, N)
> > > > -#define T_PSEUDO_ERRVAL(SYMBOL, NAME, N)   PSEUDO_ERRVAL (SYMBOL, NAME, N)
> > > > +/* If PSEUDOS_HAVE_4_ARGS is defined, PSEUDO macros have 4 arguments.  */
> > > > +#ifndef PSEUDOS_HAVE_4_ARGS
> > > > +# undef SYSCALL_ULONG_ARG_1
> > > > +# define SYSCALL_ULONG_ARG_1 0
> > > > +#endif
> > >
> > > PSEUDOS_HAVE_4_ARGS should be PSEUDOS_HAVE_ULONG_INDICES or something
> > > like that.  And a comment that briefly explains all the macro
> > > arguments.  (Not sure if T_PSEUDO is documented somewhere else
> > > already.)
> >
> > Fixed.
> >
> > >
> > > > diff --git a/sysdeps/unix/syscalls.list b/sysdeps/unix/syscalls.list
> > > > index 01c4a0e6b1..e63a8b9d23 100644
> > > > --- a/sysdeps/unix/syscalls.list
> > > > +++ b/sysdeps/unix/syscalls.list
> > > > @@ -37,11 +37,11 @@ kill            -       kill            i:ii    __kill          kill
> > > >  link               -       link            i:ss    __link          link
> > > >  listen             -       listen          i:ii    __listen        listen
> > > >  lseek              -       lseek           i:iii   __libc_lseek    __lseek lseek
> > > > -madvise            -       madvise         i:pii   __madvise       madvise
> > > > +madvise            -       madvise         i:pUi   __madvise       madvise
> > > >  mkdir              -       mkdir           i:si    __mkdir         mkdir
> > > >  mmap               -       mmap            b:aniiii __mmap         mmap
> > > > -mprotect   -       mprotect        i:aii   __mprotect      mprotect
> > > > -munmap             -       munmap          i:ai    __munmap        munmap
> > > > +mprotect   -       mprotect        i:aUi   __mprotect      mprotect
> > > > +munmap             -       munmap          i:aU    __munmap        munmap
> > > >  open               -       open            Ci:siv  __libc_open __open open
> > > >  profil             -       profil          i:piii  __profil        profil
> > > >  ptrace             -       ptrace          i:iiii  ptrace
> > >
> > > What about read, readlink, write, etc.?
> >
> > I updated readlink and readlinkat.  read/write are implemented as inlined
> > SYSCALLs.
> >
> > >
> > > > diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> > > > index a37d520f86..b8a74ad2c2 100644
> > > > --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> > > > +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> > > > @@ -26,7 +26,25 @@
> > > >  #undef LO_HI_LONG
> > > >  #define LO_HI_LONG(val) (val)
> > > >
> > > > -#ifndef __ASSEMBLER__
> > > > +#ifdef __ASSEMBLER__
> > > > +# undef ZERO_EXTEND_1
> > > > +# define ZERO_EXTEND_1 movl %edi, %edi;
> > > > +# undef ZERO_EXTEND_2
> > > > +# define ZERO_EXTEND_2 movl %esi, %esi;
> > > > +# undef ZERO_EXTEND_3
> > > > +# define ZERO_EXTEND_3 movl %edx, %edx;
> > > > +# if SYSCALL_ULONG_ARG_1 == 4 || SYSCALL_ULONG_ARG_2 == 4
> > > > +#  undef DOARGS_4
> > > > +#  define DOARGS_4 movl %ecx, %r10d;
> > > > +# else
> > > > +#  undef ZERO_EXTEND_4
> > > > +#  define ZERO_EXTEND_4 movl %r10d, %r10d;
> > > > +# endif
> > > > +# undef ZERO_EXTEND_5
> > > > +# define ZERO_EXTEND_5 movl %r8d, %r8d;
> > > > +# undef ZERO_EXTEND_6
> > > > +# define ZERO_EXTEND_6 movl %r9d, %r9d;
> > > > +#else /* !__ASSEMBLER__ */
> > > >  # undef ARGIFY
> > > >  /* Enforce zero-extension for pointers and array system call arguments.
> > > >     For integer types, extend to int64_t (the full register) using a
> > >
> > > The comment should come before the newly added changes, I think.
> >
> > This comment is C specific.  I added
> >
> > /* Zero-extend 32-bit unsigned long int arguments to 64 bits.  */
> >
> > to the newly added ZERO_EXTEND_X micros.
> >
> > Here is the updated patch.  OK for master?
> >
> > Thanks.
> >
> >
> > H.J.
> > ---
> > X32 has 32-bit long and pointer with 64-bit off_t.  Since x32 psABI
> > requires that pointers passed in registers must be zero-extended to
> > 64bit, x32 can share many syscall interfaces with LP64.  When a LP64
> > syscall with long and unsigned long int arguments is used for x32, these
> > arguments must be properly extended to 64-bit.  Otherwise if the upper
> > 32 bits of the register have undefined value, such a syscall will be
> > rejected by kernel.
> >
> > For syscalls implemented in assembly codes, 'U' is added to syscall
> > signature key letters for unsigned long, which is zero-extended to
> > 64-bit types.  SYSCALL_ULONG_ARG_1 and SYSCALL_ULONG_ARG_2 are passed
> > to syscall-template.S for the first and the second unsigned long int
> > arguments if PSEUDOS_HAVE_ULONG_INDICES is defined.  They are used by
> > x32 to zero-extend 32-bit arguments to 64 bits.
> >
> > Tested on i386, x86-64 and x32 as well as with build-many-glibcs.py.
> > ---
> >  sysdeps/unix/make-syscalls.sh               | 24 +++++++
> >  sysdeps/unix/syscall-template.S             | 49 +++++++++++++-
> >  sysdeps/unix/syscalls.list                  |  8 +--
> >  sysdeps/unix/sysv/linux/syscalls.list       | 16 ++---
> >  sysdeps/unix/sysv/linux/x86_64/sysdep.h     | 71 +++++++++++++++++----
> >  sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h | 21 +++++-
> >  6 files changed, 159 insertions(+), 30 deletions(-)
>
> PING:
>
> https://sourceware.org/pipermail/libc-alpha/2020-April/113036.html
>
> I want to keep changes to make-syscalls.sh and syscall-template.S to
> minimum so that they can be backported.  Any improvement to syscall
> generation should be done after this bug fix.
>
>

PING.

-- 
H.J.

Re: V3 [PATCH 1/2] Add SYSCALL_ULONG_ARG_[12] to pass long to syscall [BZ #25810]

[Florian Weimer]

* H. J. Lu via Libc-alpha:

> diff --git a/sysdeps/unix/make-syscalls.sh b/sysdeps/unix/make-syscalls.sh
> index c07626677f..4f6c3490a2 100644
> --- a/sysdeps/unix/make-syscalls.sh
> +++ b/sysdeps/unix/make-syscalls.sh
> @@ -30,6 +30,7 @@
>  # P: optionally-NULL pointer to typed object (e.g., 3rd argument to sigaction)
>  # s: non-NULL string (e.g., 1st arg to open)
>  # S: optionally-NULL string (e.g., 1st arg to acct)
> +# U: unsigned long int (32-bit types are zero-extended to 64-bit types)
>  # v: vararg scalar (e.g., optional 3rd arg to open)
>  # V: byte-per-page vector (3rd arg to mincore)
>  # W: wait status, optionally-NULL pointer to int (e.g., 2nd arg of wait4)
> @@ -184,6 +185,27 @@ while read file srcfile caller syscall args strong weak; do
>    ?:?????????) nargs=9;;
>    esac
>  
> +  # Derive the unsigned long int arguments from the argument signature
> +  ulong_arg_1=0
> +  ulong_arg_2=0
> +  ulong_count=0
> +  for U in $(echo $args | sed -e "s/.*:/:/" | grep -ob U)
> +  do
> +    ulong_count=$(expr $ulong_count + 1)
> +    ulong_arg=$(echo $U | sed -e "s/:U//")
> +    case $ulong_count in
> +    1)
> +      ulong_arg_1=$ulong_arg
> +      ;;
> +    2)
> +      ulong_arg_2=$ulong_arg
> +      ;;
> +    *)
> +      echo >&2 "$0: Too many unsigned long int arguments for syscall ($strong $weak)"
> +      exit 2
> +    esac
> +  done

This version is much better.  -ob isn't specific to GNU grep
(FreeBSD's base system grep has it as well), so maybe we don't need to
update INSTALL.

> diff --git a/sysdeps/unix/syscalls.list b/sysdeps/unix/syscalls.list
> index 01c4a0e6b1..0cf290076d 100644
> --- a/sysdeps/unix/syscalls.list
> +++ b/sysdeps/unix/syscalls.list
> @@ -37,16 +37,16 @@ kill		-	kill		i:ii	__kill		kill
>  link		-	link		i:ss	__link		link
>  listen		-	listen		i:ii	__listen	listen
>  lseek		-	lseek		i:iii	__libc_lseek	__lseek lseek
> -madvise		-	madvise		i:pii	__madvise	madvise
> +madvise		-	madvise		i:pUi	__madvise	madvise
>  mkdir		-	mkdir		i:si	__mkdir		mkdir
>  mmap		-	mmap		b:aniiii __mmap		mmap
> -mprotect	-	mprotect	i:aii	__mprotect	mprotect
> -munmap		-	munmap		i:ai	__munmap	munmap
> +mprotect	-	mprotect	i:aUi	__mprotect	mprotect
> +munmap		-	munmap		i:aU	__munmap	munmap
>  open		-	open		Ci:siv	__libc_open __open open
>  profil		-	profil		i:piii	__profil	profil
>  ptrace		-	ptrace		i:iiii	ptrace
>  read		-	read		Ci:ibn	__libc_read	__read read
> -readlink	-	readlink	i:spi	__readlink	readlink
> +readlink	-	readlink	i:spU	__readlink	readlink
>  readv		-	readv		Ci:ipi	__readv		readv
>  reboot		-	reboot		i:i	reboot
>  recv		-	recv		Ci:ibni	__libc_recv	recv


I went through the list of syscalls, and the following have size
arguments which need markup (even though they may not be used on Linux):

bind
connect
mmap
read
recv
recvrom
recvmsg
send
sendmsg
sendto (twice)
write

getdomainname, getgroups, gethostname, sethostname, setsockopt are
exceptions, they have int size argument in userspace and on the kernel
side and should therefore not be changed.

fstatfs and statfs do not match the Linux interface, so the correct
setting is unclear.

There's a mismatch between the kernel and userspace definitions for
readv, writev, setgroups (but not getgroups).


> diff --git a/sysdeps/unix/sysv/linux/syscalls.list b/sysdeps/unix/sysv/linux/syscalls.list
> index e40f993495..1b1010d4c8 100644
> --- a/sysdeps/unix/sysv/linux/syscalls.list
> +++ b/sysdeps/unix/sysv/linux/syscalls.list
> @@ -32,12 +32,12 @@ ioperm		-	ioperm		i:iii	ioperm
>  iopl		-	iopl		i:i	iopl
>  klogctl		EXTRA	syslog		i:isi	klogctl
>  lchown		-	lchown		i:sii	__lchown	lchown
> -mincore		-	mincore		i:anV	mincore
> -mlock		-	mlock		i:bn	mlock
> +mincore		-	mincore		i:aUV	mincore
> +mlock		-	mlock		i:bU	mlock
>  mlockall	-	mlockall	i:i	mlockall
> -mount		EXTRA	mount		i:sssip	__mount	mount
> -mremap		EXTRA	mremap		b:ainip	__mremap	mremap
> -munlock		-	munlock		i:ai	munlock
> +mount		EXTRA	mount		i:sssUp	__mount	mount
> +mremap		EXTRA	mremap		b:aUUip	__mremap	mremap
> +munlock		-	munlock		i:aU	munlock
>  munlockall	-	munlockall	i:	munlockall
>  nfsservctl	EXTRA	nfsservctl	i:ipp	__compat_nfsservctl	nfsservctl@GLIBC_2.0:GLIBC_2.28
>  pipe		-	pipe		i:f	__pipe		pipe
> @@ -46,7 +46,7 @@ pivot_root	EXTRA	pivot_root	i:ss	pivot_root
>  prctl		EXTRA	prctl		i:iiiii	__prctl		prctl
>  query_module	EXTRA	query_module	i:sipip	__compat_query_module	query_module@GLIBC_2.0:GLIBC_2.23
>  quotactl	EXTRA	quotactl	i:isip	quotactl
> -remap_file_pages -	remap_file_pages i:piiii	__remap_file_pages remap_file_pages
> +remap_file_pages -	remap_file_pages i:pUiUi	__remap_file_pages remap_file_pages
>  sched_getp	-	sched_getparam	i:ip	__sched_getparam	sched_getparam
>  sched_gets	-	sched_getscheduler	i:i	__sched_getscheduler	sched_getscheduler
>  sched_primax	-	sched_get_priority_max	i:i	__sched_get_priority_max	sched_get_priority_max
> @@ -54,7 +54,7 @@ sched_primin	-	sched_get_priority_min	i:i	__sched_get_priority_min	sched_get_pri
>  sched_setp	-	sched_setparam	i:ip	__sched_setparam	sched_setparam
>  sched_sets	-	sched_setscheduler	i:iip	__sched_setscheduler	sched_setscheduler
>  sched_yield	-	sched_yield	i:	__sched_yield	sched_yield
> -sendfile	-	sendfile	i:iipi	sendfile
> +sendfile	-	sendfile	i:iipU	sendfile
>  sendfile64	-	sendfile64	i:iipi	sendfile64
>  setfsgid	EXTRA	setfsgid	i:i	setfsgid
>  setfsuid	EXTRA	setfsuid	i:i	setfsuid
> @@ -71,7 +71,7 @@ chown		-	chown		i:sii	__libc_chown	__chown chown
>  fchownat	-	fchownat	i:isiii	fchownat
>  linkat		-	linkat		i:isisi	linkat
>  mkdirat		-	mkdirat		i:isi	mkdirat
> -readlinkat	-	readlinkat	i:issi	readlinkat
> +readlinkat	-	readlinkat	i:issU	readlinkat
>  symlinkat	-	symlinkat	i:sis	symlinkat
>  unlinkat	-	unlinkat	i:isi	unlinkat

Missing updates:

ioperm
sendfile64
setxattr
setxattr
lsetxattr
fsetxattr
getxattr
lgetxattr
fgetxattr
listxattr
llistxattr
flistxattr

prctl looks busted (too many arguments).  It will need a C wrapper, I
think.  Likewise process_vm_readv, process_vm_writev.  These can be a
separate patches, I guess.

epoll_create is special (int size argument).

The rest of the patch looks good to me.  It's okay to push this if you
can verify that stripped libc.so.6 does not change on i686 and x86-64.

Re: V3 [PATCH 1/2] Add SYSCALL_ULONG_ARG_[12] to pass long to syscall [BZ #25810]

[H.J. Lu]

On Wed, Apr 29, 2020 at 5:14 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>
> * H. J. Lu via Libc-alpha:
>
> > diff --git a/sysdeps/unix/make-syscalls.sh b/sysdeps/unix/make-syscalls.sh
> > index c07626677f..4f6c3490a2 100644
> > --- a/sysdeps/unix/make-syscalls.sh
> > +++ b/sysdeps/unix/make-syscalls.sh
> > @@ -30,6 +30,7 @@
> >  # P: optionally-NULL pointer to typed object (e.g., 3rd argument to sigaction)
> >  # s: non-NULL string (e.g., 1st arg to open)
> >  # S: optionally-NULL string (e.g., 1st arg to acct)
> > +# U: unsigned long int (32-bit types are zero-extended to 64-bit types)
> >  # v: vararg scalar (e.g., optional 3rd arg to open)
> >  # V: byte-per-page vector (3rd arg to mincore)
> >  # W: wait status, optionally-NULL pointer to int (e.g., 2nd arg of wait4)
> > @@ -184,6 +185,27 @@ while read file srcfile caller syscall args strong weak; do
> >    ?:?????????) nargs=9;;
> >    esac
> >
> > +  # Derive the unsigned long int arguments from the argument signature
> > +  ulong_arg_1=0
> > +  ulong_arg_2=0
> > +  ulong_count=0
> > +  for U in $(echo $args | sed -e "s/.*:/:/" | grep -ob U)
> > +  do
> > +    ulong_count=$(expr $ulong_count + 1)
> > +    ulong_arg=$(echo $U | sed -e "s/:U//")
> > +    case $ulong_count in
> > +    1)
> > +      ulong_arg_1=$ulong_arg
> > +      ;;
> > +    2)
> > +      ulong_arg_2=$ulong_arg
> > +      ;;
> > +    *)
> > +      echo >&2 "$0: Too many unsigned long int arguments for syscall ($strong $weak)"
> > +      exit 2
> > +    esac
> > +  done
>
> This version is much better.  -ob isn't specific to GNU grep
> (FreeBSD's base system grep has it as well), so maybe we don't need to
> update INSTALL.
>
> > diff --git a/sysdeps/unix/syscalls.list b/sysdeps/unix/syscalls.list
> > index 01c4a0e6b1..0cf290076d 100644
> > --- a/sysdeps/unix/syscalls.list
> > +++ b/sysdeps/unix/syscalls.list
> > @@ -37,16 +37,16 @@ kill              -       kill            i:ii    __kill          kill
> >  link         -       link            i:ss    __link          link
> >  listen               -       listen          i:ii    __listen        listen
> >  lseek                -       lseek           i:iii   __libc_lseek    __lseek lseek
> > -madvise              -       madvise         i:pii   __madvise       madvise
> > +madvise              -       madvise         i:pUi   __madvise       madvise
> >  mkdir                -       mkdir           i:si    __mkdir         mkdir
> >  mmap         -       mmap            b:aniiii __mmap         mmap
> > -mprotect     -       mprotect        i:aii   __mprotect      mprotect
> > -munmap               -       munmap          i:ai    __munmap        munmap
> > +mprotect     -       mprotect        i:aUi   __mprotect      mprotect
> > +munmap               -       munmap          i:aU    __munmap        munmap
> >  open         -       open            Ci:siv  __libc_open __open open
> >  profil               -       profil          i:piii  __profil        profil
> >  ptrace               -       ptrace          i:iiii  ptrace
> >  read         -       read            Ci:ibn  __libc_read     __read read
> > -readlink     -       readlink        i:spi   __readlink      readlink
> > +readlink     -       readlink        i:spU   __readlink      readlink
> >  readv                -       readv           Ci:ipi  __readv         readv
> >  reboot               -       reboot          i:i     reboot
> >  recv         -       recv            Ci:ibni __libc_recv     recv
>
>
> I went through the list of syscalls, and the following have size
> arguments which need markup (even though they may not be used on Linux):
>
> bind
> connect
> mmap
> read
> recv
> recvrom
> recvmsg
> send
> sendmsg
> sendto (twice)
> write

I will look into them.

> getdomainname, getgroups, gethostname, sethostname, setsockopt are
> exceptions, they have int size argument in userspace and on the kernel
> side and should therefore not be changed.
>
> fstatfs and statfs do not match the Linux interface, so the correct
> setting is unclear.
>
> There's a mismatch between the kernel and userspace definitions for
> readv, writev, setgroups (but not getgroups).
>

I will leave them alone.

> > diff --git a/sysdeps/unix/sysv/linux/syscalls.list b/sysdeps/unix/sysv/linux/syscalls.list
> > index e40f993495..1b1010d4c8 100644
> > --- a/sysdeps/unix/sysv/linux/syscalls.list
> > +++ b/sysdeps/unix/sysv/linux/syscalls.list
> > @@ -32,12 +32,12 @@ ioperm            -       ioperm          i:iii   ioperm
> >  iopl         -       iopl            i:i     iopl
> >  klogctl              EXTRA   syslog          i:isi   klogctl
> >  lchown               -       lchown          i:sii   __lchown        lchown
> > -mincore              -       mincore         i:anV   mincore
> > -mlock                -       mlock           i:bn    mlock
> > +mincore              -       mincore         i:aUV   mincore
> > +mlock                -       mlock           i:bU    mlock
> >  mlockall     -       mlockall        i:i     mlockall
> > -mount                EXTRA   mount           i:sssip __mount mount
> > -mremap               EXTRA   mremap          b:ainip __mremap        mremap
> > -munlock              -       munlock         i:ai    munlock
> > +mount                EXTRA   mount           i:sssUp __mount mount
> > +mremap               EXTRA   mremap          b:aUUip __mremap        mremap
> > +munlock              -       munlock         i:aU    munlock
> >  munlockall   -       munlockall      i:      munlockall
> >  nfsservctl   EXTRA   nfsservctl      i:ipp   __compat_nfsservctl     nfsservctl@GLIBC_2.0:GLIBC_2.28
> >  pipe         -       pipe            i:f     __pipe          pipe
> > @@ -46,7 +46,7 @@ pivot_root  EXTRA   pivot_root      i:ss    pivot_root
> >  prctl                EXTRA   prctl           i:iiiii __prctl         prctl
> >  query_module EXTRA   query_module    i:sipip __compat_query_module   query_module@GLIBC_2.0:GLIBC_2.23
> >  quotactl     EXTRA   quotactl        i:isip  quotactl
> > -remap_file_pages -   remap_file_pages i:piiii        __remap_file_pages remap_file_pages
> > +remap_file_pages -   remap_file_pages i:pUiUi        __remap_file_pages remap_file_pages
> >  sched_getp   -       sched_getparam  i:ip    __sched_getparam        sched_getparam
> >  sched_gets   -       sched_getscheduler      i:i     __sched_getscheduler    sched_getscheduler
> >  sched_primax -       sched_get_priority_max  i:i     __sched_get_priority_max        sched_get_priority_max
> > @@ -54,7 +54,7 @@ sched_primin        -       sched_get_priority_min  i:i     __sched_get_priority_min        sched_get_pri
> >  sched_setp   -       sched_setparam  i:ip    __sched_setparam        sched_setparam
> >  sched_sets   -       sched_setscheduler      i:iip   __sched_setscheduler    sched_setscheduler
> >  sched_yield  -       sched_yield     i:      __sched_yield   sched_yield
> > -sendfile     -       sendfile        i:iipi  sendfile
> > +sendfile     -       sendfile        i:iipU  sendfile
> >  sendfile64   -       sendfile64      i:iipi  sendfile64
> >  setfsgid     EXTRA   setfsgid        i:i     setfsgid
> >  setfsuid     EXTRA   setfsuid        i:i     setfsuid
> > @@ -71,7 +71,7 @@ chown               -       chown           i:sii   __libc_chown    __chown chown
> >  fchownat     -       fchownat        i:isiii fchownat
> >  linkat               -       linkat          i:isisi linkat
> >  mkdirat              -       mkdirat         i:isi   mkdirat
> > -readlinkat   -       readlinkat      i:issi  readlinkat
> > +readlinkat   -       readlinkat      i:issU  readlinkat
> >  symlinkat    -       symlinkat       i:sis   symlinkat
> >  unlinkat     -       unlinkat        i:isi   unlinkat
>
> Missing updates:
>
> ioperm
> sendfile64
> setxattr
> setxattr
> lsetxattr
> fsetxattr
> getxattr
> lgetxattr
> fgetxattr
> listxattr
> llistxattr
> flistxattr

I will look into them.

> prctl looks busted (too many arguments).  It will need a C wrapper, I
> think.  Likewise process_vm_readv, process_vm_writev.  These can be a
> separate patches, I guess.

I will into them.

> epoll_create is special (int size argument).
>
> The rest of the patch looks good to me.  It's okay to push this if you
> can verify that stripped libc.so.6 does not change on i686 and x86-64.

There are no changes in stripped libc.so.6 and ld.so on i686 and x86-64.

Pushed into master branch.  Is it OK to backport to release branches?

Thanks.

-- 
H.J.

Re: V3 [PATCH 1/2] Add SYSCALL_ULONG_ARG_[12] to pass long to syscall [BZ #25810]

[Florian Weimer]

* H. J. Lu:

> Pushed into master branch.  Is it OK to backport to release branches?

Yes please.  Maybe wait a week for potential fallout before you do the
backports?

Re: V3 [PATCH 1/2] Add SYSCALL_ULONG_ARG_[12] to pass long to syscall [BZ #25810]

[H.J. Lu]

On Wed, Apr 29, 2020 at 5:56 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>
> * H. J. Lu:
>
> > Pushed into master branch.  Is it OK to backport to release branches?
>
> Yes please.  Maybe wait a week for potential fallout before you do the
> backports?

OK.

Thanks.

-- 
H.J.

Re: V3 [PATCH 1/2] Add SYSCALL_ULONG_ARG_[12] to pass long to syscall [BZ #25810]

[H.J. Lu]

On Wed, Apr 29, 2020 at 5:14 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>
> * H. J. Lu via Libc-alpha:
>
> > diff --git a/sysdeps/unix/make-syscalls.sh b/sysdeps/unix/make-syscalls.sh
> > index c07626677f..4f6c3490a2 100644
> > --- a/sysdeps/unix/make-syscalls.sh
> > +++ b/sysdeps/unix/make-syscalls.sh
> > @@ -30,6 +30,7 @@
> >  # P: optionally-NULL pointer to typed object (e.g., 3rd argument to sigaction)
> >  # s: non-NULL string (e.g., 1st arg to open)
> >  # S: optionally-NULL string (e.g., 1st arg to acct)
> > +# U: unsigned long int (32-bit types are zero-extended to 64-bit types)
> >  # v: vararg scalar (e.g., optional 3rd arg to open)
> >  # V: byte-per-page vector (3rd arg to mincore)
> >  # W: wait status, optionally-NULL pointer to int (e.g., 2nd arg of wait4)
> > @@ -184,6 +185,27 @@ while read file srcfile caller syscall args strong weak; do
> >    ?:?????????) nargs=9;;
> >    esac
> >
> > +  # Derive the unsigned long int arguments from the argument signature
> > +  ulong_arg_1=0
> > +  ulong_arg_2=0
> > +  ulong_count=0
> > +  for U in $(echo $args | sed -e "s/.*:/:/" | grep -ob U)
> > +  do
> > +    ulong_count=$(expr $ulong_count + 1)
> > +    ulong_arg=$(echo $U | sed -e "s/:U//")
> > +    case $ulong_count in
> > +    1)
> > +      ulong_arg_1=$ulong_arg
> > +      ;;
> > +    2)
> > +      ulong_arg_2=$ulong_arg
> > +      ;;
> > +    *)
> > +      echo >&2 "$0: Too many unsigned long int arguments for syscall ($strong $weak)"
> > +      exit 2
> > +    esac
> > +  done
>
> This version is much better.  -ob isn't specific to GNU grep
> (FreeBSD's base system grep has it as well), so maybe we don't need to
> update INSTALL.
>
> > diff --git a/sysdeps/unix/syscalls.list b/sysdeps/unix/syscalls.list
> > index 01c4a0e6b1..0cf290076d 100644
> > --- a/sysdeps/unix/syscalls.list
> > +++ b/sysdeps/unix/syscalls.list
> > @@ -37,16 +37,16 @@ kill              -       kill            i:ii    __kill          kill
> >  link         -       link            i:ss    __link          link
> >  listen               -       listen          i:ii    __listen        listen
> >  lseek                -       lseek           i:iii   __libc_lseek    __lseek lseek
> > -madvise              -       madvise         i:pii   __madvise       madvise
> > +madvise              -       madvise         i:pUi   __madvise       madvise
> >  mkdir                -       mkdir           i:si    __mkdir         mkdir
> >  mmap         -       mmap            b:aniiii __mmap         mmap
> > -mprotect     -       mprotect        i:aii   __mprotect      mprotect
> > -munmap               -       munmap          i:ai    __munmap        munmap
> > +mprotect     -       mprotect        i:aUi   __mprotect      mprotect
> > +munmap               -       munmap          i:aU    __munmap        munmap
> >  open         -       open            Ci:siv  __libc_open __open open
> >  profil               -       profil          i:piii  __profil        profil
> >  ptrace               -       ptrace          i:iiii  ptrace
> >  read         -       read            Ci:ibn  __libc_read     __read read
> > -readlink     -       readlink        i:spi   __readlink      readlink
> > +readlink     -       readlink        i:spU   __readlink      readlink
> >  readv                -       readv           Ci:ipi  __readv         readv
> >  reboot               -       reboot          i:i     reboot
> >  recv         -       recv            Ci:ibni __libc_recv     recv
>
>
> I went through the list of syscalls, and the following have size
> arguments which need markup (even though they may not be used on Linux):
>
> bind

Kernel has

SYSCALL_DEFINE3(bind, int, fd, struct sockaddr __user *, umyaddr, int, addrlen)
{
        return __sys_bind(fd, umyaddr, addrlen);
}

No change is needed.

> connect

SYSCALL_DEFINE3(connect, int, fd, struct sockaddr __user *, uservaddr,
                int, addrlen)
{
        return __sys_connect(fd, uservaddr, addrlen);
}

> mmap

Will fix.

> read

Will fix.

> recv

Will fix.

> recvrom

Will fix

> recvmsg

Kernel has

SYSCALL_DEFINE3(recvmsg, int, fd, struct user_msghdr __user *, msg,
                unsigned int, flags)
{
        return __sys_recvmsg(fd, msg, flags, true);
}

No change is needed.

> send

Will fix.

> sendmsg

Kernel has

SYSCALL_DEFINE3(sendmsg, int, fd, struct user_msghdr __user *, msg,
unsigned int, flags)
{
        return __sys_sendmsg(fd, msg, flags, true);
}

No change is needed.

> sendto (twice)

Kernel has

SYSCALL_DEFINE6(sendto, int, fd, void __user *, buff, size_t, len,
                unsigned int, flags, struct sockaddr __user *, addr,
                int, addr_len)
{
        return __sys_sendto(fd, buff, len, flags, addr, addr_len);
}

There is only one size_t.

> write

Will fix.

> getdomainname, getgroups, gethostname, sethostname, setsockopt are
> exceptions, they have int size argument in userspace and on the kernel
> side and should therefore not be changed.
>
> fstatfs and statfs do not match the Linux interface, so the correct
> setting is unclear.

No change.

> There's a mismatch between the kernel and userspace definitions for
> readv, writev, setgroups (but not getgroups).

No change.

>
> > diff --git a/sysdeps/unix/sysv/linux/syscalls.list b/sysdeps/unix/sysv/linux/syscalls.list
> > index e40f993495..1b1010d4c8 100644
> > --- a/sysdeps/unix/sysv/linux/syscalls.list
> > +++ b/sysdeps/unix/sysv/linux/syscalls.list
> > @@ -32,12 +32,12 @@ ioperm            -       ioperm          i:iii   ioperm
> >  iopl         -       iopl            i:i     iopl
> >  klogctl              EXTRA   syslog          i:isi   klogctl
> >  lchown               -       lchown          i:sii   __lchown        lchown
> > -mincore              -       mincore         i:anV   mincore
> > -mlock                -       mlock           i:bn    mlock
> > +mincore              -       mincore         i:aUV   mincore
> > +mlock                -       mlock           i:bU    mlock
> >  mlockall     -       mlockall        i:i     mlockall
> > -mount                EXTRA   mount           i:sssip __mount mount
> > -mremap               EXTRA   mremap          b:ainip __mremap        mremap
> > -munlock              -       munlock         i:ai    munlock
> > +mount                EXTRA   mount           i:sssUp __mount mount
> > +mremap               EXTRA   mremap          b:aUUip __mremap        mremap
> > +munlock              -       munlock         i:aU    munlock
> >  munlockall   -       munlockall      i:      munlockall
> >  nfsservctl   EXTRA   nfsservctl      i:ipp   __compat_nfsservctl     nfsservctl@GLIBC_2.0:GLIBC_2.28
> >  pipe         -       pipe            i:f     __pipe          pipe
> > @@ -46,7 +46,7 @@ pivot_root  EXTRA   pivot_root      i:ss    pivot_root
> >  prctl                EXTRA   prctl           i:iiiii __prctl         prctl
> >  query_module EXTRA   query_module    i:sipip __compat_query_module   query_module@GLIBC_2.0:GLIBC_2.23
> >  quotactl     EXTRA   quotactl        i:isip  quotactl
> > -remap_file_pages -   remap_file_pages i:piiii        __remap_file_pages remap_file_pages
> > +remap_file_pages -   remap_file_pages i:pUiUi        __remap_file_pages remap_file_pages
> >  sched_getp   -       sched_getparam  i:ip    __sched_getparam        sched_getparam
> >  sched_gets   -       sched_getscheduler      i:i     __sched_getscheduler    sched_getscheduler
> >  sched_primax -       sched_get_priority_max  i:i     __sched_get_priority_max        sched_get_priority_max
> > @@ -54,7 +54,7 @@ sched_primin        -       sched_get_priority_min  i:i     __sched_get_priority_min        sched_get_pri
> >  sched_setp   -       sched_setparam  i:ip    __sched_setparam        sched_setparam
> >  sched_sets   -       sched_setscheduler      i:iip   __sched_setscheduler    sched_setscheduler
> >  sched_yield  -       sched_yield     i:      __sched_yield   sched_yield
> > -sendfile     -       sendfile        i:iipi  sendfile
> > +sendfile     -       sendfile        i:iipU  sendfile
> >  sendfile64   -       sendfile64      i:iipi  sendfile64
> >  setfsgid     EXTRA   setfsgid        i:i     setfsgid
> >  setfsuid     EXTRA   setfsuid        i:i     setfsuid
> > @@ -71,7 +71,7 @@ chown               -       chown           i:sii   __libc_chown    __chown chown
> >  fchownat     -       fchownat        i:isiii fchownat
> >  linkat               -       linkat          i:isisi linkat
> >  mkdirat              -       mkdirat         i:isi   mkdirat
> > -readlinkat   -       readlinkat      i:issi  readlinkat
> > +readlinkat   -       readlinkat      i:issU  readlinkat
> >  symlinkat    -       symlinkat       i:sis   symlinkat
> >  unlinkat     -       unlinkat        i:isi   unlinkat
>
> Missing updates:
>
> ioperm

Will fix.

> sendfile64

Will fix.

> setxattr
> setxattr
> lsetxattr
> fsetxattr
> getxattr
> lgetxattr
> fgetxattr
> listxattr
> llistxattr
> flistxattr

Will fix them.

> prctl looks busted (too many arguments).  It will need a C wrapper, I
> think.  Likewise process_vm_readv, process_vm_writev.  These can be a
> separate patches, I guess.

Will do.

> epoll_create is special (int size argument).
>
> The rest of the patch looks good to me.  It's okay to push this if you
> can verify that stripped libc.so.6 does not change on i686 and x86-64.

Thanks.

-- 
H.J.

Re: V3 [PATCH 1/2] Add SYSCALL_ULONG_ARG_[12] to pass long to syscall [BZ #25810]

[Florian Weimer]

* H. J. Lu:

> Kernel has
>
> SYSCALL_DEFINE3(bind, int, fd, struct sockaddr __user *, umyaddr, int, addrlen)
> {
>         return __sys_bind(fd, umyaddr, addrlen);
> }
>
> No change is needed.
>
>> connect
>
> SYSCALL_DEFINE3(connect, int, fd, struct sockaddr __user *, uservaddr,
>                 int, addrlen)
> {
>         return __sys_connect(fd, uservaddr, addrlen);
> }

Ah, those are different bugs then because socklen_t in userspace is
unsigned int.  It's not related to x32 at all.

>> recvmsg
>
> Kernel has
>
> SYSCALL_DEFINE3(recvmsg, int, fd, struct user_msghdr __user *, msg,
>                 unsigned int, flags)
> {
>         return __sys_recvmsg(fd, msg, flags, true);
> }
>
> No change is needed.

>> sendmsg
>
> Kernel has
>
> SYSCALL_DEFINE3(sendmsg, int, fd, struct user_msghdr __user *, msg,
> unsigned int, flags)
> {
>         return __sys_sendmsg(fd, msg, flags, true);
> }
>
> No change is needed.

Agreed, I mixed those up.

>> sendto (twice)
>
> Kernel has
>
> SYSCALL_DEFINE6(sendto, int, fd, void __user *, buff, size_t, len,
>                 unsigned int, flags, struct sockaddr __user *, addr,
>                 int, addr_len)
> {
>         return __sys_sendto(fd, buff, len, flags, addr, addr_len);
> }
>
> There is only one size_t.

The addr_len argument is socklen_t in userspace, so the bind/connect
problem applies as well.