Skip to content

Commit b8b27a9

Browse files
authored
Fix #15117 (MSBuild arithmetic intrinsics: signed overflow UB in Add/Subtract/Multiply) (cppcheck-opensource#8938)
The $([MSBuild]::Add/Subtract/Multiply) intrinsics in applyMSBuildStaticFunction computed a+b, a-b and a*b directly on long long operands from the project file, so values like $([MSBuild]::Add(9223372036854775807, 1)) were signed-overflow UB (reported by -fsanitize=signed-integer-overflow). MSBuild evaluates these in .NET's unchecked context, so on overflow the result wraps. The arithmetic is now done on unsigned long long and cast back, which gives the same two's complement result without UB. Divide/Modulo of LLONG_MIN by -1 throw OverflowException in .NET, so those return the empty string with a debug message, the same as the existing division-by-zero handling. Regression tests added to testMSBuildStaticFunctions.
1 parent 37a06d2 commit b8b27a9

2 files changed

Lines changed: 36 additions & 3 deletions

File tree

‎lib/importproject.cpp‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
#include <algorithm>
2929
#include <cctype>
30+
#include <climits>
3031
#include <cmath>
3132
#include <cstdlib>
3233
#include <cstring>
@@ -848,6 +849,22 @@ static std::string safeFormat(const char *fmt, Args... args) {
848849
#pragma clang diagnostic pop
849850
#endif
850851

852+
// MSBuild evaluates the [MSBuild]::Add/Subtract/Multiply intrinsics in .NET's
853+
// default unchecked context, so on overflow the result wraps (two's complement).
854+
// Do the arithmetic on unsigned operands to get the same result without
855+
// signed-overflow UB.
856+
static long long wrapAdd(long long a, long long b) {
857+
return static_cast<long long>(static_cast<unsigned long long>(a) + static_cast<unsigned long long>(b));
858+
}
859+
860+
static long long wrapSub(long long a, long long b) {
861+
return static_cast<long long>(static_cast<unsigned long long>(a) - static_cast<unsigned long long>(b));
862+
}
863+
864+
static long long wrapMul(long long a, long long b) {
865+
return static_cast<long long>(static_cast<unsigned long long>(a) * static_cast<unsigned long long>(b));
866+
}
867+
851868
// Evaluate a $([ClassName]::Method(args)) static property function.
852869
// Returns an empty string for unknown or unimplementable functions rather
853870
// than throwing, so import can continue gracefully.
@@ -894,23 +911,31 @@ std::string ImportProject::applyMSBuildStaticFunction(const std::string &classNa
894911
long long a = 0, b = 0;
895912
if (toInt(args[0], a) && toInt(args[1], b)) {
896913
if (caseInsensitiveStringCompare(member, "Add") == 0)
897-
return std::to_string(a + b);
914+
return std::to_string(wrapAdd(a, b));
898915
if (caseInsensitiveStringCompare(member, "Subtract") == 0)
899-
return std::to_string(a - b);
916+
return std::to_string(wrapSub(a, b));
900917
if (caseInsensitiveStringCompare(member, "Multiply") == 0)
901-
return std::to_string(a * b);
918+
return std::to_string(wrapMul(a, b));
902919
if (caseInsensitiveStringCompare(member, "Divide") == 0) {
903920
if (b == 0) {
904921
addDebug("MSBuild::Divide: division by zero");
905922
return std::string();
906923
}
924+
if (a == LLONG_MIN && b == -1) {
925+
addDebug("MSBuild::Divide: integer overflow");
926+
return std::string();
927+
}
907928
return std::to_string(a / b);
908929
}
909930
if (caseInsensitiveStringCompare(member, "Modulo") == 0) {
910931
if (b == 0) {
911932
addDebug("MSBuild::Modulo: division by zero");
912933
return std::string();
913934
}
935+
if (a == LLONG_MIN && b == -1) {
936+
addDebug("MSBuild::Modulo: integer overflow");
937+
return std::string();
938+
}
914939
return std::to_string(a % b);
915940
}
916941
}

‎test/testimportproject.cpp‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1211,6 +1211,14 @@ class TestImportProject : public TestFixture {
12111211
ASSERT_EQUALS("3", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::Divide(9, 3))"));
12121212
ASSERT_EQUALS("2", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::Modulo(5, 3))"));
12131213

1214+
// Overflow wraps around like MSBuild (.NET unchecked arithmetic).
1215+
ASSERT_EQUALS("-9223372036854775808", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::Add(9223372036854775807, 1))"));
1216+
ASSERT_EQUALS("9223372036854775807", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::Subtract(-9223372036854775808, 1))"));
1217+
ASSERT_EQUALS("7766279611452241921", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::Multiply(9999999999, 9999999999))"));
1218+
// .NET throws OverflowException for LLONG_MIN / -1 and LLONG_MIN % -1.
1219+
ASSERT_EQUALS("", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::Divide(-9223372036854775808, -1))"));
1220+
ASSERT_EQUALS("", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::Modulo(-9223372036854775808, -1))"));
1221+
12141222
// --- $([MSBuild]::...) path helpers ---
12151223
ASSERT_EQUALS("foo/", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::EnsureTrailingSlash('foo'))"));
12161224
ASSERT_EQUALS("foo/", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::EnsureTrailingSlash('foo/'))"));

0 commit comments

Comments
 (0)