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