From owner-svn-src-all@freebsd.org Wed Aug 24 18:15:07 2016 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id BF6BFBC4DEC; Wed, 24 Aug 2016 18:15:07 +0000 (UTC) (envelope-from bdrewery@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:1900:2254:206c::16:87]) by mx1.freebsd.org (Postfix) with ESMTP id A2D931C0B; Wed, 24 Aug 2016 18:15:07 +0000 (UTC) (envelope-from bdrewery@FreeBSD.org) Received: from mail.xzibition.com (localhost [IPv6:::1]) by freefall.freebsd.org (Postfix) with ESMTP id 947881C8B; Wed, 24 Aug 2016 18:15:07 +0000 (UTC) (envelope-from bdrewery@FreeBSD.org) Received: from mail.xzibition.com (localhost [172.31.3.2]) by mail.xzibition.com (Postfix) with ESMTP id 479CC1A753; Wed, 24 Aug 2016 18:15:07 +0000 (UTC) X-Virus-Scanned: amavisd-new at mail.xzibition.com Received: from mail.xzibition.com ([172.31.3.2]) by mail.xzibition.com (mail.xzibition.com [172.31.3.2]) (amavisd-new, port 10026) with LMTP id i6saX_oUbrby; Wed, 24 Aug 2016 18:14:53 +0000 (UTC) Subject: Re: svn commit: r303988 - head/lib/libc/gen DKIM-Filter: OpenDKIM Filter v2.9.2 mail.xzibition.com 5D9D41A747 To: Ed Schouten , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org, jilles@FreeBSD.org References: <201608120703.u7C73whf007189@repo.freebsd.org> From: Bryan Drewery Organization: FreeBSD Message-ID: <9ae1c2eb-02ad-b8fe-6aff-7e17e955607a@FreeBSD.org> Date: Wed, 24 Aug 2016 11:14:52 -0700 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="UUORrO2PcT4TggUvbSEaPbri6KAKR8DNf" X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 24 Aug 2016 18:15:07 -0000 This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --UUORrO2PcT4TggUvbSEaPbri6KAKR8DNf Content-Type: multipart/mixed; boundary="03tp5HJPoQK4vGt2F9Mff9hb8JOaMtpH4" From: Bryan Drewery To: Ed Schouten , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org, jilles@FreeBSD.org Message-ID: <9ae1c2eb-02ad-b8fe-6aff-7e17e955607a@FreeBSD.org> Subject: Re: svn commit: r303988 - head/lib/libc/gen References: <201608120703.u7C73whf007189@repo.freebsd.org> In-Reply-To: --03tp5HJPoQK4vGt2F9Mff9hb8JOaMtpH4 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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 >> >> 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 dirnam= e(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 alread= y >> 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 CloudA= BI's >> C library. This version scans through the pathname string from left = to >> right, normalizing it, while discarding the last pathname component.= >> =20 >> Reviewed by: emaste, jilles >> Differential Revision: 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 >> >=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, cop= y source) >> +++ head/lib/libc/gen/dirname_compat.c Fri Aug 12 07:03:58 2016 (r3039= 88) >> @@ -26,7 +26,7 @@ __FBSDID("$FreeBSD$"); >> #include >> =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. >=20 > Example scenario: >=20 > 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 tryin= g > this too). >=20 > Buildworld builds usr.bin/xinstall in the bootstrap-tools phase *for th= e > 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. >=20 > 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. >=20 > I mimiced this by building a releng/11.0 xinstall from head but the > result is the same for building an older stable/9: >=20 >> # 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@FBS= D_1.5 (3) >> 108: 0000000000000000 247 FUNC GLOBAL DEFAULT UND dirname@@FB= SD_1.5 >=20 > 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@FBS= D_1.5 (3) >> 108: 0000000000000000 247 FUNC GLOBAL DEFAULT UND dirname@@FB= SD_1.5 >=20 >=20 > The xinstall being built lacks the fix to expect dirname/basename to > modify its arguments (r303450). >=20 > So later when the *old* xinstall runs with *new host libc* in > installworld it gets the wrong result from basename/dirname: >=20 >> ~/git/freebsd/lib/libcxxrt # INSTALL=3D/usr/obj/root/svn/releng/11.0/u= sr.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 wh= eel -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/bl= ah/lib/libcxxrt.so.1 /tmp/blah/usr/lib/libcxxrt.so >> xinstall: symlink ../../lib/libcxxrt.so.1 -> /tmp/blah/usr/lib: File e= xists >> *** Error code 71 >=20 >=20 > So how can we fix? >=20 > 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 an= y > ideas currently. >=20 >=20 > 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: >=20 >> # readelf -a /usr/obj/root/svn/stable/10/tmp/legacy/usr/bin/install | = grep dirname >> 112: 00000000006f4160 8 OBJECT LOCAL DEFAULT 15 dirname.dna= me >> 1893: 000000000040d950 268 FUNC GLOBAL DEFAULT 3 dirname >=20 >> ~/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 = 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 whe= el -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/bla= h/lib/libcxxrt.so.1 /tmp/blah/usr/lib/libcxxrt.so >=20 > But as soon as I do another buildworld on that checkout it will break > the binary. >=20 > [1] > https://lists.freebsd.org/pipermail/freebsd-current/2016-August/063023.= html >=20 I'm drawing blanks, hoping someone else has an idea. Otherwise I think 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 reasons as listed. --=20 Regards, Bryan Drewery --03tp5HJPoQK4vGt2F9Mff9hb8JOaMtpH4-- --UUORrO2PcT4TggUvbSEaPbri6KAKR8DNf 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 iQEcBAEBCgAGBQJXveQdAAoJEDXXcbtuRpfPE8gIAKI1akGr9m3rY7uHDMBlVI7S TpuhlkRz25li4MxITPV+Q7EBp9GIqDuaYrGIc1eP0VyX2NY+G0QS1m3qjv/nJhBL 4flfvIOFsrGNGOkYXgzVifMAyZPTb4debR3y+VhkCC1Eokyapxyb4QPTXyX/vfZb +jOK1Knpm6SJzJprbmc2kFt6Q5obssIfXPZllXCYlBdzea7AwA0XeRabjebVEQBQ +dWG4XcFPI0TITMZTCXA/OpdPb9/NUCVECYVpMtfSp5h+wDT69WvGM4393Q9tEkc lDfQJJ0b0hHWIYRJZCLjPBSPEBPBF/HXT08yj4NCj5OLGmFJelZTyAu4T3VaTB4= =XqLt -----END PGP SIGNATURE----- --UUORrO2PcT4TggUvbSEaPbri6KAKR8DNf--