Re: [src] cvs commit: src/include unistd.h src/lib/libc/sys readlink.2 src/sys/compat/freebsd32 syscalls.master src/sys/kern syscalls.master vfs_syscalls.c src/sys/sys syscallsubr.h

On Thu, Feb 14, 2008 at 07:38:50PM +0200, Kostik Belousov wrote:
> On Wed, Feb 13, 2008 at 02:35:30PM +0300, Ruslan Ermilov wrote:
> > [ Replying to the list. ]
> > 
> > On Tue, Feb 12, 2008 at 10:48:04PM +0200, Kostik Belousov wrote:
> > > > -int	 readlink(const char *, char *, int);
> > > > +ssize_t	 readlink(const char *, char *, size_t);
> > > You do understand that this changes the ABI ? size_t and int have different
> > > sizes on 64-bit arches, and now upper-half of the register used to pass
> > > the third arg is used.  Amd64, fortunately, makes very hard to load a
> > > non-zero into the upper half, I am not so sure about IA64/sparc64.
> > 
> > I considered that.  I've tested locally on amd64 and sparc64, and
> > ia64 on pluto2.freebsd.org.  Since this is only a third argument,
> > it's passed in a 64-bit register, and for any meaningful value of it
> > (0 .. INT_MAX), there's no ABI change at all.  I compared .s files.
> > 
> > : // cc -S a.c ; mv a.s a.s~ ; cc -S -DNEW a.c ; diff -u a.s~ a.s
> > : #include <sys/types.h>
> > : #include <sys/limits.h>
> > : 
> > : #ifdef NEW
> > : ssize_t readlink(const char *, char *, size_t);
> > : #else
> > : int readlink(const char *, char *, int);
> > : #endif
> > : 
> > : void
> > : foo(void)
> > : {
> > : 	int i;
> > : 	char buf[1024];
> > : 
> > : 	i = readlink("foo", buf, INT_MAX);
> > : }
> > 
> > > This change, IMHO, requires symbol version compat shims.
> > 
> > I don't think so.
> > 
> 
> The slightly contrived example below works on RELENG_7 amd64, relevant
> output from the truss is
> 	readlink("/usr/X11R6","l",1)                     = 1 (0x1)
> on the CURRENT gives
> 	readlink("/usr/X11R6","l",1)                     = -4294967295 (0xffffffff00000001)
> [also please note wrong output for the third readlink arg; ktrace/kdump works
> ok].
> 
> 	.text
> 	.globl	main
> main:	movq	$0xffffffff00000001, %rax
> 	movq	%rax, %rdx
> 	movq	$buf, %rax
> 	movq	%rax, %rsi
> 	movq	$path, %rax
> 	movq	%rax, %rdi
> 	call	readlink
> 	xorl	%edi, %edi
> 	call	exit
> 	
> 	.section	.rodata
> path:	.asciz	"/usr/X11R6"
> 
> 	.data
> 	.comm	buf, 0x80

This is because uio_resid is still "int".

: int
: kern_readlink(struct thread *td, char *path, enum uio_seg pathseg, char *buf,
:     enum uio_seg bufseg, size_t count)
[...]
:                 auio.uio_resid = count;
[...]
:         td->td_retval[0] = count - auio.uio_resid;

uio_resid gets the (truncated) value of "1", VOP_READLINK()
reads 1 char, td_retval[0] gets the value 0xffffffff00000001.
Any meaningful value of the third argument will work OTOH.

As for the wrong output, truss(1) is known to be very broken
in this respect (it doesn't know about long/size_t arguments).


Cheers,
-- 
Ruslan Ermilov
ru_at_FreeBSD.org
FreeBSD committer