svn commit: r303988 - head/lib/libc/gen

Bryan Drewery bdrewery at FreeBSD.org
Wed Aug 24 18:30:28 UTC 2016


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,
> 
> We could solve this by simple mfcing the change I made for xinstall, right?
> 
> Ed
> 
> 
> On 24 Aug 2016 20:15, "Bryan Drewery" <bdrewery at freebsd.org
> <mailto:bdrewery at freebsd.org>> 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
>     <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
>     <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 <sys/param.h>
>     >>
>     >>  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 at 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 at 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 at 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 at 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
>     <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
> 


-- 
Regards,
Bryan Drewery

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 496 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freebsd.org/pipermail/svn-src-all/attachments/20160824/d23bbe3f/attachment.sig>


More information about the svn-src-all mailing list