Szelethus accepted this revision.
Szelethus added a comment.
This revision is now accepted and ready to land.
LGTM! It makes much more sense to do the checking in `preCall` and the modeling
in `evalCall`, the comments in the patch are precise and helpful, the tests
cover everything. While I feel fairly confident that this patch is great both
in concept and in execution, please leave some time for others to respond -- if
that doesn't happen in a day or two, I think this is ready to land.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:64-70
+/// Get the value of the stream argument out of the passed call event.
+/// The call should contain a function that is described by Desc.
+SVal getStreamArg(const FnDescription *Desc, const CallEvent &Call) {
+ assert(Desc && Desc->StreamArgNo != ArgNone &&
+ "Try to get a non-existing stream argument.");
+ return Call.getArgSVal(Desc->StreamArgNo);
+}
----------------
You could make this a method of `FnDescription`.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:119
+ /// If it can only be NULL a fatal error is emitted and nullptr returned.
+ /// Otherwise a new state where the stream is constrained to be non-null.
+ ProgramStateRef ensureStreamNonNull(SVal StreamVal, CheckerContext &C,
----------------
/// If it can only be NULL a fatal error is emitted and nullptr returned.
/// Otherwise the return value is a new state where the stream is constrained
to be non-null.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:130
+
+ /// Check that the stream is in opened state.
+ /// If the stream is known to be not opened an error is generated
----------------
/// Check that the stream is in an opened state.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:148-151
+ // Ensure that not a member function is called.
+ const auto *FD = dyn_cast_or_null<FunctionDecl>(Call.getDecl());
+ if (!FD || FD->getKind() != Decl::Function)
+ return nullptr;
----------------
I vaguely recall us having this conversation once, but isn't this redundant
with
```lang=c++
if (!Call.isGlobalCFunction())
return nullptr;
```
? I don't mind not addressing this before commiting.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75163/new/
https://reviews.llvm.org/D75163
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
