On Wed, 23 Jan 2019 at 16:28, Jonathan Wakely <jwak...@redhat.com> wrote: > > On 09/01/19 13:53 +0100, Christophe Lyon wrote: > >On Wed, 9 Jan 2019 at 11:11, Jonathan Wakely <jwak...@redhat.com> 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 > >> >><christophe.l...@linaro.org> wrote: > >> >>> > >> >>>On Mon, 7 Jan 2019 at 13:39, Jonathan Wakely <jwak...@redhat.com> 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 <jwak...@redhat.com> > >> >>>> >>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 <unistd.h> 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
Yes, it uses glibc-2.28 That build has this in its libstdc++.log: /home/tcwg-buildslave/workspace/tcwg-buildfarm_0/snapshots/gcc.git~master_rev_2e9ceebcd7618d0e068e0029b43cd75d679022d7/libstdc++-v3/testsuite/27_io/filesystem/path/compare/strings.cc:39: void test01(): Assertion 'p.compare(p0) == p.compare(s0)' failed. timeout: the monitored command dumped core FAIL: 27_io/filesystem/path/compare/strings.cc execution test > > The test should be reducable to simply: > > // { dg-options "-std=gnu++17" } > #include <string> > #include <string_view> > > 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. > I get: as path:-102 as string:-102