[cfe-commits] r157721 - /cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

[Anna Zaks]

On May 30, 2012, at 4:22 PM, David Blaikie wrote:

> 
> 
> On Wed, May 30, 2012 at 4:14 PM, Anna Zaks <ganna at apple.com> wrote:
> Author: zaks
> Date: Wed May 30 18:14:48 2012
> New Revision: 157721
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=157721&view=rev
> Log:
> [analyzer]Fix another occurrence of iterator invalidation (LocalTUDecls)
> 
> Follow up in r155693, r155680.
> 
> Prevents a hard to reproduce crash with the following stack trace:
> 3  libsystem_c.dylib 0x00007ff55a835050 _sigtramp + 18446744029881443184
> 4  clang             0x0000000106218e97 (anonymous
> namespace)::AnalysisConsumer::HandleTranslationUnit(clang::ASTContext&)
> + 519
> 5  clang             0x0000000105cf3002 clang::ParseAST(clang::Sema&,
> bool, bool) + 690
> 6  clang             0x00000001059a41d8
> clang::ASTFrontendAction::ExecuteAction() + 312
> 7  clang             0x00000001059a3df7 clang::FrontendAction::Execute()
> + 231
> 8  clang             0x00000001059b0ecc
> clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) + 860
> 9  clang             0x000000010595e451
> clang::ExecuteCompilerInvocation(clang::CompilerInstance*) + 961
> 10 clang             0x0000000105947f29 cc1_main(char const**, char
> const**, char const*, void*) + 969
> 11 clang             0x0000000105958259 main + 473
> 12 clang             0x0000000105947b34 start + 52
> 
> Modified:
>    cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
> 
> Modified: cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp?rev=157721&r1=157720&r2=157721&view=diff
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp Wed May 30 18:14:48 2012
> @@ -315,10 +315,16 @@
>   // Otherwise, use the Callgraph to derive the order.
>   // Build the Call Graph.
>   CallGraph CG;
> +
>   // Add all the top level declarations to the graph.
> -  for (SetOfDecls::iterator I = LocalTUDecls.begin(),
> -                            E = LocalTUDecls.end(); I != E; ++I)
> -    CG.addToCallGraph(*I);
> +  // Note: TraverseDecl may modify LocalTUDecls, but only by appending more
> +  // entries.  
> 
> Do you want to visit the newly added entries? (I assume not, but just figured I would check since it wasn't clear from the comment)
>  
> Thus we don't use an iterator, but rely on LocalTUDecls
> +  // random access.  By doing so, we automatically compensate for iterators
> +  // possibly being invalidated, although this is a bit slower.
> +  const unsigned n = LocalTUDecls.size();
> +  for (unsigned i = 0 ; i < n ; ++i) {
> 
> If you really don't want to visit the newly added elements, the common way to keep the precomputed 'end' in Clang and LLVM is:
> 
> for (unsigned i = 0, n = LocalTUDecls.size(); i < n; ++i) {
> 
> rather than declaring an extra variable with a wider scope

Thanks for the review David!

We do not want to go through the newly added elements; after thinking about this more I've found another optimization/issue - the number of elements we should go through should be locked in even earlier (r157762).

Cheers,
Anna.

>  
> +    CG.addToCallGraph(LocalTUDecls[i]);
> +  }
> 
>   // Find the top level nodes - children of root + the unreachable (parentless)
>   // nodes.
> 
> 
> _______________________________________________
> 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/20120531/f1097349/attachment.html>