From c8da41390cdd66d84a7a1a93487dbe3401b53c13 Mon Sep 17 00:00:00 2001 From: Philip Top Date: Tue, 11 Jun 2019 07:37:47 -0700 Subject: [PATCH] change the checked_multiply function to not use undefined behavior to check for potential undefined behavior and wrapping. --- include/CLI/Validators.hpp | 18 +++++++++++++----- tests/HelpersTest.cpp | 10 ++++++++++ 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/include/CLI/Validators.hpp b/include/CLI/Validators.hpp index 96253a489..56f2c3737 100644 --- a/include/CLI/Validators.hpp +++ b/include/CLI/Validators.hpp @@ -1,5 +1,4 @@ #pragma once - // Distributed under the 3-Clause BSD License. See accompanying // file LICENSE or https://github.com/CLIUtils/CLI11 for details. @@ -9,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -509,18 +509,26 @@ auto search(const T &set, const V &val, const std::function &filter_functi }); return {(it != std::end(setref)), it}; } +/// Generate the absolute value of a number +template inline typename std::enable_if::value, T>::type absval(T a) { + return static_cast((std::abs)(a)); +} +/// unsigned values just return the value +template inline typename std::enable_if::value, T>::type absval(T a) { return a; } /// Performs a *= b; if it doesn't cause integer overflow. Returns false otherwise. template typename std::enable_if::value, bool>::type checked_multiply(T &a, T b) { - if(a == 0 || b == 0) { + if(a == 0 || b == 0 || a == 1 || b == 1) { a *= b; return true; } - T c = a * b; - if(c / a != b) { + if(a == (std::numeric_limits::min)() || b == (std::numeric_limits::min)()) { return false; } - a = c; + if((std::numeric_limits::max)() / absval(a) < absval(b)) { + return false; + } + a *= b; return true; } diff --git a/tests/HelpersTest.cpp b/tests/HelpersTest.cpp index c7fa8eb18..595e0804a 100644 --- a/tests/HelpersTest.cpp +++ b/tests/HelpersTest.cpp @@ -423,10 +423,20 @@ TEST(CheckedMultiply, Int) { ASSERT_FALSE(CLI::detail::checked_multiply(a, b)); ASSERT_EQ(a, std::numeric_limits::min()); + b = std::numeric_limits::min(); + a = -1; + ASSERT_FALSE(CLI::detail::checked_multiply(a, b)); + ASSERT_EQ(a, -1); + a = std::numeric_limits::min() / 100; b = 99; ASSERT_TRUE(CLI::detail::checked_multiply(a, b)); ASSERT_EQ(a, std::numeric_limits::min() / 100 * 99); + + a = std::numeric_limits::min() / 100; + b = -101; + ASSERT_FALSE(CLI::detail::checked_multiply(a, b)); + ASSERT_EQ(a, std::numeric_limits::min() / 100); } TEST(CheckedMultiply, SizeT) {