[PATCH] D104844: [Analyzer][solver] Fix crashes during symbol simplification

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2314-2315
 
+  if (SymbolRef SimplifiedSym = simplify(St, Sym))
+    Sym = SimplifiedSym;
+
----------------
vsavchenko wrote:
> I don't like the idea of duplicating it into every `assume` method.  This way we drastically increase our chances to forget it (like you did with `assumeSymGE` and `assumeSymLE`).
> I think the better place for it is in `RangedConstraintManager::assumeSymRel` and neighboring methods, though still not perfect.
> I don't really get why we get not simplified symbol to begin with.
> 
`assumeSymRel` is not enough, because e.g. `assumeSymGE` is called also e.g. from `assumeSymUnsupported`. Perhaps we could change the signature of `assumeSymEQ/NE/GT/GE/LT/LE` to take an auxiliary `Simplifier` wrapper object instead of `SymbolRef`?

```
  ProgramStateRef assumeSymNE(ProgramStateRef State, Simplifier S,
                                      const llvm::APSInt &V,
                                      const llvm::APSInt &Adjustment);

```
And for the Simplifier something like:
```
struct Simplifier {
  SymbolRef SimplifiedSym = nullptr;
  Simplifier(SymbolRef Sym) : SimplifiedSym(simplify(Sym)) {}
  
};
```



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104844/new/

https://reviews.llvm.org/D104844