[cfe-commits] r160767 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp test/Analysis/misc-ps-cxx0x.cpp

[Jordan Rose]

Nice. So that's what's going on with CallAndMessageChecker!

On Jul 25, 2012, at 5:22 PM, Ted Kremenek <kremenek at apple.com> wrote:

> Author: kremenek
> Date: Wed Jul 25 19:22:32 2012
> New Revision: 160767
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=160767&view=rev
> Log:
> Add static analyzer check for calling a C++ instance method with a null/uninitialized pointer.
> 
> Modified:
>    cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
>    cfe/trunk/test/Analysis/misc-ps-cxx0x.cpp
> 
> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp?rev=160767&r1=160766&r2=160767&view=diff
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp Wed Jul 25 19:22:32 2012
> @@ -31,6 +31,8 @@
>                     check::PreCall > {
>   mutable OwningPtr<BugType> BT_call_null;
>   mutable OwningPtr<BugType> BT_call_undef;
> +  mutable OwningPtr<BugType> BT_cxx_call_null;
> +  mutable OwningPtr<BugType> BT_cxx_call_undef;
>   mutable OwningPtr<BugType> BT_call_arg;
>   mutable OwningPtr<BugType> BT_msg_undef;
>   mutable OwningPtr<BugType> BT_objc_prop_undef;
> @@ -239,14 +241,35 @@
> 
> void CallAndMessageChecker::checkPreCall(const CallEvent &Call,
>                                          CheckerContext &C) const {
> +  // If this is a call to a C++ method, check if the callee is null or
> +  // undefined.
> +  // FIXME: Generalize this to CXXInstanceCall once it supports
> +  // getCXXThisVal().

Right now, all CallEvents support getCXXThisVal(); it's type-dispatched. But outside of CXXInstanceCall, "UndefinedVal" is a placeholder for "no 'this' object". Any CXXInstanceCall should have a valid this-val though.


> +  if (const CXXMemberCall *CC = dyn_cast<CXXMemberCall>(&Call)) {
> +    SVal V = CC->getCXXThisVal();
> +    if (V.isUndef()) {
> +      if (!BT_cxx_call_undef)
> +        BT_cxx_call_undef.reset(new BuiltinBug("Called C++ object pointer is "
> +                                               "uninitialized"));
> +      EmitBadCall(BT_cxx_call_undef.get(), C, CC->getOriginExpr());
> +      return;
> +    }
> +    if (V.isZeroConstant()) {
> +      if (!BT_cxx_call_null)
> +        BT_cxx_call_null.reset(new BuiltinBug("Called C++ object pointer "
> +                                              "is null"));
> +      EmitBadCall(BT_cxx_call_null.get(), C, CC->getOriginExpr());
> +      return;
> +    }
> +  }
> +

This is probably the place to assume V is non-null if we call a method on it. Do we want to do that?

>   // 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()));
> +    !(C.getAnalysisManager().shouldInlineCall() && (D && D->getBody())); 

Yeah, this should be refactored into CallEvent anyway, either CallEvent::mayBeInlined or CallEvent::willBeInlined. The test is wrong because a CallEvent's decl is often not the decl with the definition.