<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div>Hi Jordan,</div><div><br></div><div>It's a cute idea, but I'm opting for simplicity at this point, and I'm not even certain if your suggestion would really make the implementation cleaner. In some ways I like the logic being there explicitly in the loop. At this point I'm also trying to minimize as possible the differences (aside from obvious glaring issues) between the old and new technique where those differences aren't necessary. That way I can focus on the parts that truly are functionally different without introducing behavioral regressions. The code I've checked in now isn't supposed to be smart; the goal is to generate all the arrows. The interesting part of the algorithm, which is where I think most of the effort will be anyway, is on optimizing those edges away. I'd like to getting something working without focusing too much time on refactoring the existing implementation I've copied (that we know works).</div><div><br></div><div>Ted</div><br><div><div>On May 3, 2013, at 1:38 PM, Jordan Rose <<a href="mailto:jordan_rose@apple.com">jordan_rose@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div>While building a cute new algorithm, would it be a good idea to move the loop events into visitors instead of mixing them with the main traversal and edge logic?</div><div><br></div><div>Jordan</div><div><br></div><br><div><div>On May 3, 2013, at 11:25 , Ted Kremenek <<a href="mailto:kremenek@apple.com">kremenek@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">Author: kremenek<br>Date: Fri May 3 13:25:33 2013<br>New Revision: 181040<br><br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project?rev=181040&view=rev">http://llvm.org/viewvc/llvm-project?rev=181040&view=rev</a><br>Log:<br>[analyzer] Start hacking up alternate control-flow edge generation. WIP. Not guaranteed to do anything useful yet.<br><br>Modified:<br> cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h<br> cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp<br><br>Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h<br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h?rev=181040&r1=181039&r2=181040&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h?rev=181040&r1=181039&r2=181040&view=diff</a><br>==============================================================================<br>--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h (original)<br>+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h Fri May 3 13:25:33 2013<br>@@ -94,7 +94,7 @@ public:<br><br> void HandlePathDiagnostic(PathDiagnostic *D);<br><br>- enum PathGenerationScheme { None, Minimal, Extensive };<br>+ enum PathGenerationScheme { None, Minimal, Extensive, AlternateExtensive };<br> virtual PathGenerationScheme getGenerationScheme() const { return Minimal; }<br> virtual bool supportsLogicalOpControlFlow() const { return false; }<br> virtual bool supportsAllBlockEdges() const { return false; }<br><br>Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp<br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=181040&r1=181039&r2=181040&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp?rev=181040&r1=181039&r2=181040&view=diff</a><br>==============================================================================<br>--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp (original)<br>+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporter.cpp Fri May 3 13:25:33 2013<br>@@ -1530,6 +1530,212 @@ static bool GenerateExtensivePathDiagnos<br> return PDB.getBugReport()->isValid();<br>}<br><br>+/// \brief Adds a sanitized control-flow diagnostic edge to a path.<br>+static void addEdgeToPath(PathPieces &path,<br>+ PathDiagnosticLocation &PrevLoc,<br>+ PathDiagnosticLocation NewLoc,<br>+ const LocationContext *LC) {<br>+ if (NewLoc.asLocation().isMacroID())<br>+ return;<br>+<br>+ if (!PrevLoc.isValid()) {<br>+ PrevLoc = NewLoc;<br>+ return;<br>+ }<br>+<br>+ const PathDiagnosticLocation &PrevLocClean = cleanUpLocation(PrevLoc, LC);<br>+ if (PrevLocClean.asLocation().isInvalid()) {<br>+ PrevLoc = NewLoc;<br>+ return;<br>+ }<br>+<br>+ const PathDiagnosticLocation &NewLocClean = cleanUpLocation(NewLoc, LC);<br>+ if (NewLocClean.asLocation() == PrevLocClean.asLocation())<br>+ return;<br>+<br>+ // FIXME: ignore intra-macro edges for now.<br>+ if (NewLocClean.asLocation().getExpansionLoc() ==<br>+ PrevLocClean.asLocation().getExpansionLoc())<br>+ return;<br>+<br>+ path.push_front(new PathDiagnosticControlFlowPiece(NewLocClean,<br>+ PrevLocClean));<br>+ PrevLoc = NewLoc;<br>+}<br>+<br>+static bool<br>+GenerateAlternateExtensivePathDiagnostic(PathDiagnostic& PD,<br>+ PathDiagnosticBuilder &PDB,<br>+ const ExplodedNode *N,<br>+ LocationContextMap &LCM,<br>+ ArrayRef<BugReporterVisitor *> visitors) {<br>+<br>+ BugReport *report = PDB.getBugReport();<br>+ const SourceManager& SM = PDB.getSourceManager();<br>+ StackDiagVector CallStack;<br>+ InterestingExprs IE;<br>+<br>+ // Record the last location for a given visited stack frame.<br>+ llvm::DenseMap<const StackFrameContext *, PathDiagnosticLocation><br>+ PrevLocMap;<br>+<br>+ const ExplodedNode *NextNode = N->getFirstPred();<br>+ while (NextNode) {<br>+ N = NextNode;<br>+ NextNode = N->getFirstPred();<br>+ ProgramPoint P = N->getLocation();<br>+ const LocationContext *LC = N->getLocationContext();<br>+ PathDiagnosticLocation &PrevLoc = PrevLocMap[LC->getCurrentStackFrame()];<br>+<br>+ do {<br>+ if (Optional<PostStmt> PS = P.getAs<PostStmt>()) {<br>+ // For expressions, make sure we propagate the<br>+ // interesting symbols correctly.<br>+ if (const Expr *Ex = PS->getStmtAs<Expr>())<br>+ reversePropagateIntererstingSymbols(*PDB.getBugReport(), IE,<br>+ N->getState().getPtr(), Ex,<br>+ N->getLocationContext());<br>+ break;<br>+ }<br>+<br>+ // Have we encountered an exit from a function call?<br>+ if (Optional<CallExitEnd> CE = P.getAs<CallExitEnd>()) {<br>+ const Stmt *S = CE->getCalleeContext()->getCallSite();<br>+ // Propagate the interesting symbols accordingly.<br>+ if (const Expr *Ex = dyn_cast_or_null<Expr>(S)) {<br>+ reversePropagateIntererstingSymbols(*PDB.getBugReport(), IE,<br>+ N->getState().getPtr(), Ex,<br>+ N->getLocationContext());<br>+ }<br>+<br>+ // We are descending into a call (backwards). Construct<br>+ // a new call piece to contain the path pieces for that call.<br>+ PathDiagnosticCallPiece *C =<br>+ PathDiagnosticCallPiece::construct(N, *CE, SM);<br>+<br>+ // Record the location context for this call piece.<br>+ LCM[C] = CE->getCalleeContext();<br>+<br>+ // Add the edge to the return site.<br>+ addEdgeToPath(PD.getActivePath(), PrevLoc, C->callReturn, LC);<br>+<br>+ // Make the contents of the call the active path for now.<br>+ PD.pushActivePath(&C->path);<br>+ CallStack.push_back(StackDiagPair(C, N));<br>+ break;<br>+ }<br>+<br>+ // Have we encountered an entrance to a call? It may be<br>+ // the case that we have not encountered a matching<br>+ // call exit before this point. This means that the path<br>+ // terminated within the call itself.<br>+ if (Optional<CallEnter> CE = P.getAs<CallEnter>()) {<br>+ // Add an edge to the start of the function.<br>+ const Decl *D = CE->getCalleeContext()->getDecl();<br>+ addEdgeToPath(PD.getActivePath(), PrevLoc,<br>+ PathDiagnosticLocation::createBegin(D, SM), LC);<br>+<br>+ // Did we visit an entire call?<br>+ bool VisitedEntireCall = PD.isWithinCall();<br>+ PD.popActivePath();<br>+<br>+ PathDiagnosticCallPiece *C;<br>+ if (VisitedEntireCall) {<br>+ C = cast<PathDiagnosticCallPiece>(PD.getActivePath().front());<br>+ } else {<br>+ const Decl *Caller = CE->getLocationContext()->getDecl();<br>+ C = PathDiagnosticCallPiece::construct(PD.getActivePath(), Caller);<br>+ LCM[C] = CE->getCalleeContext();<br>+ }<br>+ C->setCallee(*CE, SM);<br>+<br>+ if (!CallStack.empty()) {<br>+ assert(CallStack.back().first == C);<br>+ CallStack.pop_back();<br>+ }<br>+ break;<br>+ }<br>+<br>+ // Block edges.<br>+ if (Optional<BlockEdge> BE = P.getAs<BlockEdge>()) {<br>+ // Does this represent entering a call? If so, look at propagating<br>+ // interesting symbols across call boundaries.<br>+ if (NextNode) {<br>+ const LocationContext *CallerCtx = NextNode->getLocationContext();<br>+ const LocationContext *CalleeCtx = PDB.LC;<br>+ if (CallerCtx != CalleeCtx) {<br>+ reversePropagateInterestingSymbols(*PDB.getBugReport(), IE,<br>+ N->getState().getPtr(),<br>+ CalleeCtx, CallerCtx);<br>+ }<br>+ }<br>+<br>+ // Are we jumping to the head of a loop? Add a special diagnostic.<br>+ if (const Stmt *Loop = BE->getSrc()->getLoopTarget()) {<br>+ PathDiagnosticLocation L(Loop, SM, PDB.LC);<br>+ const CompoundStmt *CS = NULL;<br>+<br>+ if (const ForStmt *FS = dyn_cast<ForStmt>(Loop))<br>+ CS = dyn_cast<CompoundStmt>(FS->getBody());<br>+ else if (const WhileStmt *WS = dyn_cast<WhileStmt>(Loop))<br>+ CS = dyn_cast<CompoundStmt>(WS->getBody());<br>+<br>+ PathDiagnosticEventPiece *p =<br>+ new PathDiagnosticEventPiece(L, "Looping back to the head "<br>+ "of the loop");<br>+ p->setPrunable(true);<br>+<br>+ addEdgeToPath(PD.getActivePath(), PrevLoc, p->getLocation(), LC);<br>+<br>+ if (CS) {<br>+ addEdgeToPath(PD.getActivePath(), PrevLoc,<br>+ PathDiagnosticLocation::createEndBrace(CS, SM), LC);<br>+ }<br>+ }<br>+ <br>+ const CFGBlock *BSrc = BE->getSrc();<br>+ ParentMap &PM = PDB.getParentMap();<br>+<br>+ if (const Stmt *Term = BSrc->getTerminator()) {<br>+ // Are we jumping past the loop body without ever executing the<br>+ // loop (because the condition was false)?<br>+ if (isLoopJumpPastBody(Term, &*BE) &&<br>+ !isInLoopBody(PM,<br>+ getStmtBeforeCond(PM,<br>+ BSrc->getTerminatorCondition(),<br>+ N),<br>+ Term))<br>+ {<br>+ PathDiagnosticLocation L(Term, SM, PDB.LC);<br>+ PathDiagnosticEventPiece *PE =<br>+ new PathDiagnosticEventPiece(L, "Loop body executed 0 times");<br>+ PE->setPrunable(true);<br>+ addEdgeToPath(PD.getActivePath(), PrevLoc,<br>+ PE->getLocation(), LC);<br>+ }<br>+ }<br>+ break;<br>+ }<br>+ } while (0);<br>+<br>+ if (!NextNode)<br>+ continue;<br>+<br>+ // Add pieces from custom visitors.<br>+ BugReport *R = PDB.getBugReport();<br>+ for (ArrayRef<BugReporterVisitor *>::iterator I = visitors.begin(),<br>+ E = visitors.end();<br>+ I != E; ++I) {<br>+ if (PathDiagnosticPiece *p = (*I)->VisitNode(N, NextNode, PDB, *R)) {<br>+ addEdgeToPath(PD.getActivePath(), PrevLoc, p->getLocation(), LC);<br>+ updateStackPiecesWithMessage(p, CallStack);<br>+ }<br>+ }<br>+ }<br>+<br>+ return report->isValid();<br>+}<br>+<br>//===----------------------------------------------------------------------===//<br>// Methods for BugType and subclasses.<br>//===----------------------------------------------------------------------===//<br>@@ -2108,6 +2314,13 @@ bool GRBugReporter::generatePathDiagnost<br> typedef PathDiagnosticConsumer::PathGenerationScheme PathGenerationScheme;<br> PathGenerationScheme ActiveScheme = PC.getGenerationScheme();<br><br>+ if (ActiveScheme == PathDiagnosticConsumer::Extensive) {<br>+ AnalyzerOptions &options = getEngine().getAnalysisManager().options;<br>+ if (options.getBooleanOption("path-diagnostics-alternate", false)) {<br>+ ActiveScheme = PathDiagnosticConsumer::AlternateExtensive;<br>+ }<br>+ }<br>+<br> TrimmedGraph TrimG(&getGraph(), errorNodes);<br> ReportGraph ErrorGraph;<br><br>@@ -2169,6 +2382,9 @@ bool GRBugReporter::generatePathDiagnost<br> LCM.clear();<br><br> switch (ActiveScheme) {<br>+ case PathDiagnosticConsumer::AlternateExtensive:<br>+ GenerateAlternateExtensivePathDiagnostic(PD, PDB, N, LCM, visitors);<br>+ break;<br> case PathDiagnosticConsumer::Extensive:<br> GenerateExtensivePathDiagnostic(PD, PDB, N, LCM, visitors);<br> break;<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">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a></div></blockquote></div></div></blockquote></div><br></body></html>