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

[Anna Zaks]

Looks good to me. See a comment below. CC-ing Jordan.

Anna.
On Jun 17, 2013, at 11:54 AM, Pavel Labath <labath at google.com> wrote:

> 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.
> 
> http://llvm-reviews.chandlerc.com/D994
> 
> Files:
>  lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
>  lib/StaticAnalyzer/Core/CallEvent.cpp
>  test/Analysis/fewer-args.cpp
> 
> Index: lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
> ===================================================================
> --- lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
> +++ lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
> @@ -40,6 +40,7 @@
>   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,32 @@
>     State = StNonNull;
>   }
> 
> +  const Decl *D = Call.getDecl();
> +  if (D && isa<FunctionDecl>(D)) {
> +    // If we have a declaration, we can make sure we pass enough parameters to
> +    // the function.
> +    unsigned Params = cast<FunctionDecl>(D)->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(s) called with less ("

"arguments(s)" -> (Params > 1) ? "arguments" : "argument"

also a verb is missing:

"Function taking 5 arguments called with " -> "Function taking 5 arguments is called with "

> +         << 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()));
> 
> Index: lib/StaticAnalyzer/Core/CallEvent.cpp
> ===================================================================
> --- lib/StaticAnalyzer/Core/CallEvent.cpp
> +++ lib/StaticAnalyzer/Core/CallEvent.cpp
> @@ -272,8 +272,9 @@
>                                          CallEvent::param_iterator E) {
>   MemRegionManager &MRMgr = SVB.getRegionManager();
> 
> +  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?");
> 
> Index: test/Analysis/fewer-args.cpp
> ===================================================================
> --- /dev/null
> +++ test/Analysis/fewer-args.cpp
> @@ -0,0 +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)}}
> +}
> <D994.1.patch>_______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130617/96ac2a3c/attachment.html>