svn commit: r303988 - head/lib/libc/gen
Bryan Drewery
bdrewery at FreeBSD.org
Wed Aug 24 18:15:07 UTC 2016
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 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
>
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
-------------- 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-head/attachments/20160824/900afeff/attachment.sig>
More information about the svn-src-head
mailing list