Fix bugs in last_write_time implementation.

* Fix passing a negative number as either tv_usec or tv_nsec. When file_time_type
  is negative and has a non-zero sub-second value we subtract 1 from tv_sec
  and make the sub-second duration positive.

* Detect and report when 'file_time_type' cannot be represented by time_t. This
  happens when using large/small file_time_type values with a 32 bit time_t.

There is more work to be done in the implementation. It should start to use
stat's st_mtim or st_mtimeval if it's provided as an extension. That way
we can provide a better resolution.



git-svn-id: https://llvm.org/svn/llvm-project/libcxx/trunk@273103 91177308-0d34-0410-b5e6-96231b3b80d8

src/experimental/filesystem/operations.cpp

@@ -486,9 +486,45 @@ bool __fs_is_empty(const path& p, std::error_code *ec)
 }
 
 
+namespace detail { namespace {
+
+template <class CType, class ChronoType>
+bool checked_set(CType* out, ChronoType time) {
+    using Lim = numeric_limits<CType>;
+    if (time > Lim::max() || time < Lim::min())
+        return false;
+    *out = static_cast<CType>(time);
+    return true;
+}
+
+constexpr long long min_seconds = file_time_type::duration::min().count()
+    / file_time_type::period::den;
+
+template <class SubSecDurT, class SubSecT>
+bool set_times_checked(time_t* sec_out, SubSecT* subsec_out, file_time_type tp) {
+    using namespace chrono;
+    auto dur = tp.time_since_epoch();
+    auto sec_dur = duration_cast<seconds>(dur);
+    auto subsec_dur = duration_cast<SubSecDurT>(dur - sec_dur);
+    // The tv_nsec and tv_usec fields must not be negative so adjust accordingly
+    if (subsec_dur.count() < 0) {
+        if (sec_dur.count() > min_seconds) {
+
+            sec_dur -= seconds(1);
+            subsec_dur += seconds(1);
+        } else {
+            subsec_dur = SubSecDurT::zero();
+        }
+    }
+    return checked_set(sec_out, sec_dur.count())
+        && checked_set(subsec_out, subsec_dur.count());
+}
+
+}} // end namespace detail
+
+
 file_time_type __last_write_time(const path& p, std::error_code *ec)
 {
-    using Clock = file_time_type::clock;
     std::error_code m_ec;
     struct ::stat st;
     detail::posix_stat(p, st, &m_ec);
@@ -497,10 +533,9 @@ file_time_type __last_write_time(const path& p, std::error_code *ec)
         return file_time_type::min();
     }
     if (ec) ec->clear();
-    return Clock::from_time_t(static_cast<std::time_t>(st.st_mtime));
+    return file_time_type::clock::from_time_t(st.st_mtime);
 }
 
-
 void __last_write_time(const path& p, file_time_type new_time,
                        std::error_code *ec)
 {
@@ -513,34 +548,38 @@ void __last_write_time(const path& p, file_time_type new_time,
     // This implementation has a race condition between determining the
     // last access time and attempting to set it to the same value using
     // ::utimes
-    using Clock = file_time_type::clock;
     struct ::stat st;
     file_status fst = detail::posix_stat(p, st, &m_ec);
     if (m_ec && !status_known(fst)) {
         set_or_throw(m_ec, ec, "last_write_time", p);
         return;
     }
-    auto write_dur = new_time.time_since_epoch();
-    auto write_sec = duration_cast<seconds>(write_dur);
-    auto access_dur = Clock::from_time_t(st.st_atime).time_since_epoch();
-    auto access_sec = duration_cast<seconds>(access_dur);
     struct ::timeval tbuf[2];
-    tbuf[0].tv_sec = access_sec.count();
+    tbuf[0].tv_sec = st.st_atime;
-    tbuf[0].tv_usec = duration_cast<microseconds>(access_dur - access_sec).count();
+    tbuf[0].tv_usec = 0;
-    tbuf[1].tv_sec = write_sec.count();
+    const bool overflowed = !detail::set_times_checked<microseconds>(
-    tbuf[1].tv_usec = duration_cast<microseconds>(write_dur - write_sec).count();
+        &tbuf[1].tv_sec, &tbuf[1].tv_usec, new_time);
+
+    if (overflowed) {
+        set_or_throw(make_error_code(errc::invalid_argument), ec,
+                     "last_write_time", p);
+        return;
+    }
     if (::utimes(p.c_str(), tbuf) == -1) {
         m_ec = detail::capture_errno();
     }
 #else
-    auto dur_since_epoch = new_time.time_since_epoch();
-    auto sec_since_epoch = duration_cast<seconds>(dur_since_epoch);
-    auto ns_since_epoch = duration_cast<nanoseconds>(dur_since_epoch - sec_since_epoch);
     struct ::timespec tbuf[2];
     tbuf[0].tv_sec = 0;
     tbuf[0].tv_nsec = UTIME_OMIT;
-    tbuf[1].tv_sec = sec_since_epoch.count();
+
-    tbuf[1].tv_nsec = ns_since_epoch.count();
+    const bool overflowed = !detail::set_times_checked<nanoseconds>(
+        &tbuf[1].tv_sec, &tbuf[1].tv_nsec, new_time);
+    if (overflowed) {
+        set_or_throw(make_error_code(errc::invalid_argument),
+            ec, "last_write_time", p);
+        return;
+    }
     if (::utimensat(AT_FDCWD, p.c_str(), tbuf, 0) == -1) {
         m_ec = detail::capture_errno();
     }

test/std/experimental/filesystem/fs.op.funcs/fs.op.last_write_time/last_write_time.pass.cpp

@@ -22,7 +22,6 @@
 #include <type_traits>
 #include <chrono>
 #include <fstream>
-#include <iostream>
 #include <cstdlib>
 
 #include "test_macros.h"
@@ -30,6 +29,7 @@
 #include "filesystem_test_helper.hpp"
 
 #include <sys/stat.h>
+#include <iostream>
 
 using namespace std::experimental::filesystem;
 
@@ -72,6 +72,13 @@ std::pair<std::time_t, std::time_t> GetSymlinkTimes(path const& p) {
     return {st.st_atime, st.st_mtime};
 }
 
+inline bool TimeIsRepresentableAsTimeT(file_time_type tp) {
+    using namespace std::chrono;
+    using Lim = std::numeric_limits<std::time_t>;
+    auto sec = duration_cast<seconds>(tp.time_since_epoch()).count();
+    return (sec >= Lim::min() && sec <= Lim::max());
+}
+
 
 TEST_SUITE(exists_test_suite)
 
@@ -162,37 +169,58 @@ TEST_CASE(set_last_write_time_dynamic_env_test)
     using Sec = std::chrono::seconds;
     using Hours = std::chrono::hours;
     using Minutes = std::chrono::minutes;
-
+    using MicroSec = std::chrono::microseconds;
     scoped_test_env env;
 
     const path file = env.create_file("file", 42);
     const path dir = env.create_dir("dir");
+    const auto now = Clock::now();
+    const file_time_type epoch_time = now - now.time_since_epoch();
 
-    const file_time_type future_time = Clock::now() + Hours(3);
+    const file_time_type future_time = now + Hours(3) + Sec(42) + MicroSec(17);
-    const file_time_type past_time = Clock::now() - Minutes(3);
+    const file_time_type past_time = now - Minutes(3) - Sec(42) - MicroSec(17);
+    const file_time_type before_epoch_time = epoch_time - Minutes(3) - Sec(42) - MicroSec(17);
+    // FreeBSD has a bug in their utimes implementation where the time is not update
+    // when the number of seconds is '-1'.
+#if defined(__FreeBSD__)
+    const file_time_type just_before_epoch_time = epoch_time - Sec(2) - MicroSec(17);
+#else
+    const file_time_type just_before_epoch_time = epoch_time - MicroSec(17);
+#endif
 
     struct TestCase {
       path p;
       file_time_type new_time;
     } cases[] = {
+        {file, epoch_time},
+        {dir, epoch_time},
         {file, future_time},
         {dir, future_time},
         {file, past_time},
-        {dir, past_time}
+        {dir, past_time},
+        {file, before_epoch_time},
+        {dir, before_epoch_time},
+        {file, just_before_epoch_time},
+        {dir, just_before_epoch_time}
     };
     for (const auto& TC : cases) {
         const auto old_times = GetTimes(TC.p);
+        file_time_type old_time(Sec(old_times.second));
 
         std::error_code ec = GetTestEC();
         last_write_time(TC.p, TC.new_time, ec);
         TEST_CHECK(!ec);
 
-        const std::time_t new_time_t = Clock::to_time_t(TC.new_time);
         file_time_type  got_time = last_write_time(TC.p);
-        std::time_t got_time_t = Clock::to_time_t(got_time);
 
-        TEST_CHECK(got_time_t != old_times.second);
+        TEST_CHECK(got_time != old_time);
-        TEST_CHECK(got_time_t == new_time_t);
+        if (TC.new_time < epoch_time) {
+            TEST_CHECK(got_time <= TC.new_time);
+            TEST_CHECK(got_time > TC.new_time - Sec(1));
+        } else {
+            TEST_CHECK(got_time <= TC.new_time + Sec(1));
+            TEST_CHECK(got_time >= TC.new_time - Sec(1));
+        }
         TEST_CHECK(LastAccessTime(TC.p) == old_times.first);
     }
 }
@@ -229,41 +257,79 @@ TEST_CASE(last_write_time_symlink_test)
     TEST_CHECK(GetSymlinkTimes(sym) == old_sym_times);
 }
 
+
+TEST_CASE(test_write_min_time)
+{
+    using Clock = file_time_type::clock;
+    using Sec = std::chrono::seconds;
+    using MicroSec = std::chrono::microseconds;
+    using Lim = std::numeric_limits<std::time_t>;
+    scoped_test_env env;
+    const path p = env.create_file("file", 42);
+
+    std::error_code ec = GetTestEC();
+    file_time_type last_time = last_write_time(p);
+    file_time_type new_time = file_time_type::min();
+
+    last_write_time(p, new_time, ec);
+    file_time_type tt = last_write_time(p);
+
+    if (!TimeIsRepresentableAsTimeT(new_time)) {
+        TEST_CHECK(ec);
+        TEST_CHECK(ec != GetTestEC());
+        TEST_CHECK(tt == last_time);
+    } else {
+        TEST_CHECK(!ec);
+        TEST_CHECK(tt >= new_time);
+        TEST_CHECK(tt < new_time + Sec(1));
+    }
+
+    ec = GetTestEC();
+    last_write_time(p, Clock::now());
+    last_time = last_write_time(p);
+
+    new_time = file_time_type::min() + MicroSec(1);
+
+    last_write_time(p, new_time, ec);
+    tt = last_write_time(p);
+
+    if (!TimeIsRepresentableAsTimeT(new_time)) {
+        TEST_CHECK(ec);
+        TEST_CHECK(ec != GetTestEC());
+        TEST_CHECK(tt == last_time);
+    } else {
+        TEST_CHECK(!ec);
+        TEST_CHECK(tt >= new_time);
+        TEST_CHECK(tt < new_time + Sec(1));
+    }
+}
+
+
+
 TEST_CASE(test_write_min_max_time)
 {
     using Clock = file_time_type::clock;
     using Sec = std::chrono::seconds;
     using Hours = std::chrono::hours;
+    using Lim = std::numeric_limits<std::time_t>;
     scoped_test_env env;
     const path p = env.create_file("file", 42);
 
-    file_time_type last_time = last_write_time(p);
-
-    file_time_type new_time = file_time_type::min();
     std::error_code ec = GetTestEC();
-    last_write_time(p, new_time, ec);
+    file_time_type last_time = last_write_time(p);
-    file_time_type tt = last_write_time(p);
+    file_time_type new_time = file_time_type::max();
-    if (ec) {
-        TEST_CHECK(ec != GetTestEC());
-        TEST_CHECK(tt == last_time);
-    } else {
-        file_time_type max_allowed = new_time + Sec(1);
-        TEST_CHECK(tt >= new_time);
-        TEST_CHECK(tt < max_allowed);
-    }
 
-    last_time = tt;
-    new_time = file_time_type::max();
     ec = GetTestEC();
     last_write_time(p, new_time, ec);
+    file_time_type tt = last_write_time(p);
 
-    tt = last_write_time(p);
+    if (!TimeIsRepresentableAsTimeT(new_time)) {
-    if (ec) {
+        TEST_CHECK(ec);
         TEST_CHECK(ec != GetTestEC());
         TEST_CHECK(tt == last_time);
     } else {
-        file_time_type min_allowed = new_time - Sec(1);
+        TEST_CHECK(!ec);
-        TEST_CHECK(tt > min_allowed);
+        TEST_CHECK(tt > new_time - Sec(1));
         TEST_CHECK(tt <= new_time);
     }
 }