Date: Wed, 24 Aug 2016 11:06:32 -0700 From: Bryan Drewery <bdrewery@FreeBSD.org> To: Ed Schouten <ed@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org, jilles@FreeBSD.org Subject: Re: svn commit: r303988 - head/lib/libc/gen Message-ID: <d23b295a-1902-193c-dee6-ba49ebd77280@FreeBSD.org> In-Reply-To: <201608120703.u7C73whf007189@repo.freebsd.org> References: <201608120703.u7C73whf007189@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --5JJ1vNJMdtrVfE1bWEte5sJbcUmPNgvMx Content-Type: multipart/mixed; boundary="kMGWq1BCq8WCPNaxK2TWuMqn10H8hpRfu" From: Bryan Drewery <bdrewery@FreeBSD.org> To: Ed Schouten <ed@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org, jilles@FreeBSD.org Message-ID: <d23b295a-1902-193c-dee6-ba49ebd77280@FreeBSD.org> Subject: Re: svn commit: r303988 - head/lib/libc/gen References: <201608120703.u7C73whf007189@repo.freebsd.org> In-Reply-To: <201608120703.u7C73whf007189@repo.freebsd.org> --kMGWq1BCq8WCPNaxK2TWuMqn10H8hpRfu Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 8/12/16 12:03 AM, Ed Schouten wrote: > Author: ed > Date: Fri Aug 12 07:03:58 2016 > New Revision: 303988 > URL: https://svnweb.freebsd.org/changeset/base/303988 >=20 > Log: > Reimplement dirname(3) to be thread-safe. > =20 > Now that we've updated the prototypes of the basename(3) and dirname(= 3) > functions to conform to POSIX, let's go ahead and reimplement dirname= (3) > in such a way that it's thread-safe, but also guaranteed to succeed. = C > libraries like glibc, musl and the one that's part of Solaris already= > follow such an approach. > =20 > Move the existing implementation to another source file, > freebsd11_dirname.c to keep existing users of the API that pass in a > constant string happy, using symbol versioning. > =20 > Put a new version of the function in dirname.c, obtained from CloudAB= I's > C library. This version scans through the pathname string from left t= o > right, normalizing it, while discarding the last pathname component. > =20 > Reviewed by: emaste, jilles > Differential Revision: https://reviews.freebsd.org/D7355 >=20 > Added: > head/lib/libc/gen/dirname_compat.c > - copied, changed from r303452, head/lib/libc/gen/dirname.c > Modified: > head/lib/libc/gen/Makefile.inc > head/lib/libc/gen/Symbol.map > head/lib/libc/gen/dirname.3 > head/lib/libc/gen/dirname.c >=20 [...] >=20 > Copied and modified: head/lib/libc/gen/dirname_compat.c (from r303452, = head/lib/libc/gen/dirname.c) > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D > --- head/lib/libc/gen/dirname.c Thu Jul 28 16:54:12 2016 (r303452, copy= source) > +++ head/lib/libc/gen/dirname_compat.c Fri Aug 12 07:03:58 2016 (r30398= 8) > @@ -26,7 +26,7 @@ __FBSDID("$FreeBSD$"); > #include <sys/param.h> > =20 > char * > -dirname(char *path) > +__freebsd11_dirname(char *path) > { > static char *dname =3D NULL; > size_t len; > @@ -75,3 +75,5 @@ dirname(char *path) > dname[len] =3D '\0'; > return (dname); > } > + > +__sym_compat(dirname, __freebsd11_dirname, FBSD_1.0); >=20 This creates an interesting situation [1] that breaks building older sources and installing them into a chroot. The dirname@FBSD_1.0 is _not_ used in this case and isn't expected to be. This is coming up most often lately for nanonbsd and staged installs. Example scenario: Host: Head after this commit Src: stable/9 Building to install on another system over NFS or chroot to tar, whatever. Not trying to downgrade the host (though one report was trying this too). Buildworld builds usr.bin/xinstall in the bootstrap-tools phase *for the host* to run during the build and to use for installation later in installworld. This uses *the host libc* and picks up the new dirname@FBSD_1.5 symbol. The reasoning for this is so we can add new flags into the build that install needs and have a newly boostrapped xinstall to use them. I mimiced this by building a releng/11.0 xinstall from head but the result is the same for building an older stable/9: > # readelf -a /usr/obj/root/svn/releng/11.0/usr.bin/xinstall/xinstall | = grep dirname > 000000607200 000300000007 R_X86_64_JUMP_SLOT 0000000000000000 dirname = + 0 > 3: 0000000000000000 247 FUNC GLOBAL DEFAULT UND dirname@FBSD= _1.5 (3) > 108: 0000000000000000 247 FUNC GLOBAL DEFAULT UND dirname@@FBS= D_1.5 Stable/9 too: > ~/svn/stable/9/usr.bin/xinstall # readelf -a $(make whereobj)/xinstall = | grep dirname > 000000607200 000300000007 R_X86_64_JUMP_SLOT 0000000000000000 dirname = + 0 > 3: 0000000000000000 247 FUNC GLOBAL DEFAULT UND dirname@FBSD= _1.5 (3) > 108: 0000000000000000 247 FUNC GLOBAL DEFAULT UND dirname@@FBS= D_1.5 The xinstall being built lacks the fix to expect dirname/basename to modify its arguments (r303450). So later when the *old* xinstall runs with *new host libc* in installworld it gets the wrong result from basename/dirname: > ~/git/freebsd/lib/libcxxrt # INSTALL=3D/usr/obj/root/svn/releng/11.0/us= r.bin/xinstall/xinstall make install DESTDIR=3D/tmp/blah > /usr/obj/root/svn/releng/11.0/usr.bin/xinstall/xinstall -C -o root -g = wheel -m 444 libcxxrt.a /tmp/blah/usr/lib/ > /usr/obj/root/svn/releng/11.0/usr.bin/xinstall/xinstall -C -o root -g = wheel -m 444 libcxxrt_p.a /tmp/blah/usr/lib/ > /usr/obj/root/svn/releng/11.0/usr.bin/xinstall/xinstall -s -o root -g = wheel -m 444 libcxxrt.so.1 /tmp/blah/lib/ > /usr/obj/root/svn/releng/11.0/usr.bin/xinstall/xinstall -o root -g whe= el -m 444 libcxxrt.so.1.debug /tmp/blah/usr/lib/debug/lib/ > /usr/obj/root/svn/releng/11.0/usr.bin/xinstall/xinstall -l rs /tmp/bla= h/lib/libcxxrt.so.1 /tmp/blah/usr/lib/libcxxrt.so > xinstall: symlink ../../lib/libcxxrt.so.1 -> /tmp/blah/usr/lib: File ex= ists > *** Error code 71 So how can we fix? We can't expect older builds to expect basename(3)/dirname(3) to change arguments. We could fix the tips of branches and all relengs/, but not non-tips and I doubt so@ would care to EN something into all relengs/. Nor can we change the xinstall bootstrapping in older builds for the same reasons. We need a fix in head to handle this but I don't have any ideas currently. Interestingly I have an older stable/10 build that has an xinstall before the dirname version was moved and it works fine as it is defaulting to FBSD_1.0: > # readelf -a /usr/obj/root/svn/stable/10/tmp/legacy/usr/bin/install | g= rep dirname > 112: 00000000006f4160 8 OBJECT LOCAL DEFAULT 15 dirname.dnam= e > 1893: 000000000040d950 268 FUNC GLOBAL DEFAULT 3 dirname > ~/git/freebsd/lib/libcxxrt # INSTALL=3D/usr/obj/root/svn/stable/10/tmp/= legacy/usr/bin/install make install DESTDIR=3D/tmp/blah > /usr/obj/root/svn/stable/10/tmp/legacy/usr/bin/install -C -o root -g w= heel -m 444 libcxxrt.a /tmp/blah/usr/lib/ > /usr/obj/root/svn/stable/10/tmp/legacy/usr/bin/install -C -o root -g w= heel -m 444 libcxxrt_p.a /tmp/blah/usr/lib/ > /usr/obj/root/svn/stable/10/tmp/legacy/usr/bin/install -s -o root -g w= heel -m 444 libcxxrt.so.1 /tmp/blah/lib/ > /usr/obj/root/svn/stable/10/tmp/legacy/usr/bin/install -o root -g whee= l -m 444 libcxxrt.so.1.debug /tmp/blah/usr/lib/debug/lib/ > /usr/obj/root/svn/stable/10/tmp/legacy/usr/bin/install -l rs /tmp/blah= /lib/libcxxrt.so.1 /tmp/blah/usr/lib/libcxxrt.so But as soon as I do another buildworld on that checkout it will break the binary. [1] https://lists.freebsd.org/pipermail/freebsd-current/2016-August/063023.ht= ml --=20 Regards, Bryan Drewery --kMGWq1BCq8WCPNaxK2TWuMqn10H8hpRfu-- --5JJ1vNJMdtrVfE1bWEte5sJbcUmPNgvMx Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Comment: GPGTools - https://gpgtools.org iQEcBAEBCgAGBQJXveIpAAoJEDXXcbtuRpfPM8AH/0flfVs7ViHCk4GatAANnQUn 0op9GHb0zMhDXcMnly8Lx/IMIBQkLlPZPL7WGJTyCFMgh0VbgZE9KKsD0YHvM0r/ z/NC+9MXnqQK0pzQ4cuKSjuqnjs/Y7x6Rlrey/q75qJwEGB5jBnxV2mubLqpFH5t WcQhy16tdF3cGA7u6mxudxAoMX9uM8PKQJS9STC8wG+Rp1EAs22UqJhRk72YxQZI cJOjeztW5WAdha7XHEXiiIFHaMdO4mCaAr9jOVbk5XLv17WYw1ftbtZSA9joyzyB oP+6LWT59MmyTMnP8Vq7QS7ouMioKYR6EO/KLRudgKWa6fOCAZDTRI5hxjz7tic= =q7KX -----END PGP SIGNATURE----- --5JJ1vNJMdtrVfE1bWEte5sJbcUmPNgvMx--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?d23b295a-1902-193c-dee6-ba49ebd77280>