From: Pavel Labath <labath@google.com>
Date: Wed, 19 Jun 2013 08:19:56 +0000 (+0000)
Subject: Fix a crash in the static analyzer (bug #16307)
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=37926da411d5a0047240b3ffd4dad0c4838aac57;p=clang

Fix a crash in the static analyzer (bug #16307)

Summary:
When processing a call to a function, which got passed less arguments than it
expects, the analyzer would crash.

I've also added a test for that and a analyzer warning which detects these
cases.

CC: cfe-commits

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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@184288 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
index 4965d22996..c3c5b5e087 100644
--- a/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
@@ -40,6 +40,7 @@ class CallAndMessageChecker
   mutable OwningPtr<BugType> BT_objc_subscript_undef;
   mutable OwningPtr<BugType> BT_msg_arg;
   mutable OwningPtr<BugType> BT_msg_ret;
+  mutable OwningPtr<BugType> BT_call_few_args;
 public:
 
   void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
@@ -280,11 +281,33 @@ void CallAndMessageChecker::checkPreCall(const CallEvent &Call,
     State = StNonNull;
   }
 
+  const Decl *D = Call.getDecl();
+  if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D)) {
+    // If we have a declaration, we can make sure we pass enough parameters to
+    // the function.
+    unsigned Params = FD->getNumParams();
+    if (Call.getNumArgs() < Params) {
+      ExplodedNode *N = C.generateSink();
+      if (!N)
+        return;
+
+      LazyInit_BT("Function call with too few arguments", BT_call_few_args);
+
+      SmallString<512> Str;
+      llvm::raw_svector_ostream os(Str);
+      os << "Function taking " << Params << " argument"
+         << (Params == 1 ? "" : "s") << " is called with less ("
+         << Call.getNumArgs() << ")";
+
+      BugReport *R = new BugReport(*BT_call_few_args, os.str(), N);
+      C.emitReport(R);
+    }
+  }
+
   // Don't check for uninitialized field values in arguments if the
   // caller has a body that is available and we have the chance to inline it.
   // This is a hack, but is a reasonable compromise betweens sometimes warning
   // and sometimes not depending on if we decide to inline a function.
-  const Decl *D = Call.getDecl();
   const bool checkUninitFields =
     !(C.getAnalysisManager().shouldInlineCall() && (D && D->getBody()));
 
diff --git a/lib/StaticAnalyzer/Core/CallEvent.cpp b/lib/StaticAnalyzer/Core/CallEvent.cpp
index dd33e014c3..9ab6874756 100644
--- a/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ b/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -272,8 +272,11 @@ static void addParameterValuesToBindings(const StackFrameContext *CalleeCtx,
                                          CallEvent::param_iterator E) {
   MemRegionManager &MRMgr = SVB.getRegionManager();
 
+  // If the function has fewer parameters than the call has arguments, we simply
+  // do not bind any values to them.
+  unsigned NumArgs = Call.getNumArgs();
   unsigned Idx = 0;
-  for (; I != E; ++I, ++Idx) {
+  for (; I != E && Idx < NumArgs; ++I, ++Idx) {
     const ParmVarDecl *ParamDecl = *I;
     assert(ParamDecl && "Formal parameter has no decl?");
 
diff --git a/test/Analysis/inline.c b/test/Analysis/inline.c
index 8cba63f17e..9ecdce33a3 100644
--- a/test/Analysis/inline.c
+++ b/test/Analysis/inline.c
@@ -110,3 +110,9 @@ void never_called_by_anyone() {
   clang_analyzer_checkInlined(0); // no-warning
 }
 
+
+void knr_one_argument(a) int a; { }
+
+void call_with_less_arguments() {
+  knr_one_argument(); // expected-warning{{too few arguments}} expected-warning{{Function taking 1 argument}}
+}
diff --git a/test/Analysis/inline.cpp b/test/Analysis/inline.cpp
index 909e18017b..fad77b3bee 100644
--- a/test/Analysis/inline.cpp
+++ b/test/Analysis/inline.cpp
@@ -420,3 +420,10 @@ namespace rdar12409977  {
     clang_analyzer_eval(obj.getThis()->x == 42); // expected-warning{{TRUE}}
   }
 }
+
+namespace bug16307 {
+  void one_argument(int a) { }
+  void call_with_less() {
+    reinterpret_cast<void (*)()>(one_argument)(); // expected-warning{{Function taking 1 argument}}
+  }
+}