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+naona...@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+naona...@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 = _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits