[clang] [clang][StaticAnalyzer] Fix crash in SimpleSValBuilder with unsigned __int128 and negative literals (PR #150225)
Cả thế giới là Rust via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 23 22:46:25 PDT 2025
https://github.com/naoNao89 updated https://github.com/llvm/llvm-project/pull/150225
>From 2d9ae771247af025d6319c044e9f8727e9bebfd8 Mon Sep 17 00:00:00 2001
From: naoNao89 <90588855+naoNao89 at users.noreply.github.com>
Date: Wed, 23 Jul 2025 20:54:02 +0700
Subject: [PATCH 1/2] [clang][StaticAnalyzer] Fix crash in SimpleSValBuilder
with unsigned __int128 and negative literals
Fix a crash in SimpleSValBuilder::MakeSymIntVal when processing overflow
builtins like __builtin_mul_overflow with unsigned __int128 and negative
literal operands.
The issue occurred when converting negative values to very large unsigned
types (>64 bits). The original logic would convert the negative value to
the large unsigned type first (creating a very large positive number),
then attempt to negate it, which could cause overflow issues.
The fix adds a special case for large unsigned types where we take the
absolute value of the negative operand first, then convert it to the
target type, avoiding the problematic intermediate conversion.
Fixes #150206
---
.../StaticAnalyzer/Core/SimpleSValBuilder.cpp | 12 +++++++++++-
clang/test/Analysis/builtin_overflow.c | 19 +++++++++++++++++++
2 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
index 84a9c43d3572e..63c2bb785744b 100644
--- a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -219,7 +219,17 @@ SVal SimpleSValBuilder::MakeSymIntVal(const SymExpr *LHS,
// subtraction/addition of the negated value.
APSIntType resultIntTy = BasicVals.getAPSIntType(resultTy);
if (isNegationValuePreserving(RHS, resultIntTy)) {
- ConvertedRHS = BasicVals.getValue(-resultIntTy.convert(RHS));
+ // For large unsigned types, we need to be careful about the conversion
+ // to avoid issues with very large intermediate values
+ if (resultIntTy.isUnsigned() && resultIntTy.getBitWidth() > 64) {
+ // For large unsigned types, convert the absolute value directly
+ // instead of converting the negative value and then negating
+ llvm::APSInt AbsRHS = RHS;
+ AbsRHS.negate();
+ ConvertedRHS = BasicVals.Convert(resultTy, AbsRHS);
+ } else {
+ ConvertedRHS = BasicVals.getValue(-resultIntTy.convert(RHS));
+ }
op = (op == BO_Add) ? BO_Sub : BO_Add;
} else {
ConvertedRHS = BasicVals.Convert(resultTy, RHS);
diff --git a/clang/test/Analysis/builtin_overflow.c b/clang/test/Analysis/builtin_overflow.c
index d290333071dc9..f2bd08d25ad1f 100644
--- a/clang/test/Analysis/builtin_overflow.c
+++ b/clang/test/Analysis/builtin_overflow.c
@@ -164,3 +164,22 @@ void test_bool_assign(void)
// should return _Bool, but not int.
_Bool ret = __builtin_mul_overflow(10, 20, &res); // no crash
}
+
+void test_unsigned_int128_negative_literal(void)
+{
+ unsigned __int128 a = 42;
+
+ // This should not crash the static analyzer.
+ // Reproduces issue from GitHub #150206 where __builtin_mul_overflow
+ // with unsigned __int128 and negative literal caused a crash in
+ // SimpleSValBuilder::MakeSymIntVal.
+ __builtin_mul_overflow(a, -16, &a); // no crash
+
+ // Test other overflow builtins with the same pattern
+ __builtin_add_overflow(a, -16, &a); // no crash
+ __builtin_sub_overflow(a, -16, &a); // no crash
+
+ // Test with different negative values
+ __builtin_mul_overflow(a, -1, &a); // no crash
+ __builtin_mul_overflow(a, -255, &a); // no crash
+}
>From 764787c1af58a5865bedc69b5deb60d828f95adb Mon Sep 17 00:00:00 2001
From: naoNao89 <90588855+naoNao89 at users.noreply.github.com>
Date: Thu, 24 Jul 2025 11:50:45 +0700
Subject: [PATCH 2/2] Fix crash in SimpleSValBuilder with unsigned __int128
overflow builtins
- Add null check for resultTy in SimpleSValBuilder::evalBinOpNN
- Add null check for resultTy in BuiltinFunctionChecker::handleOverflowBuiltin
- Add conservative handling for __int128 multiplication in BasicValueFactory::evalAPSInt
- Add unknown value checks in BuiltinFunctionChecker::checkOverflow
This fixes the crash when using __builtin_mul_overflow with unsigned __int128
and negative literals, which was causing segmentation faults in the static analyzer.
---
.../Checkers/BuiltinFunctionChecker.cpp | 16 +++++++++++++
.../StaticAnalyzer/Core/BasicValueFactory.cpp | 13 ++++++++++
.../StaticAnalyzer/Core/SimpleSValBuilder.cpp | 24 ++++++++++++-------
3 files changed, 45 insertions(+), 8 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
index 8eb68b1fa638e..ee62f9401346e 100644
--- a/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/BuiltinFunctionChecker.cpp
@@ -149,6 +149,12 @@ BuiltinFunctionChecker::checkOverflow(CheckerContext &C, SVal RetVal,
// Calling a builtin with a non-integer type result produces compiler error.
assert(Res->isIntegerType());
+ // If RetVal is unknown or undefined, we can't determine overflow
+ if (RetVal.isUnknown() || RetVal.isUndef()) {
+ // Return both possibilities as true (may overflow, may not overflow)
+ return {true, true};
+ }
+
unsigned BitWidth = C.getASTContext().getIntWidth(Res);
bool IsUnsigned = Res->isUnsignedIntegerType();
@@ -164,6 +170,11 @@ BuiltinFunctionChecker::checkOverflow(CheckerContext &C, SVal RetVal,
SVal IsLeMax = SVB.evalBinOp(State, BO_LE, RetVal, MaxVal, Res);
SVal IsGeMin = SVB.evalBinOp(State, BO_GE, RetVal, MinVal, Res);
+ // If the comparison results are unknown, be conservative
+ if (IsLeMax.isUnknown() || IsGeMin.isUnknown()) {
+ return {true, true};
+ }
+
auto [MayNotOverflow, MayOverflow] =
State->assume(IsLeMax.castAs<DefinedOrUnknownSVal>());
auto [MayNotUnderflow, MayUnderflow] =
@@ -202,6 +213,11 @@ void BuiltinFunctionChecker::handleOverflowBuiltin(const CallEvent &Call,
// Calling a builtin with an incorrect argument count produces compiler error.
assert(Call.getNumArgs() == 3);
+ // If ResultType is null, we can't proceed with the evaluation
+ if (ResultType.isNull()) {
+ return;
+ }
+
ProgramStateRef State = C.getState();
SValBuilder &SVB = C.getSValBuilder();
diff --git a/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp b/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
index 02f34bc30f554..e0001acdf8f8e 100644
--- a/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
@@ -250,6 +250,19 @@ BasicValueFactory::evalAPSInt(BinaryOperator::Opcode Op, const llvm::APSInt &V1,
llvm_unreachable("Invalid Opcode.");
case BO_Mul:
+ // For large bit widths (like __int128), check for potential crashes
+ if (V1.getBitWidth() >= 128 || V2.getBitWidth() >= 128) {
+ // If either operand is zero, result is zero
+ if (V1 == 0 || V2 == 0) {
+ return getValue(llvm::APSInt(llvm::APInt::getZero(std::max(V1.getBitWidth(), V2.getBitWidth())),
+ V1.isUnsigned() && V2.isUnsigned()));
+ }
+
+ // For __int128 types, be conservative to avoid crashes in APInt multiplication
+ // This happens when multiplying unsigned __int128 with large values (like negative
+ // numbers converted to unsigned)
+ return std::nullopt;
+ }
return getValue(V1 * V2);
case BO_Div:
diff --git a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
index 63c2bb785744b..29a711c81dd4f 100644
--- a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -217,8 +217,12 @@ SVal SimpleSValBuilder::MakeSymIntVal(const SymExpr *LHS,
// Change a+(-N) into a-N, and a-(-N) into a+N
// Adjust addition/subtraction of negative value, to
// subtraction/addition of the negated value.
- APSIntType resultIntTy = BasicVals.getAPSIntType(resultTy);
- if (isNegationValuePreserving(RHS, resultIntTy)) {
+ // Check if resultTy is valid before using it
+ if (resultTy.isNull()) {
+ ConvertedRHS = BasicVals.Convert(resultTy, RHS);
+ } else {
+ APSIntType resultIntTy = BasicVals.getAPSIntType(resultTy);
+ if (isNegationValuePreserving(RHS, resultIntTy)) {
// For large unsigned types, we need to be careful about the conversion
// to avoid issues with very large intermediate values
if (resultIntTy.isUnsigned() && resultIntTy.getBitWidth() > 64) {
@@ -230,9 +234,10 @@ SVal SimpleSValBuilder::MakeSymIntVal(const SymExpr *LHS,
} else {
ConvertedRHS = BasicVals.getValue(-resultIntTy.convert(RHS));
}
- op = (op == BO_Add) ? BO_Sub : BO_Add;
- } else {
- ConvertedRHS = BasicVals.Convert(resultTy, RHS);
+ op = (op == BO_Add) ? BO_Sub : BO_Add;
+ } else {
+ ConvertedRHS = BasicVals.Convert(resultTy, RHS);
+ }
}
} else
ConvertedRHS = BasicVals.Convert(resultTy, RHS);
@@ -548,9 +553,12 @@ SVal SimpleSValBuilder::evalBinOpNN(ProgramStateRef state,
CompareType.apply(LHSValue);
CompareType.apply(RHSValue);
} else if (!BinaryOperator::isShiftOp(op)) {
- APSIntType IntType = BasicVals.getAPSIntType(resultTy);
- IntType.apply(LHSValue);
- IntType.apply(RHSValue);
+ // Check if resultTy is valid before using it
+ if (!resultTy.isNull()) {
+ APSIntType IntType = BasicVals.getAPSIntType(resultTy);
+ IntType.apply(LHSValue);
+ IntType.apply(RHSValue);
+ }
}
std::optional<APSIntPtr> Result =
More information about the cfe-commits
mailing list