From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 75032 invoked by alias); 28 Sep 2016 18:02:35 -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 74633 invoked by uid 89); 28 Sep 2016 18:02:31 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.9 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=century, H*Ad:U*eric, 1525, utime.h X-Spam-User: qpsmtpd, 2 recipients 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, 28 Sep 2016 18:02:21 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 0A1EA31B32E; Wed, 28 Sep 2016 18:02:20 +0000 (UTC) Received: from localhost (ovpn-116-23.ams2.redhat.com [10.36.116.23]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u8SI2IMR012588; Wed, 28 Sep 2016 14:02:19 -0400 Date: Wed, 28 Sep 2016 18:04:00 -0000 From: Jonathan Wakely To: libstdc++@gcc.gnu.org, gcc-patches@gcc.gnu.org Cc: eric@efcs.ca Subject: [PATCH] Check for overflow in filesystem::last_write_time Message-ID: <20160928180218.GA4826@redhat.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="d6Gm4EdcadzBjdND" Content-Disposition: inline X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.7.0 (2016-08-17) X-SW-Source: 2016-09/txt/msg02173.txt.bz2 --d6Gm4EdcadzBjdND Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline Content-length: 920 A system_clock::time_point can't be used to represent all valid values of a 64-bit time_t because nearly half the bits are used up for the nanoseconds part. This makes filesystem::last_write_time() check the time_t values and report an error if it can't be converted to the time_point type. This means we don't get undefined behaviour for files with mtime after the 24th century :-) Thanks to Eric Fiselier for point it out. * include/experimental/bits/fs_fwd.h (file_time_type): Simplify definition. * src/filesystem/ops.cc (file_time): Take error_code parameter and check for overflow. (do_copy_file, last_write_time): Pass error_code in file_time calls. * testsuite/experimental/filesystem/operations/last_write_time.cc: New. * testsuite/util/testsuite_fs.h (scoped_file): Define RAII helper. Tested x86_64-linux, committed to trunk. Backports for this (and some other filesystem changes) to follow soon. --d6Gm4EdcadzBjdND Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="patch.txt" Content-length: 7765 commit 3a59a5aabe310ca9e659dcf73829b3f89454f3ab Author: Jonathan Wakely Date: Wed Sep 28 13:51:09 2016 +0100 Check for overflow in filesystem::last_write_time * include/experimental/bits/fs_fwd.h (file_time_type): Simplify definition. * src/filesystem/ops.cc (file_time): Take error_code parameter and check for overflow. (do_copy_file, last_write_time): Pass error_code in file_time calls. * testsuite/experimental/filesystem/operations/last_write_time.cc: New. * testsuite/util/testsuite_fs.h (scoped_file): Define RAII helper. diff --git a/libstdc++-v3/include/experimental/bits/fs_fwd.h b/libstdc++-v3/include/experimental/bits/fs_fwd.h index 57aa4d3..b9cc041 100644 --- a/libstdc++-v3/include/experimental/bits/fs_fwd.h +++ b/libstdc++-v3/include/experimental/bits/fs_fwd.h @@ -253,7 +253,7 @@ _GLIBCXX_END_NAMESPACE_CXX11 operator^=(directory_options& __x, directory_options __y) noexcept { return __x = __x ^ __y; } - typedef chrono::time_point file_time_type; + using file_time_type = std::chrono::system_clock::time_point; // operational functions diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc index 0ecb8b9..659cfbb 100644 --- a/libstdc++-v3/src/filesystem/ops.cc +++ b/libstdc++-v3/src/filesystem/ops.cc @@ -288,16 +288,24 @@ namespace } inline fs::file_time_type - file_time(const stat_type& st) noexcept + file_time(const stat_type& st, std::error_code& ec) noexcept { using namespace std::chrono; - return fs::file_time_type{ #ifdef _GLIBCXX_USE_ST_MTIM - seconds{st.st_mtim.tv_sec} + nanoseconds{st.st_mtim.tv_nsec} + time_t s = st.st_mtim.tv_sec; + nanoseconds ns{st.st_mtim.tv_nsec}; #else - seconds{st.st_mtime} + time_t s = st.st_mtime; + nanoseconds ns{}; #endif - }; + + if (s >= (nanoseconds::max().count() / 1e9)) + { + ec = std::make_error_code(std::errc::value_too_large); // EOVERFLOW + return fs::file_time_type::min(); + } + ec.clear(); + return fs::file_time_type{seconds{s} + ns}; } // Returns true if the file descriptor was successfully closed, @@ -373,11 +381,11 @@ namespace } else if (is_set(option, opts::update_existing)) { - if (file_time(*from_st) <= file_time(*to_st)) - { - ec.clear(); - return false; - } + const auto from_mtime = file_time(*from_st, ec); + if (ec) + return false; + if ((from_mtime <= file_time(*to_st, ec)) || ec) + return false; } else if (!is_set(option, opts::overwrite_existing)) { @@ -1036,7 +1044,7 @@ fs::last_write_time(const path& p) fs::file_time_type fs::last_write_time(const path& p, error_code& ec) noexcept { - return do_stat(p, ec, [](const auto& st) { return file_time(st); }, + return do_stat(p, ec, [&ec](const auto& st) { return file_time(st, ec); }, file_time_type::min()); } diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/last_write_time.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/last_write_time.cc new file mode 100644 index 0000000..b1aea20 --- /dev/null +++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/last_write_time.cc @@ -0,0 +1,111 @@ +// Copyright (C) 2016 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . + +// { dg-options "-lstdc++fs" } +// { dg-do run { target c++11 } } +// { dg-require-filesystem-ts "" } + +// 15.25 Permissions [fs.op.last_write_time] + +#include +#include +#include + +#ifdef _GLIBCXX_HAVE_FCNTL_H +# include +#endif +#if _GLIBCXX_HAVE_UTIME_H +# include +#endif + +void +test01() +{ + bool test __attribute__((unused)) = true; + using time_type = std::experimental::filesystem::file_time_type; + + auto p = __gnu_test::nonexistent_path(); + std::error_code ec; + time_type mtime = last_write_time(p, ec); + VERIFY( ec ); + VERIFY( ec == std::make_error_code(std::errc::no_such_file_or_directory) ); +#if __cpp_exceptions + bool caught = false; + try { + mtime = last_write_time(p); + } catch (std::system_error const& e) { + caught = true; + ec = e.code(); + } + VERIFY( caught ); + VERIFY( ec ); + VERIFY( ec == std::make_error_code(std::errc::no_such_file_or_directory) ); +#endif + + __gnu_test::scoped_file file(p); + VERIFY( exists(p) ); + mtime = last_write_time(p, ec); + VERIFY( !ec ); + VERIFY( mtime <= time_type::clock::now() ); + VERIFY( mtime == last_write_time(p) ); + + auto end_of_time = time_type::duration::max(); + auto last_second + = std::chrono::duration_cast(end_of_time).count(); + if (last_second > std::numeric_limits::max()) + return; // can't test overflow + +#if _GLIBCXX_USE_UTIMENSAT + struct ::timespec ts[2]; + ts[0].tv_sec = 0; + ts[0].tv_nsec = UTIME_NOW; + ts[1].tv_sec = std::numeric_limits::max() - 1; + ts[1].tv_nsec = 0; + VERIFY( !::utimensat(AT_FDCWD, p.c_str(), ts, 0) ); +#elif _GLIBCXX_HAVE_UTIME_H + ::utimbuf times; + times.modtime = std::numeric_limits::max() - 1; + times.actime = std::numeric_limits::max() - 1; + VERIFY( !::utime(p.c_str(), ×) ); +#else + return; +#endif + + mtime = last_write_time(p, ec); + VERIFY( ec ); + VERIFY( ec == std::make_error_code(std::errc::value_too_large) ); + VERIFY( mtime == time_type::min() ); + +#if __cpp_exceptions + caught = false; + try { + mtime = last_write_time(p); + } catch (std::system_error const& e) { + caught = true; + ec = e.code(); + } + VERIFY( caught ); + VERIFY( ec ); + VERIFY( ec == std::make_error_code(std::errc::value_too_large) ); +#endif +} + +int +main() +{ + test01(); +} diff --git a/libstdc++-v3/testsuite/util/testsuite_fs.h b/libstdc++-v3/testsuite/util/testsuite_fs.h index f1e0bfc..5b36670 100644 --- a/libstdc++-v3/testsuite/util/testsuite_fs.h +++ b/libstdc++-v3/testsuite/util/testsuite_fs.h @@ -23,7 +23,7 @@ #define _TESTSUITE_FS_H 1 #include -#include +#include #include #include #include @@ -40,7 +40,6 @@ namespace __gnu_test compare_paths(const std::experimental::filesystem::path& p1, const std::experimental::filesystem::path& p2) { - // std::cout << "Comparing " << p1 << " and " << p2 << std::endl; PATH_CHK( p1, p2, string ); PATH_CHK( p1, p2, empty ); PATH_CHK( p1, p2, has_root_path ); @@ -95,5 +94,23 @@ namespace __gnu_test return p; } + // RAII helper to remove a file on scope exit. + struct scoped_file + { + using path_type = std::experimental::filesystem::path; + + enum adopt_file_t { adopt_file }; + + explicit + scoped_file(const path_type& p = nonexistent_path()) : path(p) + { std::ofstream{p.native()}; } + + scoped_file(path_type p, adopt_file_t) : path(p) { } + + ~scoped_file() { remove(path); } + + path_type path; + }; + } // namespace __gnu_test #endif --d6Gm4EdcadzBjdND--