[clang] [analyzer] Enforce not making overly complicated symbols (PR #144327)

Balázs Benics via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 27 07:53:12 PDT 2025


balazs-benics-sonarsource wrote:

> The parts of the flamegraph that you highlight are presumably coming from the _other_ operand whose simplification is skipped due to the presence of the `UnknownVal`.

Yes, this is my theory, but this still wouldn't explain why I didn't see these simplifys on the baseline flamegraph.

> If you really want to bother with performance improvements that specifically target this 0.05% of the entrypoints, then you can insert one more early return here at the beginning of `evalBinOp` to skip some calculations if you encounter a `SymbolOverlyComplex`.

This is the point. I don't think I can special case these because the computations still make sense to do.
So I'm concerned about adding something like this:
```diff
diff --git a/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp b/clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -492,6 +492,10 @@ SVal SValBuilder::evalBinOp(ProgramStateRef state, BinaryOperator::Opcode op,
   if (lhs.isUnknown() || rhs.isUnknown())
     return UnknownVal();
 
+  if (isa_and_nonnull<SymbolOverlyComplex>(lhs.getAsSymbol()) ||
+      isa_and_nonnull<SymbolOverlyComplex>(rhs.getAsSymbol()))
+    return UnknownVal();
+
   if (isa<nonloc::LazyCompoundVal>(lhs) || isa<nonloc::LazyCompoundVal>(rhs)) {
     return UnknownVal();
   }
```
Btw this would solve the performance problem (at least on the sample I shared), and it's technically a correct implementation, but I still find it unfair. As far as combining `SymbolOverlyComplex` with any other symbol, it should behave just like any other `SymbolData`.

Let me know if you means something like this when you referred to "you can insert one more early return".

https://github.com/llvm/llvm-project/pull/144327


More information about the cfe-commits mailing list