Szelethus added inline comments. Herald added a subscriber: manas.
================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:156-157 // the bug is reported. - virtual std::string describe(ProgramStateRef State, + virtual std::string describe(DescritptionKind DK, ProgramStateRef State, const Summary &Summary) const { // There are some descendant classes that are not used as argument ---------------- How about we turn this into a print-like function and instead of returning with a string, we take an `llvm::raw_ostream` object as argument? `SmallString` + `raw_svector_stream` is how we construct most of our checker message strings. ================ Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:857 + if (BR.isInteresting(ArgSVal)) + OS << Msg; + })); ---------------- steakhal wrote: > 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. >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? Thats how I'd expect this to work. This shouldn't be a burden on the checker developer (certainly not this kind of a checker), but rather be handled by `PathSensitiveBugReport`. So I think this is fine as it is. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101526/new/ https://reviews.llvm.org/D101526 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits