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<double> 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<double> 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

@@ -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

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: "
+  cmCTestOptionalLog(
-                       << std::chrono::duration_cast<std::chrono::seconds>(
+    this->CTest, DEBUG, "End test (elapsed: "
-                            std::chrono::steady_clock::now() - sttime)
+      << cmDurationTo<unsigned int>(std::chrono::steady_clock::now() - sttime)
-                            .count()
+      << "s)" << std::endl,
-                       << "s)" << std::endl,
+    this->Quiet);
-                     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: "
+  cmCTestOptionalLog(
-                       << std::chrono::duration_cast<std::chrono::seconds>(
+    this->CTest, DEBUG, "End test (elapsed: "
-                            std::chrono::steady_clock::now() - sttime)
+      << cmDurationTo<unsigned int>(std::chrono::steady_clock::now() - sttime)
-                            .count()
+      << "s)" << std::endl,
-                       << "s)" << std::endl,
+    this->Quiet);
-                     this->Quiet);
   if (defects) {
     // only put the output of Bounds Checker if there were
     // errors or leaks detected

Source/CTest/cmCTestRunTest.cxx

@@ -621,17 +621,11 @@ bool cmCTestRunTest::ForkProcess(cmDuration testTimeOut, bool explicitTimeout,
   if (testTimeOut == cmDuration::zero() && explicitTimeout) {
     timeout = cmDuration::zero();
   }
-  cmCTestOptionalLog(
+  cmCTestOptionalLog(this->CTest, HANDLER_VERBOSE_OUTPUT, this->Index
-    this->CTest, HANDLER_VERBOSE_OUTPUT, this->Index
+                       << ": "
-      << ": "
+                       << "Test timeout computed to be: "
-      << "Test timeout computed to be: "
+                       << cmDurationTo<unsigned int>(timeout) << "\n",
-      << (timeout == cmCTest::MaxDuration()
+                     this->TestHandler->GetQuiet());
-            ? std::string("infinite")
-            : std::to_string(
-                std::chrono::duration_cast<std::chrono::seconds>(timeout)
-                  .count()))
-      << "\n",
-    this->TestHandler->GetQuiet());
 
   this->TestProcess->SetTimeout(timeout);
 

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::seconds>(
+    auto itime = cmDurationTo<unsigned int>(std::chrono::steady_clock::now() -
-      std::chrono::steady_clock::now() - this->ScriptStartTime);
+                                            this->ScriptStartTime);
-    auto timeString = std::to_string(itime.count());
+    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<std::chrono::seconds>(
+        auto sleepTime =
-                           minimumInterval - interval)
+          cmDurationTo<unsigned int>(minimumInterval - interval);
-                           .count();
+        this->SleepInSeconds(sleepTime);
-        this->SleepInSeconds(static_cast<unsigned int>(sleepTime));
       }
       if (this->EmptyBinDirOnce) {
         this->EmptyBinDir = false;

Source/cmCTest.cxx

@@ -1092,14 +1092,11 @@ int cmCTest::RunTest(std::vector<const char*> argv, std::string* output,
   if (timeout <= cmDuration::zero()) {
     timeout = std::chrono::seconds(1);
   }
-  cmCTestLog(
+  cmCTestLog(this, HANDLER_VERBOSE_OUTPUT, "Test timeout computed to be: "
-    this, HANDLER_VERBOSE_OUTPUT, "Test timeout computed to be: "
+               << (timeout == cmCTest::MaxDuration()
-      << (timeout == cmCTest::MaxDuration()
+                     ? std::string("infinite")
-            ? std::string("infinite")
+                     : std::to_string(cmDurationTo<unsigned int>(timeout)))
-            : std::to_string(
+               << "\n");
-                std::chrono::duration_cast<std::chrono::seconds>(timeout)
-                  .count()))
-      << "\n");
   if (cmSystemTools::SameFile(argv[0], cmSystemTools::GetCTestCommand()) &&
       !this->ForceNewCTestProcess) {
     cmCTest inst;
@@ -1121,8 +1118,7 @@ int cmCTest::RunTest(std::vector<const char*> argv, std::string* output,
             timeout > cmDuration::zero()) {
           args.push_back("--test-timeout");
           std::ostringstream msg;
-          msg << std::chrono::duration_cast<std::chrono::seconds>(timeout)
+          msg << cmDurationTo<unsigned int>(timeout);
-                   .count();
           args.push_back(msg.str());
         }
         args.push_back(i);

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 <typename T>
+T cmDurationTo(const cmDuration& duration)
+{
+  /* This works because the comparison operators for duration rely on
+   * std::common_type.
+   * So for example duration<int>::max() gets promoted to a duration<double>,
+   * which can then be safely compared.
+   */
+  if (duration >= std::chrono::duration<T>::max()) {
+    return std::chrono::duration<T>::max().count();
+  }
+  if (duration <= std::chrono::duration<T>::min()) {
+    return std::chrono::duration<T>::min().count();
+  }
+  // Ensure number of seconds by defining ratio<1>
+  return std::chrono::duration_cast<std::chrono::duration<T, std::ratio<1>>>(
+           duration)
+    .count();
+}
+
+template int cmDurationTo<int>(const cmDuration&);
+template unsigned int cmDurationTo<unsigned int>(const cmDuration&);

Source/cmDuration.h

@@ -6,3 +6,19 @@
 #include <ratio>
 
 typedef std::chrono::duration<double, std::ratio<1>> cmDuration;
+
+/*
+ * This function will return number of seconds in the requested type T.
+ *
+ * A duration_cast from duration<double> to duration<T> 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 <typename T>
+T cmDurationTo(const cmDuration& duration);
+
+#ifndef CMDURATION_CPP
+extern template int cmDurationTo<int>(const cmDuration&);
+extern template unsigned int cmDurationTo<unsigned int>(const cmDuration&);
+#endif