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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to