[PATCH] D20811: [analyzer] Model some library functions

Anna Zaks via cfe-commits cfe-commits at lists.llvm.org
Sat Jul 23 14:55:34 PDT 2016


zaks.anna added inline comments.

================
Comment at: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:191
@@ +190,3 @@
+    // impossible to verify this precisely, but at least
+    // this check avoids potential crashes.
+    bool matchesCall(const CallExpr *CE) const;
----------------
Do we need to talk about crashes when describing what this does?
Also, please, use oxygen throughout.

================
Comment at: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:205
@@ +204,3 @@
+  }
+  static QualType getArgType(const CallEvent &Call, ArgNoTy ArgNo) {
+    return ArgNo == Ret ? Call.getResultType().getCanonicalType()
----------------
We could either provide these APIs in CallEvent or at least have variants that return canonical types. Maybe we already do some of that?

================
Comment at: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:445
@@ +444,3 @@
+
+  const FunctionSpecTy &Spec = FSMI->second;
+  if (!Spec.matchesCall(CE))
----------------
When can this go wrong? Are we checking if there is a mismatch between the function declaration and the call expression? It is strange that findFunctionSpec takes both of those. Couldn't you always get FunctionDecl out of CallExpr?

================
Comment at: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:508
@@ +507,3 @@
+  FunctionSpecMap = {
+    // The isascii() family of functions.
+    { "isalnum",
----------------
you could also use /*NameOfTheField*/ convention to name the arguments if that would make this map more readable.

================
Comment at: test/Analysis/std-library-functions.c:3
@@ +2,3 @@
+
+void clang_analyzer_eval(int);
+int glob;
----------------
Why are you not testing all of the functions?


https://reviews.llvm.org/D20811





More information about the cfe-commits mailing list