[PATCH] D106644: [clang][analyzer] Add standard streams to alpha.unix.Stream checker.
Balázs Kéri via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 6 04:02:27 PDT 2021
balazske added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:227-236
+ for (Decl *D : LookupRes) {
+ D = D->getCanonicalDecl();
+ if (!C.getSourceManager().isInSystemHeader(D->getLocation()))
+ continue;
+ if (auto *VD = dyn_cast<VarDecl>(D)) {
+ if (FILEType && !ACtx.hasSameType(*FILEType, VD->getType()))
+ continue;
----------------
steakhal wrote:
> It will early return and uses one fewer `continue`.
The intent was that if no `FILEType` is found we find just the first `VarDecl` and do not check the type. This recommended change returns always null if no `FILEType` is found.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:504
+
+ OrigStdin = findStdStream("stdin", C);
+ OrigStdout = findStdStream("stdout", C);
----------------
steakhal wrote:
> martong wrote:
> > We should be careful, to cache the results (either here, or deeper in the call stack).
> > I mean, we certainly don't want to do a lookup of "stdin" every time a function is evaluated. We should do this lazily, only once.
> I agree. We should do this only for the first top-level function, instead of doing this for every top-level function.
I am not sure if the `SymbolRef` values are safe to store between top-level function analyses. The `FILE` type and std stream declarations are safe to cache, but the SymbolRef values of these variables probably not.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D106644/new/
https://reviews.llvm.org/D106644
More information about the cfe-commits
mailing list