Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 24 Aug 2016 20:27:53 +0200
From:      Ed Schouten <ed@nuxi.nl>
To:        Bryan Drewery <bdrewery@freebsd.org>
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:  <CABh_MKkxD3OTF7VO9Rq_eZyqHPN%2BxVws3q3dsH2R3DfZ343kFw@mail.gmail.com>
In-Reply-To: <9ae1c2eb-02ad-b8fe-6aff-7e17e955607a@FreeBSD.org>
References:  <201608120703.u7C73whf007189@repo.freebsd.org> <d23b295a-1902-193c-dee6-ba49ebd77280@FreeBSD.org> <9ae1c2eb-02ad-b8fe-6aff-7e17e955607a@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
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@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
> >>
> >> 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 <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@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
>
>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CABh_MKkxD3OTF7VO9Rq_eZyqHPN%2BxVws3q3dsH2R3DfZ343kFw>