donat.nagy added a comment.
Overall this seems to be a promising checker; I added a few minor remarks as
inline comments. Unfortunately @steakhal's comment that
> The `check::PostCall` handler wants to mark some memory region immutable.
> Currently, the checker creates a new //symbolic// memregion, spawned into the
> //immutable// memory space. After this it simply re-binds the return value.
> However, only `eval::Call` handler is supposed (**must**) to bind the return
> value, so the current implementation cannot land.
highlights question that is as far as I see a blocking obstacle. I hope that
you can find a solution for that technical difficulty.
Moreover, I agree with @martong's comment that you should probably create two
commits for these two checkers. (One-way dependency between the checkers is not
an obstacle: simply pay attention to merging the commits in the right order.)
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StoreToImmutableChecker.cpp:68-72
+ if (const auto *BP = dyn_cast<BinaryOperator>(S))
+ if (const auto *UP = dyn_cast<UnaryOperator>(BP->getLHS())) {
+ bugreporter::trackExpressionValue(
+ Report->getErrorNode(), UP->getSubExpr()->IgnoreImpCasts(), *Report);
+ }
----------------
This code is a bit hard to understand, perhaps add a comment like
`// If the binding statement looks like "*ptr = ...", then track the pointer
"ptr"`
================
Comment at:
clang/lib/StaticAnalyzer/Checkers/cert/ModelConstQualifiedReturnChecker.cpp:39
+ // SEI CERT ENV30-C
+ const CallDescriptionMap<HandlerFn> ConstQualifiedReturnFunctions = {
+ {{"getenv", 1},
----------------
If you don't have concrete plans for extending this checker to cases where you
need a different HandlerFn, then I strongly suggest replacing this map with a
set (or a CallDescriptionMap<bool> where the bool is just a dummy placeholder).
There is no reason to juggle member function pointers when they are always
pointing to the same function.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124244/new/
https://reviews.llvm.org/D124244
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
