* [PATCH 0/3] x32: Properly pass long to syscall [BZ #25810]
@ 2020-04-13 14:25 H.J. Lu
2020-04-13 14:25 ` [PATCH 1/3] Add SYSCALL_ULONG_ARG_[12] to " H.J. Lu
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: H.J. Lu @ 2020-04-13 14:25 UTC (permalink / raw)
To: libc-alpha
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.
For x32 INLINE_SYSCALLs, cast
1. pointer to unsigned long (32 bit).
2. 32-bit unsigned integer to unsigned long long (64 bit).
3. 32-bit integer to long long (64 bit).
For
void *mmap(void *addr, size_t length, int prot, int flags,
int fd, off_t offset);
we now generate
0: 41 f7 c1 ff 0f 00 00 test $0xfff,%r9d
7: 75 1f jne 28 <__mmap64+0x28>
9: 48 63 d2 movslq %edx,%rdx
c: 89 f6 mov %esi,%esi
e: 4d 63 c0 movslq %r8d,%r8
11: 4c 63 d1 movslq %ecx,%r10
14: b8 09 00 00 40 mov $0x40000009,%eax
19: 0f 05 syscall
That is
1. addr is unchanged.
2. length is zero-extend to 64 bits.
3. prot is sign-extend to 64 bits.
4. flags is sign-extend to 64 bits.
5. fd is sign-extend to 64 bits.
6. offset is unchanged.
For int arguments, since kernel uses only the lower 32 bits and ignores
the upper 32 bits in 64-bit registers, these work correctly.
Tested on i386, x86-64 and x32 as well as with build-many-glibcs.py.
H.J. Lu (3):
Add SYSCALL_ULONG_ARG_[12] to pass long to syscall [BZ #25810]
x32: Properly 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 | 80 ++++++++---
sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h | 38 +++++
8 files changed, 385 insertions(+), 32 deletions(-)
create mode 100644 misc/tst-syscalls.c
--
2.25.2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/3] Add SYSCALL_ULONG_ARG_[12] to pass long to syscall [BZ #25810]
2020-04-13 14:25 [PATCH 0/3] x32: Properly pass long to syscall [BZ #25810] H.J. Lu
@ 2020-04-13 14:25 ` H.J. Lu
2020-04-13 14:25 ` [PATCH 2/3] x32: Properly " H.J. Lu
2020-04-13 14:25 ` [PATCH 3/3] Add a syscall test for " H.J. Lu
2 siblings, 0 replies; 13+ messages in thread
From: H.J. Lu @ 2020-04-13 14:25 UTC (permalink / raw)
To: libc-alpha
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, 214 insertions(+), 27 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 24d8b8ec20..06e39212ee 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 5bf9eed80b..2b5cf0c0ea 100644
--- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
+++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
@@ -26,4 +26,24 @@
#undef LO_HI_LONG
#define LO_HI_LONG(val) (val)
+#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;
+#endif /* __ASSEMBLER__ */
+
#endif /* linux/x86_64/x32/sysdep.h */
--
2.25.2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/3] x32: Properly pass long to syscall [BZ #25810]
2020-04-13 14:25 [PATCH 0/3] x32: Properly pass long to syscall [BZ #25810] H.J. Lu
2020-04-13 14:25 ` [PATCH 1/3] Add SYSCALL_ULONG_ARG_[12] to " H.J. Lu
@ 2020-04-13 14:25 ` H.J. Lu
2020-04-13 14:43 ` Florian Weimer
2020-04-13 14:25 ` [PATCH 3/3] Add a syscall test for " H.J. Lu
2 siblings, 1 reply; 13+ messages in thread
From: H.J. Lu @ 2020-04-13 14:25 UTC (permalink / raw)
To: libc-alpha
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 x32 INLINE_SYSCALLs, cast
1. pointer to unsigned long (32 bit).
2. 32-bit unsigned integer to unsigned long long (64 bit).
3. 32-bit integer to long long (64 bit).
For
void *mmap(void *addr, size_t length, int prot, int flags,
int fd, off_t offset);
we now generate
0: 41 f7 c1 ff 0f 00 00 test $0xfff,%r9d
7: 75 1f jne 28 <__mmap64+0x28>
9: 48 63 d2 movslq %edx,%rdx
c: 89 f6 mov %esi,%esi
e: 4d 63 c0 movslq %r8d,%r8
11: 4c 63 d1 movslq %ecx,%r10
14: b8 09 00 00 40 mov $0x40000009,%eax
19: 0f 05 syscall
That is
1. addr is unchanged.
2. length is zero-extend to 64 bits.
3. prot is sign-extend to 64 bits.
4. flags is sign-extend to 64 bits.
5. fd is sign-extend to 64 bits.
6. offset is unchanged.
For int arguments, since kernel uses only the lower 32 bits and ignores
the upper 32 bits in 64-bit registers, these work correctly.
Tested on x86-64 and x32. There are no code changes on x86-64.
---
sysdeps/unix/sysv/linux/x86_64/sysdep.h | 10 ++++++----
sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h | 18 ++++++++++++++++++
2 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
index 06e39212ee..93dad97c8f 100644
--- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h
+++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
@@ -219,12 +219,14 @@
/* Registers clobbered by syscall. */
# define REGISTERS_CLOBBERED_BY_SYSCALL "cc", "r11", "cx"
-/* Create a variable 'name' based on type 'X' to avoid explicit types.
- This is mainly used set use 64-bits arguments in x32. */
-#define TYPEFY(X, name) __typeof__ ((X) - (X)) name
+/* This also works when X is an array. */
+#define TYPEFY1(X) __typeof__ ((X) - (X))
/* Explicit cast the argument to avoid integer from pointer warning on
x32. */
-#define ARGIFY(X) ((__typeof__ ((X) - (X))) (X))
+#define ARGIFY(X) ((TYPEFY1 (X)) (X))
+/* Create a variable 'name' based on type 'X' to avoid explicit types.
+ This is mainly used set use 64-bits arguments in x32. */
+#define TYPEFY(X, name) __typeof__ (ARGIFY (X) - ARGIFY (X)) name
#undef INTERNAL_SYSCALL
#define INTERNAL_SYSCALL(name, nr, args...) \
diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
index 2b5cf0c0ea..f3bc7e89df 100644
--- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
+++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
@@ -44,6 +44,24 @@
# define ZERO_EXTEND_5 movl %r8d, %r8d;
# undef ZERO_EXTEND_6
# define ZERO_EXTEND_6 movl %r9d, %r9d;
+#else /*! __ASSEMBLER__ */
+# undef ARGIFY
+/* For x32, cast
+ 1. pointer to unsigned long (32 bit).
+ 2. 32-bit unsigned integer to unsigned long long (64 bit).
+ 3. 32-bit integer to long long (64 bit).
+ */
+# define ARGIFY(X) \
+ ({ \
+ _Pragma ("GCC diagnostic push"); \
+ _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \
+ (__builtin_classify_type (X) == 5 \
+ ? (unsigned long) (X) \
+ : ((sizeof (X) <= 4 && (TYPEFY1 (X)) 0 < (TYPEFY1 (X)) -1) \
+ ? (unsigned long long) (X) \
+ : (long long) (X))); \
+ _Pragma ("GCC diagnostic pop"); \
+ })
#endif /* __ASSEMBLER__ */
#endif /* linux/x86_64/x32/sysdep.h */
--
2.25.2
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/3] Add a syscall test for [BZ #25810]
2020-04-13 14:25 [PATCH 0/3] x32: Properly pass long to syscall [BZ #25810] H.J. Lu
2020-04-13 14:25 ` [PATCH 1/3] Add SYSCALL_ULONG_ARG_[12] to " H.J. Lu
2020-04-13 14:25 ` [PATCH 2/3] x32: Properly " H.J. Lu
@ 2020-04-13 14:25 ` H.J. Lu
2 siblings, 0 replies; 13+ messages in thread
From: H.J. Lu @ 2020-04-13 14:25 UTC (permalink / raw)
To: libc-alpha
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] x32: Properly pass long to syscall [BZ #25810]
2020-04-13 14:25 ` [PATCH 2/3] x32: Properly " H.J. Lu
@ 2020-04-13 14:43 ` Florian Weimer
2020-04-13 15:03 ` H.J. Lu
0 siblings, 1 reply; 13+ messages in thread
From: Florian Weimer @ 2020-04-13 14:43 UTC (permalink / raw)
To: H.J. Lu via Libc-alpha
* H. J. Lu via Libc-alpha:
> diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> index 06e39212ee..93dad97c8f 100644
> --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> @@ -219,12 +219,14 @@
> /* Registers clobbered by syscall. */
> # define REGISTERS_CLOBBERED_BY_SYSCALL "cc", "r11", "cx"
>
> -/* Create a variable 'name' based on type 'X' to avoid explicit types.
> - This is mainly used set use 64-bits arguments in x32. */
> -#define TYPEFY(X, name) __typeof__ ((X) - (X)) name
> +/* This also works when X is an array. */
> +#define TYPEFY1(X) __typeof__ ((X) - (X))
I think the comment is now misleading. For an array X, (X) - (X) is
of type ptrdiff_t, which is signed, so it triggers sign extension, not
zero extension, in the x32 case. I think this is the reason why my
proposed construct went astray in your experiments.
Is it really necessary to support arrays for system call arguments?
> diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> index 2b5cf0c0ea..f3bc7e89df 100644
> --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> @@ -44,6 +44,24 @@
> # define ZERO_EXTEND_5 movl %r8d, %r8d;
> # undef ZERO_EXTEND_6
> # define ZERO_EXTEND_6 movl %r9d, %r9d;
> +#else /*! __ASSEMBLER__ */
> +# undef ARGIFY
> +/* For x32, cast
> + 1. pointer to unsigned long (32 bit).
> + 2. 32-bit unsigned integer to unsigned long long (64 bit).
> + 3. 32-bit integer to long long (64 bit).
> + */
> +# define ARGIFY(X) \
> + ({ \
> + _Pragma ("GCC diagnostic push"); \
> + _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \
> + (__builtin_classify_type (X) == 5 \
> + ? (unsigned long) (X) \
> + : ((sizeof (X) <= 4 && (TYPEFY1 (X)) 0 < (TYPEFY1 (X)) -1) \
> + ? (unsigned long long) (X) \
> + : (long long) (X))); \
> + _Pragma ("GCC diagnostic pop"); \
> + })
> #endif /* __ASSEMBLER__ */
This should use long int instead long everywhere.
__builtin_classify_type is undocumented. I'm not sure if its behavior
and the constant 5 is actually stable across GCC versions, but the
powerpc uses it as well, for a slightly difference purpose (to
recognize array arguments, it seems).
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] x32: Properly pass long to syscall [BZ #25810]
2020-04-13 14:43 ` Florian Weimer
@ 2020-04-13 15:03 ` H.J. Lu
2020-04-13 15:17 ` Florian Weimer
0 siblings, 1 reply; 13+ messages in thread
From: H.J. Lu @ 2020-04-13 15:03 UTC (permalink / raw)
To: Florian Weimer; +Cc: H.J. Lu via Libc-alpha
On Mon, Apr 13, 2020 at 7:43 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>
> * H. J. Lu via Libc-alpha:
>
> > diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> > index 06e39212ee..93dad97c8f 100644
> > --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> > +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> > @@ -219,12 +219,14 @@
> > /* Registers clobbered by syscall. */
> > # define REGISTERS_CLOBBERED_BY_SYSCALL "cc", "r11", "cx"
> >
> > -/* Create a variable 'name' based on type 'X' to avoid explicit types.
> > - This is mainly used set use 64-bits arguments in x32. */
> > -#define TYPEFY(X, name) __typeof__ ((X) - (X)) name
> > +/* This also works when X is an array. */
> > +#define TYPEFY1(X) __typeof__ ((X) - (X))
>
> I think the comment is now misleading. For an array X, (X) - (X) is
> of type ptrdiff_t, which is signed, so it triggers sign extension, not
I will update the comments.
> zero extension, in the x32 case. I think this is the reason why my
> proposed construct went astray in your experiments.
>
> Is it really necessary to support arrays for system call arguments?
int
fexecve (int fd, char *const argv[], char *const envp[])
{
if (fd < 0 || argv == NULL || envp == NULL)
{
__set_errno (EINVAL);
return -1;
}
#ifdef __NR_execveat
/* Avoid implicit array coercion in syscall macros. */
INLINE_SYSCALL (execveat, 5, fd, "", &argv[0], &envp[0], AT_EMPTY_PATH);
"" is an array.
> > diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> > index 2b5cf0c0ea..f3bc7e89df 100644
> > --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> > +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> > @@ -44,6 +44,24 @@
> > # define ZERO_EXTEND_5 movl %r8d, %r8d;
> > # undef ZERO_EXTEND_6
> > # define ZERO_EXTEND_6 movl %r9d, %r9d;
> > +#else /*! __ASSEMBLER__ */
> > +# undef ARGIFY
> > +/* For x32, cast
> > + 1. pointer to unsigned long (32 bit).
> > + 2. 32-bit unsigned integer to unsigned long long (64 bit).
> > + 3. 32-bit integer to long long (64 bit).
> > + */
> > +# define ARGIFY(X) \
> > + ({ \
> > + _Pragma ("GCC diagnostic push"); \
> > + _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \
> > + (__builtin_classify_type (X) == 5 \
> > + ? (unsigned long) (X) \
> > + : ((sizeof (X) <= 4 && (TYPEFY1 (X)) 0 < (TYPEFY1 (X)) -1) \
> > + ? (unsigned long long) (X) \
> > + : (long long) (X))); \
> > + _Pragma ("GCC diagnostic pop"); \
> > + })
> > #endif /* __ASSEMBLER__ */
>
> This should use long int instead long everywhere.
I will update.
> __builtin_classify_type is undocumented. I'm not sure if its behavior
> and the constant 5 is actually stable across GCC versions, but the
> powerpc uses it as well, for a slightly difference purpose (to
> recognize array arguments, it seems).
include/libc-pointer-arith.h has
/* 1 if 'type' is a pointer type, 0 otherwise. */
# define __pointer_type(type) (__builtin_classify_type ((type) 0) == 5)
If __builtin_classify_type ever changes, glibc won't build correctly.
--
H.J.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] x32: Properly pass long to syscall [BZ #25810]
2020-04-13 15:03 ` H.J. Lu
@ 2020-04-13 15:17 ` Florian Weimer
2020-04-13 16:33 ` V2 " H.J. Lu
0 siblings, 1 reply; 13+ messages in thread
From: Florian Weimer @ 2020-04-13 15:17 UTC (permalink / raw)
To: H.J. Lu; +Cc: H.J. Lu via Libc-alpha
* H. J. Lu:
> On Mon, Apr 13, 2020 at 7:43 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>>
>> * H. J. Lu via Libc-alpha:
>>
>> > diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
>> > index 06e39212ee..93dad97c8f 100644
>> > --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h
>> > +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
>> > @@ -219,12 +219,14 @@
>> > /* Registers clobbered by syscall. */
>> > # define REGISTERS_CLOBBERED_BY_SYSCALL "cc", "r11", "cx"
>> >
>> > -/* Create a variable 'name' based on type 'X' to avoid explicit types.
>> > - This is mainly used set use 64-bits arguments in x32. */
>> > -#define TYPEFY(X, name) __typeof__ ((X) - (X)) name
>> > +/* This also works when X is an array. */
>> > +#define TYPEFY1(X) __typeof__ ((X) - (X))
>>
>> I think the comment is now misleading. For an array X, (X) - (X) is
>> of type ptrdiff_t, which is signed, so it triggers sign extension, not
>
> I will update the comments.
Thanks.
>> zero extension, in the x32 case. I think this is the reason why my
>> proposed construct went astray in your experiments.
>>
>> Is it really necessary to support arrays for system call arguments?
>
> int
> fexecve (int fd, char *const argv[], char *const envp[])
> {
> if (fd < 0 || argv == NULL || envp == NULL)
> {
> __set_errno (EINVAL);
> return -1;
> }
>
> #ifdef __NR_execveat
> /* Avoid implicit array coercion in syscall macros. */
> INLINE_SYSCALL (execveat, 5, fd, "", &argv[0], &envp[0], AT_EMPTY_PATH);
>
> "" is an array.
If that were the only reason for enforcing pointer decay, I'd say we'd
change the fexecve implementation. But see below.
>> > diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
>> > index 2b5cf0c0ea..f3bc7e89df 100644
>> > --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
>> > +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
>> > @@ -44,6 +44,24 @@
>> > # define ZERO_EXTEND_5 movl %r8d, %r8d;
>> > # undef ZERO_EXTEND_6
>> > # define ZERO_EXTEND_6 movl %r9d, %r9d;
>> > +#else /*! __ASSEMBLER__ */
>> > +# undef ARGIFY
>> > +/* For x32, cast
>> > + 1. pointer to unsigned long (32 bit).
>> > + 2. 32-bit unsigned integer to unsigned long long (64 bit).
>> > + 3. 32-bit integer to long long (64 bit).
>> > + */
>> > +# define ARGIFY(X) \
>> > + ({ \
>> > + _Pragma ("GCC diagnostic push"); \
>> > + _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \
>> > + (__builtin_classify_type (X) == 5 \
>> > + ? (unsigned long) (X) \
>> > + : ((sizeof (X) <= 4 && (TYPEFY1 (X)) 0 < (TYPEFY1 (X)) -1) \
>> > + ? (unsigned long long) (X) \
>> > + : (long long) (X))); \
>> > + _Pragma ("GCC diagnostic pop"); \
>> > + })
>> > #endif /* __ASSEMBLER__ */
>>
>> This should use long int instead long everywhere.
>
> I will update.
>
>> __builtin_classify_type is undocumented. I'm not sure if its behavior
>> and the constant 5 is actually stable across GCC versions, but the
>> powerpc uses it as well, for a slightly difference purpose (to
>> recognize array arguments, it seems).
>
> include/libc-pointer-arith.h has
>
> /* 1 if 'type' is a pointer type, 0 otherwise. */
> # define __pointer_type(type) (__builtin_classify_type ((type) 0) == 5)
>
> If __builtin_classify_type ever changes, glibc won't build correctly.
I see. In this case, can we solely rely on __builtin_classify_type?
I dont't think the inner ternary operator is necessary.
/* Enforce zero-extension for pointers and array system call
arguments. For integer types, extend to long long int (the full
register) using a regular cast, resulting in zero or sign extension
based on the signed-ness of the original type. */
#define ARGIFY(X) \
({ \
_Pragma ("GCC diagnostic push"); \
_Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \
(__builtin_classify_type (X) == 5 \
? (long long int) (uintptr_t) (X) \
: (long long int) (X))); \
_Pragma ("GCC diagnostic pop"); \
})
(Untested, sorry.)
^ permalink raw reply [flat|nested] 13+ messages in thread
* V2 [PATCH 2/3] x32: Properly pass long to syscall [BZ #25810]
2020-04-13 15:17 ` Florian Weimer
@ 2020-04-13 16:33 ` H.J. Lu
2020-04-13 17:00 ` Florian Weimer
2020-04-13 18:34 ` Adhemerval Zanella
0 siblings, 2 replies; 13+ messages in thread
From: H.J. Lu @ 2020-04-13 16:33 UTC (permalink / raw)
To: Florian Weimer; +Cc: H.J. Lu via Libc-alpha
[-- Attachment #1: Type: text/plain, Size: 5001 bytes --]
On Mon, Apr 13, 2020 at 8:17 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>
> * H. J. Lu:
>
> > On Mon, Apr 13, 2020 at 7:43 AM Florian Weimer <fw@deneb.enyo.de> wrote:
> >>
> >> * H. J. Lu via Libc-alpha:
> >>
> >> > diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> >> > index 06e39212ee..93dad97c8f 100644
> >> > --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> >> > +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> >> > @@ -219,12 +219,14 @@
> >> > /* Registers clobbered by syscall. */
> >> > # define REGISTERS_CLOBBERED_BY_SYSCALL "cc", "r11", "cx"
> >> >
> >> > -/* Create a variable 'name' based on type 'X' to avoid explicit types.
> >> > - This is mainly used set use 64-bits arguments in x32. */
> >> > -#define TYPEFY(X, name) __typeof__ ((X) - (X)) name
> >> > +/* This also works when X is an array. */
> >> > +#define TYPEFY1(X) __typeof__ ((X) - (X))
> >>
> >> I think the comment is now misleading. For an array X, (X) - (X) is
> >> of type ptrdiff_t, which is signed, so it triggers sign extension, not
> >
> > I will update the comments.
>
> Thanks.
>
> >> zero extension, in the x32 case. I think this is the reason why my
> >> proposed construct went astray in your experiments.
> >>
> >> Is it really necessary to support arrays for system call arguments?
> >
> > int
> > fexecve (int fd, char *const argv[], char *const envp[])
> > {
> > if (fd < 0 || argv == NULL || envp == NULL)
> > {
> > __set_errno (EINVAL);
> > return -1;
> > }
> >
> > #ifdef __NR_execveat
> > /* Avoid implicit array coercion in syscall macros. */
> > INLINE_SYSCALL (execveat, 5, fd, "", &argv[0], &envp[0], AT_EMPTY_PATH);
> >
> > "" is an array.
>
> If that were the only reason for enforcing pointer decay, I'd say we'd
> change the fexecve implementation. But see below.
>
> >> > diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> >> > index 2b5cf0c0ea..f3bc7e89df 100644
> >> > --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> >> > +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> >> > @@ -44,6 +44,24 @@
> >> > # define ZERO_EXTEND_5 movl %r8d, %r8d;
> >> > # undef ZERO_EXTEND_6
> >> > # define ZERO_EXTEND_6 movl %r9d, %r9d;
> >> > +#else /*! __ASSEMBLER__ */
> >> > +# undef ARGIFY
> >> > +/* For x32, cast
> >> > + 1. pointer to unsigned long (32 bit).
> >> > + 2. 32-bit unsigned integer to unsigned long long (64 bit).
> >> > + 3. 32-bit integer to long long (64 bit).
> >> > + */
> >> > +# define ARGIFY(X) \
> >> > + ({ \
> >> > + _Pragma ("GCC diagnostic push"); \
> >> > + _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \
> >> > + (__builtin_classify_type (X) == 5 \
> >> > + ? (unsigned long) (X) \
> >> > + : ((sizeof (X) <= 4 && (TYPEFY1 (X)) 0 < (TYPEFY1 (X)) -1) \
> >> > + ? (unsigned long long) (X) \
> >> > + : (long long) (X))); \
> >> > + _Pragma ("GCC diagnostic pop"); \
> >> > + })
> >> > #endif /* __ASSEMBLER__ */
> >>
> >> This should use long int instead long everywhere.
> >
> > I will update.
> >
> >> __builtin_classify_type is undocumented. I'm not sure if its behavior
> >> and the constant 5 is actually stable across GCC versions, but the
> >> powerpc uses it as well, for a slightly difference purpose (to
> >> recognize array arguments, it seems).
> >
> > include/libc-pointer-arith.h has
> >
> > /* 1 if 'type' is a pointer type, 0 otherwise. */
> > # define __pointer_type(type) (__builtin_classify_type ((type) 0) == 5)
> >
> > If __builtin_classify_type ever changes, glibc won't build correctly.
>
> I see. In this case, can we solely rely on __builtin_classify_type?
> I dont't think the inner ternary operator is necessary.
>
> /* Enforce zero-extension for pointers and array system call
> arguments. For integer types, extend to long long int (the full
> register) using a regular cast, resulting in zero or sign extension
> based on the signed-ness of the original type. */
> #define ARGIFY(X) \
> ({ \
> _Pragma ("GCC diagnostic push"); \
> _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \
> (__builtin_classify_type (X) == 5 \
> ? (long long int) (uintptr_t) (X) \
> : (long long int) (X))); \
> _Pragma ("GCC diagnostic pop"); \
> })
>
> (Untested, sorry.)
Here is the updated patch.
--
H.J.
[-- Attachment #2: 0002-x32-Properly-pass-long-to-syscall-BZ-25810.patch --]
[-- Type: text/x-patch, Size: 4189 bytes --]
From fd210e7ada784986f5f5bff3e4892478fc998fea Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Mon, 13 Apr 2020 07:02:46 -0700
Subject: V2 [PATCH 2/3] x32: Properly pass long to syscall [BZ #25810]
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.
Enforce zero-extension for pointers and array system call arguments.
For integer types, extend to int64_t (the full register) using a
regular cast, resulting in zero or sign extension based on the
signedness of the original type.
For
void *mmap(void *addr, size_t length, int prot, int flags,
int fd, off_t offset);
we now generate
0: 41 f7 c1 ff 0f 00 00 test $0xfff,%r9d
7: 75 1f jne 28 <__mmap64+0x28>
9: 48 63 d2 movslq %edx,%rdx
c: 89 f6 mov %esi,%esi
e: 4d 63 c0 movslq %r8d,%r8
11: 4c 63 d1 movslq %ecx,%r10
14: b8 09 00 00 40 mov $0x40000009,%eax
19: 0f 05 syscall
That is
1. addr is unchanged.
2. length is zero-extend to 64 bits.
3. prot is sign-extend to 64 bits.
4. flags is sign-extend to 64 bits.
5. fd is sign-extend to 64 bits.
6. offset is unchanged.
For int arguments, since kernel uses only the lower 32 bits and ignores
the upper 32 bits in 64-bit registers, these work correctly.
Tested on x86-64 and x32. There are no code changes on x86-64.
---
sysdeps/unix/sysv/linux/x86_64/sysdep.h | 15 +++++++++------
sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h | 14 ++++++++++++++
2 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
index 06e39212ee..3596273c94 100644
--- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h
+++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
@@ -219,12 +219,15 @@
/* Registers clobbered by syscall. */
# define REGISTERS_CLOBBERED_BY_SYSCALL "cc", "r11", "cx"
-/* Create a variable 'name' based on type 'X' to avoid explicit types.
- This is mainly used set use 64-bits arguments in x32. */
-#define TYPEFY(X, name) __typeof__ ((X) - (X)) name
-/* Explicit cast the argument to avoid integer from pointer warning on
- x32. */
-#define ARGIFY(X) ((__typeof__ ((X) - (X))) (X))
+/* NB: This also works when X is an array. For an array X, type of
+ (X) - (X) is ptrdiff_t, which is signed, since size of ptrdiff_t
+ == size of pointer, cast is a NOP. */
+#define TYPEFY1(X) __typeof__ ((X) - (X))
+/* Explicit cast the argument. */
+#define ARGIFY(X) ((TYPEFY1 (X)) (X))
+/* Create a variable 'name' based on type of variable 'X' to avoid
+ explicit types. */
+#define TYPEFY(X, name) __typeof__ (ARGIFY (X)) name
#undef INTERNAL_SYSCALL
#define INTERNAL_SYSCALL(name, nr, args...) \
diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
index 2b5cf0c0ea..67335c2a7f 100644
--- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
+++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
@@ -44,6 +44,20 @@
# 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
+ regular cast, resulting in zero or sign extension based on the
+ signedness of the original type. */
+# define ARGIFY(X) \
+ ({ \
+ _Pragma ("GCC diagnostic push"); \
+ _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \
+ (__builtin_classify_type (X) == 5 \
+ ? (uintptr_t) (X) : (int64_t) (X)); \
+ _Pragma ("GCC diagnostic pop"); \
+ })
#endif /* __ASSEMBLER__ */
#endif /* linux/x86_64/x32/sysdep.h */
--
2.25.2
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: V2 [PATCH 2/3] x32: Properly pass long to syscall [BZ #25810]
2020-04-13 16:33 ` V2 " H.J. Lu
@ 2020-04-13 17:00 ` Florian Weimer
2020-04-13 18:34 ` Adhemerval Zanella
1 sibling, 0 replies; 13+ messages in thread
From: Florian Weimer @ 2020-04-13 17:00 UTC (permalink / raw)
To: H.J. Lu via Libc-alpha
* H. J. Lu via Libc-alpha:
> From: "H.J. Lu" <hjl.tools@gmail.com>
> Date: Mon, 13 Apr 2020 07:02:46 -0700
> Subject: V2 [PATCH 2/3] x32: Properly pass long to syscall [BZ #25810]
>
> 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.
This version looks okay to me, thanks.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: V2 [PATCH 2/3] x32: Properly pass long to syscall [BZ #25810]
2020-04-13 16:33 ` V2 " H.J. Lu
2020-04-13 17:00 ` Florian Weimer
@ 2020-04-13 18:34 ` Adhemerval Zanella
2020-04-13 20:43 ` H.J. Lu
1 sibling, 1 reply; 13+ messages in thread
From: Adhemerval Zanella @ 2020-04-13 18:34 UTC (permalink / raw)
To: libc-alpha
On 13/04/2020 13:33, H.J. Lu via Libc-alpha wrote:
> On Mon, Apr 13, 2020 at 8:17 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>>
>> * H. J. Lu:
>>
>>> On Mon, Apr 13, 2020 at 7:43 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>>>>
>>>> * H. J. Lu via Libc-alpha:
>>>>
>>>>> diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
>>>>> index 06e39212ee..93dad97c8f 100644
>>>>> --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h
>>>>> +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
>>>>> @@ -219,12 +219,14 @@
>>>>> /* Registers clobbered by syscall. */
>>>>> # define REGISTERS_CLOBBERED_BY_SYSCALL "cc", "r11", "cx"
>>>>>
>>>>> -/* Create a variable 'name' based on type 'X' to avoid explicit types.
>>>>> - This is mainly used set use 64-bits arguments in x32. */
>>>>> -#define TYPEFY(X, name) __typeof__ ((X) - (X)) name
>>>>> +/* This also works when X is an array. */
>>>>> +#define TYPEFY1(X) __typeof__ ((X) - (X))
>>>>
>>>> I think the comment is now misleading. For an array X, (X) - (X) is
>>>> of type ptrdiff_t, which is signed, so it triggers sign extension, not
>>>
>>> I will update the comments.
>>
>> Thanks.
>>
>>>> zero extension, in the x32 case. I think this is the reason why my
>>>> proposed construct went astray in your experiments.
>>>>
>>>> Is it really necessary to support arrays for system call arguments?
>>>
>>> int
>>> fexecve (int fd, char *const argv[], char *const envp[])
>>> {
>>> if (fd < 0 || argv == NULL || envp == NULL)
>>> {
>>> __set_errno (EINVAL);
>>> return -1;
>>> }
>>>
>>> #ifdef __NR_execveat
>>> /* Avoid implicit array coercion in syscall macros. */
>>> INLINE_SYSCALL (execveat, 5, fd, "", &argv[0], &envp[0], AT_EMPTY_PATH);
>>>
>>> "" is an array.
>>
>> If that were the only reason for enforcing pointer decay, I'd say we'd
>> change the fexecve implementation. But see below.
>>
>>>>> diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
>>>>> index 2b5cf0c0ea..f3bc7e89df 100644
>>>>> --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
>>>>> +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
>>>>> @@ -44,6 +44,24 @@
>>>>> # define ZERO_EXTEND_5 movl %r8d, %r8d;
>>>>> # undef ZERO_EXTEND_6
>>>>> # define ZERO_EXTEND_6 movl %r9d, %r9d;
>>>>> +#else /*! __ASSEMBLER__ */
>>>>> +# undef ARGIFY
>>>>> +/* For x32, cast
>>>>> + 1. pointer to unsigned long (32 bit).
>>>>> + 2. 32-bit unsigned integer to unsigned long long (64 bit).
>>>>> + 3. 32-bit integer to long long (64 bit).
>>>>> + */
>>>>> +# define ARGIFY(X) \
>>>>> + ({ \
>>>>> + _Pragma ("GCC diagnostic push"); \
>>>>> + _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \
>>>>> + (__builtin_classify_type (X) == 5 \
>>>>> + ? (unsigned long) (X) \
>>>>> + : ((sizeof (X) <= 4 && (TYPEFY1 (X)) 0 < (TYPEFY1 (X)) -1) \
>>>>> + ? (unsigned long long) (X) \
>>>>> + : (long long) (X))); \
>>>>> + _Pragma ("GCC diagnostic pop"); \
>>>>> + })
>>>>> #endif /* __ASSEMBLER__ */
>>>>
>>>> This should use long int instead long everywhere.
>>>
>>> I will update.
>>>
>>>> __builtin_classify_type is undocumented. I'm not sure if its behavior
>>>> and the constant 5 is actually stable across GCC versions, but the
>>>> powerpc uses it as well, for a slightly difference purpose (to
>>>> recognize array arguments, it seems).
>>>
>>> include/libc-pointer-arith.h has
>>>
>>> /* 1 if 'type' is a pointer type, 0 otherwise. */
>>> # define __pointer_type(type) (__builtin_classify_type ((type) 0) == 5)
>>>
>>> If __builtin_classify_type ever changes, glibc won't build correctly.
>>
>> I see. In this case, can we solely rely on __builtin_classify_type?
>> I dont't think the inner ternary operator is necessary.
>>
>> /* Enforce zero-extension for pointers and array system call
>> arguments. For integer types, extend to long long int (the full
>> register) using a regular cast, resulting in zero or sign extension
>> based on the signed-ness of the original type. */
>> #define ARGIFY(X) \
>> ({ \
>> _Pragma ("GCC diagnostic push"); \
>> _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \
>> (__builtin_classify_type (X) == 5 \
>> ? (long long int) (uintptr_t) (X) \
>> : (long long int) (X))); \
>> _Pragma ("GCC diagnostic pop"); \
>> })
>>
>> (Untested, sorry.)
>
> Here is the updated patch.
>
> diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> index 2b5cf0c0ea..67335c2a7f 100644
> --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> @@ -44,6 +44,20 @@
> # 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
> + regular cast, resulting in zero or sign extension based on the
> + signedness of the original type. */
> +# define ARGIFY(X) \
> + ({ \
> + _Pragma ("GCC diagnostic push"); \
> + _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \
> + (__builtin_classify_type (X) == 5 \
> + ? (uintptr_t) (X) : (int64_t) (X)); \
> + _Pragma ("GCC diagnostic pop"); \
> + })
> #endif /* __ASSEMBLER__ */
>
> #endif /* linux/x86_64/x32/sysdep.h */
I face a similar issue when fixing BZ#12683 for x32, and one possible
solution I found was:
#define __SSC(__x) \
({ \
__syscall_arg_t __arg = sizeof (1 ? (__x) : 0ULL) < 8 \
? (unsigned long int) (uintptr_t)(__x) \
: (__syscall_arg_t) (__typeof__ ((__x) - (__x))) (__x); \
__arg; \
})
Which was for argument passing in the internal cancellable bridge.
Later you suggested a solution I have also devised sometime ago [1].
Couldn't we use any of them instead of explicitly disable/enable
compile warnings?
[1] https://sourceware.org/legacy-ml/libc-alpha/2017-12/msg00315.html
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: V2 [PATCH 2/3] x32: Properly pass long to syscall [BZ #25810]
2020-04-13 18:34 ` Adhemerval Zanella
@ 2020-04-13 20:43 ` H.J. Lu
2020-04-13 21:13 ` Florian Weimer
0 siblings, 1 reply; 13+ messages in thread
From: H.J. Lu @ 2020-04-13 20:43 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: GNU C Library
On Mon, Apr 13, 2020 at 11:35 AM Adhemerval Zanella via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
>
>
> On 13/04/2020 13:33, H.J. Lu via Libc-alpha wrote:
> > On Mon, Apr 13, 2020 at 8:17 AM Florian Weimer <fw@deneb.enyo.de> wrote:
> >>
> >> * H. J. Lu:
> >>
> >>> On Mon, Apr 13, 2020 at 7:43 AM Florian Weimer <fw@deneb.enyo.de> wrote:
> >>>>
> >>>> * H. J. Lu via Libc-alpha:
> >>>>
> >>>>> diff --git a/sysdeps/unix/sysv/linux/x86_64/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> >>>>> index 06e39212ee..93dad97c8f 100644
> >>>>> --- a/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> >>>>> +++ b/sysdeps/unix/sysv/linux/x86_64/sysdep.h
> >>>>> @@ -219,12 +219,14 @@
> >>>>> /* Registers clobbered by syscall. */
> >>>>> # define REGISTERS_CLOBBERED_BY_SYSCALL "cc", "r11", "cx"
> >>>>>
> >>>>> -/* Create a variable 'name' based on type 'X' to avoid explicit types.
> >>>>> - This is mainly used set use 64-bits arguments in x32. */
> >>>>> -#define TYPEFY(X, name) __typeof__ ((X) - (X)) name
> >>>>> +/* This also works when X is an array. */
> >>>>> +#define TYPEFY1(X) __typeof__ ((X) - (X))
> >>>>
> >>>> I think the comment is now misleading. For an array X, (X) - (X) is
> >>>> of type ptrdiff_t, which is signed, so it triggers sign extension, not
> >>>
> >>> I will update the comments.
> >>
> >> Thanks.
> >>
> >>>> zero extension, in the x32 case. I think this is the reason why my
> >>>> proposed construct went astray in your experiments.
> >>>>
> >>>> Is it really necessary to support arrays for system call arguments?
> >>>
> >>> int
> >>> fexecve (int fd, char *const argv[], char *const envp[])
> >>> {
> >>> if (fd < 0 || argv == NULL || envp == NULL)
> >>> {
> >>> __set_errno (EINVAL);
> >>> return -1;
> >>> }
> >>>
> >>> #ifdef __NR_execveat
> >>> /* Avoid implicit array coercion in syscall macros. */
> >>> INLINE_SYSCALL (execveat, 5, fd, "", &argv[0], &envp[0], AT_EMPTY_PATH);
> >>>
> >>> "" is an array.
> >>
> >> If that were the only reason for enforcing pointer decay, I'd say we'd
> >> change the fexecve implementation. But see below.
> >>
> >>>>> diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> >>>>> index 2b5cf0c0ea..f3bc7e89df 100644
> >>>>> --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> >>>>> +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> >>>>> @@ -44,6 +44,24 @@
> >>>>> # define ZERO_EXTEND_5 movl %r8d, %r8d;
> >>>>> # undef ZERO_EXTEND_6
> >>>>> # define ZERO_EXTEND_6 movl %r9d, %r9d;
> >>>>> +#else /*! __ASSEMBLER__ */
> >>>>> +# undef ARGIFY
> >>>>> +/* For x32, cast
> >>>>> + 1. pointer to unsigned long (32 bit).
> >>>>> + 2. 32-bit unsigned integer to unsigned long long (64 bit).
> >>>>> + 3. 32-bit integer to long long (64 bit).
> >>>>> + */
> >>>>> +# define ARGIFY(X) \
> >>>>> + ({ \
> >>>>> + _Pragma ("GCC diagnostic push"); \
> >>>>> + _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \
> >>>>> + (__builtin_classify_type (X) == 5 \
> >>>>> + ? (unsigned long) (X) \
> >>>>> + : ((sizeof (X) <= 4 && (TYPEFY1 (X)) 0 < (TYPEFY1 (X)) -1) \
> >>>>> + ? (unsigned long long) (X) \
> >>>>> + : (long long) (X))); \
> >>>>> + _Pragma ("GCC diagnostic pop"); \
> >>>>> + })
> >>>>> #endif /* __ASSEMBLER__ */
> >>>>
> >>>> This should use long int instead long everywhere.
> >>>
> >>> I will update.
> >>>
> >>>> __builtin_classify_type is undocumented. I'm not sure if its behavior
> >>>> and the constant 5 is actually stable across GCC versions, but the
> >>>> powerpc uses it as well, for a slightly difference purpose (to
> >>>> recognize array arguments, it seems).
> >>>
> >>> include/libc-pointer-arith.h has
> >>>
> >>> /* 1 if 'type' is a pointer type, 0 otherwise. */
> >>> # define __pointer_type(type) (__builtin_classify_type ((type) 0) == 5)
> >>>
> >>> If __builtin_classify_type ever changes, glibc won't build correctly.
> >>
> >> I see. In this case, can we solely rely on __builtin_classify_type?
> >> I dont't think the inner ternary operator is necessary.
> >>
> >> /* Enforce zero-extension for pointers and array system call
> >> arguments. For integer types, extend to long long int (the full
> >> register) using a regular cast, resulting in zero or sign extension
> >> based on the signed-ness of the original type. */
> >> #define ARGIFY(X) \
> >> ({ \
> >> _Pragma ("GCC diagnostic push"); \
> >> _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \
> >> (__builtin_classify_type (X) == 5 \
> >> ? (long long int) (uintptr_t) (X) \
> >> : (long long int) (X))); \
> >> _Pragma ("GCC diagnostic pop"); \
> >> })
> >>
> >> (Untested, sorry.)
> >
> > Here is the updated patch.
> >
>
> > diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> > index 2b5cf0c0ea..67335c2a7f 100644
> > --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> > +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> > @@ -44,6 +44,20 @@
> > # 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
> > + regular cast, resulting in zero or sign extension based on the
> > + signedness of the original type. */
> > +# define ARGIFY(X) \
> > + ({ \
> > + _Pragma ("GCC diagnostic push"); \
> > + _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \
> > + (__builtin_classify_type (X) == 5 \
> > + ? (uintptr_t) (X) : (int64_t) (X)); \
> > + _Pragma ("GCC diagnostic pop"); \
> > + })
> > #endif /* __ASSEMBLER__ */
> >
> > #endif /* linux/x86_64/x32/sysdep.h */
>
> I face a similar issue when fixing BZ#12683 for x32, and one possible
> solution I found was:
>
> #define __SSC(__x) \
> ({ \
> __syscall_arg_t __arg = sizeof (1 ? (__x) : 0ULL) < 8 \
> ? (unsigned long int) (uintptr_t)(__x) \
> : (__syscall_arg_t) (__typeof__ ((__x) - (__x))) (__x); \
> __arg; \
> })
This is similar to what I have checked in.
> Which was for argument passing in the internal cancellable bridge.
> Later you suggested a solution I have also devised sometime ago [1].
>
> Couldn't we use any of them instead of explicitly disable/enable
> compile warnings?
>
> [1] https://sourceware.org/legacy-ml/libc-alpha/2017-12/msg00315.html
This doesn't work on array, like "".
--
H.J.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: V2 [PATCH 2/3] x32: Properly pass long to syscall [BZ #25810]
2020-04-13 20:43 ` H.J. Lu
@ 2020-04-13 21:13 ` Florian Weimer
2020-04-13 21:19 ` H.J. Lu
0 siblings, 1 reply; 13+ messages in thread
From: Florian Weimer @ 2020-04-13 21:13 UTC (permalink / raw)
To: H.J. Lu via Libc-alpha
* H. J. Lu via Libc-alpha:
>> > diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
>> > index 2b5cf0c0ea..67335c2a7f 100644
>> > --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
>> > +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
>> > @@ -44,6 +44,20 @@
>> > # 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
>> > + regular cast, resulting in zero or sign extension based on the
>> > + signedness of the original type. */
>> > +# define ARGIFY(X) \
>> > + ({ \
>> > + _Pragma ("GCC diagnostic push"); \
>> > + _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \
>> > + (__builtin_classify_type (X) == 5 \
>> > + ? (uintptr_t) (X) : (int64_t) (X)); \
>> > + _Pragma ("GCC diagnostic pop"); \
>> > + })
>> > #endif /* __ASSEMBLER__ */
>> >
>> > #endif /* linux/x86_64/x32/sysdep.h */
>>
>> I face a similar issue when fixing BZ#12683 for x32, and one possible
>> solution I found was:
>>
>> #define __SSC(__x) \
>> ({ \
>> __syscall_arg_t __arg = sizeof (1 ? (__x) : 0ULL) < 8 \
>> ? (unsigned long int) (uintptr_t)(__x) \
>> : (__syscall_arg_t) (__typeof__ ((__x) - (__x))) (__x); \
>> __arg; \
>> })
>
> This is similar to what I have checked in.
I think Adhemerval specifically meant the additional cast using
__typeof__ to suppress the warning (which I did not think of).
The _Pragma construct as written is a Clang porting hazard because it
causes the statement expression to return void (which is a Clang/GCC
discrepancy). Rather than introducing a temporary, using the
__typeof__ hack is probably the better way of fixing it.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: V2 [PATCH 2/3] x32: Properly pass long to syscall [BZ #25810]
2020-04-13 21:13 ` Florian Weimer
@ 2020-04-13 21:19 ` H.J. Lu
0 siblings, 0 replies; 13+ messages in thread
From: H.J. Lu @ 2020-04-13 21:19 UTC (permalink / raw)
To: Florian Weimer; +Cc: H.J. Lu via Libc-alpha, Adhemerval Zanella
On Mon, Apr 13, 2020 at 2:13 PM Florian Weimer <fw@deneb.enyo.de> wrote:
>
> * H. J. Lu via Libc-alpha:
>
> >> > diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> >> > index 2b5cf0c0ea..67335c2a7f 100644
> >> > --- a/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> >> > +++ b/sysdeps/unix/sysv/linux/x86_64/x32/sysdep.h
> >> > @@ -44,6 +44,20 @@
> >> > # 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
> >> > + regular cast, resulting in zero or sign extension based on the
> >> > + signedness of the original type. */
> >> > +# define ARGIFY(X) \
> >> > + ({ \
> >> > + _Pragma ("GCC diagnostic push"); \
> >> > + _Pragma ("GCC diagnostic ignored \"-Wpointer-to-int-cast\""); \
> >> > + (__builtin_classify_type (X) == 5 \
> >> > + ? (uintptr_t) (X) : (int64_t) (X)); \
> >> > + _Pragma ("GCC diagnostic pop"); \
> >> > + })
> >> > #endif /* __ASSEMBLER__ */
> >> >
> >> > #endif /* linux/x86_64/x32/sysdep.h */
> >>
> >> I face a similar issue when fixing BZ#12683 for x32, and one possible
> >> solution I found was:
> >>
> >> #define __SSC(__x) \
> >> ({ \
> >> __syscall_arg_t __arg = sizeof (1 ? (__x) : 0ULL) < 8 \
> >> ? (unsigned long int) (uintptr_t)(__x) \
> >> : (__syscall_arg_t) (__typeof__ ((__x) - (__x))) (__x); \
> >> __arg; \
> >> })
> >
> > This is similar to what I have checked in.
>
> I think Adhemerval specifically meant the additional cast using
> __typeof__ to suppress the warning (which I did not think of).
His patch:
https://sourceware.org/pipermail/libc-alpha/2020-April/112520.html
also needs to suppress -Wpointer-to-int-cast
> The _Pragma construct as written is a Clang porting hazard because it
> causes the statement expression to return void (which is a Clang/GCC
> discrepancy). Rather than introducing a temporary, using the
> __typeof__ hack is probably the better way of fixing it.
I don't think it works on array.
--
H.J.
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-04-13 21:19 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-13 14:25 [PATCH 0/3] x32: Properly pass long to syscall [BZ #25810] H.J. Lu
2020-04-13 14:25 ` [PATCH 1/3] Add SYSCALL_ULONG_ARG_[12] to " H.J. Lu
2020-04-13 14:25 ` [PATCH 2/3] x32: Properly " H.J. Lu
2020-04-13 14:43 ` Florian Weimer
2020-04-13 15:03 ` H.J. Lu
2020-04-13 15:17 ` Florian Weimer
2020-04-13 16:33 ` V2 " H.J. Lu
2020-04-13 17:00 ` Florian Weimer
2020-04-13 18:34 ` Adhemerval Zanella
2020-04-13 20:43 ` H.J. Lu
2020-04-13 21:13 ` Florian Weimer
2020-04-13 21:19 ` H.J. Lu
2020-04-13 14:25 ` [PATCH 3/3] Add a syscall test for " H.J. Lu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).