[PATCH] Fix a crash in the static analyzer bug #16307

Pavel Labath labath at google.com
Wed Jun 19 01:28:53 PDT 2013


  I'm not sure really sure about the workflow here yet. I don't know if this counted as ACK, but i have already committed this. :D
  If that's wrong I can revert.


================
Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:275-277
@@ -274,4 +274,5 @@
 
+  unsigned NumArgs = Call.getNumArgs();
   unsigned Idx = 0;
-  for (; I != E; ++I, ++Idx) {
+  for (; I != E && Idx < NumArgs; ++I, ++Idx) {
     const ParmVarDecl *ParamDecl = *I;
----------------
Jordan Rose wrote:
> This implies we won't bind any parameter values when calling a K&R C function. I guess that's correct behavior...we can't just not inline such a function because people declare C functions as `void foo()`.
I've added a small comment about this.

================
Comment at: test/Analysis/fewer-args.cpp:1-7
@@ +1,7 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core %s -verify
+
+void f(int a) { }
+
+void g() {
+  reinterpret_cast<void (*)()>(f)(); // expected-warning{{Function taking 1 argument(s) called with less (0)}}
+}
----------------
Jordan Rose wrote:
> I'm not sure this deserves a whole file of its own...the cost of a separate test invocation is non-zero. Please add this to, say, inline.cpp, even though it's not just about inline functions, and please add the K&R test to inline.c.
Done.

================
Comment at: lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp:285
@@ +284,3 @@
+  const Decl *D = Call.getDecl();
+  if (D && isa<FunctionDecl>(D)) {
+    // If we have a declaration, we can make sure we pass enough parameters to
----------------
Jordan Rose wrote:
> Since you cast D again below, the best way to write this is probably as follows:
> 
> ```if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D))```
> 
> This doesn't check blocks, which could easily have the same problem, but I guess that's okay for now. Your check in CallEvent will at least ensure they aren't called with the wrong values.
> 
Done.


http://llvm-reviews.chandlerc.com/D994

COMMIT
  http://llvm-reviews.chandlerc.com/rL184288



More information about the cfe-commits mailing list