From 8c80299a75f67c459985ff335ed3bb6e3dda2ded Mon Sep 17 00:00:00 2001 From: nussaiba shaikh Date: Fri, 13 Nov 2026 21:22:22 +0530 Subject: [PATCH] fix signed overflow in calculate() and MathLib constant folding --- Makefile | 2 +- lib/calculate.h | 42 +++++++++++++++++++++++++++++++++---- lib/mathlib.cpp | 22 +++++++++++-------- lib/valueflow.cpp | 5 +++-- oss-fuzz/Makefile | 2 +- test/testmathlib.cpp | 7 +++++++ test/testsymboldatabase.cpp | 15 +++++++++++++ test/testvalueflow.cpp | 4 ++++ 8 files changed, 82 insertions(+), 17 deletions(-) diff --git a/Makefile b/Makefile index 6ea7afab1a5..fe249b730e0 100644 --- a/Makefile +++ b/Makefile @@ -628,7 +628,7 @@ $(libcppdir)/keywords.o: lib/keywords.cpp lib/config.h lib/keywords.h lib/standa $(libcppdir)/library.o: lib/library.cpp externals/tinyxml2/tinyxml2.h lib/astutils.h lib/config.h lib/errortypes.h lib/library.h lib/mathlib.h lib/path.h lib/refthunk.h lib/smallvector.h lib/sourcelocation.h lib/standards.h lib/symboldatabase.h lib/templatesimplifier.h lib/token.h lib/tokenlist.h lib/utils.h lib/vfvalue.h lib/xml.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/library.cpp -$(libcppdir)/mathlib.o: lib/mathlib.cpp externals/simplecpp/simplecpp.h lib/config.h lib/errortypes.h lib/mathlib.h lib/smallvector.h lib/templatesimplifier.h lib/token.h lib/utils.h lib/vfvalue.h +$(libcppdir)/mathlib.o: lib/mathlib.cpp externals/simplecpp/simplecpp.h lib/calculate.h lib/config.h lib/errortypes.h lib/mathlib.h lib/smallvector.h lib/templatesimplifier.h lib/token.h lib/utils.h lib/vfvalue.h $(CXX) ${INCLUDE_FOR_LIB} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/mathlib.cpp $(libcppdir)/path.o: lib/path.cpp externals/simplecpp/simplecpp.h lib/config.h lib/path.h lib/standards.h lib/utils.h diff --git a/lib/calculate.h b/lib/calculate.h index c33e37fd3b7..8dbd2a279ba 100644 --- a/lib/calculate.h +++ b/lib/calculate.h @@ -23,6 +23,7 @@ #include "errortypes.h" #include #include +#include template bool isEqual(T x, T y) @@ -46,6 +47,39 @@ bool isZero(T x) return isEqual(x, T(0)); } +/** + * x + y, x - y or x * y. Signed integer overflow is undefined behaviour, so for a signed integer + * type the operation is done on the unsigned type and the result wraps. + */ +template +T calculateArithmetic(char op, T x, T y, std::true_type /*isSignedInteger*/) +{ + using U = typename std::make_unsigned::type; + const U ux = static_cast(x); + const U uy = static_cast(y); + if (op == '+') + return static_cast(ux + uy); + if (op == '-') + return static_cast(ux - uy); + return static_cast(ux * uy); +} + +template +T calculateArithmetic(char op, T x, T y, std::false_type /*isSignedInteger*/) +{ + if (op == '+') + return x + y; + if (op == '-') + return x - y; + return x * y; +} + +template +T calculateArithmetic(char op, T x, T y) +{ + return calculateArithmetic(op, x, y, std::integral_constant::value && std::is_signed::value>{}); +} + /** * @throws InternalError thrown in case of unknown operator */ @@ -60,11 +94,11 @@ R calculate(const std::string& s, const T& x, const T& y, bool* error = nullptr) constexpr MathLib::bigint maxBitsSignedShift = maxBitsShift - 1; switch (MathLib::encodeMultiChar(s)) { case '+': - return wrap(x + y); + return wrap(calculateArithmetic('+', x, y)); case '-': - return wrap(x - y); + return wrap(calculateArithmetic('-', x, y)); case '*': - return wrap(x * y); + return wrap(calculateArithmetic('*', x, y)); case '/': if (isZero(y) || (std::is_signed{} && y < 0)) { if (error) @@ -116,7 +150,7 @@ R calculate(const std::string& s, const T& x, const T& y, bool* error = nullptr) case '<=': return wrap(x <= y); case '<=>': - return wrap(x - y); + return wrap(calculateArithmetic('-', x, y)); } throw InternalError(nullptr, "Unknown operator: " + s); } diff --git a/lib/mathlib.cpp b/lib/mathlib.cpp index 9aba7118858..77bec7eb465 100644 --- a/lib/mathlib.cpp +++ b/lib/mathlib.cpp @@ -18,6 +18,7 @@ #include "mathlib.h" +#include "calculate.h" #include "errortypes.h" #include "token.h" #include "utils.h" @@ -186,13 +187,9 @@ MathLib::value MathLib::value::calc(char op, const MathLib::value &v1, const Mat } else { switch (op) { case '+': - temp.mIntValue += v2.mIntValue; - break; case '-': - temp.mIntValue -= v2.mIntValue; - break; case '*': - temp.mIntValue *= v2.mIntValue; + temp.mIntValue = calculateArithmetic(op, temp.mIntValue, v2.mIntValue); break; case '/': if (v2.mIntValue == 0) @@ -204,7 +201,11 @@ MathLib::value MathLib::value::calc(char op, const MathLib::value &v1, const Mat case '%': if (v2.mIntValue == 0) throw InternalError(nullptr, "Internal Error: Division by zero"); - temp.mIntValue %= v2.mIntValue; + // x % -1 is 0, but the division overflows for the minimum value + if (v2.mIntValue == -1) + temp.mIntValue = 0; + else + temp.mIntValue %= v2.mIntValue; break; case '&': temp.mIntValue &= v2.mIntValue; @@ -1119,7 +1120,7 @@ std::string MathLib::add(const std::string & first, const std::string & second) return (value(first) + value(second)).str(); #else if (MathLib::isInt(first) && MathLib::isInt(second)) { - return MathLib::toString(toBigNumber(first) + toBigNumber(second)) + intsuffix(first, second); + return MathLib::toString(calculateArithmetic('+', toBigNumber(first), toBigNumber(second))) + intsuffix(first, second); } double d1 = toDoubleNumber(first); @@ -1141,7 +1142,7 @@ std::string MathLib::subtract(const std::string &first, const std::string &secon return (value(first) - value(second)).str(); #else if (MathLib::isInt(first) && MathLib::isInt(second)) { - return MathLib::toString(toBigNumber(first) - toBigNumber(second)) + intsuffix(first, second); + return MathLib::toString(calculateArithmetic('-', toBigNumber(first), toBigNumber(second))) + intsuffix(first, second); } if (first == second) @@ -1189,7 +1190,7 @@ std::string MathLib::multiply(const std::string &first, const std::string &secon return (value(first) * value(second)).str(); #else if (MathLib::isInt(first) && MathLib::isInt(second)) { - return MathLib::toString(toBigNumber(first) * toBigNumber(second)) + intsuffix(first, second); + return MathLib::toString(calculateArithmetic('*', toBigNumber(first), toBigNumber(second))) + intsuffix(first, second); } return toString(toDoubleNumber(first) * toDoubleNumber(second)); #endif @@ -1204,6 +1205,9 @@ std::string MathLib::mod(const std::string &first, const std::string &second) const bigint b = toBigNumber(second); if (b == 0) throw InternalError(nullptr, "Internal Error: Division by zero"); + // x % -1 is 0, but the division overflows for the minimum value + if (b == -1) + return "0" + intsuffix(first, second); return MathLib::toString(toBigNumber(first) % b) + intsuffix(first, second); } return toString(std::fmod(toDoubleNumber(first),toDoubleNumber(second))); diff --git a/lib/valueflow.cpp b/lib/valueflow.cpp index 62d4012b823..1f4a7b4d855 100644 --- a/lib/valueflow.cpp +++ b/lib/valueflow.cpp @@ -1146,12 +1146,13 @@ static void valueFlowEnumValue(SymbolDatabase & symboldatabase, const Settings & if (rhs && rhs->hasKnownIntValue()) { enumerator.value = rhs->getKnownIntValue(); enumerator.value_known = true; - value = enumerator.value + 1; + value = calculateArithmetic('+', enumerator.value, 1); prev_enum_is_known = true; } else prev_enum_is_known = false; } else if (prev_enum_is_known) { - enumerator.value = value++; + enumerator.value = value; + value = calculateArithmetic('+', value, 1); enumerator.value_known = true; } } diff --git a/oss-fuzz/Makefile b/oss-fuzz/Makefile index a71b026618b..a61752ae290 100644 --- a/oss-fuzz/Makefile +++ b/oss-fuzz/Makefile @@ -297,7 +297,7 @@ $(libcppdir)/keywords.o: ../lib/keywords.cpp ../lib/config.h ../lib/keywords.h . $(libcppdir)/library.o: ../lib/library.cpp ../externals/tinyxml2/tinyxml2.h ../lib/astutils.h ../lib/config.h ../lib/errortypes.h ../lib/library.h ../lib/mathlib.h ../lib/path.h ../lib/refthunk.h ../lib/smallvector.h ../lib/sourcelocation.h ../lib/standards.h ../lib/symboldatabase.h ../lib/templatesimplifier.h ../lib/token.h ../lib/tokenlist.h ../lib/utils.h ../lib/vfvalue.h ../lib/xml.h $(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/library.cpp -$(libcppdir)/mathlib.o: ../lib/mathlib.cpp ../externals/simplecpp/simplecpp.h ../lib/config.h ../lib/errortypes.h ../lib/mathlib.h ../lib/smallvector.h ../lib/templatesimplifier.h ../lib/token.h ../lib/utils.h ../lib/vfvalue.h +$(libcppdir)/mathlib.o: ../lib/mathlib.cpp ../externals/simplecpp/simplecpp.h ../lib/calculate.h ../lib/config.h ../lib/errortypes.h ../lib/mathlib.h ../lib/smallvector.h ../lib/templatesimplifier.h ../lib/token.h ../lib/utils.h ../lib/vfvalue.h $(CXX) ${LIB_FUZZING_ENGINE} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ $(libcppdir)/mathlib.cpp $(libcppdir)/path.o: ../lib/path.cpp ../externals/simplecpp/simplecpp.h ../lib/config.h ../lib/path.h ../lib/standards.h ../lib/utils.h diff --git a/test/testmathlib.cpp b/test/testmathlib.cpp index 800334926e5..77eb66d7b94 100644 --- a/test/testmathlib.cpp +++ b/test/testmathlib.cpp @@ -157,6 +157,13 @@ class TestMathLib : public TestFixture { ASSERT_THROW_INTERNAL_EQUALS(MathLib::divide("-9223372036854775808", "-1"), INTERNAL, "Internal Error: Division overflow"); // #4520 - out of range => throw ASSERT_EQUALS("4611686018427387904", MathLib::divide("-9223372036854775808", "-2")); // #6679 + // out of range results wrap without signed integer overflow + ASSERT_EQUALS("-9223372036854775808", MathLib::add("9223372036854775807", "1")); + ASSERT_EQUALS("9223372036854775807", MathLib::subtract("-9223372036854775808", "1")); + ASSERT_EQUALS("-2", MathLib::multiply("9223372036854775807", "2")); + ASSERT_EQUALS("0", MathLib::mod("-9223372036854775808", "-1")); + ASSERT_EQUALS("0", MathLib::calculate("-9223372036854775808", "-1", '%')); + // invoke for each supported action ASSERT_EQUALS("3", MathLib::calculate("2", "1", '+')); diff --git a/test/testsymboldatabase.cpp b/test/testsymboldatabase.cpp index 85ee8430a77..35e7656df3c 100644 --- a/test/testsymboldatabase.cpp +++ b/test/testsymboldatabase.cpp @@ -28,6 +28,7 @@ #include "tokenlist.h" #include +#include #include #include #include @@ -470,6 +471,7 @@ class TestSymbolDatabase : public TestFixture { TEST_CASE(enum18); TEST_CASE(enum19); TEST_CASE(enum20); // #14419 + TEST_CASE(enum21); TEST_CASE(struct1); @@ -7113,6 +7115,19 @@ class TestSymbolDatabase : public TestFixture { } } + void enum21() { // implicit values past the maximum wrap without signed integer overflow + GET_SYMBOL_DB("enum E { A = 0x7FFFFFFFFFFFFFFE, B, C };\n"); + ASSERT(db != nullptr); + const Scope* scope = db->findScopeByName("E"); + ASSERT(scope); + const Enumerator* B = scope->findEnumerator("B"); + ASSERT(B && B->value_known); + ASSERT_EQUALS(0x7FFFFFFFFFFFFFFELL + 1, B->value); + const Enumerator* C = scope->findEnumerator("C"); + ASSERT(C && C->value_known); + ASSERT_EQUALS(LLONG_MIN, C->value); + } + void struct1() { GET_SYMBOL_DB_C("struct deer {\n" " uint16_t a;\n" diff --git a/test/testvalueflow.cpp b/test/testvalueflow.cpp index 1e953af1535..c2bf4c9f194 100644 --- a/test/testvalueflow.cpp +++ b/test/testvalueflow.cpp @@ -1140,6 +1140,10 @@ class TestValueFlow : public TestFixture { ASSERT_EQUALS(10, valueOfTok("x = static_cast(10);\n", "( 10 )").intvalue); ASSERT_EQUALS(0, valueOfTok("x = sizeof (struct {int a;}) * 0;\n", "*").intvalue); + // out of range results wrap without signed integer overflow + ASSERT_EQUALS(LLONG_MIN, valueOfTok("x = 0x7fffffffffffffff + 1;\n", "+").intvalue); + ASSERT_EQUALS(-2, valueOfTok("x = 0x7fffffffffffffff * 2;\n", "*").intvalue); + // Don't calculate or crash if there is UB or invalid operations ASSERT(tokenValues(";-1<<10;\n","<<").empty()); ASSERT(tokenValues(";10<<-1;\n","<<").empty());