[PATCH] D101526: [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg constraints
Balázs Benics via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon May 3 04:28:33 PDT 2021
steakhal added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:688-703
std::string StdLibraryFunctionsChecker::BufferSizeConstraint::describe(
- ProgramStateRef State, const Summary &Summary) const {
+ DescritptionKind DK, ProgramStateRef State, const Summary &Summary) const {
SmallString<96> Result;
- Result += "The size of the ";
+ const auto Violation = ValueConstraint::DescritptionKind::Violation;
+ Result += "the size of the ";
Result += getArgDesc(ArgN);
+ Result += DK == Violation ? " should be " : " is ";
----------------
I don't know. Report message construction always seemed clunky.
Clang's or ClangTidy's approach seems superior in this regard.
Do we have anything better for this @NoQ?
Maybe `llvm::format()` could be an option.
Regarding this patch: It's fine. Better than it was before!
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:854
+ NewState, NewNode,
+ C.getNoteTag([Msg, ArgSVal](PathSensitiveBugReport &BR,
+ llvm::raw_ostream &OS) {
----------------
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:857
+ if (BR.isInteresting(ArgSVal))
+ OS << Msg;
+ }));
----------------
Ah, there is a slight issue.
You should mark some stuff interesting here, to make this interestingness propagate back transitively.
Let's say `ArgSVal` is `x + y` which is considered to be out of range `[42,52]`. We should mark both `x` and `y` interesting because they themselves could have been constrained by the StdLibChecker previously. So, they must be interesting as well.
On the same token, IMO `PathSensitiveBugReport::markInteresting(symbol)` should be //transitive//. So that all `SymbolData` in that symbolic expression tree are considered interesting. What do you think @NoQ?
If we were doing this, @martong - you could simply acquire the assumption you constructed for the given `ValueConstraint`, and mark that interesting. Then all `SymbolData`s on both sides of the logic operator would become implicitly interesting.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101526/new/
https://reviews.llvm.org/D101526
More information about the cfe-commits
mailing list