From: Jonathan Wakely <jwakely@redhat.com>
To: Christophe Lyon <christophe.lyon@linaro.org>
Cc: libstdc++@gcc.gnu.org, gcc Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH 2/2] PR libstdc++/86756 Move rest of std::filesystem to libstdc++.so
Date: Thu, 24 Jan 2019 13:32:00 -0000 [thread overview]
Message-ID: <20190124130747.GJ15627@redhat.com> (raw)
In-Reply-To: <CAKdteOamNjdTw5cvirVzhgvDqpTyPRjwuTGeWJcADXLduOCLVg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4781 bytes --]
On 23/01/19 17:01 +0100, Christophe Lyon wrote:
>On Wed, 23 Jan 2019 at 16:28, Jonathan Wakely <jwakely@redhat.com> wrote:
>>
>> On 09/01/19 13:53 +0100, Christophe Lyon wrote:
>> >On Wed, 9 Jan 2019 at 11:11, Jonathan Wakely <jwakely@redhat.com> wrote:
>> >>
>> >> On 09/01/19 10:09 +0000, Jonathan Wakely wrote:
>> >> >On 08/01/19 11:13 +0100, Christophe Lyon wrote:
>> >> >>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
I have no idea what's going on here, so I think I will just change the
test to only care about the sign of the result, as in the attached
patch.
[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 1110 bytes --]
commit 84fc5b9d432788414e75df1f62bf8645db57f395
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Thu Jan 24 12:53:45 2019 +0000
Fix failing test due to inconsistent strcmp results
* testsuite/27_io/filesystem/path/compare/strings.cc: Only compare
sign of results.
diff --git a/libstdc++-v3/testsuite/27_io/filesystem/path/compare/strings.cc b/libstdc++-v3/testsuite/27_io/filesystem/path/compare/strings.cc
index 3f0aa4bde06..83487ae35b6 100644
--- a/libstdc++-v3/testsuite/27_io/filesystem/path/compare/strings.cc
+++ b/libstdc++-v3/testsuite/27_io/filesystem/path/compare/strings.cc
@@ -26,6 +26,8 @@
using std::filesystem::path;
+int sign(int i) { return i > 0 ? 1 : i < 0 ? -1 : 0; }
+
void
test01()
{
@@ -36,8 +38,8 @@ test01()
path p(s);
VERIFY( p.compare(s) == 0 );
VERIFY( p.compare(s.c_str()) == 0 );
- VERIFY( p.compare(p0) == p.compare(s0) );
- VERIFY( p.compare(p0) == p.compare(s0.c_str()) );
+ VERIFY( sign(p.compare(p0)) == sign(p.compare(s0)) );
+ VERIFY( sign(p.compare(p0)) == sign(p.compare(s0.c_str())) );
}
}
next prev parent reply other threads:[~2019-01-24 13:07 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-06 22:35 [PATCH 1/2] PR libstdc++/86756 add std::filesystem::path " Jonathan Wakely
2019-01-06 22:36 ` [PATCH 2/2] PR libstdc++/86756 Move rest of std::filesystem " Jonathan Wakely
2019-01-07 9:25 ` Christophe Lyon
2019-01-07 9:48 ` Jonathan Wakely
2019-01-07 12:39 ` Jonathan Wakely
2019-01-07 14:14 ` Christophe Lyon
2019-01-08 10:14 ` Christophe Lyon
2019-01-09 10:09 ` Jonathan Wakely
2019-01-09 10:11 ` Jonathan Wakely
2019-01-09 12:54 ` Christophe Lyon
2019-01-09 17:02 ` Jonathan Wakely
2019-01-23 16:01 ` Jonathan Wakely
2019-01-23 16:05 ` Christophe Lyon
2019-01-24 13:32 ` Jonathan Wakely [this message]
2019-01-07 12:46 ` [PATCH 3/2] Update documentation for C++17 filesystem library Jonathan Wakely
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190124130747.GJ15627@redhat.com \
--to=jwakely@redhat.com \
--cc=christophe.lyon@linaro.org \
--cc=gcc-patches@gcc.gnu.org \
--cc=libstdc++@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).