martong added a comment.
Nice work!
================
Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:159
+// Note: This pointer has type 'const MemRegion *'
+REGISTER_TRAIT_WITH_PROGRAMSTATE(EnvPtrRegion, const void *)
+
----------------
Why is it `const void *`? Why can't we use `const MemRegion *` and get rid of
the ugly reinterpret_cast? There must be a reason ,which I'd like to see
documented in the comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:163
+// memory region returned by previous call of this function
+REGISTER_MAP_WITH_PROGRAMSTATE(PreviousCallResultMap, const char *,
+ const MemRegion *)
----------------
I think we could use the canonical `FunctionDecl*` as the key instead of `const
char *`. Then all the `eval` functions like `evalGetenv` and alike could be
substituted with one simple function.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:221-224
+ const auto *SymRegOfRetVal = dyn_cast<SymbolicRegion>(RetVal.getAsRegion());
+ State = State->set<PreviousCallResultMap>(
+ FunctionName.data(),
+ const_cast<MemRegion *>(SymRegOfRetVal->getBaseRegion()));
----------------
Would it be possible to add a `NoteTag` here and eliminate the
`PreviousCallVisitor`?
================
Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:342
+
+void InvalidPtrChecker::checkDeadSymbols(SymbolReaper &SymReaper,
+ CheckerContext &C) const {
----------------
I think we should delete the code of this callback, since we can't use it.
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
