<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div><div>On May 30, 2012, at 4:22 PM, David Blaikie wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><br><br><div class="gmail_quote">On Wed, May 30, 2012 at 4:14 PM, Anna Zaks <span dir="ltr"><<a href="mailto:ganna@apple.com" target="_blank">ganna@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex; position: static; z-index: auto; ">
Author: zaks<br>
Date: Wed May 30 18:14:48 2012<br>
New Revision: 157721<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=157721&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=157721&view=rev</a><br>
Log:<br>
[analyzer]Fix another occurrence of iterator invalidation (LocalTUDecls)<br>
<br>
Follow up in r155693, r155680.<br>
<br>
Prevents a hard to reproduce crash with the following stack trace:<br>
3  libsystem_c.dylib 0x00007ff55a835050 _sigtramp + 18446744029881443184<br>
4  clang             0x0000000106218e97 (anonymous<br>
namespace)::AnalysisConsumer::HandleTranslationUnit(clang::ASTContext&)<br>
+ 519<br>
5  clang             0x0000000105cf3002 clang::ParseAST(clang::Sema&,<br>
bool, bool) + 690<br>
6  clang             0x00000001059a41d8<br>
clang::ASTFrontendAction::ExecuteAction() + 312<br>
7  clang             0x00000001059a3df7 clang::FrontendAction::Execute()<br>
+ 231<br>
8  clang             0x00000001059b0ecc<br>
clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) + 860<br>
9  clang             0x000000010595e451<br>
clang::ExecuteCompilerInvocation(clang::CompilerInstance*) + 961<br>
10 clang             0x0000000105947f29 cc1_main(char const**, char<br>
const**, char const*, void*) + 969<br>
11 clang             0x0000000105958259 main + 473<br>
12 clang             0x0000000105947b34 start + 52<br>
<br>
Modified:<br>
    cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp<br>
<br>
Modified: cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp?rev=157721&r1=157720&r2=157721&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp?rev=157721&r1=157720&r2=157721&view=diff</a><br>

==============================================================================<br>
--- cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp (original)<br>
+++ cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp Wed May 30 18:14:48 2012<br>
@@ -315,10 +315,16 @@<br>
   // Otherwise, use the Callgraph to derive the order.<br>
   // Build the Call Graph.<br>
   CallGraph CG;<br>
+<br>
   // Add all the top level declarations to the graph.<br>
-  for (SetOfDecls::iterator I = LocalTUDecls.begin(),<br>
-                            E = LocalTUDecls.end(); I != E; ++I)<br>
-    CG.addToCallGraph(*I);<br>
+  // Note: TraverseDecl may modify LocalTUDecls, but only by appending more<br>
+  // entries.  </blockquote><div><br></div><div>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)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Thus we don't use an iterator, but rely on LocalTUDecls<br>
+  // random access.  By doing so, we automatically compensate for iterators<br>
+  // possibly being invalidated, although this is a bit slower.<br>
+  const unsigned n = LocalTUDecls.size();<br>
+  for (unsigned i = 0 ; i < n ; ++i) {<br></blockquote><div><br></div><div>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:<br>
<br>for (unsigned i = 0, n = LocalTUDecls.size(); i < n; ++i) {<br><br>rather than declaring an extra variable with a wider scope</div></div></blockquote><div><br></div>Thanks for the review David!<div><br></div><div>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).<br><div></div></div><div><br></div><div>Cheers,</div><div>Anna.</div><div><br></div><blockquote type="cite"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

+    CG.addToCallGraph(LocalTUDecls[i]);<br>
+  }<br>
<br>
   // Find the top level nodes - children of root + the unreachable (parentless)<br>
   // nodes.<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br>
</blockquote></div><br></div></body></html>