r174245 - [analyzer] Always inline functions with bodies generated by BodyFarm.

[Jordan Rose]

On Feb 1, 2013, at 16:30 , Anna Zaks <ganna at apple.com> wrote:

> Author: zaks
> Date: Fri Feb  1 18:30:04 2013
> New Revision: 174245
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=174245&view=rev
> Log:
> [analyzer] Always inline functions with bodies generated by BodyFarm.
> 
> Inlining these functions is essential for correctness. We often have
> cases where we do not inline calls. For example, the shallow mode and
> when reanalyzing previously inlined ObjC methods as top level.
> 
> Modified:
>    cfe/trunk/include/clang/Analysis/AnalysisContext.h
>    cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp
>    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
>    cfe/trunk/test/Analysis/NSString.m
> 
> Modified: cfe/trunk/include/clang/Analysis/AnalysisContext.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/AnalysisContext.h?rev=174245&r1=174244&r2=174245&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Analysis/AnalysisContext.h (original)
> +++ cfe/trunk/include/clang/Analysis/AnalysisContext.h Fri Feb  1 18:30:04 2013
> @@ -133,7 +133,21 @@ public:
>   void registerForcedBlockExpression(const Stmt *stmt);
>   const CFGBlock *getBlockForRegisteredExpression(const Stmt *stmt);
> 
> +  /// \brief Get the body of the Declaration.
>   Stmt *getBody() const;
> +
> +  /// \brief Get the body of the Declaration.
> +  /// \param IsAutosynthesized Output parameter that specifies if the body is
> +  /// auto-generated by the BodyFarm.

Nitpick: you can specify output parameters in Doxygen with \param[out].


> +  Stmt *getBody(bool &IsAutosynthesized) const;
> +
> +  /// \brief Checks if the body of the Decl is generated by the BodyFarm.
> +  ///
> +  /// Note, the lookup is not free. We are going to call getBody behind
> +  /// the sceines.

Typo: "sceines"

> +  /// \sa getBody
> +  bool isBodyAutosynthesized() const;
> +
>   CFG *getCFG();
> 
>   CFGStmtMap *getCFGStmtMap();
> 
> Modified: cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp?rev=174245&r1=174244&r2=174245&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp (original)
> +++ cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp Fri Feb  1 18:30:04 2013
> @@ -86,11 +86,13 @@ static BodyFarm &getBodyFarm(ASTContext 
>   return *BF;
> }
> 
> -Stmt *AnalysisDeclContext::getBody() const {
> +Stmt *AnalysisDeclContext::getBody(bool &IsAutosynthesized) const {
>   if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
>     Stmt *Body = FD->getBody();
> -    if (!Body && Manager && Manager->synthesizeBodies())
> +    if (!Body && Manager && Manager->synthesizeBodies()) {
> +      IsAutosynthesized = true;
>       return getBodyFarm(getASTContext()).getBody(FD);
> +    }
>     return Body;
>   }
>   else if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D))
> @@ -104,6 +106,17 @@ Stmt *AnalysisDeclContext::getBody() con
>   llvm_unreachable("unknown code decl");
> }
> 
> +Stmt *AnalysisDeclContext::getBody() const {
> +  bool Tmp;
> +  return getBody(Tmp);
> +}
> +
> +bool AnalysisDeclContext::isBodyAutosynthesized() const {
> +  bool Tmp;
> +  getBody(Tmp);
> +  return Tmp;
> +}
> +
> const ImplicitParamDecl *AnalysisDeclContext::getSelfDecl() const {
>   if (const ObjCMethodDecl *MD = dyn_cast<ObjCMethodDecl>(D))
>     return MD->getSelfDecl();
> 
> Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp?rev=174245&r1=174244&r2=174245&view=diff
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp Fri Feb  1 18:30:04 2013
> @@ -733,13 +733,27 @@ void ExprEngine::conservativeEvalCall(co
>   Bldr.generateNode(Call.getProgramPoint(), State, Pred);
> }
> 
> +static bool isEssentialToInline(const CallEvent &Call) {
> +  const Decl *D = Call.getDecl();
> +  if (D) {
> +    AnalysisDeclContext *AD =
> +      Call.getLocationContext()->getAnalysisDeclContext()->
> +      getManager()->getContext(D);
> +
> +    // The auto-synthesized bodies are essential to inline as they are
> +    // usually small and commonly used.
> +    return AD->isBodyAutosynthesized();
> +  }
> +  return false;
> +}
> +
> void ExprEngine::defaultEvalCall(NodeBuilder &Bldr, ExplodedNode *Pred,
>                                  const CallEvent &CallTemplate) {
>   // Make sure we have the most recent state attached to the call.
>   ProgramStateRef State = Pred->getState();
>   CallEventRef<> Call = CallTemplate.cloneWithState(State);
> 
> -  if (HowToInline == Inline_None) {
> +  if (HowToInline == Inline_None && !isEssentialToInline(CallTemplate)) {
>     conservativeEvalCall(*Call, Bldr, Pred, State);
>     return;
>   }
> 
> Modified: cfe/trunk/test/Analysis/NSString.m
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/NSString.m?rev=174245&r1=174244&r2=174245&view=diff
> ==============================================================================
> --- cfe/trunk/test/Analysis/NSString.m (original)
> +++ cfe/trunk/test/Analysis/NSString.m Fri Feb  1 18:30:04 2013
> @@ -404,3 +404,27 @@ void testOSCompareAndSwapXXBarrier_param
>   else    
>     return;
> }
> +
> + at interface AlwaysInlineBodyFarmBodies : NSObject {
> +  NSString *_value;
> +}
> +  - (NSString *)_value;
> +  - (void)callValue;
> + at end
> +
> + at implementation AlwaysInlineBodyFarmBodies
> +
> +- (NSString *)_value {
> +  if (!_value) {
> +    NSString *s = [[NSString alloc] init];
> +    if (!OSAtomicCompareAndSwapPtr(0, s, (void**)&_value)) {
> +      [s release];
> +    }
> +  }
> +  return _value;
> +}
> +
> +- (void)callValue {
> +  [self _value];
> +}
> + at end

It would be nice to have a note here that this is only testing the feature because of the re-analyzing behavior for Objective-C methods. Alternately, maybe the test should be moved to an explicit no-inlining file…except that we probably have a control higher up that will stop inlining altogether in this case.

Jordan