[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