diff options
-rw-r--r-- | include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h | 3 | ||||
-rw-r--r-- | lib/StaticAnalyzer/Core/BugReporter.cpp | 68 | ||||
-rw-r--r-- | test/Analysis/inlining/path-notes.m | 265 |
3 files changed, 310 insertions, 26 deletions
diff --git a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h b/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h index 2e3b0d15c5..3ee7925304 100644 --- a/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h +++ b/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/lib/StaticAnalyzer/Core/BugReporter.cpp b/lib/StaticAnalyzer/Core/BugReporter.cpp index bc5e7ddc64..d5e5f05c48 100644 --- a/lib/StaticAnalyzer/Core/BugReporter.cpp +++ b/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<PathDiagnosticPiece> 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<PathDiagnosticMacroPiece>(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<PathDiagnosticCallPiece>(*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/test/Analysis/inlining/path-notes.m b/test/Analysis/inlining/path-notes.m index 429d857776..3929261e82 100644 --- a/test/Analysis/inlining/path-notes.m +++ b/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: <key>diagnostics</key> // CHECK-NEXT: <array> @@ -815,4 +835,249 @@ int testDispatchSyncInlining() { // CHECK-NEXT: <key>file</key><integer>0</integer> // CHECK-NEXT: </dict> // CHECK-NEXT: </dict> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>path</key> +// CHECK-NEXT: <array> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>kind</key><string>control</string> +// CHECK-NEXT: <key>edges</key> +// CHECK-NEXT: <array> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>start</key> +// CHECK-NEXT: <array> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>line</key><integer>51</integer> +// CHECK-NEXT: <key>col</key><integer>3</integer> +// CHECK-NEXT: <key>file</key><integer>0</integer> +// CHECK-NEXT: </dict> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>line</key><integer>51</integer> +// CHECK-NEXT: <key>col</key><integer>8</integer> +// CHECK-NEXT: <key>file</key><integer>0</integer> +// CHECK-NEXT: </dict> +// CHECK-NEXT: </array> +// CHECK-NEXT: <key>end</key> +// CHECK-NEXT: <array> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>line</key><integer>56</integer> +// CHECK-NEXT: <key>col</key><integer>3</integer> +// CHECK-NEXT: <key>file</key><integer>0</integer> +// CHECK-NEXT: </dict> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>line</key><integer>56</integer> +// CHECK-NEXT: <key>col</key><integer>15</integer> +// CHECK-NEXT: <key>file</key><integer>0</integer> +// CHECK-NEXT: </dict> +// CHECK-NEXT: </array> +// CHECK-NEXT: </dict> +// CHECK-NEXT: </array> +// CHECK-NEXT: </dict> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>kind</key><string>event</string> +// CHECK-NEXT: <key>location</key> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>line</key><integer>56</integer> +// CHECK-NEXT: <key>col</key><integer>3</integer> +// CHECK-NEXT: <key>file</key><integer>0</integer> +// CHECK-NEXT: </dict> +// CHECK-NEXT: <key>ranges</key> +// CHECK-NEXT: <array> +// CHECK-NEXT: <array> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>line</key><integer>56</integer> +// CHECK-NEXT: <key>col</key><integer>3</integer> +// CHECK-NEXT: <key>file</key><integer>0</integer> +// CHECK-NEXT: </dict> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>line</key><integer>62</integer> +// CHECK-NEXT: <key>col</key><integer>4</integer> +// CHECK-NEXT: <key>file</key><integer>0</integer> +// CHECK-NEXT: </dict> +// CHECK-NEXT: </array> +// CHECK-NEXT: </array> +// CHECK-NEXT: <key>depth</key><integer>0</integer> +// CHECK-NEXT: <key>extended_message</key> +// CHECK-NEXT: <string>Calling 'dispatch_sync'</string> +// CHECK-NEXT: <key>message</key> +// CHECK-NEXT: <string>Calling 'dispatch_sync'</string> +// CHECK-NEXT: </dict> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>kind</key><string>event</string> +// CHECK-NEXT: <key>location</key> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>line</key><integer>7</integer> +// CHECK-NEXT: <key>col</key><integer>1</integer> +// CHECK-NEXT: <key>file</key><integer>0</integer> +// CHECK-NEXT: </dict> +// CHECK-NEXT: <key>depth</key><integer>1</integer> +// CHECK-NEXT: <key>extended_message</key> +// CHECK-NEXT: <string>Entered call from 'testDispatchSyncInliningNoPruning'</string> +// CHECK-NEXT: <key>message</key> +// CHECK-NEXT: <string>Entered call from 'testDispatchSyncInliningNoPruning'</string> +// CHECK-NEXT: </dict> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>kind</key><string>event</string> +// CHECK-NEXT: <key>location</key> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>line</key><integer>7</integer> +// CHECK-NEXT: <key>col</key><integer>1</integer> +// CHECK-NEXT: <key>file</key><integer>0</integer> +// CHECK-NEXT: </dict> +// CHECK-NEXT: <key>depth</key><integer>1</integer> +// CHECK-NEXT: <key>extended_message</key> +// CHECK-NEXT: <string>Calling anonymous block</string> +// CHECK-NEXT: <key>message</key> +// CHECK-NEXT: <string>Calling anonymous block</string> +// CHECK-NEXT: </dict> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>kind</key><string>event</string> +// CHECK-NEXT: <key>location</key> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>line</key><integer>56</integer> +// CHECK-NEXT: <key>col</key><integer>30</integer> +// CHECK-NEXT: <key>file</key><integer>0</integer> +// CHECK-NEXT: </dict> +// CHECK-NEXT: <key>depth</key><integer>2</integer> +// CHECK-NEXT: <key>extended_message</key> +// CHECK-NEXT: <string>Entered call from 'dispatch_sync'</string> +// CHECK-NEXT: <key>message</key> +// CHECK-NEXT: <string>Entered call from 'dispatch_sync'</string> +// CHECK-NEXT: </dict> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>kind</key><string>control</string> +// CHECK-NEXT: <key>edges</key> +// CHECK-NEXT: <array> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>start</key> +// CHECK-NEXT: <array> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>line</key><integer>56</integer> +// CHECK-NEXT: <key>col</key><integer>30</integer> +// CHECK-NEXT: <key>file</key><integer>0</integer> +// CHECK-NEXT: </dict> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>line</key><integer>56</integer> +// CHECK-NEXT: <key>col</key><integer>30</integer> +// CHECK-NEXT: <key>file</key><integer>0</integer> +// CHECK-NEXT: </dict> +// CHECK-NEXT: </array> +// CHECK-NEXT: <key>end</key> +// CHECK-NEXT: <array> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>line</key><integer>58</integer> +// CHECK-NEXT: <key>col</key><integer>5</integer> +// CHECK-NEXT: <key>file</key><integer>0</integer> +// CHECK-NEXT: </dict> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>line</key><integer>58</integer> +// CHECK-NEXT: <key>col</key><integer>7</integer> +// CHECK-NEXT: <key>file</key><integer>0</integer> +// CHECK-NEXT: </dict> +// CHECK-NEXT: </array> +// CHECK-NEXT: </dict> +// CHECK-NEXT: </array> +// CHECK-NEXT: </dict> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>kind</key><string>event</string> +// CHECK-NEXT: <key>location</key> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>line</key><integer>58</integer> +// CHECK-NEXT: <key>col</key><integer>5</integer> +// CHECK-NEXT: <key>file</key><integer>0</integer> +// CHECK-NEXT: </dict> +// CHECK-NEXT: <key>ranges</key> +// CHECK-NEXT: <array> +// CHECK-NEXT: <array> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>line</key><integer>58</integer> +// CHECK-NEXT: <key>col</key><integer>5</integer> +// CHECK-NEXT: <key>file</key><integer>0</integer> +// CHECK-NEXT: </dict> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>line</key><integer>58</integer> +// CHECK-NEXT: <key>col</key><integer>9</integer> +// CHECK-NEXT: <key>file</key><integer>0</integer> +// CHECK-NEXT: </dict> +// CHECK-NEXT: </array> +// CHECK-NEXT: </array> +// CHECK-NEXT: <key>depth</key><integer>2</integer> +// CHECK-NEXT: <key>extended_message</key> +// CHECK-NEXT: <string>Variable 'x' declared without an initial value</string> +// CHECK-NEXT: <key>message</key> +// CHECK-NEXT: <string>Variable 'x' declared without an initial value</string> +// CHECK-NEXT: </dict> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>kind</key><string>control</string> +// CHECK-NEXT: <key>edges</key> +// CHECK-NEXT: <array> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>start</key> +// CHECK-NEXT: <array> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>line</key><integer>58</integer> +// CHECK-NEXT: <key>col</key><integer>5</integer> +// CHECK-NEXT: <key>file</key><integer>0</integer> +// CHECK-NEXT: </dict> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>line</key><integer>58</integer> +// CHECK-NEXT: <key>col</key><integer>7</integer> +// CHECK-NEXT: <key>file</key><integer>0</integer> +// CHECK-NEXT: </dict> +// CHECK-NEXT: </array> +// CHECK-NEXT: <key>end</key> +// CHECK-NEXT: <array> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>line</key><integer>60</integer> +// CHECK-NEXT: <key>col</key><integer>5</integer> +// CHECK-NEXT: <key>file</key><integer>0</integer> +// CHECK-NEXT: </dict> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>line</key><integer>60</integer> +// CHECK-NEXT: <key>col</key><integer>5</integer> +// CHECK-NEXT: <key>file</key><integer>0</integer> +// CHECK-NEXT: </dict> +// CHECK-NEXT: </array> +// CHECK-NEXT: </dict> +// CHECK-NEXT: </array> +// CHECK-NEXT: </dict> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>kind</key><string>event</string> +// CHECK-NEXT: <key>location</key> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>line</key><integer>60</integer> +// CHECK-NEXT: <key>col</key><integer>5</integer> +// CHECK-NEXT: <key>file</key><integer>0</integer> +// CHECK-NEXT: </dict> +// CHECK-NEXT: <key>ranges</key> +// CHECK-NEXT: <array> +// CHECK-NEXT: <array> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>line</key><integer>60</integer> +// CHECK-NEXT: <key>col</key><integer>12</integer> +// CHECK-NEXT: <key>file</key><integer>0</integer> +// CHECK-NEXT: </dict> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>line</key><integer>60</integer> +// CHECK-NEXT: <key>col</key><integer>12</integer> +// CHECK-NEXT: <key>file</key><integer>0</integer> +// CHECK-NEXT: </dict> +// CHECK-NEXT: </array> +// CHECK-NEXT: </array> +// CHECK-NEXT: <key>depth</key><integer>2</integer> +// CHECK-NEXT: <key>extended_message</key> +// CHECK-NEXT: <string>Variable 'x' is uninitialized when captured by block</string> +// CHECK-NEXT: <key>message</key> +// CHECK-NEXT: <string>Variable 'x' is uninitialized when captured by block</string> +// CHECK-NEXT: </dict> +// CHECK-NEXT: </array> +// CHECK-NEXT: <key>description</key><string>Variable 'x' is uninitialized when captured by block</string> +// CHECK-NEXT: <key>category</key><string>Logic error</string> +// CHECK-NEXT: <key>type</key><string>uninitialized variable captured by block</string> +// CHECK-NEXT: <key>location</key> +// CHECK-NEXT: <dict> +// CHECK-NEXT: <key>line</key><integer>60</integer> +// CHECK-NEXT: <key>col</key><integer>5</integer> +// CHECK-NEXT: <key>file</key><integer>0</integer> +// CHECK-NEXT: </dict> +// CHECK-NEXT: </dict> // CHECK-NEXT: </array> |