This is an archive of the discontinued LLVM Phabricator instance.

Differential D45117

[analyzer] Fix diagnostics in callees of interesting callees.
ClosedPublic

Authored by NoQ on Mar 30 2018, 5:55 PM.

Details

Reviewers
dcoughlin
xazax.hun
a.sidorin
george.karpenkov
szepet
Commits
rGf7281b475225: [analyzer] RetainCount: Accept more "safe" CFRetain wrappers.
rGc8b1d5f32963: [analyzer] Fix diagnostics in callees of interesting callees.
rC330375: [analyzer] RetainCount: Accept more "safe" CFRetain wrappers.
rL330375: [analyzer] RetainCount: Accept more "safe" CFRetain wrappers.
rC329102: [analyzer] Fix diagnostics in callees of interesting callees.
rL329102: [analyzer] Fix diagnostics in callees of interesting callees.
Summary

removeUnneededCalls() is responsible for removing path diagnostic pieces within functions that don't contain "interesting" events. It is the functionality that's controlled by the `-analyzer-config prune-paths and it makes bug reports much tidier (though sometimes it removes too much, and in this case it's great to disable this option when debugging).

While relatively simple, it contains a bug: when a stack frame is known to be interesting, the function doesn't descend into it to prune anything within it, even other callees that are totally boring.

This is rarely apparent, but in C++ there are a lot of implicit calls that suddenly become visible because of this bug.

Diff Detail

Event Timeline

NoQ created this revision.Mar 30 2018, 5:55 PM
Herald added subscribers: cfe-commits, rnkovacs. · View Herald TranscriptMar 30 2018, 5:55 PM
This revision was not accepted when it landed; it landed in state Needs Review.Apr 3 2018, 11:56 AM
Closed by commit rL329102: [analyzer] Fix diagnostics in callees of interesting callees. (authored by NoQ). · Explain Why
This revision was automatically updated to reflect the committed changes.
Herald added a subscriber: llvm-commits. · View Herald TranscriptApr 3 2018, 11:56 AM
NoQ added a comment.Apr 19 2018, 4:08 PM

rC330375 has nothing to do with this review; i misplaced the link in the commit message again.

Herald added a subscriber: baloghadamsoftware. · View Herald TranscriptApr 19 2018, 4:08 PM

Revision Contents

PathSize
lib/
StaticAnalyzer/
Core/
15 lines
test/
Analysis/
inlining/
302 lines

Diff 140513

lib/StaticAnalyzer/Core/BugReporter.cpp

Show First 20 Lines • Show All 186 Lines • ▼ Show 20 Lines
/// function call it represents. /// function call it represents.
using LocationContextMap = using LocationContextMap =
llvm::DenseMap<const PathPieces *, const LocationContext *>; llvm::DenseMap<const PathPieces *, const LocationContext *>;
/// Recursively scan through a path and prune out calls and macros pieces /// Recursively scan through a path and prune out calls and macros pieces
/// that aren't needed. Return true if afterwards the path contains /// that aren't needed. Return true if afterwards the path contains
/// "interesting stuff" which means it shouldn't be pruned from the parent path. /// "interesting stuff" which means it shouldn't be pruned from the parent path.
static bool removeUnneededCalls(PathPieces &pieces, BugReport *R, static bool removeUnneededCalls(PathPieces &pieces, BugReport *R,
LocationContextMap &LCM) { LocationContextMap &LCM,
bool containsSomethingInteresting = false; bool IsInteresting = false) {
bool containsSomethingInteresting = IsInteresting;
const unsigned N = pieces.size(); const unsigned N = pieces.size();
for (unsigned i = 0 ; i < N ; ++i) { for (unsigned i = 0 ; i < N ; ++i) {
// Remove the front piece from the path. If it is still something we // Remove the front piece from the path. If it is still something we
// want to keep once we are done, we will push it back on the end. // want to keep once we are done, we will push it back on the end.
auto piece = std::move(pieces.front()); auto piece = std::move(pieces.front());
pieces.pop_front(); pieces.pop_front();
switch (piece->getKind()) { switch (piece->getKind()) {
case PathDiagnosticPiece::Call: { case PathDiagnosticPiece::Call: {
auto &call = cast<PathDiagnosticCallPiece>(*piece); auto &call = cast<PathDiagnosticCallPiece>(*piece);
// Check if the location context is interesting. // Check if the location context is interesting.
assert(LCM.count(&call.path)); assert(LCM.count(&call.path));
if (R->isInteresting(LCM[&call.path])) { if (!removeUnneededCalls(call.path, R, LCM,
containsSomethingInteresting = true; R->isInteresting(LCM[&call.path])))
break;
}
if (!removeUnneededCalls(call.path, R, LCM))
continue; continue;
containsSomethingInteresting = true; containsSomethingInteresting = true;
break; break;
} }
case PathDiagnosticPiece::Macro: { case PathDiagnosticPiece::Macro: {
auto &macro = cast<PathDiagnosticMacroPiece>(*piece); auto &macro = cast<PathDiagnosticMacroPiece>(*piece);
if (!removeUnneededCalls(macro.subPieces, R, LCM)) if (!removeUnneededCalls(macro.subPieces, R, LCM, IsInteresting))
continue; continue;
containsSomethingInteresting = true; containsSomethingInteresting = true;
break; break;
} }
case PathDiagnosticPiece::Event: { case PathDiagnosticPiece::Event: {
auto &event = cast<PathDiagnosticEventPiece>(*piece); auto &event = cast<PathDiagnosticEventPiece>(*piece);
// We never throw away an event, but we do throw it away wholesale // We never throw away an event, but we do throw it away wholesale
▲ Show 20 Lines • Show All 3,560 Lines • Show Last 20 Lines

test/Analysis/inlining/path-notes.c

Show First 20 Lines • Show All 134 Lines • ▼ Show 20 Lines
void test4(int **p) { void test4(int **p) {
if (*p) return; // expected-note {{Taking false branch}} if (*p) return; // expected-note {{Taking false branch}}
// expected-note@-1 {{Assuming pointer value is null}} // expected-note@-1 {{Assuming pointer value is null}}
**p = 1; // expected-warning {{Dereference of null pointer}} **p = 1; // expected-warning {{Dereference of null pointer}}
// expected-note@-1 {{Dereference of null pointer}} // expected-note@-1 {{Dereference of null pointer}}
} }
void boringCallee() {
}
void interestingCallee(int *x) {
*x = 0; // expected-note{{The value 0 is assigned to 'x'}}
boringCallee(); // no-note
}
int testBoringCalleeOfInterestingCallee() {
int x;
interestingCallee(&x); // expected-note{{Calling 'interestingCallee'}}
// expected-note@-1{{Returning from 'interestingCallee'}}
return 1 / x; // expected-warning{{Division by zero}}
// expected-note@-1{{Division by zero}}
}
// CHECK: <key>diagnostics</key> // CHECK: <key>diagnostics</key>
// CHECK-NEXT: <array> // CHECK-NEXT: <array>
// CHECK-NEXT: <dict> // CHECK-NEXT: <dict>
// CHECK-NEXT: <key>path</key> // CHECK-NEXT: <key>path</key>
// CHECK-NEXT: <array> // CHECK-NEXT: <array>
// CHECK-NEXT: <dict> // CHECK-NEXT: <dict>
// CHECK-NEXT: <key>kind</key><string>event</string> // CHECK-NEXT: <key>kind</key><string>event</string>
// CHECK-NEXT: <key>location</key> // CHECK-NEXT: <key>location</key>
▲ Show 20 Lines • Show All 3,221 Lines • ▼ Show 20 Lines
// CHECK-NEXT: <key>issue_hash_function_offset</key><string>3</string> // CHECK-NEXT: <key>issue_hash_function_offset</key><string>3</string>
// CHECK-NEXT: <key>location</key> // CHECK-NEXT: <key>location</key>
// CHECK-NEXT: <dict> // CHECK-NEXT: <dict>
// CHECK-NEXT: <key>line</key><integer>139</integer> // CHECK-NEXT: <key>line</key><integer>139</integer>
// CHECK-NEXT: <key>col</key><integer>7</integer> // CHECK-NEXT: <key>col</key><integer>7</integer>
// CHECK-NEXT: <key>file</key><integer>0</integer> // CHECK-NEXT: <key>file</key><integer>0</integer>
// CHECK-NEXT: </dict> // CHECK-NEXT: </dict>
// 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>152</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>152</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: <key>end</key>
// CHECK-NEXT: <array>
// CHECK-NEXT: <dict>
// CHECK-NEXT: <key>line</key><integer>153</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>153</integer>
// CHECK-NEXT: <key>col</key><integer>19</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>153</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>153</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>153</integer>
// CHECK-NEXT: <key>col</key><integer>23</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 &apos;interestingCallee&apos;</string>
// CHECK-NEXT: <key>message</key>
// CHECK-NEXT: <string>Calling &apos;interestingCallee&apos;</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>146</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 &apos;testBoringCalleeOfInterestingCallee&apos;</string>
// CHECK-NEXT: <key>message</key>
// CHECK-NEXT: <string>Entered call from &apos;testBoringCalleeOfInterestingCallee&apos;</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>146</integer>
// CHECK-NEXT: <key>col</key><integer>1</integer>
// CHECK-NEXT: <key>file</key><integer>0</integer>
// CHECK-NEXT: </dict>
// CHECK-NEXT: <dict>
// CHECK-NEXT: <key>line</key><integer>146</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: <key>end</key>
// CHECK-NEXT: <array>
// CHECK-NEXT: <dict>
// CHECK-NEXT: <key>line</key><integer>147</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>147</integer>
// CHECK-NEXT: <key>col</key><integer>3</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>147</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>147</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>147</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: </array>
// CHECK-NEXT: <key>depth</key><integer>1</integer>
// CHECK-NEXT: <key>extended_message</key>
// CHECK-NEXT: <string>The value 0 is assigned to &apos;x&apos;</string>
// CHECK-NEXT: <key>message</key>
// CHECK-NEXT: <string>The value 0 is assigned to &apos;x&apos;</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>147</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>147</integer>
// CHECK-NEXT: <key>col</key><integer>3</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>148</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>148</integer>
// CHECK-NEXT: <key>col</key><integer>14</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>153</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>153</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>153</integer>
// CHECK-NEXT: <key>col</key><integer>23</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>Returning from &apos;interestingCallee&apos;</string>
// CHECK-NEXT: <key>message</key>
// CHECK-NEXT: <string>Returning from &apos;interestingCallee&apos;</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>153</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>153</integer>
// CHECK-NEXT: <key>col</key><integer>19</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>155</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>155</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: </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>155</integer>
// CHECK-NEXT: <key>col</key><integer>12</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>155</integer>
// CHECK-NEXT: <key>col</key><integer>10</integer>
// CHECK-NEXT: <key>file</key><integer>0</integer>
// CHECK-NEXT: </dict>
// CHECK-NEXT: <dict>
// CHECK-NEXT: <key>line</key><integer>155</integer>
// CHECK-NEXT: <key>col</key><integer>14</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>Division by zero</string>
// CHECK-NEXT: <key>message</key>
// CHECK-NEXT: <string>Division by zero</string>
// CHECK-NEXT: </dict>
// CHECK-NEXT: </array>
// CHECK-NEXT: <key>description</key><string>Division by zero</string>
// CHECK-NEXT: <key>category</key><string>Logic error</string>
// CHECK-NEXT: <key>type</key><string>Division by zero</string>
// CHECK-NEXT: <key>check_name</key><string>core.DivideZero</string>
// CHECK-NEXT: <!-- This hash is experimental and going to change! -->
// CHECK-NEXT: <key>issue_hash_content_of_line_in_context</key><string>fcd480c0f73d071bac6f908387893e26</string>
// CHECK-NEXT: <key>issue_context_kind</key><string>function</string>
// CHECK-NEXT: <key>issue_context</key><string>testBoringCalleeOfInterestingCallee</string>
// CHECK-NEXT: <key>issue_hash_function_offset</key><string>4</string>
// CHECK-NEXT: <key>location</key>
// CHECK-NEXT: <dict>
// CHECK-NEXT: <key>line</key><integer>155</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: </array> // CHECK-NEXT: </array>