From 654d63dfae872db7cd9a1e9245a930c76cfcc8bb Mon Sep 17 00:00:00 2001 From: "Dexter.k" <164054284+rootvector2@users.noreply.github.com> Date: Tue, 2 Jun 2026 21:28:20 +0000 Subject: [PATCH] Avoid undefined std::abs(INT_MIN) in integer_parser (#498) * Avoid undefined std::abs(INT_MIN) in integer_parser When parsing a negative integer into a signed type whose minimum value equals INTMAX_MIN (e.g. int64_t on typical platforms), integer_parser computes the limit as std::abs(static_cast(min())). Negating INTMAX_MIN cannot be represented in intmax_t, so the std::abs call is undefined behaviour. UBSan flags this on every negative int64_t parse, including a trivial one like "-1": runtime error: negation of -9223372036854775808 cannot be represented in type 'long'; cast to an unsigned type to negate this value to itself Build the same unsigned magnitude (|min| == max + 1 for two's-complement) directly in the unsigned arithmetic type instead. For unsigned T the expression wraps to 0, which matches the previous std::abs(0) value, so behaviour for unsigned types is preserved. * test: cover INT64_MIN parse and run parser tests under UBSan * test: skip UBSan flags on Windows clang-cl/MinGW builds --- include/cxxopts.hpp | 6 +++++- test/CMakeLists.txt | 9 +++++++++ test/options.cpp | 3 +++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/include/cxxopts.hpp b/include/cxxopts.hpp index 891e78a..6bf57a6 100644 --- a/include/cxxopts.hpp +++ b/include/cxxopts.hpp @@ -1057,7 +1057,11 @@ integer_parser(const std::string& text, T& value) US limit = 0; if (negative) { - limit = static_cast(std::abs(static_cast((std::numeric_limits::min)()))); + // |min| equals max + 1 for two's-complement signed types; computing it + // via std::abs(min) is undefined behaviour (e.g. abs(INT64_MIN)). Build + // the same value in unsigned arithmetic instead. For unsigned T this + // intentionally wraps to 0, matching the previous std::abs(0) result. + limit = static_cast(static_cast((std::numeric_limits::max)()) + US{1}); } else { diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 757b081..f97f283 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -27,6 +27,15 @@ target_link_libraries(options_test_noregex cxxopts) target_compile_definitions(options_test_noregex PRIVATE CXXOPTS_NO_REGEX) add_test(options_no_regex options_test_noregex) +# Run the parser tests under UBSan so undefined behaviour in integer parsing +# (e.g. std::abs(INT64_MIN)) fails the build instead of going unnoticed. +if(CMAKE_CXX_COMPILER_ID MATCHES "Clang|GNU" AND NOT WIN32) + foreach(test_target options_test options_test_noregex) + target_compile_options(${test_target} PRIVATE -fsanitize=undefined -fno-sanitize-recover=undefined) + set_target_properties(${test_target} PROPERTIES LINK_FLAGS "-fsanitize=undefined -fno-sanitize-recover=undefined") + endforeach() +endif() + # test if the targets are findable from the build directory add_test(find-package-test ${CMAKE_CTEST_COMMAND} -C ${CMAKE_BUILD_TYPE} diff --git a/test/options.cpp b/test/options.cpp index fb495ed..b41d883 100644 --- a/test/options.cpp +++ b/test/options.cpp @@ -2,6 +2,7 @@ #include #include +#include #include "cxxopts.hpp" @@ -921,6 +922,8 @@ TEST_CASE("Overflow on boundary", "[integer]") CHECK_THROWS_AS((integer_parser("-42769", si16)), cxxopts::exceptions::incorrect_argument_type); CHECK_THROWS_AS((integer_parser("-75536", si16)), cxxopts::exceptions::incorrect_argument_type); + CHECK_NOTHROW((integer_parser("-9223372036854775808", si64))); + CHECK(si64 == (std::numeric_limits::min)()); CHECK_THROWS_AS((integer_parser("18446744073709551616", ui64)), cxxopts::exceptions::incorrect_argument_type); CHECK_THROWS_AS((integer_parser("28446744073709551616", ui64)), cxxopts::exceptions::incorrect_argument_type); CHECK_THROWS_AS((integer_parser("9223372036854775808", si64)), cxxopts::exceptions::incorrect_argument_type);