[cfe-commits] r123056 - in /cfe/trunk: lib/Analysis/ReachableCode.cpp lib/Sema/AnalysisBasedWarnings.cpp test/SemaCXX/return-noreturn.cpp

[Ted Kremenek]

Hi Chandler,

Thanks for investigating this.  BTW, do you have any code examples that triggered the infinite loop that we could put into the test suite?

Ted

On Jan 7, 2011, at 10:54 PM, Chandler Carruth wrote:

> Author: chandlerc
> Date: Sat Jan  8 00:54:40 2011
> New Revision: 123056
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=123056&view=rev
> Log:
> Remove a kludge from analysis based warnings that used to detect
> temporaries with no-return destructors. The CFG now properly supports
> temporaries and implicit destructors which both makes this kludge no
> longer work, and conveniently removes the need for it.
> 
> Turn on CFG handling of implicit destructors and initializers. Several
> ad-hoc benchmarks don't indicate any measurable performance impact from
> growing the CFG, and it fixes real correctness problems with warnings.
> 
> As a result of turning on these CFG elements, we started to tickle an
> inf-loop in the unreachable code logic used for warnings. The fix is
> trivial.
> 
> Modified:
>    cfe/trunk/lib/Analysis/ReachableCode.cpp
>    cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
>    cfe/trunk/test/SemaCXX/return-noreturn.cpp
> 
> Modified: cfe/trunk/lib/Analysis/ReachableCode.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ReachableCode.cpp?rev=123056&r1=123055&r2=123056&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Analysis/ReachableCode.cpp (original)
> +++ cfe/trunk/lib/Analysis/ReachableCode.cpp Sat Jan  8 00:54:40 2011
> @@ -30,12 +30,11 @@
>   unsigned sn = 0;
>   R1 = R2 = SourceRange();
> 
> -top:
>   if (sn < b.size()) {
>     CFGStmt CS = b[sn].getAs<CFGStmt>();
>     if (!CS)
> -      goto top;
> -    
> +      return SourceLocation();
> +
>     S = CS.getStmt(); 
>   } else if (b.getTerminator())
>     S = b.getTerminator();
> 
> Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=123056&r1=123055&r2=123056&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original)
> +++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Sat Jan  8 00:54:40 2011
> @@ -177,19 +177,6 @@
>         }
>       }
>     }
> -    // FIXME: Remove this hack once temporaries and their destructors are
> -    // modeled correctly by the CFG.
> -    if (ExprWithCleanups *E = dyn_cast<ExprWithCleanups>(S)) {
> -      for (unsigned I = 0, N = E->getNumTemporaries(); I != N; ++I) {
> -        const FunctionDecl *FD = E->getTemporary(I)->getDestructor();
> -        if (FD->hasAttr<NoReturnAttr>() ||
> -            FD->getType()->getAs<FunctionType>()->getNoReturnAttr()) {
> -          NoReturnEdge = true;
> -          HasFakeEdge = true;
> -          break;
> -        }
> -      }
> -    }
>     // FIXME: Add noreturn message sends.
>     if (NoReturnEdge == false)
>       HasPlainEdge = true;
> @@ -405,7 +392,8 @@
> 
>   // Don't generate EH edges for CallExprs as we'd like to avoid the n^2
>   // explosion for destrutors that can result and the compile time hit.
> -  AnalysisContext AC(D, 0, false);
> +  AnalysisContext AC(D, 0, /*useUnoptimizedCFG=*/false, /*addehedges=*/false,
> +                     /*addImplicitDtors=*/true, /*addInitializers=*/true);
> 
>   // Warning: check missing 'return'
>   if (P.enableCheckFallThrough) {
> 
> Modified: cfe/trunk/test/SemaCXX/return-noreturn.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/return-noreturn.cpp?rev=123056&r1=123055&r2=123056&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/return-noreturn.cpp (original)
> +++ cfe/trunk/test/SemaCXX/return-noreturn.cpp Sat Jan  8 00:54:40 2011
> @@ -7,14 +7,11 @@
>     ~abort_struct() __attribute__((noreturn));
>   };
> 
> -  // FIXME: Should either of these actually warn, since the destructor is
> -  //  marked noreturn?
> -
>   int f() {
>     abort_struct();
> -  } // expected-warning{{control reaches end of non-void function}}
> +  }
> 
>   int f2() {
>     abort_struct s;
> -  } // expected-warning{{control reaches end of non-void function}}
> +  }
> }
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits