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
--
Regards,
Bryan Drewery
signature.asc
Description: OpenPGP digital signature
