Re: changes in i386 and x86_64 assembrer

Bela Lubkin <[email protected]> Sat, 25 Dec 2010 03:18:58 -0800
Newsgroups gmane.linux.lib.dietlibc
Message-ID <[email protected]>
Two notes on what Nikola Vladov wrote.  Neither of these are actually
about Nikola's change, they are other details I noticed while looking at
the change: one bug and one tiny code shrink.

1: wrong errno threshold:

> +.global __error_unified_syscall
> +__error_unified_syscall:
> 	cmp	$-124,%eax
> 	jb	.Lnoerror

This errno threshold is wrong.  Current Linux "normal" errnos go up to
132 (ERFKILL).

There are also a population of errors at 521..530, defined in kernel
source: include/linux/errno.h.  These are inside `#ifdef __KERNEL__' and
commented "These should never be seen by user programs"; but they do
leak into user space.

One of the high values is ENOTSUPP (524), which is returned by a fair
number of paths.  This simple program invokes one of them to demonstrate
that something is wrong (requires SCTP protocol driver):

   #include <sys/socket.h>
   #include <netinet/sctp.h>

   main()
   {
      int len = 4;
      int ret;
      int fd;

      fd = socket(PF_INET, SOCK_STREAM, IPPROTO_SCTP);
      getsockopt(fd, SOL_SCTP, SCTP_PARTIAL_DELIVERY_POINT, &ret, &len);
      perror("ENOTSUPP :=");
   }

   $ strace -e getsockopt sctp-enotsupp
   getsockopt(5, 0x84 /* SOL_??? */, 19, 0xbf9e16d8, 0xbf9e16dc) = -1 ENOTSUPP (Unknown error 524)
   ENOTSUPP :=: Success

glibc's syscall() compares against 0xfffff001 as if it expects errnos up
to 4095.

However, the program behaves the same with glibc since its getsockopt()
doesn't use syscall() and compares to 0xffffff83 (-125).  This is with
current kernel & libc on Ubuntu 10.10 i386.

This seems wrong in both libcs.  Maybe something is supposed to
intercept ENOTSUPP and convert it to EINVAL; maybe not; but certainly it
shouldn't pretend nothing went wrong.

getsockopt() was just one of a number of examples.  It *might* be the
only one that leaks to user space, I stopped looking when I found it.

2: save 2 bytes of code:

> 	neg	%eax
> #ifdef WANT_THREAD_SAFE
> -	movl	%eax,%ebx
> -	call	__errno_location
> -	movl	%ebx,(%eax)
> +	/* we cannot use anymore %ebp; it is recovered already above! */
> +	push	%eax
> +	call	__errno_location
> +	pop	(%eax)
> 	orl	$-1,%eax
> #else
> 	mov	%eax,errno
> 	sbb	%eax,%eax               # eax = eax - eax - CY = -1
> #endif
> .Lnoerror:

Setting %eax to return -1 can be shortened:

   	...
   	pop	(%eax)
  	stc
  #else
  	mov	%eax,errno
  #endif
  	sbb	%eax,%eax               # eax = eax - eax - CY = -1

This is 2 bytes shorter and unifies one more instruction.  Probably a
tiny bit slower, too, but not noticable on an error path.

>Bela<