Re: [PATCH] Define new filesystem::__file_clock type

[Jonathan Wakely <jwakely at redhat dot com>]

On 06/01/19 21:45 +0000, Jonathan Wakely wrote:
> On 05/01/19 20:03 +0000, Jonathan Wakely wrote:
> > In C++17 the clock used for filesystem::file_time_type is unspecified,
> > allowing it to be chrono::system_clock. The C++2a draft requires it to
> > be a distinct type, with additional member functions to convert to/from
> > other clocks (either the system clock or UTC). In order to avoid an ABI
> > change later, this patch defines a new distinct type now, which will be
> > used for std::chrono::file_clock later.
> >
> > 	* include/bits/fs_fwd.h (__file_clock): Define new clock.
> > 	(file_time_type): Redefine in terms of __file_clock.
> > 	* src/filesystem/ops-common.h (file_time): Add FIXME comment about
> > 	overflow.
> > 	* src/filesystem/std-ops.cc (is_set(perm_options, perm_options)): Give
> > 	internal linkage.
> > 	(internal_file_lock): New helper type for accessing __file_clock.
> > 	(do_copy_file): Use internal_file_lock to convert system time to
> > 	file_time_type.
> > 	(last_write_time(const path&, error_code&)): Likewise.
> > 	(last_write_time(const path&, file_time_type, error_code&)): Likewise.
> >
> > Tested powerpc64-linux, committed to trunk.
>
> There's a new failure on 32-bit x86:
>
> /home/jwakely/src/gcc/gcc/libstdc++-v3/testsuite/27_io/filesystem/operations/last_write_time.cc:148: void test02(): Assertion 'approx_equal(last_write_time(f.path), time)' failed.
> FAIL: 27_io/filesystem/operations/last_write_time.cc execution test

The problem here is 32-bit time_t. I've defined the file_clock epoch
as a date after 2038, so unrepresentable in a 32-bit time_t.

The test stores a value of file_time_type::zero() which is the epoch,
and so that value can't be converted to time_t for the utimensat call.

Fixed by skipping the parts of the test using the file clock's epoch.

Tested x86_64-linux (-m32 and -m64). Committed to trunk.

commit 68908239cf8c8987ce4693f6769709f8f5b9fbc3
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Jan 10 15:38:31 2019 +0000

    Fix filesystem::last_write_time failure with 32-bit time_t
    
            * testsuite/27_io/filesystem/operations/last_write_time.cc: Fix
            test failures on targets with 32-bit time_t.

diff --git a/libstdc++-v3/testsuite/27_io/filesystem/operations/last_write_time.cc b/libstdc++-v3/testsuite/27_io/filesystem/operations/last_write_time.cc
index 7a693a1ddcb..3f31375f51b 100644
--- a/libstdc++-v3/testsuite/27_io/filesystem/operations/last_write_time.cc
+++ b/libstdc++-v3/testsuite/27_io/filesystem/operations/last_write_time.cc
@@ -22,6 +22,7 @@
 // 15.25 Permissions [fs.op.last_write_time]
 
 #include <filesystem>
+#include <limits>
 #include <testsuite_fs.h>
 #include <testsuite_hooks.h>
 
@@ -141,14 +142,27 @@ test02()
   VERIFY( !ec );
   VERIFY( approx_equal(last_write_time(f.path), time) );
 
+  if (std::numeric_limits<std::time_t>::max()
+      < std::numeric_limits<std::int64_t>::max())
+    return; // file clock's epoch is out of range for 32-bit time_t
+
   ec = bad_ec;
+  // The file clock's epoch:
   time = time_type();
   last_write_time(f.path, time, ec);
   VERIFY( !ec );
   VERIFY( approx_equal(last_write_time(f.path), time) );
 
   ec = bad_ec;
-  time -= std::chrono::milliseconds(1000 * 60 * 10 + 15);
+  // A time after the epoch
+  time += std::chrono::milliseconds(1000 * 60 * 10 + 15);
+  last_write_time(f.path, time, ec);
+  VERIFY( !ec );
+  VERIFY( approx_equal(last_write_time(f.path), time) );
+
+  ec = bad_ec;
+  // A time before than the epoch
+  time -= std::chrono::milliseconds(1000 * 60 * 20 + 15);
   last_write_time(f.path, time, ec);
   VERIFY( !ec );
   VERIFY( approx_equal(last_write_time(f.path), time) );