From ff62b00522d1ddaeb88be241ab4a022f935b5c00 Mon Sep 17 00:00:00 2001 From: Wouter Klouwen Date: Tue, 12 Dec 2017 21:59:43 +0000 Subject: CTest: add safe conversion from cmDuration to integer types A problem area by recent refactoring of time to std::chrono has been the unsafe conversion from duration to std::chrono::seconds, which is of an unspecified integer type. This commit adds a template function that for a given type provides a safe conversion, effectively clamping a duration into what fits safely in that type. A specialisation for int and unsigned int are provided. It changes the protential problem areas to use this safe function. --- Source/CMakeLists.txt | 3 +++ Source/CTest/cmCTestMemCheckHandler.cxx | 23 +++++++++++------------ Source/CTest/cmCTestRunTest.cxx | 16 +++++----------- Source/CTest/cmCTestScriptHandler.cxx | 13 ++++++------- Source/cmCTest.cxx | 16 ++++++---------- Source/cmDuration.cxx | 27 +++++++++++++++++++++++++++ Source/cmDuration.h | 16 ++++++++++++++++ 7 files changed, 74 insertions(+), 40 deletions(-) create mode 100644 Source/cmDuration.cxx diff --git a/Source/CMakeLists.txt b/Source/CMakeLists.txt index cd1287c..da5dc60 100644 --- a/Source/CMakeLists.txt +++ b/Source/CMakeLists.txt @@ -602,6 +602,9 @@ set(SRCS cm_codecvt.hxx cm_codecvt.cxx cm_thread.hxx + + cmDuration.h + cmDuration.cxx ) SET_PROPERTY(SOURCE cmProcessOutput.cxx APPEND PROPERTY COMPILE_DEFINITIONS diff --git a/Source/CTest/cmCTestMemCheckHandler.cxx b/Source/CTest/cmCTestMemCheckHandler.cxx index 6b6c337..c3fbc0d 100644 --- a/Source/CTest/cmCTestMemCheckHandler.cxx +++ b/Source/CTest/cmCTestMemCheckHandler.cxx @@ -3,6 +3,7 @@ #include "cmCTestMemCheckHandler.h" #include "cmCTest.h" +#include "cmDuration.h" #include "cmSystemTools.h" #include "cmXMLParser.h" #include "cmXMLWriter.h" @@ -920,12 +921,11 @@ bool cmCTestMemCheckHandler::ProcessMemCheckValgrindOutput( break; // stop the copy of output if we are full } } - cmCTestOptionalLog(this->CTest, DEBUG, "End test (elapsed: " - << std::chrono::duration_cast( - std::chrono::steady_clock::now() - sttime) - .count() - << "s)" << std::endl, - this->Quiet); + cmCTestOptionalLog( + this->CTest, DEBUG, "End test (elapsed: " + << cmDurationTo(std::chrono::steady_clock::now() - sttime) + << "s)" << std::endl, + this->Quiet); log = ostr.str(); this->DefectCount += defects; return defects == 0; @@ -966,12 +966,11 @@ bool cmCTestMemCheckHandler::ProcessMemCheckBoundsCheckerOutput( results[err]++; defects++; } - cmCTestOptionalLog(this->CTest, DEBUG, "End test (elapsed: " - << std::chrono::duration_cast( - std::chrono::steady_clock::now() - sttime) - .count() - << "s)" << std::endl, - this->Quiet); + cmCTestOptionalLog( + this->CTest, DEBUG, "End test (elapsed: " + << cmDurationTo(std::chrono::steady_clock::now() - sttime) + << "s)" << std::endl, + this->Quiet); if (defects) { // only put the output of Bounds Checker if there were // errors or leaks detected diff --git a/Source/CTest/cmCTestRunTest.cxx b/Source/CTest/cmCTestRunTest.cxx index a5ec93d..8d602fa 100644 --- a/Source/CTest/cmCTestRunTest.cxx +++ b/Source/CTest/cmCTestRunTest.cxx @@ -621,17 +621,11 @@ bool cmCTestRunTest::ForkProcess(cmDuration testTimeOut, bool explicitTimeout, if (testTimeOut == cmDuration::zero() && explicitTimeout) { timeout = cmDuration::zero(); } - cmCTestOptionalLog( - this->CTest, HANDLER_VERBOSE_OUTPUT, this->Index - << ": " - << "Test timeout computed to be: " - << (timeout == cmCTest::MaxDuration() - ? std::string("infinite") - : std::to_string( - std::chrono::duration_cast(timeout) - .count())) - << "\n", - this->TestHandler->GetQuiet()); + cmCTestOptionalLog(this->CTest, HANDLER_VERBOSE_OUTPUT, this->Index + << ": " + << "Test timeout computed to be: " + << cmDurationTo(timeout) << "\n", + this->TestHandler->GetQuiet()); this->TestProcess->SetTimeout(timeout); diff --git a/Source/CTest/cmCTestScriptHandler.cxx b/Source/CTest/cmCTestScriptHandler.cxx index d5faeb5..15a81f2 100644 --- a/Source/CTest/cmCTestScriptHandler.cxx +++ b/Source/CTest/cmCTestScriptHandler.cxx @@ -159,9 +159,9 @@ void cmCTestScriptHandler::UpdateElapsedTime() { if (this->Makefile) { // set the current elapsed time - auto itime = std::chrono::duration_cast( - std::chrono::steady_clock::now() - this->ScriptStartTime); - auto timeString = std::to_string(itime.count()); + auto itime = cmDurationTo(std::chrono::steady_clock::now() - + this->ScriptStartTime); + auto timeString = std::to_string(itime); this->Makefile->AddDefinition("CTEST_ELAPSED_TIME", timeString.c_str()); } } @@ -569,10 +569,9 @@ int cmCTestScriptHandler::RunCurrentScript() auto interval = std::chrono::steady_clock::now() - startOfInterval; auto minimumInterval = cmDuration(this->MinimumInterval); if (interval < minimumInterval) { - auto sleepTime = std::chrono::duration_cast( - minimumInterval - interval) - .count(); - this->SleepInSeconds(static_cast(sleepTime)); + auto sleepTime = + cmDurationTo(minimumInterval - interval); + this->SleepInSeconds(sleepTime); } if (this->EmptyBinDirOnce) { this->EmptyBinDir = false; diff --git a/Source/cmCTest.cxx b/Source/cmCTest.cxx index 4be02ed..9e76480 100644 --- a/Source/cmCTest.cxx +++ b/Source/cmCTest.cxx @@ -1092,14 +1092,11 @@ int cmCTest::RunTest(std::vector argv, std::string* output, if (timeout <= cmDuration::zero()) { timeout = std::chrono::seconds(1); } - cmCTestLog( - this, HANDLER_VERBOSE_OUTPUT, "Test timeout computed to be: " - << (timeout == cmCTest::MaxDuration() - ? std::string("infinite") - : std::to_string( - std::chrono::duration_cast(timeout) - .count())) - << "\n"); + cmCTestLog(this, HANDLER_VERBOSE_OUTPUT, "Test timeout computed to be: " + << (timeout == cmCTest::MaxDuration() + ? std::string("infinite") + : std::to_string(cmDurationTo(timeout))) + << "\n"); if (cmSystemTools::SameFile(argv[0], cmSystemTools::GetCTestCommand()) && !this->ForceNewCTestProcess) { cmCTest inst; @@ -1121,8 +1118,7 @@ int cmCTest::RunTest(std::vector argv, std::string* output, timeout > cmDuration::zero()) { args.push_back("--test-timeout"); std::ostringstream msg; - msg << std::chrono::duration_cast(timeout) - .count(); + msg << cmDurationTo(timeout); args.push_back(msg.str()); } args.push_back(i); diff --git a/Source/cmDuration.cxx b/Source/cmDuration.cxx new file mode 100644 index 0000000..8ca5d8d --- /dev/null +++ b/Source/cmDuration.cxx @@ -0,0 +1,27 @@ +/* Distributed under the OSI-approved BSD 3-Clause License. See accompanying + file Copyright.txt or https://cmake.org/licensing for details. */ +#define CMDURATION_CPP +#include "cmDuration.h" + +template +T cmDurationTo(const cmDuration& duration) +{ + /* This works because the comparison operators for duration rely on + * std::common_type. + * So for example duration::max() gets promoted to a duration, + * which can then be safely compared. + */ + if (duration >= std::chrono::duration::max()) { + return std::chrono::duration::max().count(); + } + if (duration <= std::chrono::duration::min()) { + return std::chrono::duration::min().count(); + } + // Ensure number of seconds by defining ratio<1> + return std::chrono::duration_cast>>( + duration) + .count(); +} + +template int cmDurationTo(const cmDuration&); +template unsigned int cmDurationTo(const cmDuration&); diff --git a/Source/cmDuration.h b/Source/cmDuration.h index 5f73585..6df1455 100644 --- a/Source/cmDuration.h +++ b/Source/cmDuration.h @@ -6,3 +6,19 @@ #include typedef std::chrono::duration> cmDuration; + +/* + * This function will return number of seconds in the requested type T. + * + * A duration_cast from duration to duration will not yield what + * one might expect if the double representation does not fit into type T. + * This function aims to safely convert, by clamping the double value between + * the permissible valid values for T. + */ +template +T cmDurationTo(const cmDuration& duration); + +#ifndef CMDURATION_CPP +extern template int cmDurationTo(const cmDuration&); +extern template unsigned int cmDurationTo(const cmDuration&); +#endif -- cgit v0.12