Skip to content

Commit cd02791

Browse files
Nussu06Nussu06
committed
wrap on overflow in MSBuild Add/Subtract/Multiply to match MSBuild
1 parent 1086fac commit cd02791

2 files changed

Lines changed: 26 additions & 62 deletions

File tree

‎lib/importproject.cpp‎

Lines changed: 20 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -849,42 +849,20 @@ static std::string safeFormat(const char *fmt, Args... args) {
849849
#pragma clang diagnostic pop
850850
#endif
851851

852-
// Checked signed arithmetic for the [MSBuild]::Add/Subtract/Multiply intrinsics.
853-
// Operands come straight from the (untrusted) project file, so the result can
854-
// exceed long long and the plain a+b / a-b / a*b would be signed-overflow UB.
855-
// These return false on overflow so the caller can report it instead.
856-
static bool addChecked(long long a, long long b, long long &res) {
857-
if ((b > 0 && a > LLONG_MAX - b) || (b < 0 && a < LLONG_MIN - b))
858-
return false;
859-
res = a + b;
860-
return true;
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));
861858
}
862859

863-
static bool subChecked(long long a, long long b, long long &res) {
864-
if ((b < 0 && a > LLONG_MAX + b) || (b > 0 && a < LLONG_MIN + b))
865-
return false;
866-
res = a - b;
867-
return true;
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));
868862
}
869863

870-
static bool mulChecked(long long a, long long b, long long &res) {
871-
if (a > 0) {
872-
if (b > 0) {
873-
if (a > LLONG_MAX / b)
874-
return false;
875-
} else if (b < LLONG_MIN / a) {
876-
return false;
877-
}
878-
} else {
879-
if (b > 0) {
880-
if (a < LLONG_MIN / b)
881-
return false;
882-
} else if (a != 0 && b < LLONG_MAX / a) {
883-
return false;
884-
}
885-
}
886-
res = a * b;
887-
return true;
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));
888866
}
889867

890868
// Evaluate a $([ClassName]::Method(args)) static property function.
@@ -932,28 +910,12 @@ std::string ImportProject::applyMSBuildStaticFunction(const std::string &classNa
932910
if (args.size() == 2) {
933911
long long a = 0, b = 0;
934912
if (toInt(args[0], a) && toInt(args[1], b)) {
935-
long long r = 0;
936-
if (caseInsensitiveStringCompare(member, "Add") == 0) {
937-
if (!addChecked(a, b, r)) {
938-
addDebug("MSBuild::Add: integer overflow");
939-
return std::string();
940-
}
941-
return std::to_string(r);
942-
}
943-
if (caseInsensitiveStringCompare(member, "Subtract") == 0) {
944-
if (!subChecked(a, b, r)) {
945-
addDebug("MSBuild::Subtract: integer overflow");
946-
return std::string();
947-
}
948-
return std::to_string(r);
949-
}
950-
if (caseInsensitiveStringCompare(member, "Multiply") == 0) {
951-
if (!mulChecked(a, b, r)) {
952-
addDebug("MSBuild::Multiply: integer overflow");
953-
return std::string();
954-
}
955-
return std::to_string(r);
956-
}
913+
if (caseInsensitiveStringCompare(member, "Add") == 0)
914+
return std::to_string(wrapAdd(a, b));
915+
if (caseInsensitiveStringCompare(member, "Subtract") == 0)
916+
return std::to_string(wrapSub(a, b));
917+
if (caseInsensitiveStringCompare(member, "Multiply") == 0)
918+
return std::to_string(wrapMul(a, b));
957919
if (caseInsensitiveStringCompare(member, "Divide") == 0) {
958920
if (b == 0) {
959921
addDebug("MSBuild::Divide: division by zero");
@@ -970,8 +932,10 @@ std::string ImportProject::applyMSBuildStaticFunction(const std::string &classNa
970932
addDebug("MSBuild::Modulo: division by zero");
971933
return std::string();
972934
}
973-
if (a == LLONG_MIN && b == -1)
974-
return std::string("0");
935+
if (a == LLONG_MIN && b == -1) {
936+
addDebug("MSBuild::Modulo: integer overflow");
937+
return std::string();
938+
}
975939
return std::to_string(a % b);
976940
}
977941
}

‎test/testimportproject.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1211,13 +1211,13 @@ 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-
// Operands that would overflow long long must not compute a signed-overflow
1215-
// (undefined-behavior) result; the operation yields the empty string instead.
1216-
ASSERT_EQUALS("", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::Add(9223372036854775807, 1))"));
1217-
ASSERT_EQUALS("", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::Subtract(-9223372036854775808, 1))"));
1218-
ASSERT_EQUALS("", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::Multiply(9999999999, 9999999999))"));
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.
12191219
ASSERT_EQUALS("", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::Divide(-9223372036854775808, -1))"));
1220-
ASSERT_EQUALS("0", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::Modulo(-9223372036854775808, -1))"));
1220+
ASSERT_EQUALS("", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::Modulo(-9223372036854775808, -1))"));
12211221

12221222
// --- $([MSBuild]::...) path helpers ---
12231223
ASSERT_EQUALS("foo/", cppcheck::testing::expandMSBuildExpression("$([MSBuild]::EnsureTrailingSlash('foo'))"));

0 commit comments

Comments
 (0)