Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 15 Feb 2008 12:02:43 +0200
From:      Kostik Belousov <kostikbel@gmail.com>
To:        Ruslan Ermilov <ru@freebsd.org>
Cc:        current@freebsd.org
Subject:   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
Message-ID:  <20080215100243.GF57756@deviant.kiev.zoral.com.ua>
In-Reply-To: <20080214211744.GA80604@team.vega.ru>
References:  <200802122009.m1CK94Y8026959@repoman.freebsd.org> <20080212200911.B49F416A51C@hub.freebsd.org> <20080212204803.GT57756@deviant.kiev.zoral.com.ua> <20080213113530.GB45243@team.vega.ru> <20080214173850.GB57756@deviant.kiev.zoral.com.ua> <20080214211744.GA80604@team.vega.ru>

next in thread | previous in thread | raw e-mail | index | archive | help

--5Q2zYvwyseWqYgH3
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Fri, Feb 15, 2008 at 12:17:47AM +0300, Ruslan Ermilov wrote:
> 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. ]
> > >=20
> > > 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 d=
ifferent
> > > > 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.
> > >=20
> > > 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.
> > >=20
> > > : // 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>
> > > :=20
> > > : #ifdef NEW
> > > : ssize_t readlink(const char *, char *, size_t);
> > > : #else
> > > : int readlink(const char *, char *, int);
> > > : #endif
> > > :=20
> > > : void
> > > : foo(void)
> > > : {
> > > : 	int i;
> > > : 	char buf[1024];
> > > :=20
> > > : 	i =3D readlink("foo", buf, INT_MAX);
> > > : }
> > >=20
> > > > This change, IMHO, requires symbol version compat shims.
> > >=20
> > > I don't think so.
> > >=20
> >=20
> > The slightly contrived example below works on RELENG_7 amd64, relevant
> > output from the truss is
> > 	readlink("/usr/X11R6","l",1)                     =3D 1 (0x1)
> > on the CURRENT gives
> > 	readlink("/usr/X11R6","l",1)                     =3D -4294967295 (0xff=
ffffff00000001)
> > [also please note wrong output for the third readlink arg; ktrace/kdump=
 works
> > ok].
> >=20
> > 	.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
> > =09
> > 	.section	.rodata
> > path:	.asciz	"/usr/X11R6"
> >=20
> > 	.data
> > 	.comm	buf, 0x80
>=20
> This is because uio_resid is still "int".
>=20
> : int
> : kern_readlink(struct thread *td, char *path, enum uio_seg pathseg, char=
 *buf,
> :     enum uio_seg bufseg, size_t count)
> [...]
> :                 auio.uio_resid =3D count;
> [...]
> :         td->td_retval[0] =3D count - auio.uio_resid;
>=20
> 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.
The point of the conversation I started is exactly this: the domain of the
_reasonable_ values for the third arg is changed after your commit.
The value that was perfectly acceptable before the commit now causes
wrong effects. I consider this to be the ABI change.

--5Q2zYvwyseWqYgH3
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.8 (FreeBSD)

iEYEARECAAYFAke1Y0MACgkQC3+MBN1Mb4g9fACdGDAI5oXOQ+QeEa+A2CQ8Yk1N
bHQAn2AiV/bAKvcQ3UcLfGyYcACH6sf/
=Mulk
-----END PGP SIGNATURE-----

--5Q2zYvwyseWqYgH3--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080215100243.GF57756>