From owner-svn-src-all@freebsd.org Wed Aug 24 18:27:55 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 988ECBC512C for ; Wed, 24 Aug 2016 18:27:55 +0000 (UTC) (envelope-from ed@nuxi.nl) Received: from mail-yw0-x22d.google.com (mail-yw0-x22d.google.com [IPv6:2607:f8b0:4002:c05::22d]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 582F21725 for ; Wed, 24 Aug 2016 18:27:55 +0000 (UTC) (envelope-from ed@nuxi.nl) Received: by mail-yw0-x22d.google.com with SMTP id r9so15351434ywg.0 for ; Wed, 24 Aug 2016 11:27:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuxi-nl.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=+zRUxIXDA772R7AmgP6pABjowu76nbQ+u0VlfoVN288=; b=z9/X3i7TPDWT2eSJF75j82/MOkANiXjECK9m+omICE0wbKSop2A6fyE1XJiEaybD6i i75ZCML509tKjEGL1JnKbK1EroM+gPSa7YVkczSyrm31l9TAuFjztpPRKDouPEsFB6It kQkQoX4x7yVzAMeJOmRneH3nHUoA2kbU4b3/jGiD+SE7+5GGyxkleKVD5CkfnWdNkWXg eGL+T5fC8VOV6AKIgLGyW5DfLt67m/a+HwOVWiDX4K4AIIeIjt1dq7kJhjHjXuHdd5gy rW8UEEeBRdejojaquZLjZMLOYaOW3WBUwkM55UK6KB9qpRU1iYww3I7LpC07jxpOp0yX JwLw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=+zRUxIXDA772R7AmgP6pABjowu76nbQ+u0VlfoVN288=; b=YsQ36vzQYdFssaozaPphU+l8LGySbCrEI/ZVbjuyIPrJzB2TWerLLcigcBGRaIYtuc EwDy/oZBKtZVcPdzO8wSR2wC9eeAvRMZfxTxWq7CxcivAxALMDsftYeCVExkYBUS1Qu4 0RRLArzh8x6W+eUYkTy6ATJVwAT7YuUL++LO/4FKD3dh/jb+XLu3C4+WLxtR+xTWsz0j A+5s+LmUmaCbE5uJzLQ7BlW6DR3MhyLqNRvIqgrEVgt4Mg+sEEuuOMpFRTaNZXt5djBO VmMBX/3lv5vUxsUBw8CwxHgJ+IcjtmIbUJ/sKVJr7jQJEQRxJlnyWw6oXbqFe6UJBT4D YpBQ== X-Gm-Message-State: AE9vXwNSXM0/KWyJ3D2i9BsEdBcynFvED6p/o5a5P+g1hArEqSVW8E4wS2ticms75bCXYKRmhxMgyaBBER2gNg== X-Received: by 10.129.122.7 with SMTP id v7mr3590714ywc.219.1472063274287; Wed, 24 Aug 2016 11:27:54 -0700 (PDT) MIME-Version: 1.0 Received: by 10.13.201.71 with HTTP; Wed, 24 Aug 2016 11:27:53 -0700 (PDT) Received: by 10.13.201.71 with HTTP; Wed, 24 Aug 2016 11:27:53 -0700 (PDT) In-Reply-To: <9ae1c2eb-02ad-b8fe-6aff-7e17e955607a@FreeBSD.org> References: <201608120703.u7C73whf007189@repo.freebsd.org> <9ae1c2eb-02ad-b8fe-6aff-7e17e955607a@FreeBSD.org> From: Ed Schouten Date: Wed, 24 Aug 2016 20:27:53 +0200 Message-ID: Subject: Re: svn commit: r303988 - head/lib/libc/gen To: Bryan Drewery Cc: svn-src-head@freebsd.org, jilles@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Ed Schouten Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.22 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:27:55 -0000 Hi Bryan, We could solve this by simple mfcing the change I made for xinstall, right? Ed On 24 Aug 2016 20:15, "Bryan Drewery" wrote: > 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. > >> > >> 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 pass 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 > >> > >> 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) > >> ============================================================ > ================== > >> --- 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 > >> > >> char * > >> -dirname(char *path) > >> +__freebsd11_dirname(char *path) > >> { > >> static char *dname = NULL; > >> size_t len; > >> @@ -75,3 +75,5 @@ dirname(char *path) > >> dname[len] = '\0'; > >> return (dname); > >> } > >> + > >> +__sym_compat(dirname, __freebsd11_dirname, FBSD_1.0); > >> > > > > 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@ > @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=/usr/obj/root/svn/ > releng/11.0/usr.bin/xinstall/xinstall make install DESTDIR=/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 > /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 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 | > grep dirname > >> 112: 00000000006f4160 8 OBJECT LOCAL DEFAULT 15 dirname.dname > >> 1893: 000000000040d950 268 FUNC GLOBAL DEFAULT 3 dirname > > > >> ~/git/freebsd/lib/libcxxrt # INSTALL=/usr/obj/root/svn/ > stable/10/tmp/legacy/usr/bin/install make install DESTDIR=/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 > /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.html > > > > 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. > > -- > Regards, > Bryan Drewery > >