From 9a3391364515dec3dd89e285a8af3b91f9976420 Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Fri, 7 Dec 2012 19:56:29 +0000 Subject: [PATCH] [analyzer] Fix r168019 to work with unpruned paths as well. This is the case where the analyzer tries to print out source locations for code within a synthesized function body, which of course does not have a valid source location. The previous fix attempted to do this during diagnostic path pruning, but some diagnostics have pruning disabled, and so any diagnostic with a path that goes through a synthesized body will either hit an assertion or emit invalid output. (again) llvm-svn: 169631 --- .../StaticAnalyzer/Core/BugReporter/BugReporter.h | 3 +- clang/lib/StaticAnalyzer/Core/BugReporter.cpp | 68 ++++-- clang/test/Analysis/inlining/path-notes.m | 265 +++++++++++++++++++++ 3 files changed, 310 insertions(+), 26 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h index 2e3b0d1..3ee7925 100644 --- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h +++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h @@ -440,8 +440,7 @@ public: return true; } - bool RemoveUneededCalls(PathPieces &pieces, BugReport *R, - PathDiagnosticLocation *LastCallLocation = 0); + bool RemoveUnneededCalls(PathPieces &pieces, BugReport *R); void Register(BugType *BT); diff --git a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp index bc5e7dd..d5e5f05 100644 --- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp +++ b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -191,9 +191,8 @@ static void removeRedundantMsgs(PathPieces &path) { /// Recursively scan through a path and prune out calls and macros pieces /// that aren't needed. Return true if afterwards the path contains -/// "interesting stuff" which means it should be pruned from the parent path. -bool BugReporter::RemoveUneededCalls(PathPieces &pieces, BugReport *R, - PathDiagnosticLocation *LastCallLocation) { +/// "interesting stuff" which means it shouldn't be pruned from the parent path. +bool BugReporter::RemoveUnneededCalls(PathPieces &pieces, BugReport *R) { bool containsSomethingInteresting = false; const unsigned N = pieces.size(); @@ -203,7 +202,9 @@ bool BugReporter::RemoveUneededCalls(PathPieces &pieces, BugReport *R, IntrusiveRefCntPtr piece(pieces.front()); pieces.pop_front(); - // Throw away pieces with invalid locations. + // Throw away pieces with invalid locations. Note that we can't throw away + // calls just yet because they might have something interesting inside them. + // If so, their locations will be adjusted as necessary later. if (piece->getKind() != PathDiagnosticPiece::Call && piece->getLocation().asLocation().isInvalid()) continue; @@ -218,23 +219,7 @@ bool BugReporter::RemoveUneededCalls(PathPieces &pieces, BugReport *R, break; } - if (LastCallLocation) { - if (!call->callEnter.asLocation().isValid()) - call->callEnter = *LastCallLocation; - if (!call->callReturn.asLocation().isValid()) - call->callReturn = *LastCallLocation; - } - - // Recursively clean out the subclass. Keep this call around if - // it contains any informative diagnostics. - PathDiagnosticLocation *ThisCallLocation; - if (call->callEnterWithin.asLocation().isValid()) - ThisCallLocation = &call->callEnterWithin; - else - ThisCallLocation = &call->callEnter; - - assert(ThisCallLocation && "Outermost call has an invalid location"); - if (!RemoveUneededCalls(call->path, R, ThisCallLocation)) + if (!RemoveUnneededCalls(call->path, R)) continue; containsSomethingInteresting = true; @@ -242,7 +227,7 @@ bool BugReporter::RemoveUneededCalls(PathPieces &pieces, BugReport *R, } case PathDiagnosticPiece::Macro: { PathDiagnosticMacroPiece *macro = cast(piece); - if (!RemoveUneededCalls(macro->subPieces, R)) + if (!RemoveUnneededCalls(macro->subPieces, R)) continue; containsSomethingInteresting = true; break; @@ -265,6 +250,39 @@ bool BugReporter::RemoveUneededCalls(PathPieces &pieces, BugReport *R, return containsSomethingInteresting; } +/// Recursively scan through a path and make sure that all call pieces have +/// valid locations. Note that all other pieces with invalid locations should +/// have already been pruned out. +static void adjustCallLocations(PathPieces &Pieces, + PathDiagnosticLocation *LastCallLocation = 0) { + for (PathPieces::iterator I = Pieces.begin(), E = Pieces.end(); I != E; ++I) { + PathDiagnosticCallPiece *Call = dyn_cast(*I); + + if (!Call) { + assert((*I)->getLocation().asLocation().isValid()); + continue; + } + + if (LastCallLocation) { + if (!Call->callEnter.asLocation().isValid()) + Call->callEnter = *LastCallLocation; + if (!Call->callReturn.asLocation().isValid()) + Call->callReturn = *LastCallLocation; + } + + // Recursively clean out the subclass. Keep this call around if + // it contains any informative diagnostics. + PathDiagnosticLocation *ThisCallLocation; + if (Call->callEnterWithin.asLocation().isValid()) + ThisCallLocation = &Call->callEnterWithin; + else + ThisCallLocation = &Call->callEnter; + + assert(ThisCallLocation && "Outermost call has an invalid location"); + adjustCallLocations(Call->path, ThisCallLocation); + } +} + //===----------------------------------------------------------------------===// // PathDiagnosticBuilder and its associated routines and helper objects. //===----------------------------------------------------------------------===// @@ -2102,11 +2120,13 @@ bool GRBugReporter::generatePathDiagnostic(PathDiagnostic& PD, removeRedundantMsgs(PD.getMutablePieces()); if (R->shouldPrunePath()) { - bool hasSomethingInteresting = RemoveUneededCalls(PD.getMutablePieces(), - R); + bool hasSomethingInteresting = RemoveUnneededCalls(PD.getMutablePieces(), + R); assert(hasSomethingInteresting); (void) hasSomethingInteresting; } + + adjustCallLocations(PD.getMutablePieces()); } return true; diff --git a/clang/test/Analysis/inlining/path-notes.m b/clang/test/Analysis/inlining/path-notes.m index 429d857..3929261 100644 --- a/clang/test/Analysis/inlining/path-notes.m +++ b/clang/test/Analysis/inlining/path-notes.m @@ -44,6 +44,26 @@ int testDispatchSyncInlining() { // expected-note@-1 {{Division by zero}} } +int testDispatchSyncInliningNoPruning(int coin) { + // This tests exactly the same case as above, except on a bug report where + // path pruning is disabled (an uninitialized variable capture). + // In this case + extern dispatch_queue_t globalQueue; + + __block int y; + + // expected-note@+1 {{Calling 'dispatch_sync'}} + dispatch_sync(globalQueue, ^{ + // expected-note@7 {{Calling anonymous block}} + int x; + // expected-note@-1 {{Variable 'x' declared without an initial value}} + ^{ y = x; }(); // expected-warning{{Variable 'x' is uninitialized when captured by block}} + // expected-note@-1 {{Variable 'x' is uninitialized when captured by block}} + }); + + return y; +} + // CHECK: diagnostics // CHECK-NEXT: @@ -815,4 +835,249 @@ int testDispatchSyncInlining() { // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: path +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindcontrol +// CHECK-NEXT: edges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: start +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line51 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line51 +// CHECK-NEXT: col8 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: end +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line56 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line56 +// CHECK-NEXT: col15 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line56 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: ranges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line56 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line62 +// CHECK-NEXT: col4 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: depth0 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Calling 'dispatch_sync' +// CHECK-NEXT: message +// CHECK-NEXT: Calling 'dispatch_sync' +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line7 +// CHECK-NEXT: col1 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: depth1 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Entered call from 'testDispatchSyncInliningNoPruning' +// CHECK-NEXT: message +// CHECK-NEXT: Entered call from 'testDispatchSyncInliningNoPruning' +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line7 +// CHECK-NEXT: col1 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: depth1 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Calling anonymous block +// CHECK-NEXT: message +// CHECK-NEXT: Calling anonymous block +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line56 +// CHECK-NEXT: col30 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: depth2 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Entered call from 'dispatch_sync' +// CHECK-NEXT: message +// CHECK-NEXT: Entered call from 'dispatch_sync' +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindcontrol +// CHECK-NEXT: edges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: start +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line56 +// CHECK-NEXT: col30 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line56 +// CHECK-NEXT: col30 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: end +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line58 +// CHECK-NEXT: col5 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line58 +// CHECK-NEXT: col7 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line58 +// CHECK-NEXT: col5 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: ranges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line58 +// CHECK-NEXT: col5 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line58 +// CHECK-NEXT: col9 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: depth2 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Variable 'x' declared without an initial value +// CHECK-NEXT: message +// CHECK-NEXT: Variable 'x' declared without an initial value +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindcontrol +// CHECK-NEXT: edges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: start +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line58 +// CHECK-NEXT: col5 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line58 +// CHECK-NEXT: col7 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: end +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line60 +// CHECK-NEXT: col5 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line60 +// CHECK-NEXT: col5 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line60 +// CHECK-NEXT: col5 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: ranges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line60 +// CHECK-NEXT: col12 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line60 +// CHECK-NEXT: col12 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: depth2 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Variable 'x' is uninitialized when captured by block +// CHECK-NEXT: message +// CHECK-NEXT: Variable 'x' is uninitialized when captured by block +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: descriptionVariable 'x' is uninitialized when captured by block +// CHECK-NEXT: categoryLogic error +// CHECK-NEXT: typeuninitialized variable captured by block +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line60 +// CHECK-NEXT: col5 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: // CHECK-NEXT: -- 2.7.4