Re: [UGLY PATCH v3] Support for no-legacy-syscalls archs

[Rich Felker <dalias@libc.org>]

On Tue, May 27, 2014 at 01:26:25AM -0400, Rich Felker wrote:
> diff --git a/src/exit/exit.c b/src/exit/exit.c
> index 353f50b..6ac90c3 100644
> --- a/src/exit/exit.c
> +++ b/src/exit/exit.c
> @@ -1,5 +1,6 @@
>  #include <stdlib.h>
>  #include <stdint.h>
> +#include "futex.h"
>  #include "libc.h"
>  #include "atomic.h"
>  #include "syscall.h"
> @@ -24,7 +25,12 @@ _Noreturn void exit(int code)
>  	static int lock;
>  
>  	/* If more than one thread calls exit, hang until _Exit ends it all */
> -	while (a_swap(&lock, 1)) __syscall(SYS_pause);
> +	while (a_swap(&lock, 1))
> +#ifdef SYS_pausex
> +		__syscall(SYS_pause);
> +#else
> +		__syscall(SYS_futex, &lock, FUTEX_WAIT, 1, 0);
> +#endif

Removed this change simply by dropping the code (see commit
2e55da911896a91e95b24ab5dc8a9d9b0718f4de).

> diff --git a/src/process/posix_spawn.c b/src/process/posix_spawn.c
> index 08644f5..6fa33cb 100644
> --- a/src/process/posix_spawn.c
> +++ b/src/process/posix_spawn.c
> @@ -22,6 +22,18 @@ struct args {
>  
>  void __get_handler_set(sigset_t *);
>  
> +static int do_dup2(int old, int new)
> +{
> +	int r;
> +#ifdef SYS_dup2
> +	while ((r=__syscall(SYS_dup2, old, new))==-EBUSY);
> +#else
> +	while ((r=__syscall(SYS_dup3, old, new, 0))==-EBUSY);
> +	if (r==-EINVAL && old==new) return new;
> +#endif
> +	return r;
> +}
> +

This is overkill; I mistakenly thought the use of dup2 in posix_spawn
was missing the EBUSY check and loop and added all this mess. But the
EBUSY condition cannot happen in a single-threaded process that does
not share (CLONE_FILES) its descriptor table.

In fact, it seems like the EBUSY condition might be something we can
drop completely from dup2.c and dup3.c too. I'm not 100% sure yet but
it looks like it can only happen when the application invokes UB by
misusing dup2/dup3 in a multithreaded process when the destination
descriptor is not already known by the caller to be occupied.

> diff --git a/src/select/select.c b/src/select/select.c
> index f93597b..7b5f6dc 100644
> --- a/src/select/select.c
> +++ b/src/select/select.c
> @@ -1,8 +1,26 @@
>  #include <sys/select.h>
> +#include <signal.h>
> +#include <stdint.h>
> +#include <errno.h>
>  #include "syscall.h"
>  #include "libc.h"
>  
>  int select(int n, fd_set *restrict rfds, fd_set *restrict wfds, fd_set *restrict efds, struct timeval *restrict tv)
>  {
> +#ifdef SYS_select
>  	return syscall_cp(SYS_select, n, rfds, wfds, efds, tv);
> +#else
> +	syscall_arg_t data[2] = { 0, _NSIG/8 };
> +	struct timespec ts;
> +	if (tv) {
> +		if (tv->tv_sec < 0 || tv->tv_usec < 0)
> +			return __syscall_ret(-EINVAL);
> +		time_t extra_secs = tv->tv_usec / 1000000;
> +		ts.tv_nsec = tv->tv_usec % 1000000 * 1000;
> +		const time_t max_time = (1ULL<<8*sizeof(time_t)-1)-1;
> +		ts.tv_sec = extra_secs > max_time - tv->tv_sec ?
> +			max_time : tv->tv_sec + extra_secs;
> +	}
> +	return syscall_cp(SYS_pselect6, n, rfds, wfds, efds, tv ? &ts : 0, data);
> +#endif

This is a lot uglier than I'd prefer, but I don't see any way to
improve it. Blame the kernel folks...

>  }
> diff --git a/src/stat/chmod.c b/src/stat/chmod.c
> index beb66e5..d4f53c5 100644
> --- a/src/stat/chmod.c
> +++ b/src/stat/chmod.c
> @@ -1,7 +1,12 @@
>  #include <sys/stat.h>
> +#include <fcntl.h>
>  #include "syscall.h"
>  
>  int chmod(const char *path, mode_t mode)
>  {
> +#ifdef SYS_chmod
>  	return syscall(SYS_chmod, path, mode);
> +#else
> +	return syscall(SYS_fchmodat, AT_FDCWD, path, mode);
> +#endif
>  }
> diff --git a/src/stat/fchmod.c b/src/stat/fchmod.c
> index 6d28141..93e1b64 100644
> --- a/src/stat/fchmod.c
> +++ b/src/stat/fchmod.c
> @@ -13,5 +13,9 @@ int fchmod(int fd, mode_t mode)
>  
>  	char buf[15+3*sizeof(int)];
>  	__procfdname(buf, fd);
> +#ifdef SYS_chmod
>  	return syscall(SYS_chmod, buf, mode);
> +#else
> +	return syscall(SYS_fchmodat, AT_FDCWD, buf, mode);
> +#endif
>  }
> diff --git a/src/stat/fchmodat.c b/src/stat/fchmodat.c
> index 12e7ff0..52029ec 100644
> --- a/src/stat/fchmodat.c
> +++ b/src/stat/fchmodat.c
> @@ -29,7 +29,7 @@ int fchmodat(int fd, const char *path, mode_t mode, int flag)
>  
>  	__procfdname(proc, fd2);
>  	if (!(ret = __syscall(SYS_stat, proc, &st)) && !S_ISLNK(st.st_mode))
> -		ret = __syscall(SYS_chmod, proc, mode);
> +		ret = __syscall(SYS_fchmodat, AT_FDCWD, proc, mode);
>  
>  	__syscall(SYS_close, fd2);
>  	return __syscall_ret(ret);
> diff --git a/src/stat/futimesat.c b/src/stat/futimesat.c
> index dbefc84..5990aa6 100644
> --- a/src/stat/futimesat.c
> +++ b/src/stat/futimesat.c
> @@ -1,10 +1,23 @@
>  #define _GNU_SOURCE
>  #include <sys/time.h>
> +#include <sys/stat.h>
> +#include <errno.h>
>  #include "syscall.h"
> +#include "libc.h"
>  
> -#ifdef SYS_futimesat
> -int futimesat(int dirfd, const char *pathname, const struct timeval times[2])
> +int __futimesat(int dirfd, const char *pathname, const struct timeval times[2])
>  {
> -	return syscall(SYS_futimesat, dirfd, pathname, times);
> +	struct timespec ts[2];
> +	if (times) {
> +		int i;
> +		for (i=0; i<2; i++) {
> +			if (times[i].tv_usec >= 1000000U)

I think there's a bug here; whether comparison against 1000000U will
promote the LHS to unsigned depends on the definition of suseconds_t.

> diff --git a/src/stat/utimensat.c b/src/stat/utimensat.c
> index 929698b..abc8d74 100644
> --- a/src/stat/utimensat.c
> +++ b/src/stat/utimensat.c
> @@ -1,7 +1,40 @@
>  #include <sys/stat.h>
> +#include <sys/time.h>
> +#include <fcntl.h>
> +#include <errno.h>
>  #include "syscall.h"
>  
>  int utimensat(int fd, const char *path, const struct timespec times[2], int flags)
>  {
> -	return syscall(SYS_utimensat, fd, path, times, flags);
> +	int r = __syscall(SYS_utimensat, fd, path, times, flags);
> +	if (r != -ENOSYS || flags) return __syscall_ret(r);
> +

Not a big deal, but this check can be moved inside the #ifdef:

> +#ifdef SYS_futimesat
> [...]
> 
> diff --git a/src/unistd/dup2.c b/src/unistd/dup2.c
> index 87a0d44..ed04a89 100644
> --- a/src/unistd/dup2.c
> +++ b/src/unistd/dup2.c
> @@ -5,6 +5,11 @@
>  int dup2(int old, int new)
>  {
>  	int r;
> +#ifdef SYS_dup2
>  	while ((r=__syscall(SYS_dup2, old, new))==-EBUSY);
> +#else
> +	while ((r=__syscall(SYS_dup3, old, new, 0))==-EBUSY);
> +	if (r==-EINVAL && old==new) return new;
> +#endif

Sadly this implementation is not correct since the kernel checks for
EINVAL before EBADF; it causes dup2(fd,fd) to always report success,
even if fd is not a valid file descriptor. So I think we really need
an ugly check for validity first...

Rich