[PATCH] D67706: [clang][analyzer] Using CallDescription in StreamChecker.

[Balázs Kéri via Phabricator via cfe-commits]

balazske marked 3 inline comments as done.
balazske added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:109-110
 bool StreamChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
+  if (!Call.isGlobalCFunction())
+    return false;
+
----------------
Szelethus wrote:
> Isn't this redundant with my other inline about parameter types?
Probably change to `isInSystemHeader` or use both?


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:127-131
+  for (auto P : Call.parameters()) {
+    QualType T = P->getType();
+    if (!T->isIntegralOrEnumerationType() && !T->isPointerType())
+      return nullptr;
+  }
----------------
Szelethus wrote:
> I'm not sure why we need this, is it true that *all* stream related functions return a pointer or a numerical value? Are we actually checking whether this really is a library function? If so, this looks pretty arbitrary.
This comes from code of CStringChecker:
```
  // Pro-actively check that argument types are safe to do arithmetic upon.
  // We do not want to crash if someone accidentally passes a structure
  // into, say, a C++ overload of any of these functions. We could not check
  // that for std::copy because they may have arguments of other types.
```
Still I am not sure that the checker works correct with code that contains similar named but "arbitrary" functions.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:156
+void StreamChecker::evalFread(const CallEvent &Call, CheckerContext &C) const {
+  (void)CheckNullStream(Call.getArgSVal(3), C);
 }
----------------
Szelethus wrote:
> Why the need for `(void)`? `CheckNullSteam` doesn't seem to have an `LLVM_NODISCARD` attribute.
I wanted to be sure to get no buildbot compile errors (where -Werror is used).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67706/new/

https://reviews.llvm.org/D67706