[PATCH] D127306: [analyzer] Treat system globals as mutable if they are not const

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

In D127306#3581814 <https://reviews.llvm.org/D127306#3581814>, @martong wrote:

> In D127306#3580981 <https://reviews.llvm.org/D127306#3580981>, @steakhal wrote:
>
>> - Modify the `GenericTaintChecker::isStdin()` to look through //derived symbols//, to mitigate the effect of invalidations.
>
> So, the taint property is still not propagated by the engine after the invalidation. BUT, since we have the
>
>   static bool isTaintedOrPointsToTainted(const Expr *E, .... {
>     if (isTainted(State, E, C.getLocationContext()) || isStdin(E, C))
>       return true;
>
> condition and the modified `isStdin`, now we consider the `Expr*` associated to stdin as tainted. Please confirm my understanding is correct.

Exactly.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:98-101
+  const auto *DeclReg = dyn_cast_or_null<DeclRegion>(
+      isa<SymbolDerived>(Sym)       ? cast<SymbolDerived>(Sym)->getRegion()
+      : isa<SymbolRegionValue>(Sym) ? cast<SymbolRegionValue>(Sym)->getRegion()
+                                    : nullptr);
----------------
martong wrote:
> This code yields to a virtual function call. And we fortunately have that virtual function in the base class.
> Use `SymExpr::getOriginRegion()` and `dyn_cast_or_null` to `DeclRegion`.
Nice catch. I did not even know about this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127306