From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 95771 invoked by alias); 23 Jan 2019 15:28:16 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 95755 invoked by uid 89); 23 Jan 2019 15:28:16 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-11.9 required=5.0 tests=BAYES_00,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=stringscc, strings.cc, UD:strings.cc, tedious X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 23 Jan 2019 15:28:14 +0000 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id AFD4789AD3; Wed, 23 Jan 2019 15:28:12 +0000 (UTC) Received: from localhost (unknown [10.33.36.67]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2C81417277; Wed, 23 Jan 2019 15:28:12 +0000 (UTC) Date: Wed, 23 Jan 2019 16:01:00 -0000 From: Jonathan Wakely To: Christophe Lyon Cc: libstdc++@gcc.gnu.org, gcc Patches Subject: Re: [PATCH 2/2] PR libstdc++/86756 Move rest of std::filesystem to libstdc++.so Message-ID: <20190123152811.GA18402@redhat.com> References: <20190106223506.GA21904@redhat.com> <20190106223631.GM15627@redhat.com> <20190107094825.GN15627@redhat.com> <20190107123945.GP15627@redhat.com> <20190109100923.GV15627@redhat.com> <20190109101128.GW15627@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.10.1 (2018-07-13) X-SW-Source: 2019-01/txt/msg01377.txt.bz2 On 09/01/19 13:53 +0100, Christophe Lyon wrote: >On Wed, 9 Jan 2019 at 11:11, Jonathan Wakely wrote: >> >> On 09/01/19 10:09 +0000, Jonathan Wakely wrote: >> >On 08/01/19 11:13 +0100, Christophe Lyon wrote: >> >>On Mon, 7 Jan 2019 at 15:14, Christophe Lyon wrote: >> >>> >> >>>On Mon, 7 Jan 2019 at 13:39, Jonathan Wakely wrote: >> >>>> >> >>>> On 07/01/19 09:48 +0000, Jonathan Wakely wrote: >> >>>> >On 07/01/19 10:24 +0100, Christophe Lyon wrote: >> >>>> >>Hi Jonathan >> >>>> >> >> >>>> >>On Sun, 6 Jan 2019 at 23:37, Jonathan Wakely wrote: >> >>>> >>> >> >>>> >>>Move std::filesystem directory iterators and operations from >> >>>> >>>libstdc++fs.a to main libstdc++ library. These components have many >> >>>> >>>dependencies on OS support, which is not available on all targets. Some >> >>>> >>>additional autoconf checks and conditional compilation is needed to >> >>>> >>>ensure the files will build for all targets. Previously this code was >> >>>> >>>not compiled without --enable-libstdcxx-filesystem-ts but the C++17 >> >>>> >>>components should be available for all hosted builds. >> >>>> >>> >> >>>> >>>The tests for these components no longer need to link to libstdc++fs.a, >> >>>> >>>but are not expected to pass on all targets. To avoid numerous failures >> >>>> >>>on targets which are not expected to pass the tests (due to missing OS >> >>>> >>>functionality) leave the dg-require-filesystem-ts directives in place >> >>>> >>>for now. This will ensure the tests only run for builds where the >> >>>> >>>filesystem-ts library is built, which presumably means some level of OS >> >>>> >>>support is present. >> >>>> >>> >> >>>> >>> >> >>>> >>>Tested x86_64-linux (old/new string ABIs, 32/64 bit), x86_64-w64-mingw32. >> >>>> >>> >> >>>> >>>Committed to trunk. >> >>>> >>> >> >>>> >> >> >>>> >>After this commit (r267616), I've noticed build failures for my >> >>>> >>newlib-based toolchains: >> >>>> >>aarch64-elf, arm-eabi: >> >>>> >> >> >>>> >>In file included from >> >>>> >>/tmp/5241593_7.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libstdc++-v3/src/c++17/fs_ops.cc:57: >> >>>> >>/tmp/5241593_7.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libstdc++-v3/src/c++17/../filesystem/ops-common.h:142:11: >> >>>> >>error: '::truncate' has not been declared >> >>>> >> 142 | using ::truncate; >> >>>> >> | ^~~~~~~~ >> >>>> >>/tmp/5241593_7.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libstdc++-v3/src/c++17/fs_ops.cc: >> >>>> >>In function 'void std::filesystem::resize_file(const >> >>>> >>std::filesystem::__cxx11::path&, uintmax_t, std::error_code&)': >> >>>> >>/tmp/5241593_7.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/libstdc++-v3/src/c++17/fs_ops.cc:1274:19: >> >>>> >>error: 'truncate' is not a member of 'posix' >> >>>> >>1274 | else if (posix::truncate(p.c_str(), size)) >> >>>> >> | ^~~~~~~~ >> >>>> >>make[5]: *** [fs_ops.lo] Error 1 >> >>>> >> >> >>>> >>I'm not sure if there's an obvious fix? Note that I'm using a rather >> >>>> >>old newlib version, if that matters. >> >>>> > >> >>>> >That's probably the reason, as I didn't see this in my tests with >> >>>> >newlib builds. >> >>>> > >> >>>> >The fix is to add yet another autoconf check and guard the uses of >> >>>> >truncate with a _GLIBCXX_USE_TRUNCATE macro. I'll do that now ... >> >>>> >> >>>> >> >>>> Should be fixed with this patch, committed to trunk as r267647. >> >>>> >> >>> >> >>>Yes, it works. Thanks! >> >>> >> >> >> >>Hi Jonathan, >> >> >> >>So... this was a confirmation that the GCC build succeeded, not that >> >>the tests pass :) >> >> >> >>And there are actually a couple new errors with my newlib-based toolchains: >> >>FAIL: 27_io/filesystem/operations/all.cc (test for excess errors) >> >>FAIL: 27_io/filesystem/operations/resize_file.cc (test for excess errors) >> >>FAIL: 27_io/filesystem/path/generation/normal2.cc (test for excess errors) >> >>which are also UNRESOLVED, because of link-time undefined reference to `chdir', >> >>chmod, mkdir, pathconf and getcwd. >> > >> >Ah, I was assuming if is present, then those basic >> >functions will be present. More autoconf checks needed, I guess. >> >That isn't hard to do, just tedious. >> > >> >>On aarch64, I'm seeing an addtional: >> >>FAIL: 27_io/filesystem/path/compare/strings.cc execution test >> >>because: >> >>/libstdc++-v3/testsuite/27_io/filesystem/path/compare/strings.cc:39: >> >>void test01(): Assertion 'p.compare(p0) == p.compare(s0)' failed. >> > >> >Odd, I don't know why that would be target-specific. It's probably >> >just latent on other targets. I'll try to reproduce it on my aarch64 >> >system, but it will take a while to build current trunk. >> > >> >If you have time, could you please apply this patch, re-run that test >> >> *This* patch: >> >> --- a/libstdc++-v3/testsuite/27_io/filesystem/path/compare/strings.cc >> +++ b/libstdc++-v3/testsuite/27_io/filesystem/path/compare/strings.cc >> @@ -36,6 +36,7 @@ test01() >> path p(s); >> VERIFY( p.compare(s) == 0 ); >> VERIFY( p.compare(s.c_str()) == 0 ); >> + __builtin_printf("Comparing %s as path:%d as string:%d\n", s.c_str(), p.compare(p0), p.compare(s0)); >> VERIFY( p.compare(p0) == p.compare(s0) ); >> VERIFY( p.compare(p0) == p.compare(s0.c_str()) ); >> } >> >> >> >> >> >(cd $target/libstdc++-v3 && make check >> >RUNTESTFLAGS=conformance.exp=*/path/compare/strings/cc) and send me >> >the output from the $target/libstdc++-v3/testsuite/libstdc++.log file? >> > >> >On x86_64 I get: >> > >> >Comparing as path:-1 as string:-1 >> >Comparing / as path:-1 as string:-1 >> >Comparing // as path:-1 as string:-1 >> >Comparing /. as path:-51 as string:-51 >> >Comparing /./ as path:-51 as string:-51 >> >Comparing /a as path:-2 as string:-2 >> >Comparing /a/ as path:-1 as string:-1 >> >Comparing /a// as path:-1 as string:-1 >> >Comparing /a/b/c/d as path:1 as string:1 >> >Comparing /a//b as path:1 as string:1 >> >Comparing a as path:-1 as string:-1 >> >Comparing a/b as path:-1 as string:-1 >> >Comparing a/b/ as path:-1 as string:-1 >> >Comparing a/b/c as path:-1 as string:-1 >> >Comparing a/b/c.d as path:-1 as string:-1 >> >Comparing a/b/.. as path:-1 as string:-1 >> >Comparing a/b/c. as path:-1 as string:-1 >> >Comparing a/b/.c as path:-1 as string:-1 >> >PASS: 27_io/filesystem/path/compare/strings.cc execution test >> > > >Here is what I have on aarch64-none-elf: >Comparing as path:-1 as string:-1^M >Comparing / as path:-1 as string:-1^M >Comparing // as path:-1 as string:-1^M >Comparing /. as path:-102 as string:-51^M >/libstdc++-v3/testsuite/27_io/filesystem/path/compare/strings.cc:40: >void test01(): Assertion 'p.compare(p0) == p.compare(s0)' failed.^M >^M >*** EXIT code 4242^M >emu: host signal 6^M >FAIL: 27_io/filesystem/path/compare/strings.cc execution test This is a strange one. With my aarch64-unknown-linux-gnu trunk build I get: Comparing /. as path:-51 as string:-51 Which suggests it's a newlib vs glibc difference, but this target uses glibc (right?) and has the same FAIL: https://gcc.gnu.org/ml/gcc-testresults/2019-01/msg02276.html The test should be reducable to simply: // { dg-options "-std=gnu++17" } #include #include int main() { std::string_view s0 = "a"; std::string p0(s0); std::string p("."); __builtin_printf("as path:%d as string:%d\n", p.compare(p0), p.compare(s0)); } Again, I get -51 and -51 for this. Could you test it on aarch64-none-elf? In terms of what the standard requires, this comparison is based on strcmp, i.e. it only specifies a result less than zero, equal to zero, or greater than zero. But I'd like to know why the comparisons aren't returning the same consistent value.