cppcheck icon indicating copy to clipboard operation
cppcheck copied to clipboard

Fixes for sanitizer errors from SPEC CPU testing

Open heshpdx opened this issue 5 months ago • 6 comments

We're doing some sanitizer testing on the source code here at SPEC, and I was able to offer some patches to correct the overflow issues. These are corner cases so maybe the cppcheck community may not be so keen to accept these, but I figured I would share.

lib/infer.cpp:131:39: runtime error: signed integer overflow: 9223372036854775807 + 1 cannot be represented in type 'long long int'
lib/infer.cpp:141:39: runtime error: signed integer overflow: -9223372036854775808 - 1 cannot be represented in type 'long long int'
lib/infer.cpp:322:65: runtime error: signed integer overflow: 9223372036854775807 + 1 cannot be represented in type 'long long int'
lib/vf_common.cpp:115:96: runtime error: shift exponent 18446744073709550144 is too large for 64-bit type 'long long unsigned int'
lib/vf_common.cpp:116:47: runtime error: shift exponent 1919 is too large for 64-bit type 'long long unsigned int'
lib/token.cpp:1949:20: runtime error: signed integer overflow: -9223372036854775808 - 9223372032559808511 cannot be represented in type 'long long int'

heshpdx avatar Jul 23 '25 05:07 heshpdx

In addition, there are some final errors that I don't know how to fix.

lib/calculate.h:60:20: runtime error: signed integer overflow: 9223372036854775807 + 1 cannot be represented in type 'long long int'
lib/calculate.h:60:26: runtime error: signed integer overflow: 9223372036854775807 + 1 cannot be represented in type 'long long int'
lib/calculate.h:62:20: runtime error: signed integer overflow: 9223372036854775807 - -1 cannot be represented in type 'long long int'
lib/calculate.h:64:20: runtime error: signed integer overflow: 1152921504606846976 * 8 cannot be represented in type 'long long int'
lib/calculate.h:64:20: runtime error: signed integer overflow: 4611686018427387904 * 2 cannot be represented in type 'long long int'

I tried the following to cap the value at the limits, but this led to regressions and failures in testrunner. If someone has a better idea, please share. Thank you.

diff --git a/src/lib/calculate.h b/src/lib/calculate.h
index dbd5bff..1de9b36 100644
--- a/src/lib/calculate.h
+++ b/src/lib/calculate.h
@@ -57,10 +57,18 @@ 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.c_str())) {
     case '+':
+        if (std::numeric_limits<T>::max() - y >= x) // overflow
+            return R{std::numeric_limits<T>::max()};
         return wrap(x + y);
     case '-':
+        if (std::numeric_limits<T>::max() + y >= x) // overflow
+            return R{std::numeric_limits<T>::max()};
+        if (std::numeric_limits<T>::min() - y >= x) // underflow
+            return R{std::numeric_limits<T>::min()};
         return wrap(x - y);
     case '*':
+        if (!isZero(y) && std::numeric_limits<T>::max() / y >= x) // overflow
+            return R{std::numeric_limits<T>::max()};
         return wrap(x * y);
     case '/':
         if (isZero(y) || (std::is_integral<T>{} && std::is_signed<T>{} && isEqual(y, T(-1)) && isEqual(x, std::numeric_limits<T>::min()))) {

heshpdx avatar Jul 23 '25 05:07 heshpdx

long long is still being used in some places.

chrchr-github avatar Jul 24 '25 07:07 chrchr-github

Also see #2922.

firewave avatar Aug 04 '25 15:08 firewave

We need to add tests which trigger these warnings instead of "blindly" fixing them.

firewave avatar Aug 08 '25 12:08 firewave

See also https://trac.cppcheck.net/ticket/13446.

firewave avatar Aug 13 '25 12:08 firewave