NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs.
Once https://reviews.llvm.org/D42192 lands, i believe it should be safe to flip
the default value of `-analyzer-config c++-allocator-inlining` to `true` by
default.
My evaluation (on a large mostly internal codebase, including WebKit, more than
a day of real-world time of continuous analysis) shows that around twice as
much false positives are getting fixed (~35) as new false positives added
(~20). Additionally, the reason for newly added false positives is our
increased precision and coverage due to more aggressive inlining, which exposes
other, unrelated problems in the analyzer, which is an expected outcome of
enabling more coverage on codebases that already have high false positive rates
(such as many C++ codebases). Gradually improving C++ support should be
mitigating those. One true positive was lost due to analyzer reaching the
`max-nodes` limit due to more aggressive inlining. I didn't immediately find
any new positives that were definitely true, but some looked fairly suspicious.
The old mode would be available for a while, so it's possible, if necessary, to
turn the new mode off with `-analyzer-config c++-allocator-inlining=false`.
All changes in tests in the patch are FIXMEs that were resolved by enabling the
new mode.
Repository:
rC Clang
https://reviews.llvm.org/D42219
Files:
lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
test/Analysis/NewDelete-custom.cpp
test/Analysis/ctor.mm
test/Analysis/new.cpp
test/Analysis/virtualcall.cpp
Index: test/Analysis/virtualcall.cpp
===================================================================
--- test/Analysis/virtualcall.cpp
+++ test/Analysis/virtualcall.cpp
@@ -262,6 +262,9 @@
//expected-note-re@-2 {{{{^}}Calling default constructor for 'M'}}
#endif
Y *y = new Y;
+#if !PUREONLY
+ //expected-note-re@-2 {{{{^}}Calling default constructor for 'Y'}}
+#endif
delete y;
header::Z z;
#if !PUREONLY
Index: test/Analysis/new.cpp
===================================================================
--- test/Analysis/new.cpp
+++ test/Analysis/new.cpp
@@ -34,7 +34,7 @@
void *y = new (x) int;
clang_analyzer_eval(x == y); // expected-warning{{TRUE}};
- clang_analyzer_eval(*x == 1); // expected-warning{{UNKNOWN}};
+ clang_analyzer_eval(*x == 1); // expected-warning{{TRUE}};
return y;
}
@@ -200,8 +200,7 @@
int n;
new (&n) int;
- // Should warn that n is uninitialized.
- if (n) { // no-warning
+ if (n) { // expected-warning{{Branch condition evaluates to a garbage value}}
return 0;
}
return 1;
Index: test/Analysis/ctor.mm
===================================================================
--- test/Analysis/ctor.mm
+++ test/Analysis/ctor.mm
@@ -572,10 +572,9 @@
}
void testNew() {
- // FIXME: Pending proper implementation of constructors for 'new'.
raw_pair *pp = new raw_pair();
- clang_analyzer_eval(pp->p1 == 0); // expected-warning{{UNKNOWN}}
- clang_analyzer_eval(pp->p2 == 0); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(pp->p1 == 0); // expected-warning{{TRUE}}
+ clang_analyzer_eval(pp->p2 == 0); // expected-warning{{TRUE}}
}
void testArrayNew() {
@@ -679,8 +678,7 @@
void testDynamic() {
List *list = new List{1, 2};
- // FIXME: When we handle constructors with 'new', this will be TRUE.
- clang_analyzer_eval(list->usedInitializerList); //
expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(list->usedInitializerList); // expected-warning{{TRUE}}
}
}
Index: test/Analysis/NewDelete-custom.cpp
===================================================================
--- test/Analysis/NewDelete-custom.cpp
+++ test/Analysis/NewDelete-custom.cpp
@@ -1,7 +1,7 @@
-// RUN: %clang_analyze_cc1
-analyzer-checker=core,cplusplus.NewDelete,unix.Malloc -std=c++11 -fblocks
-verify %s
-// RUN: %clang_analyze_cc1
-analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks,unix.Malloc
-std=c++11 -DLEAKS=1 -fblocks -verify %s
-// RUN: %clang_analyze_cc1
-analyzer-checker=core,cplusplus.NewDelete,unix.Malloc -std=c++11
-analyzer-config c++-allocator-inlining=true -DALLOCATOR_INLINING=1 -fblocks
-verify %s
-// RUN: %clang_analyze_cc1
-analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks,unix.Malloc
-std=c++11 -analyzer-config c++-allocator-inlining=true -DLEAKS=1
-DALLOCATOR_INLINING=1 -fblocks -verify %s
+// RUN: %clang_analyze_cc1
-analyzer-checker=core,cplusplus.NewDelete,unix.Malloc -std=c++11
-analyzer-config c++-allocator-inlining=false -fblocks -verify %s
+// RUN: %clang_analyze_cc1
-analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks,unix.Malloc
-std=c++11 -analyzer-config c++-allocator-inlining=false -DLEAKS=1 -fblocks
-verify %s
+// RUN: %clang_analyze_cc1
-analyzer-checker=core,cplusplus.NewDelete,unix.Malloc -std=c++11
-DALLOCATOR_INLINING=1 -fblocks -verify %s
+// RUN: %clang_analyze_cc1
-analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks,unix.Malloc
-std=c++11 -DLEAKS=1 -DALLOCATOR_INLINING=1 -fblocks -verify %s
#include "Inputs/system-header-simulator-cxx.h"
#if !(LEAKS && !ALLOCATOR_INLINING)
Index: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===================================================================
--- lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -203,7 +203,7 @@
bool AnalyzerOptions::mayInlineCXXAllocator() {
return getBooleanOption(InlineCXXAllocator,
"c++-allocator-inlining",
- /*Default=*/false);
+ /*Default=*/true);
}
bool AnalyzerOptions::mayInlineCXXContainerMethods() {
Index: test/Analysis/virtualcall.cpp
===================================================================
--- test/Analysis/virtualcall.cpp
+++ test/Analysis/virtualcall.cpp
@@ -262,6 +262,9 @@
//expected-note-re@-2 {{{{^}}Calling default constructor for 'M'}}
#endif
Y *y = new Y;
+#if !PUREONLY
+ //expected-note-re@-2 {{{{^}}Calling default constructor for 'Y'}}
+#endif
delete y;
header::Z z;
#if !PUREONLY
Index: test/Analysis/new.cpp
===================================================================
--- test/Analysis/new.cpp
+++ test/Analysis/new.cpp
@@ -34,7 +34,7 @@
void *y = new (x) int;
clang_analyzer_eval(x == y); // expected-warning{{TRUE}};
- clang_analyzer_eval(*x == 1); // expected-warning{{UNKNOWN}};
+ clang_analyzer_eval(*x == 1); // expected-warning{{TRUE}};
return y;
}
@@ -200,8 +200,7 @@
int n;
new (&n) int;
- // Should warn that n is uninitialized.
- if (n) { // no-warning
+ if (n) { // expected-warning{{Branch condition evaluates to a garbage value}}
return 0;
}
return 1;
Index: test/Analysis/ctor.mm
===================================================================
--- test/Analysis/ctor.mm
+++ test/Analysis/ctor.mm
@@ -572,10 +572,9 @@
}
void testNew() {
- // FIXME: Pending proper implementation of constructors for 'new'.
raw_pair *pp = new raw_pair();
- clang_analyzer_eval(pp->p1 == 0); // expected-warning{{UNKNOWN}}
- clang_analyzer_eval(pp->p2 == 0); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(pp->p1 == 0); // expected-warning{{TRUE}}
+ clang_analyzer_eval(pp->p2 == 0); // expected-warning{{TRUE}}
}
void testArrayNew() {
@@ -679,8 +678,7 @@
void testDynamic() {
List *list = new List{1, 2};
- // FIXME: When we handle constructors with 'new', this will be TRUE.
- clang_analyzer_eval(list->usedInitializerList); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(list->usedInitializerList); // expected-warning{{TRUE}}
}
}
Index: test/Analysis/NewDelete-custom.cpp
===================================================================
--- test/Analysis/NewDelete-custom.cpp
+++ test/Analysis/NewDelete-custom.cpp
@@ -1,7 +1,7 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,unix.Malloc -std=c++11 -fblocks -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks,unix.Malloc -std=c++11 -DLEAKS=1 -fblocks -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,unix.Malloc -std=c++11 -analyzer-config c++-allocator-inlining=true -DALLOCATOR_INLINING=1 -fblocks -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks,unix.Malloc -std=c++11 -analyzer-config c++-allocator-inlining=true -DLEAKS=1 -DALLOCATOR_INLINING=1 -fblocks -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,unix.Malloc -std=c++11 -analyzer-config c++-allocator-inlining=false -fblocks -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks,unix.Malloc -std=c++11 -analyzer-config c++-allocator-inlining=false -DLEAKS=1 -fblocks -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,unix.Malloc -std=c++11 -DALLOCATOR_INLINING=1 -fblocks -verify %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDelete,cplusplus.NewDeleteLeaks,unix.Malloc -std=c++11 -DLEAKS=1 -DALLOCATOR_INLINING=1 -fblocks -verify %s
#include "Inputs/system-header-simulator-cxx.h"
#if !(LEAKS && !ALLOCATOR_INLINING)
Index: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===================================================================
--- lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -203,7 +203,7 @@
bool AnalyzerOptions::mayInlineCXXAllocator() {
return getBooleanOption(InlineCXXAllocator,
"c++-allocator-inlining",
- /*Default=*/false);
+ /*Default=*/true);
}
bool AnalyzerOptions::mayInlineCXXContainerMethods() {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
