Date: Wed, 24 Aug 2016 11:30:17 -0700 From: Bryan Drewery <bdrewery@FreeBSD.org> To: Ed Schouten <ed@nuxi.nl> Cc: svn-src-head@freebsd.org, jilles@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Ed Schouten <ed@freebsd.org> Subject: Re: svn commit: r303988 - head/lib/libc/gen Message-ID: <2632f5f8-d765-3df7-74d7-da878eb4b7a8@FreeBSD.org> In-Reply-To: <CABh_MKkxD3OTF7VO9Rq_eZyqHPN%2BxVws3q3dsH2R3DfZ343kFw@mail.gmail.com> References: <201608120703.u7C73whf007189@repo.freebsd.org> <d23b295a-1902-193c-dee6-ba49ebd77280@FreeBSD.org> <9ae1c2eb-02ad-b8fe-6aff-7e17e955607a@FreeBSD.org> <CABh_MKkxD3OTF7VO9Rq_eZyqHPN%2BxVws3q3dsH2R3DfZ343kFw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --OEj03W6pEbCEg45irbkftSAeFjmWcDW9X Content-Type: multipart/mixed; boundary="PJpGWW3Lf3hN1Wgaxjv3N4Sl3WB4XPuFx" From: Bryan Drewery <bdrewery@FreeBSD.org> To: Ed Schouten <ed@nuxi.nl> Cc: svn-src-head@freebsd.org, jilles@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Ed Schouten <ed@freebsd.org> Message-ID: <2632f5f8-d765-3df7-74d7-da878eb4b7a8@FreeBSD.org> Subject: Re: svn commit: r303988 - head/lib/libc/gen References: <201608120703.u7C73whf007189@repo.freebsd.org> <d23b295a-1902-193c-dee6-ba49ebd77280@FreeBSD.org> <9ae1c2eb-02ad-b8fe-6aff-7e17e955607a@FreeBSD.org> <CABh_MKkxD3OTF7VO9Rq_eZyqHPN+xVws3q3dsH2R3DfZ343kFw@mail.gmail.com> In-Reply-To: <CABh_MKkxD3OTF7VO9Rq_eZyqHPN+xVws3q3dsH2R3DfZ343kFw@mail.gmail.com> --PJpGWW3Lf3hN1Wgaxjv3N4Sl3WB4XPuFx Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable That would only fix stable/11, stable/10, stable/9, releng/11.0. It won't fix releng/10.3, releng/10.2, releng/10.1, releng/9.3, etc... without an EN. It won't fix stable/11 - 1, stable/10 - 1, etc. It will never fix releng/8.4 (unsupported releases) since so@ won't EN to those. People do sometimes need to build these older releases still. It creates a line in the sand where we can never build checkouts older than where the fix was at. So I don't think it is the appropriate fix. On 8/24/16 11:27 AM, Ed Schouten wrote: > Hi Bryan, >=20 > We could solve this by simple mfcing the change I made for xinstall, ri= ght? >=20 > Ed >=20 >=20 > On 24 Aug 2016 20:15, "Bryan Drewery" <bdrewery@freebsd.org > <mailto:bdrewery@freebsd.org>> wrote: >=20 > On 8/24/16 11:06 AM, Bryan Drewery wrote: > > 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 > <https://svnweb.freebsd.org/changeset/base/303988>; > >> > >> Log: > >> Reimplement dirname(3) to be thread-safe. > >> > >> 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. > >> > >> Move the existing implementation to another source file, > >> freebsd11_dirname.c to keep existing users of the API that pas= s > in a > >> constant string happy, using symbol versioning. > >> > >> Put a new version of the function in dirname.c, obtained from > CloudABI's > >> C library. This version scans through the pathname string from= > left to > >> right, normalizing it, while discarding the last pathname > component. > >> > >> Reviewed by: emaste, jilles > >> Differential Revision: https://reviews.freebsd.org/D7355 > <https://reviews.freebsd.org/D7355>; > >> > >> 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 > >> > > > > [...] > > > >> > >> 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 (r303988) > >> @@ -26,7 +26,7 @@ __FBSDID("$FreeBSD$"); > >> #include <sys/param.h> > >> > >> 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); > >> > > > > This creates an interesting situation [1] that breaks building ol= der > > 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 i= n > > 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 t= he > > result is the same for building an older stable/9: > > > >> # readelf -a > /usr/obj/root/svn/releng/11.0/usr.bin/xinstall/xinstall | grep dirn= ame > >> 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@@FBSD_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@@FBSD_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/usr.bin/xinstall/xinstall m= ake > 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 wheel -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=20 > /tmp/blah/lib/libcxxrt.so.1 /tmp/blah/usr/lib/libcxxrt.so > >> xinstall: symlink ../../lib/libcxxrt.so.1 -> /tmp/blah/usr/lib: > File exists > >> *** 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 rele= ngs/. > > 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 xinstal= l > > 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 | grep dirna= me > >> 112: 00000000006f4160 8 OBJECT LOCAL DEFAULT 15 > dirname.dname > >> 1893: 000000000040d950 268 FUNC GLOBAL DEFAULT 3 dirna= me > > > >> ~/git/freebsd/lib/libcxxrt # > INSTALL=3D/usr/obj/root/svn/stable/10/tmp/legacy/usr/bin/install ma= ke > install DESTDIR=3D/tmp/blah > >> /usr/obj/root/svn/stable/10/tmp/legacy/usr/bin/install -C -o > root -g wheel -m 444 libcxxrt.a /tmp/blah/usr/lib/ > >> /usr/obj/root/svn/stable/10/tmp/legacy/usr/bin/install -C -o > root -g wheel -m 444 libcxxrt_p.a /tmp/blah/usr/lib/ > >> /usr/obj/root/svn/stable/10/tmp/legacy/usr/bin/install -s -o > root -g wheel -m 444 libcxxrt.so.1 /tmp/blah/lib/ > >> /usr/obj/root/svn/stable/10/tmp/legacy/usr/bin/install -o root > -g wheel -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=20 > /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 b= reak > > the binary. > > > > [1] > > > https://lists.freebsd.org/pipermail/freebsd-current/2016-August/063= 023.html > <https://lists.freebsd.org/pipermail/freebsd-current/2016-August/06= 3023.html> > > >=20 > I'm drawing blanks, hoping someone else has an idea. Otherwise I t= hink > we should revert this until a solution is thought up. Even putting= a > safe dirname(3)/basename(3) in xinstall won't help for the same rea= sons > as listed. >=20 > -- > Regards, > Bryan Drewery >=20 --=20 Regards, Bryan Drewery --PJpGWW3Lf3hN1Wgaxjv3N4Sl3WB4XPuFx-- --OEj03W6pEbCEg45irbkftSAeFjmWcDW9X 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 iQEcBAEBCgAGBQJXvee5AAoJEDXXcbtuRpfPfmIH/2muV1ThaC+D4hO+Q2sMTyMF Lh+Mocqlq5l2rq2QJfknoTa0G6gedwCKGG2tg+ZROQcIWQBaYoMr+LAgCoBgXQsq LQ8CkzsrFX1EDjJfolz4vHZsb1pag6hQO71i0xTjZ+cnKsG02a+r/dVDTy5JfKpO VOsvND3Xy5tzfGXvReqlgV/hNarNwj7b6KIPGPosXSkjJ6khb1dQTu/822f9VEKW xNQ7PqCgoxBSjkGKFcYOxtK3OGAUtWmqW/RHETCEzIPAE60ImEtIJRSLTKL0NXux LZ7jT1LwagXIh3QvcYFHTEKzzG8tSODPci2JvJZKiLeWLgAlRnEqP3Seyie5rwY= =vlXm -----END PGP SIGNATURE----- --OEj03W6pEbCEg45irbkftSAeFjmWcDW9X--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?2632f5f8-d765-3df7-74d7-da878eb4b7a8>