zukatsinadze marked an inline comment as done and an inline comment as not done.
zukatsinadze added inline comments.
================
Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:947
+
+} // end "alpha.cert.env"
+
----------------
balazske wrote:
> I have multiple issues with the documentation texts but they look not final
> anyway. And the package and checker names could be better. There are already
> checkers for CERT rules that do not reside in the cert package, it is not
> good to have some checkers in a `cert` package and some not. It is likely
> that checkers will check for more rules or parts of the rules. Checker
> aliases are a better solution for the cert checker names. (And now the `cert`
> would contain checker with name not matching a rule: `InvalidPtr`.) The name
> `InvalidPtr` sounds too general, even if in an `env˙ package.
Agreed. We can come back to wordsmithing later.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:227
+ if (const auto *Reg =
+ State->get<PreviousCallResultMap>(FunctionName.data())) {
+ const auto *PrevReg = *Reg;
----------------
balazske wrote:
> balazske wrote:
> > `auto` is not to be used if the type is not clearly visible in the context.
> > And this would be exactly `auto **` here. (But better is `const MemRegion
> > **`.) Makes the later statement (assign to `PrevReg`) "messy" (and `auto`
> > is bad there too).
> `.data` is unnecessary here?
`.data` seems to be necessary.
`no known conversion from 'llvm::StringRef' to 'typename
ProgramStateTrait<PreviousCallResultMap>::key_type' (aka 'const char *') for
1st argument`
================
Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:341
+ State = State->set<EnvPtrRegion>(
+ reinterpret_cast<void *>(const_cast<MemRegion *>(EnvpReg)));
+ C.addTransition(State);
----------------
balazske wrote:
> Are these casts really needed?
Yes, as steakhal mentioned above, trait is specialized for void*
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97699/new/
https://reviews.llvm.org/D97699
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
