[clang] [clang][StaticAnalyzer] Fix crash in SimpleSValBuilder with unsigned __int128 and negative literals (PR #150225)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-static-analyzer-1

Author: Cả thế giới là Rust (naoNao89)

<details>
<summary>Changes</summary>

## Summary

This PR fixes a crash in the Clang static analyzer when processing overflow 
builtins like `__builtin_mul_overflow` with `unsigned __int128` and negative 
literal operands.

## Problem

The issue was reported in #<!-- -->150206. The crash occurred in 
`SimpleSValBuilder::MakeSymIntVal` when the static analyzer tried to process 
code like:

```c
void foo(unsigned __int128 a) {
  __builtin_mul_overflow(a, -16, &amp;a);
}
```

The root cause was in the type conversion logic when handling negative values 
with very large unsigned types (&gt;64 bits). The original logic would:

1. Convert the negative value (-16) to the large unsigned type first (creating 
a very large positive number: 2^128 - 16)
2. Then attempt to negate that very large number, which could cause overflow 
issues in the conversion process

## Solution

The fix adds a special case for large unsigned types where we:

1. Take the absolute value of the negative operand first (|-16| = 16)
2. Then convert it to the target type (16 → unsigned __int128)
3. This avoids the problematic intermediate conversion while maintaining the 
same mathematical result

The fix is targeted and only affects:
- Unsigned types with bit width &gt; 64
- When processing negative values in additive operations
- Preserves all existing behavior for other cases

## Testing

- Added comprehensive test cases in `clang/test/Analysis/builtin_overflow.c`
- Tests cover `__builtin_mul_overflow`, `__builtin_add_overflow`, and 
`__builtin_sub_overflow`
- Tests various negative literal values (-1, -16, -255)
- Verified the fix doesn't affect smaller types or signed types

## Files Changed

- `clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp`: Core fix implementation
- `clang/test/Analysis/builtin_overflow.c`: Added regression tests

Fixes #<!-- -->150206

---
Full diff: https://github.com/llvm/llvm-project/pull/150225.diff


2 Files Affected:

- (modified) clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp (+11-1) 
- (modified) clang/test/Analysis/builtin_overflow.c (+19) 


``````````diff
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
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/150225
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits