NoQ created this revision.
NoQ added reviewers: zaks.anna, dcoughlin, xazax.hun, a.sidorin, teemperor,
v.g.vassilev.
NoQ added a subscriber: cfe-commits.
This patch allows injecting extra note-like diagnostics into bug reports, which
are separate from path diagnostics. Previously, we could only display
single-piece warnings and auto-generated path diagnostics; the latter could
have been extended with the means of bug reporter visitors, however diagnostics
injected by the visitors were still tied to the execution paths.
The primary purpose of this patch is to get the Clone checker integrated into
the clang static analyzer's bug reporter mechanism. It is absolutely vital to
this checker to display a warning that refers to multiple source ranges (code
clones). This patch properly integrates the Clone checker, so that its warnings
were visible in the analyzer GUIs such as scan-view.
Other checkers, even path-sensitive ones, could also benefit from the ability
to add extra diagnostic pieces that are separate from path events. For example,
if a bug report refers to a declaration of a class member, it is a good idea to
highlight the relevant declaration, which may be far away from the execution
path. This patch applies the new technique to the ObjCDealloc checker. Here we
also see how the new extra notes interact with other path pieces.
HTML diagnostic consumer (`-analyzer-output=html`) is updated to use the new
technique. Extra notes appear as blue planks (unlike the yellow path event
planks and grey control flow planks), they are not numbered, and cannot be
navigated through arrows, but they are included in the header. This represents
the idea of how extra notes should ideally look like. The color is, of course,
is chosen arbitrarily. Here are some examples of the newly generated HTML
reports: {F2409277} {F2409279} {F2409280} {F2409281}
Null diagnostic consumer (with `-analyzer-output` unspecified) displays extra
notes as "note:" diagnostics. It makes testing extra note pieces easier. Note
that normal path notes are not displayed at this mode, so i'm not absolutely
sure this is the right decision; however, this lets clone checker tests work
without changes.
Text diagnostic consumer (with `-analyzer-output=text`) displays extra notes as
"note:" diagnostics before the path diagnostics.
Plist diagnostic consumer was not updated yet(!) Currently, it ignores extra
notes. This is because it is important to agree upon the format, so this
decision will be delayed. Feedback on this matter is welcome :)
An option is added, `-analyzer-config extra-notes-as-events=true` (defaults to
`false`), which converts extra notes to path events before sending to any
diagnostic consumer. This allows to see the newly added notes in the Plist
diagnostic consumer and display them without any changes to the viewer
application (such as Xcode), however the looks would probably not be ideal or
user-friendly (see also "HTML diagnostic consumer" above). The extra notes are
put at the beginning, before any path events.
I slightly fixed the clone checker diagnostics. In particular, diagnostics
cannot end with a period, and i capitalized the first letter. Also made slight
changes to the suspicious clone structure, because path pieces require more
than source ranges to operate (didn't look why though).
This patch includes a minor refactoring in `FlushReport()`, which introduces
the `BugReport::isPathSensitive()` method and uses it. This should have no
functional change, however it adds a stronger assertion, and, most importantly,
this refactoring was useful for me to understand this piece of code and realize
that i'm putting my code in the right place, so i decided that i should keep it.
https://reviews.llvm.org/D24278
Files:
include/clang/Analysis/CloneDetection.h
include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
lib/Analysis/CloneDetection.cpp
lib/Rewrite/HTMLRewrite.cpp
lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
lib/StaticAnalyzer/Checkers/CloneChecker.cpp
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
lib/StaticAnalyzer/Core/BugReporter.cpp
lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
lib/StaticAnalyzer/Core/PathDiagnostic.cpp
lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
test/Analysis/DeallocMissingRelease.m
test/Analysis/PR2978.m
test/Analysis/copypaste/blocks.cpp
test/Analysis/copypaste/function-try-block.cpp
test/Analysis/copypaste/functions.cpp
test/Analysis/copypaste/macro-complexity.cpp
test/Analysis/copypaste/macros.cpp
test/Analysis/copypaste/objc-methods.m
test/Analysis/copypaste/plist-diagnostics-extra-notes-as-events.cpp
test/Analysis/copypaste/plist-diagnostics.cpp
test/Analysis/copypaste/sub-sequences.cpp
test/Analysis/copypaste/suspicious-clones.cpp
test/Analysis/copypaste/text-diagnostics.cpp
test/Analysis/properties.m
Index: test/Analysis/properties.m
===================================================================
--- test/Analysis/properties.m
+++ test/Analysis/properties.m
@@ -134,11 +134,17 @@
NSString *_name;
}
@property (retain) NSString * name;
+#if !__has_feature(objc_arc)
+// expected-note@-2 {{The property is declared here}}
+#endif
@property (assign) id friend;
@end
@implementation Person
@synthesize name = _name;
+#if !__has_feature(objc_arc)
+// expected-note@-2 {{The property is synthesized here}}
+#endif
-(void)dealloc {
#if !__has_feature(objc_arc)
Index: test/Analysis/copypaste/text-diagnostics.cpp
===================================================================
--- /dev/null
+++ test/Analysis/copypaste/text-diagnostics.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -analyze -analyzer-output=text -std=c++11 -analyzer-checker=alpha.clone.CloneChecker -verify %s
+
+void log();
+
+int max(int a, int b) { // expected-warning{{Detected code clone}} // expected-note{{Detected code clone}}
+ log();
+ if (a > b)
+ return a;
+ return b;
+}
+
+int maxClone(int a, int b) { // expected-note{{Related code clone is here}}
+ log();
+ if (a > b)
+ return a;
+ return b;
+}
Index: test/Analysis/copypaste/suspicious-clones.cpp
===================================================================
--- test/Analysis/copypaste/suspicious-clones.cpp
+++ test/Analysis/copypaste/suspicious-clones.cpp
@@ -8,14 +8,14 @@
log();
if (a > b)
return a;
- return b; // expected-note{{suggestion is based on the usage of this variable in a similar piece of code}}
+ return b; // expected-note{{Suggestion is based on the usage of this variable in a similar piece of code}}
}
int maxClone(int x, int y, int z) {
log();
if (x > y)
return x;
- return z; // expected-warning{{suspicious code clone detected; did you mean to use 'y'?}}
+ return z; // expected-warning{{Suspicious code clone detected; did you mean to use 'y'?}}
}
// Tests finding a suspicious clone that references global variables.
@@ -33,7 +33,7 @@
while (true) {
if (m1.try_lock()) {
++i;
- m1.unlock(); // expected-note{{suggestion is based on the usage of this variable in a similar piece of code}}
+ m1.unlock(); // expected-note{{Suggestion is based on the usage of this variable in a similar piece of code}}
if (i > 1000) {
return;
}
@@ -45,7 +45,7 @@
while (true) {
if (m1.try_lock()) {
++i;
- m2.unlock(); // expected-warning{{suspicious code clone detected; did you mean to use 'm1'?}}
+ m2.unlock(); // expected-warning{{Suspicious code clone detected; did you mean to use 'm1'?}}
if (i > 1000) {
return;
}
@@ -58,14 +58,14 @@
int foo(int a, int b, int c) {
a += b + c;
b /= a + b;
- c -= b * a; // expected-warning{{suspicious code clone detected; did you mean to use 'a'?}}
+ c -= b * a; // expected-warning{{Suspicious code clone detected; did you mean to use 'a'?}}
return c;
}
int fooClone(int a, int b, int c) {
a += b + c;
b /= a + b;
- c -= a * a; // expected-note{{suggestion is based on the usage of this variable in a similar piece of code; did you mean to use 'b'?}}
+ c -= a * a; // expected-note{{Suggestion is based on the usage of this variable in a similar piece of code; did you mean to use 'b'?}}
return c;
}
@@ -77,21 +77,21 @@
long bar1(long a, long b, long c, long d) {
c = a - b;
c = c / d * a;
- d = b * b - c; // expected-warning{{suspicious code clone detected; did you mean to use 'c'?}}
+ d = b * b - c; // expected-warning{{Suspicious code clone detected; did you mean to use 'c'?}}
return d;
}
long bar2(long a, long b, long c, long d) {
c = a - b;
c = c / d * a;
- d = c * b - c; // expected-note{{suggestion is based on the usage of this variable in a similar piece of code; did you mean to use 'b'?}} \
- // expected-warning{{suspicious code clone detected; did you mean to use 'a'?}}
+ d = c * b - c; // expected-note{{Suggestion is based on the usage of this variable in a similar piece of code; did you mean to use 'b'?}} \
+ // expected-warning{{Suspicious code clone detected; did you mean to use 'a'?}}
return d;
}
long bar3(long a, long b, long c, long d) {
c = a - b;
c = c / d * a;
- d = a * b - c; // expected-note{{suggestion is based on the usage of this variable in a similar piece of code; did you mean to use 'c'?}}
+ d = a * b - c; // expected-note{{Suggestion is based on the usage of this variable in a similar piece of code; did you mean to use 'c'?}}
return d;
}
Index: test/Analysis/copypaste/sub-sequences.cpp
===================================================================
--- test/Analysis/copypaste/sub-sequences.cpp
+++ test/Analysis/copypaste/sub-sequences.cpp
@@ -7,14 +7,14 @@
int max(int a, int b) {
log2(a);
- log(); // expected-warning{{Detected code clone.}}
+ log(); // expected-warning{{Detected code clone}}
if (a > b)
return a;
return b;
}
int maxClone(int a, int b) {
- log(); // expected-note{{Related code clone is here.}}
+ log(); // expected-note{{Related code clone is here}}
if (a > b)
return a;
return b;
Index: test/Analysis/copypaste/plist-diagnostics.cpp
===================================================================
--- /dev/null
+++ test/Analysis/copypaste/plist-diagnostics.cpp
@@ -0,0 +1,68 @@
+// RUN: %clang_cc1 -analyze -analyzer-output=plist -o %t.plist -std=c++11 -analyzer-checker=alpha.clone.CloneChecker -verify %s
+// RUN: FileCheck --input-file=%t.plist %s
+
+void log();
+
+int max(int a, int b) { // expected-warning{{Detected code clone}}
+ log();
+ if (a > b)
+ return a;
+ return b;
+}
+
+int maxClone(int a, int b) { // expected-note{{Related code clone is here}}
+ log();
+ if (a > b)
+ return a;
+ return b;
+}
+
+// CHECK: <key>diagnostics</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>path</key>
+// CHECK-NEXT: <array>
+// 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>6</integer>
+// CHECK-NEXT: <key>col</key><integer>23</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>6</integer>
+// CHECK-NEXT: <key>col</key><integer>23</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>11</integer>
+// CHECK-NEXT: <key>col</key><integer>1</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>Detected code clone</string>
+// CHECK-NEXT: <key>message</key>
+// CHECK-NEXT: <string>Detected code clone</string>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: <key>description</key><string>Detected code clone</string>
+// CHECK-NEXT: <key>category</key><string>Code clone</string>
+// CHECK-NEXT: <key>type</key><string>Exact code clone</string>
+// CHECK-NEXT: <key>check_name</key><string>alpha.clone.CloneChecker</string>
+// CHECK-NEXT: <!-- This hash is experimental and going to change! -->
+// CHECK-NEXT: <key>issue_hash_content_of_line_in_context</key><string>3d15184f38c5fa57e479b744fe3f5035</string>
+// CHECK-NEXT: <key>location</key>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>6</integer>
+// CHECK-NEXT: <key>col</key><integer>23</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
Index: test/Analysis/copypaste/plist-diagnostics-extra-notes-as-events.cpp
===================================================================
--- /dev/null
+++ test/Analysis/copypaste/plist-diagnostics-extra-notes-as-events.cpp
@@ -0,0 +1,97 @@
+// RUN: %clang_cc1 -analyze -analyzer-output=plist -analyzer-config extra-notes-as-events=true -o %t.plist -std=c++11 -analyzer-checker=alpha.clone.CloneChecker -verify %s
+// RUN: FileCheck --input-file=%t.plist %s
+
+void log();
+
+int max(int a, int b) { // expected-warning{{Detected code clone}}
+ log();
+ if (a > b)
+ return a;
+ return b;
+}
+
+int maxClone(int a, int b) { // no-note (converted into event)
+ log();
+ if (a > b)
+ return a;
+ return b;
+}
+
+// CHECK: <key>diagnostics</key>
+// CHECK-NEXT: <array>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>path</key>
+// CHECK-NEXT: <array>
+// 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>13</integer>
+// CHECK-NEXT: <key>col</key><integer>28</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>13</integer>
+// CHECK-NEXT: <key>col</key><integer>28</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>18</integer>
+// CHECK-NEXT: <key>col</key><integer>1</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>Related code clone is here</string>
+// CHECK-NEXT: <key>message</key>
+// CHECK-NEXT: <string>Related code clone is here</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>6</integer>
+// CHECK-NEXT: <key>col</key><integer>23</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>6</integer>
+// CHECK-NEXT: <key>col</key><integer>23</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>11</integer>
+// CHECK-NEXT: <key>col</key><integer>1</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>Detected code clone</string>
+// CHECK-NEXT: <key>message</key>
+// CHECK-NEXT: <string>Detected code clone</string>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
+// CHECK-NEXT: <key>description</key><string>Detected code clone</string>
+// CHECK-NEXT: <key>category</key><string>Code clone</string>
+// CHECK-NEXT: <key>type</key><string>Exact code clone</string>
+// CHECK-NEXT: <key>check_name</key><string>alpha.clone.CloneChecker</string>
+// CHECK-NEXT: <!-- This hash is experimental and going to change! -->
+// CHECK-NEXT: <key>issue_hash_content_of_line_in_context</key><string>3d15184f38c5fa57e479b744fe3f5035</string>
+// CHECK-NEXT: <key>location</key>
+// CHECK-NEXT: <dict>
+// CHECK-NEXT: <key>line</key><integer>6</integer>
+// CHECK-NEXT: <key>col</key><integer>23</integer>
+// CHECK-NEXT: <key>file</key><integer>0</integer>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </dict>
+// CHECK-NEXT: </array>
Index: test/Analysis/copypaste/objc-methods.m
===================================================================
--- test/Analysis/copypaste/objc-methods.m
+++ test/Analysis/copypaste/objc-methods.m
@@ -7,7 +7,7 @@
@end
@implementation A
-- (int) setOk : (int) a : (int) b { // expected-warning{{Detected code clone.}}
+- (int) setOk : (int) a : (int) b { // expected-warning{{Detected code clone}}
if (a > b)
return a;
return b;
@@ -19,7 +19,7 @@
@end
@implementation B
-- (int) setOk : (int) a : (int) b { // expected-note{{Related code clone is here.}}
+- (int) setOk : (int) a : (int) b { // expected-note{{Related code clone is here}}
if (a > b)
return a;
return b;
Index: test/Analysis/copypaste/macros.cpp
===================================================================
--- test/Analysis/copypaste/macros.cpp
+++ test/Analysis/copypaste/macros.cpp
@@ -5,7 +5,7 @@
// to have the same complexity value. Macros have smaller complexity values
// and need to be in their own hash group.
-int foo(int a) { // expected-warning{{Detected code clone.}}
+int foo(int a) { // expected-warning{{Detected code clone}}
a = a + 1;
a = a + 1 / 1;
a = a + 1 + 1 + 1;
@@ -15,7 +15,7 @@
return a;
}
-int fooClone(int a) { // expected-note{{Related code clone is here.}}
+int fooClone(int a) { // expected-note{{Related code clone is here}}
a = a + 1;
a = a + 1 / 1;
a = a + 1 + 1 + 1;
@@ -30,7 +30,7 @@
#define ASSIGN(T, V) T = T + V
-int macro(int a) { // expected-warning{{Detected code clone.}}
+int macro(int a) { // expected-warning{{Detected code clone}}
ASSIGN(a, 1);
ASSIGN(a, 1 / 1);
ASSIGN(a, 1 + 1 + 1);
@@ -40,7 +40,7 @@
return a;
}
-int macroClone(int a) { // expected-note{{Related code clone is here.}}
+int macroClone(int a) { // expected-note{{Related code clone is here}}
ASSIGN(a, 1);
ASSIGN(a, 1 / 1);
ASSIGN(a, 1 + 1 + 1);
@@ -54,7 +54,7 @@
#define EMPTY
-int fooFalsePositiveClone(int a) { // expected-note{{Related code clone is here.}}
+int fooFalsePositiveClone(int a) { // expected-note{{Related code clone is here}}
a = EMPTY a + 1;
a = a + 1 / 1;
a = a + 1 + 1 + 1;
Index: test/Analysis/copypaste/macro-complexity.cpp
===================================================================
--- test/Analysis/copypaste/macro-complexity.cpp
+++ test/Analysis/copypaste/macro-complexity.cpp
@@ -11,11 +11,11 @@
// This confirms that with the current configuration the macro body would be
// considered large enough to pass the MinimumCloneComplexity constraint.
-int manualMacro(int a, int b) { // expected-warning{{Detected code clone.}}
+int manualMacro(int a, int b) { // expected-warning{{Detected code clone}}
return a > b ? -a * a : -b * b;
}
-int manualMacroClone(int a, int b) { // expected-note{{Related code clone is here.}}
+int manualMacroClone(int a, int b) { // expected-note{{Related code clone is here}}
return a > b ? -a * a : -b * b;
}
@@ -41,10 +41,10 @@
#define NEG(A) -(A)
-int nestedMacros() { // expected-warning{{Detected code clone.}}
+int nestedMacros() { // expected-warning{{Detected code clone}}
return NEG(NEG(NEG(NEG(NEG(NEG(NEG(NEG(NEG(NEG(1))))))))));
}
-int nestedMacrosClone() { // expected-note{{Related code clone is here.}}
+int nestedMacrosClone() { // expected-note{{Related code clone is here}}
return NEG(NEG(NEG(NEG(NEG(NEG(NEG(NEG(NEG(NEG(1))))))))));
}
Index: test/Analysis/copypaste/functions.cpp
===================================================================
--- test/Analysis/copypaste/functions.cpp
+++ test/Analysis/copypaste/functions.cpp
@@ -4,14 +4,14 @@
void log();
-int max(int a, int b) { // expected-warning{{Detected code clone.}}
+int max(int a, int b) { // expected-warning{{Detected code clone}}
log();
if (a > b)
return a;
return b;
}
-int maxClone(int x, int y) { // expected-note{{Related code clone is here.}}
+int maxClone(int x, int y) { // expected-note{{Related code clone is here}}
log();
if (x > y)
return x;
Index: test/Analysis/copypaste/function-try-block.cpp
===================================================================
--- test/Analysis/copypaste/function-try-block.cpp
+++ test/Analysis/copypaste/function-try-block.cpp
@@ -3,7 +3,7 @@
// Tests if function try blocks are correctly handled.
void nonCompoundStmt1(int& x)
- try { x += 1; } catch(...) { x -= 1; } // expected-warning{{Detected code clone.}}
+ try { x += 1; } catch(...) { x -= 1; } // expected-warning{{Detected code clone}}
void nonCompoundStmt2(int& x)
- try { x += 1; } catch(...) { x -= 1; } // expected-note{{Related code clone is here.}}
+ try { x += 1; } catch(...) { x -= 1; } // expected-note{{Related code clone is here}}
Index: test/Analysis/copypaste/blocks.cpp
===================================================================
--- test/Analysis/copypaste/blocks.cpp
+++ test/Analysis/copypaste/blocks.cpp
@@ -4,14 +4,14 @@
void log();
-auto BlockA = ^(int a, int b){ // expected-warning{{Detected code clone.}}
+auto BlockA = ^(int a, int b){ // expected-warning{{Detected code clone}}
log();
if (a > b)
return a;
return b;
};
-auto BlockB = ^(int a, int b){ // expected-note{{Related code clone is here.}}
+auto BlockB = ^(int a, int b){ // expected-note{{Related code clone is here}}
log();
if (a > b)
return a;
Index: test/Analysis/PR2978.m
===================================================================
--- test/Analysis/PR2978.m
+++ test/Analysis/PR2978.m
@@ -29,22 +29,22 @@
id _nonPropertyIvar;
}
@property(retain) id X;
-@property(retain) id Y;
-@property(assign) id Z;
+@property(retain) id Y; // expected-note{{The property is declared here}}
+@property(assign) id Z; // expected-note{{The property is declared here}}
@property(assign) id K;
@property(weak) id L;
@property(readonly) id N;
@property(retain) id M;
@property(weak) id P;
-@property(weak) id Q;
+@property(weak) id Q; // expected-note{{The property is declared here}}
@property(retain) id R;
-@property(weak, readonly) id S;
+@property(weak, readonly) id S; // expected-note{{The property is declared here}}
@property(assign, readonly) id T; // Shadowed in class extension
@property(assign) id U;
@property(retain) id V;
-@property(retain) id W;
+@property(retain) id W; // expected-note{{The property is declared here}}
-(id) O;
-(void) setO: (id) arg;
@end
@@ -56,16 +56,16 @@
@implementation MyClass
@synthesize X = _X;
-@synthesize Y = _Y;
-@synthesize Z = _Z;
+@synthesize Y = _Y; // expected-note{{The property is synthesized here}}
+@synthesize Z = _Z; // expected-note{{The property is synthesized here}}
@synthesize K = _K;
@synthesize L = _L;
@synthesize N = _N;
@synthesize M = _M;
-@synthesize Q = _Q;
+@synthesize Q = _Q; // expected-note{{The property is synthesized here}}
@synthesize R = _R;
@synthesize V = _V;
-@synthesize W = _W;
+@synthesize W = _W; // expected-note{{The property is synthesized here}}
-(id) O{ return 0; }
-(void) setO:(id)arg { }
Index: test/Analysis/DeallocMissingRelease.m
===================================================================
--- test/Analysis/DeallocMissingRelease.m
+++ test/Analysis/DeallocMissingRelease.m
@@ -80,6 +80,9 @@
@interface MyPropertyClass1 : NSObject
@property (copy) NSObject *ivar;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
@end
@implementation MyPropertyClass1
@@ -93,6 +96,9 @@
@interface MyPropertyClass2 : NSObject
@property (retain) NSObject *ivar;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
@end
@implementation MyPropertyClass2
@@ -108,10 +114,16 @@
NSObject *_ivar;
}
@property (retain) NSObject *ivar;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
@end
@implementation MyPropertyClass3
@synthesize ivar = _ivar;
+#if NON_ARC
+// expected-note@-2 {{The property is synthesized here}}
+#endif
- (void)dealloc
{
#if NON_ARC
@@ -125,13 +137,19 @@
void (^_blockPropertyIvar)(void);
}
@property (copy) void (^blockProperty)(void);
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
@property (copy) void (^blockProperty2)(void);
@property (copy) void (^blockProperty3)(void);
@end
@implementation MyPropertyClass4
@synthesize blockProperty = _blockPropertyIvar;
+#if NON_ARC
+// expected-note@-2 {{The property is synthesized here}}
+#endif
- (void)dealloc
{
#if NON_ARC
@@ -163,10 +181,16 @@
NSObject *_ivar;
}
@property (retain) NSObject *ivar;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
@end
@implementation MyPropertyClassWithReturnInDealloc
@synthesize ivar = _ivar;
+#if NON_ARC
+// expected-note@-2 {{The property is synthesized here}}
+#endif
- (void)dealloc
{
return;
@@ -182,12 +206,18 @@
MyPropertyClassWithReleaseInOtherInstance *_other;
}
@property (retain) NSObject *ivar;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
-(void)releaseIvars;
@end
@implementation MyPropertyClassWithReleaseInOtherInstance
@synthesize ivar = _ivar;
+#if NON_ARC
+// expected-note@-2 {{The property is synthesized here}}
+#endif
-(void)releaseIvars; {
#if NON_ARC
@@ -208,10 +238,16 @@
NSObject *_ivar;
}
@property (retain) NSObject *ivar;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
@end
@implementation MyPropertyClassWithNeitherReturnNorSuperDealloc
@synthesize ivar = _ivar;
+#if NON_ARC
+// expected-note@-2 {{The property is synthesized here}}
+#endif
- (void)dealloc
{
}
@@ -246,6 +282,9 @@
BOOL _ivar1;
}
@property (retain) NSObject *ivar2;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
@end
@implementation ClassWithControlFlowInRelease
@@ -287,6 +326,9 @@
@interface ClassWithNildOutIvar : NSObject
@property (retain) NSObject *ivar;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
@end
@implementation ClassWithNildOutIvar
@@ -305,6 +347,9 @@
@interface ClassWithUpdatedIvar : NSObject
@property (retain) NSObject *ivar;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
@end
@implementation ClassWithUpdatedIvar
@@ -349,6 +394,9 @@
@property (retain) NSObject *propNilledOutInFunction;
@property (retain) NSObject *ivarNeverReleased;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
- (void)invalidateInMethod;
@end
@@ -425,6 +473,9 @@
@interface ClassWhereSelfEscapesViaSynthesizedPropertyAccess : NSObject
@property (retain) NSObject *ivar;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
@property (retain) NSObject *otherIvar;
@end
@@ -442,6 +493,9 @@
@interface ClassWhereSelfEscapesViaCallToSystem : NSObject
@property (retain) NSObject *ivar1;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
@property (retain) NSObject *ivar2;
@property (retain) NSObject *ivar3;
@property (retain) NSObject *ivar4;
@@ -536,6 +590,9 @@
@interface SuperClassOfClassWithInlinedSuperDealloc : NSObject
@property (retain) NSObject *propInSuper;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
@end
@implementation SuperClassOfClassWithInlinedSuperDealloc
@@ -548,6 +605,9 @@
@interface ClassWithInlinedSuperDealloc : SuperClassOfClassWithInlinedSuperDealloc
@property (retain) NSObject *propInSub;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
@end
@implementation ClassWithInlinedSuperDealloc
@@ -605,6 +665,9 @@
@interface SuperClassOfClassThatEscapesBeforeInliningSuper : NSObject
@property (retain) NSObject *propInSuper;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
@end
@implementation SuperClassOfClassThatEscapesBeforeInliningSuper
@@ -794,6 +857,9 @@
@property(retain) NSObject *inputIvar;
@property(retain) NSObject *nonInputIvar;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
@property(retain) NSObject *inputAutoSynthesizedIvar;
@property(retain) NSObject *inputExplicitlySynthesizedToNonPrefixedIvar;
@property(retain) NSObject *nonPrefixedPropertyBackedByExplicitlySynthesizedPrefixedIvar;
@@ -803,6 +869,9 @@
@implementation ImmediateSubCIFilter
@synthesize inputIvar = inputIvar;
@synthesize nonInputIvar = nonInputIvar;
+#if NON_ARC
+// expected-note@-2 {{The property is synthesized here}}
+#endif
@synthesize inputExplicitlySynthesizedToNonPrefixedIvar = notPrefixedButBackingPrefixedProperty;
@synthesize nonPrefixedPropertyBackedByExplicitlySynthesizedPrefixedIvar = inputPrefixedButBackingNonPrefixedProperty;
@@ -841,6 +910,9 @@
}
@property(retain) NSObject *inputIvar;
+#if NON_ARC
+// expected-note@-2 {{The property is declared here}}
+#endif
@end
@implementation OverreleasingCIFilter
Index: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===================================================================
--- lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -113,13 +113,27 @@
Diag.Report(WarnLoc, WarnID) << PD->getShortDescription()
<< PD->path.back()->getRanges();
+ // First, add extra notes, even if paths should not be included.
+ for (PathPieces::const_iterator PI = PD->path.begin(),
+ PE = PD->path.end(); PI != PE; ++PI) {
+ if (!isa<PathDiagnosticExtraNotePiece>(PI->get()))
+ continue;
+
+ SourceLocation NoteLoc = (*PI)->getLocation().asLocation();
+ Diag.Report(NoteLoc, NoteID) << (*PI)->getString()
+ << (*PI)->getRanges();
+ }
+
if (!IncludePath)
continue;
+ // Then, add the path notes if necessary.
PathPieces FlatPath = PD->path.flatten(/*ShouldFlattenMacros=*/true);
for (PathPieces::const_iterator PI = FlatPath.begin(),
- PE = FlatPath.end();
- PI != PE; ++PI) {
+ PE = FlatPath.end(); PI != PE; ++PI) {
+ if (isa<PathDiagnosticExtraNotePiece>(PI->get()))
+ continue;
+
SourceLocation NoteLoc = (*PI)->getLocation().asLocation();
Diag.Report(NoteLoc, NoteID) << (*PI)->getString()
<< (*PI)->getRanges();
Index: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===================================================================
--- lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -281,6 +281,9 @@
ReportMacro(o, cast<PathDiagnosticMacroPiece>(P), FM, SM, LangOpts,
indent, depth);
break;
+ case PathDiagnosticPiece::ExtraNote:
+ // FIXME: Extend the plist format to support those.
+ break;
}
}
Index: lib/StaticAnalyzer/Core/PathDiagnostic.cpp
===================================================================
--- lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -60,6 +60,7 @@
PathDiagnosticCallPiece::~PathDiagnosticCallPiece() {}
PathDiagnosticControlFlowPiece::~PathDiagnosticControlFlowPiece() {}
PathDiagnosticMacroPiece::~PathDiagnosticMacroPiece() {}
+PathDiagnosticExtraNotePiece::~PathDiagnosticExtraNotePiece() {}
void PathPieces::flattenTo(PathPieces &Primary, PathPieces &Current,
bool ShouldFlattenMacros) const {
@@ -95,6 +96,7 @@
}
case PathDiagnosticPiece::Event:
case PathDiagnosticPiece::ControlFlow:
+ case PathDiagnosticPiece::ExtraNote:
Current.push_back(Piece);
break;
}
@@ -342,15 +344,16 @@
}
switch (X.getKind()) {
- case clang::ento::PathDiagnosticPiece::ControlFlow:
+ case PathDiagnosticPiece::ControlFlow:
return compareControlFlow(cast<PathDiagnosticControlFlowPiece>(X),
cast<PathDiagnosticControlFlowPiece>(Y));
- case clang::ento::PathDiagnosticPiece::Event:
+ case PathDiagnosticPiece::Event:
+ case PathDiagnosticPiece::ExtraNote:
return None;
- case clang::ento::PathDiagnosticPiece::Macro:
+ case PathDiagnosticPiece::Macro:
return compareMacro(cast<PathDiagnosticMacroPiece>(X),
cast<PathDiagnosticMacroPiece>(Y));
- case clang::ento::PathDiagnosticPiece::Call:
+ case PathDiagnosticPiece::Call:
return compareCall(cast<PathDiagnosticCallPiece>(X),
cast<PathDiagnosticCallPiece>(Y));
}
@@ -1098,6 +1101,10 @@
ID.Add(**I);
}
+void PathDiagnosticExtraNotePiece::Profile(llvm::FoldingSetNodeID &ID) const {
+ PathDiagnosticSpotPiece::Profile(ID);
+}
+
void PathDiagnostic::Profile(llvm::FoldingSetNodeID &ID) const {
ID.Add(getLocation());
ID.AddString(BugType);
Index: lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===================================================================
--- lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -152,13 +152,29 @@
}
// Process the path.
- unsigned n = path.size();
- unsigned max = n;
-
- for (PathPieces::const_reverse_iterator I = path.rbegin(),
- E = path.rend();
- I != E; ++I, --n)
- HandlePiece(R, FID, **I, n, max);
+ // Maintain the counts of extra note pieces separately.
+ unsigned TotalPieces = path.size();
+ unsigned TotalExtraPieces =
+ std::count_if(path.begin(), path.end(),
+ [](const IntrusiveRefCntPtr<PathDiagnosticPiece> &p) {
+ return isa<PathDiagnosticExtraNotePiece>(p.get());
+ });
+
+ unsigned TotalRegularPieces = TotalPieces - TotalExtraPieces;
+ unsigned NumRegularPieces = TotalRegularPieces;
+ unsigned NumExtraPieces = TotalExtraPieces;
+
+ for (auto I = path.rbegin(), E = path.rend(); I != E; ++I) {
+ if (isa<PathDiagnosticExtraNotePiece>(I->get())) {
+ // Will also make a second pass through those later below,
+ // when the header text is ready.
+ HandlePiece(R, FID, **I, NumExtraPieces, TotalExtraPieces);
+ --NumExtraPieces;
+ } else {
+ HandlePiece(R, FID, **I, NumRegularPieces, TotalRegularPieces);
+ --NumRegularPieces;
+ }
+ }
// Add line numbers, header, footer, etc.
@@ -192,24 +208,38 @@
int ColumnNumber = path.back()->getLocation().asLocation().getExpansionColumnNumber();
// Add the name of the file as an <h1> tag.
-
{
std::string s;
llvm::raw_string_ostream os(s);
os << "<!-- REPORTHEADER -->\n"
- << "<h3>Bug Summary</h3>\n<table class=\"simpletable\">\n"
+ << "<h3>Bug Summary</h3>\n<table class=\"simpletable\">\n"
"<tr><td class=\"rowname\">File:</td><td>"
- << html::EscapeText(DirName)
- << html::EscapeText(Entry->getName())
- << "</td></tr>\n<tr><td class=\"rowname\">Location:</td><td>"
- "<a href=\"#EndPath\">line "
- << LineNumber
- << ", column "
- << ColumnNumber
- << "</a></td></tr>\n"
- "<tr><td class=\"rowname\">Description:</td><td>"
- << D.getVerboseDescription() << "</td></tr>\n";
+ << html::EscapeText(DirName)
+ << html::EscapeText(Entry->getName())
+ << "</td></tr>\n<tr><td class=\"rowname\">Warning:</td><td>"
+ "<a href=\"#EndPath\">line "
+ << LineNumber
+ << ", column "
+ << ColumnNumber
+ << "</a><br />"
+ << D.getVerboseDescription() << "</td></tr>\n";
+
+ // The navigation across the extra notes pieces.
+ unsigned NumExtraPieces = 0;
+ for (auto I = path.begin(), E = path.end(); I != E; ++I) {
+ if (const auto *P = dyn_cast<PathDiagnosticExtraNotePiece>(I->get())) {
+ int LineNumber =
+ P->getLocation().asLocation().getExpansionLineNumber();
+ int ColumnNumber =
+ P->getLocation().asLocation().getExpansionColumnNumber();
+ os << "<tr><td class=\"rowname\">Note:</td><td>"
+ << "<a href=\"#ExtraNote" << NumExtraPieces << "\">line "
+ << LineNumber << ", column " << ColumnNumber << "</a><br />"
+ << P->getString() << "</td></tr>";
+ ++NumExtraPieces;
+ }
+ }
// Output any other meta data.
@@ -385,21 +415,30 @@
// Create the html for the message.
const char *Kind = nullptr;
+ bool IsExtraNote = false;
+ bool SuppressIndex = (max == 1);
switch (P.getKind()) {
case PathDiagnosticPiece::Call:
- llvm_unreachable("Calls should already be handled");
+ llvm_unreachable("Calls and extra notes should already be handled");
case PathDiagnosticPiece::Event: Kind = "Event"; break;
case PathDiagnosticPiece::ControlFlow: Kind = "Control"; break;
// Setting Kind to "Control" is intentional.
case PathDiagnosticPiece::Macro: Kind = "Control"; break;
+ case PathDiagnosticPiece::ExtraNote:
+ Kind = "ExtraNote";
+ IsExtraNote = true;
+ SuppressIndex = true;
+ break;
}
std::string sbuf;
llvm::raw_string_ostream os(sbuf);
os << "\n<tr><td class=\"num\"></td><td class=\"line\"><div id=\"";
- if (num == max)
+ if (IsExtraNote)
+ os << "ExtraNote" << num;
+ else if (num == max)
os << "EndPath";
else
os << "Path" << num;
@@ -461,7 +500,7 @@
os << "\">";
- if (max > 1) {
+ if (!SuppressIndex) {
os << "<table class=\"msgT\"><tr><td valign=\"top\">";
os << "<div class=\"PathIndex";
if (Kind) os << " PathIndex" << Kind;
@@ -501,7 +540,7 @@
os << "':\n";
- if (max > 1) {
+ if (!SuppressIndex) {
os << "</td>";
if (num < max) {
os << "<td><div class=\"PathNav\"><a href=\"#";
@@ -523,7 +562,7 @@
else {
os << html::EscapeText(P.getString());
- if (max > 1) {
+ if (!SuppressIndex) {
os << "</td>";
if (num < max) {
os << "<td><div class=\"PathNav\"><a href=\"#";
Index: lib/StaticAnalyzer/Core/BugReporter.cpp
===================================================================
--- lib/StaticAnalyzer/Core/BugReporter.cpp
+++ lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -112,15 +112,15 @@
path.pop_front();
switch (piece->getKind()) {
- case clang::ento::PathDiagnosticPiece::Call:
+ case PathDiagnosticPiece::Call:
removeRedundantMsgs(cast<PathDiagnosticCallPiece>(piece)->path);
break;
- case clang::ento::PathDiagnosticPiece::Macro:
+ case PathDiagnosticPiece::Macro:
removeRedundantMsgs(cast<PathDiagnosticMacroPiece>(piece)->subPieces);
break;
- case clang::ento::PathDiagnosticPiece::ControlFlow:
+ case PathDiagnosticPiece::ControlFlow:
break;
- case clang::ento::PathDiagnosticPiece::Event: {
+ case PathDiagnosticPiece::Event: {
if (i == N-1)
break;
@@ -140,6 +140,8 @@
}
break;
}
+ case PathDiagnosticPiece::ExtraNote:
+ break;
}
path.push_back(piece);
}
@@ -197,6 +199,9 @@
}
case PathDiagnosticPiece::ControlFlow:
break;
+
+ case PathDiagnosticPiece::ExtraNote:
+ break;
}
pieces.push_back(piece);
@@ -3403,25 +3408,28 @@
exampleReport->getUniqueingLocation(),
exampleReport->getUniqueingDecl()));
- MaxBugClassSize = std::max(bugReports.size(),
- static_cast<size_t>(MaxBugClassSize));
+ if (exampleReport->isPathSensitive()) {
+ // Generate the full path diagnostic, using the generation scheme
+ // specified by the PathDiagnosticConsumer. Note that we have to generate
+ // path diagnostics even for consumers which do not support paths, because
+ // the BugReporterVisitors may mark this bug as a false positive.
+ assert(!bugReports.empty());
+
+ MaxBugClassSize =
+ std::max(bugReports.size(), static_cast<size_t>(MaxBugClassSize));
- // Generate the full path diagnostic, using the generation scheme
- // specified by the PathDiagnosticConsumer. Note that we have to generate
- // path diagnostics even for consumers which do not support paths, because
- // the BugReporterVisitors may mark this bug as a false positive.
- if (!bugReports.empty())
if (!generatePathDiagnostic(*D.get(), PD, bugReports))
return;
- MaxValidBugClassSize = std::max(bugReports.size(),
- static_cast<size_t>(MaxValidBugClassSize));
+ MaxValidBugClassSize =
+ std::max(bugReports.size(), static_cast<size_t>(MaxValidBugClassSize));
- // Examine the report and see if the last piece is in a header. Reset the
- // report location to the last piece in the main source file.
- AnalyzerOptions& Opts = getAnalyzerOptions();
- if (Opts.shouldReportIssuesInMainSourceFile() && !Opts.AnalyzeAll)
- D->resetDiagnosticLocationToMainFile();
+ // Examine the report and see if the last piece is in a header. Reset the
+ // report location to the last piece in the main source file.
+ AnalyzerOptions &Opts = getAnalyzerOptions();
+ if (Opts.shouldReportIssuesInMainSourceFile() && !Opts.AnalyzeAll)
+ D->resetDiagnosticLocationToMainFile();
+ }
// If the path is empty, generate a single step path with the location
// of the issue.
@@ -3434,6 +3442,27 @@
D->setEndOfPath(std::move(piece));
}
+ PathPieces &Pieces = D->getMutablePieces();
+ if (getAnalyzerOptions().shouldDisplayExtraNotesAsEvents()) {
+ // For path diagnostic consumers that don't support extra notes,
+ // we may optionally convert those to path notes.
+ for (auto I = exampleReport->getExtraNotes().rbegin(),
+ E = exampleReport->getExtraNotes().rend(); I != E; ++I) {
+ PathDiagnosticExtraNotePiece *Piece = I->get();
+ PathDiagnosticEventPiece *ConvertedPiece =
+ new PathDiagnosticEventPiece(Piece->getLocation(),
+ Piece->getString());
+ for (const auto &R: Piece->getRanges())
+ ConvertedPiece->addRange(R);
+
+ Pieces.push_front(ConvertedPiece);
+ }
+ } else {
+ for (auto I = exampleReport->getExtraNotes().rbegin(),
+ E = exampleReport->getExtraNotes().rend(); I != E; ++I)
+ Pieces.push_front(*I);
+ }
+
// Get the meta data.
const BugReport::ExtraTextList &Meta = exampleReport->getExtraText();
for (BugReport::ExtraTextList::const_iterator i = Meta.begin(),
@@ -3518,6 +3547,13 @@
// FIXME: Print which macro is being invoked.
}
+LLVM_DUMP_METHOD void PathDiagnosticExtraNotePiece::dump() const {
+ llvm::errs() << "EXTRANOTE\n--------------\n";
+ llvm::errs() << getString() << "\n";
+ llvm::errs() << " ---- at ----\n";
+ getLocation().dump();
+}
+
LLVM_DUMP_METHOD void PathDiagnosticLocation::dump() const {
if (!isValid()) {
llvm::errs() << "<INVALID>\n";
Index: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===================================================================
--- lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -344,3 +344,11 @@
WidenLoops = getBooleanOption("widen-loops", /*Default=*/false);
return WidenLoops.getValue();
}
+
+
+bool AnalyzerOptions::shouldDisplayExtraNotesAsEvents() {
+ if (!DisplayExtraNotesAsEvents.hasValue())
+ DisplayExtraNotesAsEvents =
+ getBooleanOption("extra-notes-as-events", /*Default=*/false);
+ return DisplayExtraNotesAsEvents.getValue();
+}
Index: lib/StaticAnalyzer/Checkers/CloneChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/CloneChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CloneChecker.cpp
@@ -16,8 +16,10 @@
#include "ClangSACheckers.h"
#include "clang/Analysis/CloneDetection.h"
#include "clang/Basic/Diagnostic.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
using namespace clang;
@@ -27,6 +29,7 @@
class CloneChecker
: public Checker<check::ASTCodeBody, check::EndOfTranslationUnit> {
mutable CloneDetector Detector;
+ mutable std::unique_ptr<BugType> BT_Exact, BT_Suspicious;
public:
void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
@@ -36,12 +39,12 @@
AnalysisManager &Mgr, BugReporter &BR) const;
/// \brief Reports all clones to the user.
- void reportClones(SourceManager &SM, AnalysisManager &Mgr,
+ void reportClones(BugReporter &BR, AnalysisManager &Mgr,
int MinComplexity) const;
/// \brief Reports only suspicious clones to the user along with informaton
/// that explain why they are suspicious.
- void reportSuspiciousClones(SourceManager &SM, AnalysisManager &Mgr,
+ void reportSuspiciousClones(BugReporter &BR, AnalysisManager &Mgr,
int MinComplexity) const;
};
} // end anonymous namespace
@@ -70,79 +73,93 @@
"ReportNormalClones", true, this);
if (ReportSuspiciousClones)
- reportSuspiciousClones(BR.getSourceManager(), Mgr, MinComplexity);
+ reportSuspiciousClones(BR, Mgr, MinComplexity);
if (ReportNormalClones)
- reportClones(BR.getSourceManager(), Mgr, MinComplexity);
+ reportClones(BR, Mgr, MinComplexity);
}
-void CloneChecker::reportClones(SourceManager &SM, AnalysisManager &Mgr,
+static PathDiagnosticLocation makeLocation(const StmtSequence &S,
+ AnalysisManager &Mgr) {
+ ASTContext &ACtx = Mgr.getASTContext();
+ return PathDiagnosticLocation::createBegin(
+ S.front(), ACtx.getSourceManager(),
+ Mgr.getAnalysisDeclContext(ACtx.getTranslationUnitDecl()));
+}
+
+void CloneChecker::reportClones(BugReporter &BR, AnalysisManager &Mgr,
int MinComplexity) const {
std::vector<CloneDetector::CloneGroup> CloneGroups;
Detector.findClones(CloneGroups, MinComplexity);
- DiagnosticsEngine &DiagEngine = Mgr.getDiagnostic();
-
- unsigned WarnID = DiagEngine.getCustomDiagID(DiagnosticsEngine::Warning,
- "Detected code clone.");
-
- unsigned NoteID = DiagEngine.getCustomDiagID(DiagnosticsEngine::Note,
- "Related code clone is here.");
+ if (!BT_Exact)
+ BT_Exact.reset(new BugType(this, "Exact code clone", "Code clone"));
for (CloneDetector::CloneGroup &Group : CloneGroups) {
// We group the clones by printing the first as a warning and all others
// as a note.
- DiagEngine.Report(Group.Sequences.front().getStartLoc(), WarnID);
- for (unsigned i = 1; i < Group.Sequences.size(); ++i) {
- DiagEngine.Report(Group.Sequences[i].getStartLoc(), NoteID);
- }
+ auto R = llvm::make_unique<BugReport>(
+ *BT_Exact, "Detected code clone",
+ makeLocation(Group.Sequences.front(), Mgr));
+ R->addRange(Group.Sequences.front().getSourceRange());
+
+ for (unsigned i = 1; i < Group.Sequences.size(); ++i)
+ R->addExtraNote("Related code clone is here",
+ makeLocation(Group.Sequences[i], Mgr),
+ Group.Sequences[i].getSourceRange());
+ BR.emitReport(std::move(R));
}
}
-void CloneChecker::reportSuspiciousClones(SourceManager &SM,
+void CloneChecker::reportSuspiciousClones(BugReporter &BR,
AnalysisManager &Mgr,
int MinComplexity) const {
std::vector<CloneDetector::SuspiciousClonePair> Clones;
Detector.findSuspiciousClones(Clones, MinComplexity);
- DiagnosticsEngine &DiagEngine = Mgr.getDiagnostic();
-
- auto SuspiciousCloneWarning = DiagEngine.getCustomDiagID(
- DiagnosticsEngine::Warning, "suspicious code clone detected; did you "
- "mean to use %0?");
-
- auto RelatedCloneNote = DiagEngine.getCustomDiagID(
- DiagnosticsEngine::Note, "suggestion is based on the usage of this "
- "variable in a similar piece of code");
+ if (!BT_Suspicious)
+ BT_Suspicious.reset(
+ new BugType(this, "Suspicious code clone", "Code clone"));
- auto RelatedSuspiciousCloneNote = DiagEngine.getCustomDiagID(
- DiagnosticsEngine::Note, "suggestion is based on the usage of this "
- "variable in a similar piece of code; did you "
- "mean to use %0?");
+ ASTContext &ACtx = BR.getContext();
+ SourceManager &SM = ACtx.getSourceManager();
+ AnalysisDeclContext *ADC =
+ Mgr.getAnalysisDeclContext(ACtx.getTranslationUnitDecl());
for (CloneDetector::SuspiciousClonePair &Pair : Clones) {
// The first clone always has a suggestion and we report it to the user
// along with the place where the suggestion should be used.
- DiagEngine.Report(Pair.FirstCloneInfo.VarRange.getBegin(),
- SuspiciousCloneWarning)
- << Pair.FirstCloneInfo.VarRange << Pair.FirstCloneInfo.Suggestion;
+ auto R = llvm::make_unique<BugReport>(
+ *BT_Suspicious,
+ "Suspicious code clone detected; did you mean to use '" +
+ Pair.FirstCloneInfo.Suggestion->getNameAsString() + "'?",
+ PathDiagnosticLocation::createBegin(Pair.FirstCloneInfo.Mention, SM,
+ ADC));
+ R->addRange(Pair.FirstCloneInfo.Mention->getSourceRange());
- // The second clone can have a suggestion and if there is one, we report
- // that suggestion to the user.
if (Pair.SecondCloneInfo.Suggestion) {
- DiagEngine.Report(Pair.SecondCloneInfo.VarRange.getBegin(),
- RelatedSuspiciousCloneNote)
- << Pair.SecondCloneInfo.VarRange << Pair.SecondCloneInfo.Suggestion;
- continue;
+ // The second clone can have a suggestion and if there is one, we report
+ // that suggestion to the user.
+ R->addExtraNote("Suggestion is based on the usage of this variable in a "
+ "similar piece of code; did you mean to use '" +
+ Pair.SecondCloneInfo.Suggestion->getNameAsString() +
+ "'?",
+ PathDiagnosticLocation::createBegin(
+ Pair.SecondCloneInfo.Mention, SM, ADC),
+ Pair.SecondCloneInfo.Mention->getSourceRange());
+ } else {
+ // If there isn't a suggestion in the second clone, we only inform the
+ // user where we got the idea that his code could contain an error.
+ R->addExtraNote("Suggestion is based on the usage of this variable in a "
+ "similar piece of code",
+ PathDiagnosticLocation::createBegin(
+ Pair.SecondCloneInfo.Mention, SM, ADC),
+ Pair.SecondCloneInfo.Mention->getSourceRange());
}
- // If there isn't a suggestion in the second clone, we only inform the
- // user where we got the idea that his code could contain an error.
- DiagEngine.Report(Pair.SecondCloneInfo.VarRange.getBegin(),
- RelatedCloneNote)
- << Pair.SecondCloneInfo.VarRange;
+ BR.emitReport(std::move(R));
}
}
Index: lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
+++ lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
@@ -128,6 +128,9 @@
void checkEndFunction(CheckerContext &Ctx) const;
private:
+ void addExtraNoteForDecl(std::unique_ptr<BugReport> &BR, StringRef Msg,
+ const Decl *D) const;
+
void diagnoseMissingReleases(CheckerContext &C) const;
bool diagnoseExtraRelease(SymbolRef ReleasedValue, const ObjCMethodCall &M,
@@ -489,6 +492,18 @@
return State;
}
+/// Add an extra note piece describing a declaration that is important
+/// for understanding the bug report.
+void ObjCDeallocChecker::addExtraNoteForDecl(std::unique_ptr<BugReport> &BR,
+ StringRef Msg,
+ const Decl *D) const {
+ ASTContext &ACtx = D->getASTContext();
+ SourceManager &SM = ACtx.getSourceManager();
+ PathDiagnosticLocation Pos = PathDiagnosticLocation::createBegin(D, SM);
+ if (Pos.isValid() && Pos.asLocation().isValid())
+ BR->addExtraNote(Msg, Pos, D->getSourceRange());
+}
+
/// Report any unreleased instance variables for the current instance being
/// dealloced.
void ObjCDeallocChecker::diagnoseMissingReleases(CheckerContext &C) const {
@@ -586,6 +601,10 @@
std::unique_ptr<BugReport> BR(
new BugReport(*MissingReleaseBugType, OS.str(), ErrNode));
+ addExtraNoteForDecl(BR, "The property is declared here", PropDecl);
+
+ addExtraNoteForDecl(BR, "The property is synthesized here", PropImpl);
+
C.emitReport(std::move(BR));
}
@@ -689,11 +708,12 @@
);
const ObjCImplDecl *Container = getContainingObjCImpl(C.getLocationContext());
- OS << "The '" << *PropImpl->getPropertyIvarDecl()
- << "' ivar in '" << *Container;
+ const ObjCIvarDecl *IvarDecl = PropImpl->getPropertyIvarDecl();
+ OS << "The '" << *IvarDecl << "' ivar in '" << *Container;
+ bool ReleasedByCIFilterDealloc = isReleasedByCIFilterDealloc(PropImpl);
- if (isReleasedByCIFilterDealloc(PropImpl)) {
+ if (ReleasedByCIFilterDealloc) {
OS << "' will be released by '-[CIFilter dealloc]' but also released here";
} else {
OS << "' was synthesized for ";
@@ -710,6 +730,11 @@
new BugReport(*ExtraReleaseBugType, OS.str(), ErrNode));
BR->addRange(M.getOriginExpr()->getSourceRange());
+ addExtraNoteForDecl(BR, "The property is declared here", PropDecl);
+
+ if (!ReleasedByCIFilterDealloc)
+ addExtraNoteForDecl(BR, "The property is synthesized here", PropImpl);
+
C.emitReport(std::move(BR));
return true;
Index: lib/Rewrite/HTMLRewrite.cpp
===================================================================
--- lib/Rewrite/HTMLRewrite.cpp
+++ lib/Rewrite/HTMLRewrite.cpp
@@ -321,6 +321,7 @@
" .msgT { padding:0x; spacing:0x }\n"
" .msgEvent { background-color:#fff8b4; color:#000000 }\n"
" .msgControl { background-color:#bbbbbb; color:#000000 }\n"
+ " .msgExtraNote { background-color:#ddeeff; color:#000000 }\n"
" .mrange { background-color:#dfddf3 }\n"
" .mrange { border-bottom:1px solid #6F9DBE }\n"
" .PathIndex { font-weight: bold; padding:0px 5px; "
@@ -339,8 +340,12 @@
" border-collapse: collapse; border-spacing: 0px;\n"
" }\n"
" td.rowname {\n"
- " text-align:right; font-weight:bold; color:#444444;\n"
- " padding-right:2ex; }\n"
+ " text-align: right;\n"
+ " vertical-align: top;\n"
+ " font-weight: bold;\n"
+ " color:#444444;\n"
+ " padding-right:2ex;\n"
+ " }\n"
"</style>\n</head>\n<body>";
// Generate header
Index: lib/Analysis/CloneDetection.cpp
===================================================================
--- lib/Analysis/CloneDetection.cpp
+++ lib/Analysis/CloneDetection.cpp
@@ -82,6 +82,10 @@
SourceLocation StmtSequence::getEndLoc() const { return back()->getLocEnd(); }
+SourceRange StmtSequence::getSourceRange() const {
+ return SourceRange(getStartLoc(), getEndLoc());
+}
+
namespace {
/// \brief Analyzes the pattern of the referenced variables in a statement.
@@ -91,11 +95,11 @@
struct VariableOccurence {
/// The index of the associated VarDecl in the Variables vector.
size_t KindID;
- /// The source range in the code where the variable was referenced.
- SourceRange Range;
+ /// The statement in the code where the variable was referenced.
+ const Stmt *Mention;
- VariableOccurence(size_t KindID, SourceRange Range)
- : KindID(KindID), Range(Range) {}
+ VariableOccurence(size_t KindID, const Stmt *Mention)
+ : KindID(KindID), Mention(Mention) {}
};
/// All occurences of referenced variables in the order of appearance.
@@ -107,19 +111,19 @@
/// \brief Adds a new variable referenced to this pattern.
/// \param VarDecl The declaration of the variable that is referenced.
/// \param Range The SourceRange where this variable is referenced.
- void addVariableOccurence(const VarDecl *VarDecl, SourceRange Range) {
+ void addVariableOccurence(const VarDecl *VarDecl, const Stmt *Mention) {
// First check if we already reference this variable
for (size_t KindIndex = 0; KindIndex < Variables.size(); ++KindIndex) {
if (Variables[KindIndex] == VarDecl) {
// If yes, add a new occurence that points to the existing entry in
// the Variables vector.
- Occurences.emplace_back(KindIndex, Range);
+ Occurences.emplace_back(KindIndex, Mention);
return;
}
}
// If this variable wasn't already referenced, add it to the list of
// referenced variables and add a occurence that points to this new entry.
- Occurences.emplace_back(Variables.size(), Range);
+ Occurences.emplace_back(Variables.size(), Mention);
Variables.push_back(VarDecl);
}
@@ -134,7 +138,7 @@
// Check if S is a reference to a variable. If yes, add it to the pattern.
if (auto D = dyn_cast<DeclRefExpr>(S)) {
if (auto VD = dyn_cast<VarDecl>(D->getDecl()->getCanonicalDecl()))
- addVariableOccurence(VD, D->getSourceRange());
+ addVariableOccurence(VD, D);
}
// Recursively check all children of the given statement.
@@ -208,7 +212,7 @@
// Store information about the first clone.
FirstMismatch->FirstCloneInfo =
CloneDetector::SuspiciousClonePair::SuspiciousCloneInfo(
- Variables[ThisOccurence.KindID], ThisOccurence.Range,
+ Variables[ThisOccurence.KindID], ThisOccurence.Mention,
FirstSuggestion);
// Same as above but with the other clone. We do this for both clones as
@@ -221,7 +225,7 @@
// Store information about the second clone.
FirstMismatch->SecondCloneInfo =
CloneDetector::SuspiciousClonePair::SuspiciousCloneInfo(
- Variables[ThisOccurence.KindID], OtherOccurence.Range,
+ Variables[ThisOccurence.KindID], OtherOccurence.Mention,
SecondSuggestion);
// SuspiciousClonePair guarantees that the first clone always has a
Index: include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
===================================================================
--- include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
+++ include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
@@ -336,7 +336,7 @@
class PathDiagnosticPiece : public RefCountedBaseVPTR {
public:
- enum Kind { ControlFlow, Event, Macro, Call };
+ enum Kind { ControlFlow, Event, Macro, Call, ExtraNote };
enum DisplayHint { Above, Below };
private:
@@ -452,7 +452,8 @@
void Profile(llvm::FoldingSetNodeID &ID) const override;
static bool classof(const PathDiagnosticPiece *P) {
- return P->getKind() == Event || P->getKind() == Macro;
+ return P->getKind() == Event || P->getKind() == Macro ||
+ P->getKind() == ExtraNote;
}
};
@@ -710,6 +711,23 @@
void Profile(llvm::FoldingSetNodeID &ID) const override;
};
+class PathDiagnosticExtraNotePiece: public PathDiagnosticSpotPiece {
+public:
+ PathDiagnosticExtraNotePiece(const PathDiagnosticLocation &Pos, StringRef S,
+ bool AddPosRange = true)
+ : PathDiagnosticSpotPiece(Pos, S, ExtraNote, AddPosRange) {}
+
+ ~PathDiagnosticExtraNotePiece() override;
+
+ static inline bool classof(const PathDiagnosticPiece *P) {
+ return P->getKind() == ExtraNote;
+ }
+
+ void dump() const override;
+
+ void Profile(llvm::FoldingSetNodeID &ID) const override;
+};
+
/// PathDiagnostic - PathDiagnostic objects represent a single path-sensitive
/// diagnostic. It represents an ordered-collection of PathDiagnosticPieces,
/// each which represent the pieces of the path.
Index: include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
===================================================================
--- include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
+++ include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
@@ -66,6 +66,8 @@
typedef SmallVector<std::unique_ptr<BugReporterVisitor>, 8> VisitorList;
typedef VisitorList::iterator visitor_iterator;
typedef SmallVector<StringRef, 2> ExtraTextList;
+ typedef SmallVector<llvm::IntrusiveRefCntPtr<PathDiagnosticExtraNotePiece>, 4>
+ ExtraNoteList;
protected:
friend class BugReporter;
@@ -82,7 +84,8 @@
const ExplodedNode *ErrorNode;
SmallVector<SourceRange, 4> Ranges;
ExtraTextList ExtraText;
-
+ ExtraNoteList ExtraNotes;
+
typedef llvm::DenseSet<SymbolRef> Symbols;
typedef llvm::DenseSet<const MemRegion *> Regions;
@@ -177,6 +180,18 @@
const BugType& getBugType() const { return BT; }
BugType& getBugType() { return BT; }
+ /// \brief True when the report has an execution path associated with it.
+ ///
+ /// A report is said to be path-sensitive if it was thrown against a
+ /// particular exploded node in the path-sensitive analysis graph.
+ /// Path-sensitive reports have their intermediate path diagnostics
+ /// auto-generated, perhaps with the help of checker-defined visitors,
+ /// and may contain extra notes.
+ /// Path-insensitive reports consist only of a single warning message
+ /// in a specific location, and perhaps extra notes.
+ /// Path-sensitive checkers are allowed to throw path-insensitive reports.
+ bool isPathSensitive() const { return ErrorNode != nullptr; }
+
const ExplodedNode *getErrorNode() const { return ErrorNode; }
StringRef getDescription() const { return Description; }
@@ -245,7 +260,27 @@
void setDeclWithIssue(const Decl *declWithIssue) {
DeclWithIssue = declWithIssue;
}
-
+
+ /// Add new item to the list of additional notes that need to be attached to
+ /// this path-insensitive report. If you want to add extra notes to a
+ /// path-sensitive report, you need to use a BugReporterVisitor because it
+ /// allows you to specify where exactly in the auto-generated path diagnostic
+ /// the extra note should appear.
+ void addExtraNote(StringRef Msg, const PathDiagnosticLocation &Pos,
+ ArrayRef<SourceRange> Ranges = {}) {
+ PathDiagnosticExtraNotePiece *P =
+ new PathDiagnosticExtraNotePiece(Pos, Msg);
+
+ for (const auto &R : Ranges)
+ P->addRange(R);
+
+ ExtraNotes.push_back(P);
+ }
+
+ virtual const ExtraNoteList &getExtraNotes() {
+ return ExtraNotes;
+ }
+
/// \brief This allows for addition of meta data to the diagnostic.
///
/// Currently, only the HTMLDiagnosticClient knows how to display it.
Index: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
===================================================================
--- include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -266,6 +266,9 @@
/// \sa shouldWidenLoops
Optional<bool> WidenLoops;
+ /// \sa shouldDisplayExtraNotesAsEvents
+ Optional<bool> DisplayExtraNotesAsEvents;
+
/// A helper function that retrieves option for a given full-qualified
/// checker name.
/// Options for checkers can be specified via 'analyzer-config' command-line
@@ -534,6 +537,14 @@
/// This is controlled by the 'widen-loops' config option.
bool shouldWidenLoops();
+ /// Returns true if the bug reporter should transparently treat extra note
+ /// diagnostic pieces as event diagnostic pieces. Useful when the diagnostic
+ /// consumer doesn't support the extra note pieces.
+ ///
+ /// This is controlled by the 'extra-notes-as-events' option, which defaults
+ /// to false when unset.
+ bool shouldDisplayExtraNotesAsEvents();
+
public:
AnalyzerOptions() :
AnalysisStoreOpt(RegionStoreModel),
Index: include/clang/Analysis/CloneDetection.h
===================================================================
--- include/clang/Analysis/CloneDetection.h
+++ include/clang/Analysis/CloneDetection.h
@@ -128,6 +128,10 @@
/// This method should only be called on a non-empty StmtSequence object.
SourceLocation getEndLoc() const;
+ /// Returns the source range of the whole sequence - from the beginning
+ /// of the first statement to the end of the last statement.
+ SourceRange getSourceRange() const;
+
bool operator==(const StmtSequence &Other) const {
return std::tie(S, StartIndex, EndIndex) ==
std::tie(Other.S, Other.StartIndex, Other.EndIndex);
@@ -250,14 +254,14 @@
/// The variable which referencing in this clone was against the pattern.
const VarDecl *Variable;
/// Where the variable was referenced.
- SourceRange VarRange;
+ const Stmt *Mention;
/// The variable that should have been referenced to follow the pattern.
/// If Suggestion is a nullptr then it's not possible to fix the pattern
/// by referencing a different variable in this clone.
const VarDecl *Suggestion;
- SuspiciousCloneInfo(const VarDecl *Variable, SourceRange Range,
+ SuspiciousCloneInfo(const VarDecl *Variable, const Stmt *Mention,
const VarDecl *Suggestion)
- : Variable(Variable), VarRange(Range), Suggestion(Suggestion) {}
+ : Variable(Variable), Mention(Mention), Suggestion(Suggestion) {}
SuspiciousCloneInfo() {}
};
/// The first clone in the pair which always has a suggested variable.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
