[clang] Add option -fstdlib-hardening= (PR #78763)

[Devin Coughlin via cfe-commits]

@@ -7730,6 +7730,14 @@ def source_date_epoch : Separate<["-"], 
"source-date-epoch">,
 
 } // let Visibility = [CC1Option]
 
+def stdlib_hardening_EQ : Joined<["-"], "fstdlib-hardening=">,

devincoughlin wrote:

With the name `stdlib_hardening`, are we worried that users might think it 
applies to the C standard library as well? Should the name imply that this is 
only for C++?

https://github.com/llvm/llvm-project/pull/78763
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] abc04ff - [analyzer] Require darwin for scan-build tests

[Devin Coughlin via cfe-commits]

Author: Devin Coughlin
Date: 2019-11-04T21:17:55-08:00
New Revision: abc04ff4012c62c98aa9f0d840114b2f56855dc8

URL: 
https://github.com/llvm/llvm-project/commit/abc04ff4012c62c98aa9f0d840114b2f56855dc8
DIFF: 
https://github.com/llvm/llvm-project/commit/abc04ff4012c62c98aa9f0d840114b2f56855dc8.diff

LOG: [analyzer] Require darwin for scan-build tests

Let's at least get some coverage from these tests. We can generalize to
other platforms later.

Added: 


Modified: 
clang/test/Analysis/scan-build/exclude_directories.test
clang/test/Analysis/scan-build/help.test
clang/test/Analysis/scan-build/html_output.test
clang/test/Analysis/scan-build/plist_html_output.test
clang/test/Analysis/scan-build/plist_output.test

Removed: 




diff  --git a/clang/test/Analysis/scan-build/exclude_directories.test 
b/clang/test/Analysis/scan-build/exclude_directories.test
index 2511a42312bd..3db68d635763 100644
--- a/clang/test/Analysis/scan-build/exclude_directories.test
+++ b/clang/test/Analysis/scan-build/exclude_directories.test
@@ -1,4 +1,4 @@
-XFAIL: x86_64-scei-ps4
+REQUIRES: system-darwin
 
 RUN: rm -rf %t.output_dir && mkdir %t.output_dir
 RUN: %scan-build -o %t.output_dir %clang \

diff  --git a/clang/test/Analysis/scan-build/help.test 
b/clang/test/Analysis/scan-build/help.test
index c553d6b1b890..ce4696e0380d 100644
--- a/clang/test/Analysis/scan-build/help.test
+++ b/clang/test/Analysis/scan-build/help.test
@@ -1,3 +1,5 @@
+REQUIRES: system-darwin
+
 RUN: %scan-build -h | FileCheck %s
 RUN: %scan-build --help | FileCheck %s
 

diff  --git a/clang/test/Analysis/scan-build/html_output.test 
b/clang/test/Analysis/scan-build/html_output.test
index 20347900c9bb..f2345fa06062 100644
--- a/clang/test/Analysis/scan-build/html_output.test
+++ b/clang/test/Analysis/scan-build/html_output.test
@@ -1,4 +1,4 @@
-XFAIL: x86_64-scei-ps4
+REQUIRES: system-darwin
 
 RUN: rm -rf %t.output_dir && mkdir %t.output_dir
 RUN: %scan-build -o %t.output_dir %clang %S/Inputs/single_null_dereference.c \

diff  --git a/clang/test/Analysis/scan-build/plist_html_output.test 
b/clang/test/Analysis/scan-build/plist_html_output.test
index ec059460a395..feefa25617d3 100644
--- a/clang/test/Analysis/scan-build/plist_html_output.test
+++ b/clang/test/Analysis/scan-build/plist_html_output.test
@@ -1,4 +1,4 @@
-XFAIL: x86_64-scei-ps4
+REQUIRES: system-darwin
 
 RUN: rm -rf %t.output_dir && mkdir %t.output_dir
 RUN: %scan-build -plist-html -o %t.output_dir %clang 
%S/Inputs/single_null_dereference.c \

diff  --git a/clang/test/Analysis/scan-build/plist_output.test 
b/clang/test/Analysis/scan-build/plist_output.test
index 9812cbdcc332..ffef11656317 100644
--- a/clang/test/Analysis/scan-build/plist_output.test
+++ b/clang/test/Analysis/scan-build/plist_output.test
@@ -1,4 +1,4 @@
-XFAIL: x86_64-scei-ps4
+REQUIRES: system-darwin
 
 RUN: rm -rf %t.output_dir && mkdir %t.output_dir
 RUN: %scan-build -plist -o %t.output_dir %clang 
%S/Inputs/single_null_dereference.c \



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 48223d9 - [analyzer] Fixup scan-build tests for non-Darwin platforms.

[Devin Coughlin via cfe-commits]

Author: Devin Coughlin
Date: 2019-11-04T21:12:11-08:00
New Revision: 48223d92a98e2eb7da6844d56471953f83da191e

URL: 
https://github.com/llvm/llvm-project/commit/48223d92a98e2eb7da6844d56471953f83da191e
DIFF: 
https://github.com/llvm/llvm-project/commit/48223d92a98e2eb7da6844d56471953f83da191e.diff

LOG: [analyzer] Fixup scan-build tests for non-Darwin platforms.

This is a fix to 0aba69eb1a01c44185009f50cc633e3c648e9950 to
address failing bots.

Added: 


Modified: 
clang/test/Analysis/scan-build/exclude_directories.test
clang/test/Analysis/scan-build/help.test
clang/test/Analysis/scan-build/html_output.test
clang/test/Analysis/scan-build/plist_html_output.test
clang/test/Analysis/scan-build/plist_output.test

Removed: 




diff  --git a/clang/test/Analysis/scan-build/exclude_directories.test 
b/clang/test/Analysis/scan-build/exclude_directories.test
index db53a34aa65d..2511a42312bd 100644
--- a/clang/test/Analysis/scan-build/exclude_directories.test
+++ b/clang/test/Analysis/scan-build/exclude_directories.test
@@ -1,3 +1,5 @@
+XFAIL: x86_64-scei-ps4
+
 RUN: rm -rf %t.output_dir && mkdir %t.output_dir
 RUN: %scan-build -o %t.output_dir %clang \
 RUN: %S/Inputs/multidirectory_project/directory1/file1.c \

diff  --git a/clang/test/Analysis/scan-build/help.test 
b/clang/test/Analysis/scan-build/help.test
index 4d1972b70653..c553d6b1b890 100644
--- a/clang/test/Analysis/scan-build/help.test
+++ b/clang/test/Analysis/scan-build/help.test
@@ -11,7 +11,6 @@ CHECK: USAGE: scan-build [options]  [build 
options]
 CHECK: AVAILABLE CHECKERS:
 ...
 CHECK:optin.performance.GCDAntipattern
-CHECK:  + osx.API
 ...
 
 

diff  --git a/clang/test/Analysis/scan-build/html_output.test 
b/clang/test/Analysis/scan-build/html_output.test
index 07aa481fae16..20347900c9bb 100644
--- a/clang/test/Analysis/scan-build/html_output.test
+++ b/clang/test/Analysis/scan-build/html_output.test
@@ -1,3 +1,5 @@
+XFAIL: x86_64-scei-ps4
+
 RUN: rm -rf %t.output_dir && mkdir %t.output_dir
 RUN: %scan-build -o %t.output_dir %clang %S/Inputs/single_null_dereference.c \
 RUN: | FileCheck %s -check-prefix CHECK-STDOUT

diff  --git a/clang/test/Analysis/scan-build/plist_html_output.test 
b/clang/test/Analysis/scan-build/plist_html_output.test
index ed26b272246e..ec059460a395 100644
--- a/clang/test/Analysis/scan-build/plist_html_output.test
+++ b/clang/test/Analysis/scan-build/plist_html_output.test
@@ -1,3 +1,5 @@
+XFAIL: x86_64-scei-ps4
+
 RUN: rm -rf %t.output_dir && mkdir %t.output_dir
 RUN: %scan-build -plist-html -o %t.output_dir %clang 
%S/Inputs/single_null_dereference.c \
 RUN: | FileCheck %s -check-prefix CHECK-STDOUT

diff  --git a/clang/test/Analysis/scan-build/plist_output.test 
b/clang/test/Analysis/scan-build/plist_output.test
index 1ee331b8d0e6..9812cbdcc332 100644
--- a/clang/test/Analysis/scan-build/plist_output.test
+++ b/clang/test/Analysis/scan-build/plist_output.test
@@ -1,3 +1,5 @@
+XFAIL: x86_64-scei-ps4
+
 RUN: rm -rf %t.output_dir && mkdir %t.output_dir
 RUN: %scan-build -plist -o %t.output_dir %clang 
%S/Inputs/single_null_dereference.c \
 RUN: | FileCheck %s -check-prefix CHECK-STDOUT



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 0aba69e - [analyzer] Add test directory for scan-build.

[Devin Coughlin via cfe-commits]

Author: Devin Coughlin
Date: 2019-11-04T20:26:35-08:00
New Revision: 0aba69eb1a01c44185009f50cc633e3c648e9950

URL: 
https://github.com/llvm/llvm-project/commit/0aba69eb1a01c44185009f50cc633e3c648e9950
DIFF: 
https://github.com/llvm/llvm-project/commit/0aba69eb1a01c44185009f50cc633e3c648e9950.diff

LOG: [analyzer] Add test directory for scan-build.

The static analyzer's scan-build script is critical infrastructure but
is not well tested. To start to address this, add a new test directory under
tests/Analysis for scan-build lit tests and seed it with several tests. The
goal is that future scan-build changes will be accompanied by corresponding
tests.

Differential Revision: https://reviews.llvm.org/D69781

Added: 

clang/test/Analysis/scan-build/Inputs/multidirectory_project/directory1/file1.c

clang/test/Analysis/scan-build/Inputs/multidirectory_project/directory2/file2.c
clang/test/Analysis/scan-build/Inputs/single_null_dereference.c
clang/test/Analysis/scan-build/exclude_directories.test
clang/test/Analysis/scan-build/help.test
clang/test/Analysis/scan-build/html_output.test
clang/test/Analysis/scan-build/plist_html_output.test
clang/test/Analysis/scan-build/plist_output.test

Modified: 
clang/test/lit.cfg.py
llvm/utils/lit/lit/llvm/config.py

Removed: 




diff  --git 
a/clang/test/Analysis/scan-build/Inputs/multidirectory_project/directory1/file1.c
 
b/clang/test/Analysis/scan-build/Inputs/multidirectory_project/directory1/file1.c
new file mode 100644
index ..7fffb69e01a0
--- /dev/null
+++ 
b/clang/test/Analysis/scan-build/Inputs/multidirectory_project/directory1/file1.c
@@ -0,0 +1,9 @@
+int main() {
+  return 0;
+}
+
+void function1(int *p) {
+  if (!p) {
+*p = 7; // This will emit a null pointer diagnostic.
+  }
+}

diff  --git 
a/clang/test/Analysis/scan-build/Inputs/multidirectory_project/directory2/file2.c
 
b/clang/test/Analysis/scan-build/Inputs/multidirectory_project/directory2/file2.c
new file mode 100644
index ..ed0e17212337
--- /dev/null
+++ 
b/clang/test/Analysis/scan-build/Inputs/multidirectory_project/directory2/file2.c
@@ -0,0 +1,5 @@
+void function2(int *o) {
+  if (!o) {
+*o = 7; // This will emit a null pointer diagnostic.
+  }
+}

diff  --git a/clang/test/Analysis/scan-build/Inputs/single_null_dereference.c 
b/clang/test/Analysis/scan-build/Inputs/single_null_dereference.c
new file mode 100644
index ..21a43dfd08a5
--- /dev/null
+++ b/clang/test/Analysis/scan-build/Inputs/single_null_dereference.c
@@ -0,0 +1,5 @@
+int main() {
+  int *p = 0;
+  *p = 7; // We expect a diagnostic about this.
+  return 0;
+}

diff  --git a/clang/test/Analysis/scan-build/exclude_directories.test 
b/clang/test/Analysis/scan-build/exclude_directories.test
new file mode 100644
index ..db53a34aa65d
--- /dev/null
+++ b/clang/test/Analysis/scan-build/exclude_directories.test
@@ -0,0 +1,34 @@
+RUN: rm -rf %t.output_dir && mkdir %t.output_dir
+RUN: %scan-build -o %t.output_dir %clang \
+RUN: %S/Inputs/multidirectory_project/directory1/file1.c \
+RUN: %S/Inputs/multidirectory_project/directory2/file2.c \
+RUN: | FileCheck %s -check-prefix CHECK-NO-EXCLUDE
+
+// The purpose of this test is to ensure that the --exclude command line option
+// actually excludes reports from inside the specified directories.
+
+
+// First, let's make sure that without --exclude issues in both
+// directory1 and directory2 are found.
+CHECK-NO-EXCLUDE: scan-build: 2 bugs found.
+
+
+// Only one issue should be found when directory1 is excluded.
+RUN: rm -rf %t.output_dir && mkdir %t.output_dir
+RUN: %scan-build -o %t.output_dir --exclude directory1 %clang \
+RUN: %S/Inputs/multidirectory_project/directory1/file1.c \
+RUN: %S/Inputs/multidirectory_project/directory2/file2.c \
+RUN: | FileCheck %s -check-prefix CHECK-EXCLUDE1
+
+CHECK-EXCLUDE1: scan-build: 1 bug found.
+
+
+// When both directories are excluded, no issues should be reported.
+RUN: rm -rf %t.output_dir && mkdir %t.output_dir
+RUN: %scan-build -o %t.output_dir --exclude directory1 --exclude directory2 
%clang \
+RUN: %S/Inputs/multidirectory_project/directory1/file1.c \
+RUN: %S/Inputs/multidirectory_project/directory2/file2.c \
+RUN: | FileCheck %s -check-prefix CHECK-EXCLUDE-BOTH
+
+CHECK-EXCLUDE-BOTH: scan-build: 0 bugs found.
+

diff  --git a/clang/test/Analysis/scan-build/help.test 
b/clang/test/Analysis/scan-build/help.test
new file mode 100644
index ..4d1972b70653
--- /dev/null
+++ b/clang/test/Analysis/scan-build/help.test
@@ -0,0 +1,18 @@
+RUN: %scan-build -h | FileCheck %s
+RUN: %scan-build --help | FileCheck %s
+
+Test for help output from scan-build.
+
+
+CHECK: USAGE: scan-build [options]  [build options]
+
+...
+
+CHECK: AVAILABLE CHECKERS:
+...
+CHECK:optin.performance.GCDAntipattern
+CHECK:  + osx.API
+...
+
+
+

diff  --git 

r356308 - [analyzer] Teach scan-build to find clang when installed in /usr/local/bin/

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Fri Mar 15 18:01:29 2019
New Revision: 356308

URL: http://llvm.org/viewvc/llvm-project?rev=356308=rev
Log:
[analyzer] Teach scan-build to find clang when installed in /usr/local/bin/

Change scan-build to support the scenario where scan-build is installed in
$TOOLCHAIN/usr/local/bin/ but clang itself is installed in $TOOLCHAIN/usr/bin/.

This is restricted to when 'xcrun' is present; that is, on the Mac.

rdar://problem/48914634

Differential Revision: https://reviews.llvm.org/D59406

Modified:
cfe/trunk/tools/scan-build/bin/scan-build

Modified: cfe/trunk/tools/scan-build/bin/scan-build
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-build/bin/scan-build?rev=356308=356307=356308=diff
==
--- cfe/trunk/tools/scan-build/bin/scan-build (original)
+++ cfe/trunk/tools/scan-build/bin/scan-build Fri Mar 15 18:01:29 2019
@@ -1460,6 +1460,16 @@ sub ShellEscape {
 }
 
 
####
+# FindXcrun - searches for the 'xcrun' executable. Returns "" if not found.
+####
+
+sub FindXcrun {
+  my $xcrun = `which xcrun`;
+  chomp $xcrun;
+  return $xcrun;
+}
+
+####
 # FindClang - searches for 'clang' executable.
 
####
 
@@ -1468,6 +1478,16 @@ sub FindClang {
 $Clang = Cwd::realpath("$RealBin/bin/clang") if (-f "$RealBin/bin/clang");
 if (!defined $Clang || ! -x $Clang) {
   $Clang = Cwd::realpath("$RealBin/clang") if (-f "$RealBin/clang");
+  if (!defined $Clang || ! -x $Clang) {
+# When an Xcode toolchain is present, look for a clang in the sibling 
bin
+# of the parent of the bin directory. So if scan-build is at
+# $TOOLCHAIN/usr/local/bin/scan-build look for clang at
+# $TOOLCHAIN/usr/bin/clang.
+my $has_xcode_toolchain = FindXcrun() ne "";
+if ($has_xcode_toolchain && -f "$RealBin/../../bin/clang") {
+  $Clang = Cwd::realpath("$RealBin/../../bin/clang");
+}
+  }
 }
 if (!defined $Clang || ! -x $Clang) {
   return "error: Cannot find an executable 'clang' relative to" .
@@ -1477,8 +1497,7 @@ sub FindClang {
   }
   else {
 if ($Options{AnalyzerDiscoveryMethod} =~ /^[Xx]code$/) {
-  my $xcrun = `which xcrun`;
-  chomp $xcrun;
+  my $xcrun = FindXcrun();
   if ($xcrun eq "") {
 return "Cannot find 'xcrun' to find 'clang' for analysis.\n";
   }


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r326062 - [www] Update link to analyzer's "Building a Checker in 24 hours" video

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Sun Feb 25 16:39:25 2018
New Revision: 326062

URL: http://llvm.org/viewvc/llvm-project?rev=326062=rev
Log:
[www] Update link to analyzer's "Building a Checker in 24 hours" video

The video is now uploaded to YouTube.

Modified:
cfe/trunk/www/analyzer/alpha_checks.html
cfe/trunk/www/analyzer/checker_dev_manual.html
cfe/trunk/www/analyzer/open_projects.html

Modified: cfe/trunk/www/analyzer/alpha_checks.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/analyzer/alpha_checks.html?rev=326062=326061=326062=diff
==
--- cfe/trunk/www/analyzer/alpha_checks.html (original)
+++ cfe/trunk/www/analyzer/alpha_checks.html Sun Feb 25 16:39:25 2018
@@ -800,7 +800,7 @@ Check for misuses of stream APIs:
 fclose(demo checker, the subject of the demo
 (http://llvm.org/devmtg/2012-11/Zaks-Rose-Checker24Hours.pdf;>Slides
-,http://llvm.org/devmtg/2012-11/videos/Zaks-Rose-Checker24Hours.mp4;>Video)
 
+,https://youtu.be/kdxlsP5QVPw;>Video)
 by Anna Zaks and Jordan Rose presented at the http://llvm.org/devmtg/2012-11/;>
 2012 LLVM Developers' Meeting).
 

Modified: cfe/trunk/www/analyzer/checker_dev_manual.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/analyzer/checker_dev_manual.html?rev=326062=326061=326062=diff
==
--- cfe/trunk/www/analyzer/checker_dev_manual.html (original)
+++ cfe/trunk/www/analyzer/checker_dev_manual.html Sun Feb 25 16:39:25 2018
@@ -23,7 +23,7 @@ relies on a set of checkers to implement
 constructing specific bug reports. Anyone who is interested in implementing 
their own 
 checker, should check out the Building a Checker in 24 Hours talk 
 (http://llvm.org/devmtg/2012-11/Zaks-Rose-Checker24Hours.pdf;>slides
- http://llvm.org/devmtg/2012-11/videos/Zaks-Rose-Checker24Hours.mp4;>video)
 
+ https://youtu.be/kdxlsP5QVPw;>video)
 and refer to this page for additional information on writing a checker. The 
static analyzer is a 
 part of the Clang project, so consult http://clang.llvm.org/hacking.html;>Hacking on Clang 
 and http://llvm.org/docs/ProgrammersManual.html;>LLVM Programmer's 
Manual 
@@ -696,7 +696,7 @@ href="http://llvm.org/devmtg/2012-11;>No
 meeting. Describes the construction of SimpleStreamChecker. http://llvm.org/devmtg/2012-11/Zaks-Rose-Checker24Hours.pdf;>Slides
 and http://llvm.org/devmtg/2012-11/videos/Zaks-Rose-Checker24Hours.mp4;>video
+href="https://youtu.be/kdxlsP5QVPw;>video
 are available.
 
 

Modified: cfe/trunk/www/analyzer/open_projects.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/analyzer/open_projects.html?rev=326062=326061=326062=diff
==
--- cfe/trunk/www/analyzer/open_projects.html (original)
+++ cfe/trunk/www/analyzer/open_projects.html Sun Feb 25 16:39:25 2018
@@ -147,7 +147,7 @@ mailing list to notify other members
 A SimpleStreamChecker has been presented in the Building a Checker in 
24 
 Hours talk 
 (http://llvm.org/devmtg/2012-11/Zaks-Rose-Checker24Hours.pdf;>slides
-http://llvm.org/devmtg/2012-11/videos/Zaks-Rose-Checker24Hours.mp4;>video).
 
+https://youtu.be/kdxlsP5QVPw;>video).
 We need to implement a production version of the checker with richer set 
of 
 APIs and evaluate it by running on real codebases. 
 (Difficulty: Easy)


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r323052 - [analyzer] Provide a check name when MallocChecker enables CStringChecker

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Sat Jan 20 15:11:17 2018
New Revision: 323052

URL: http://llvm.org/viewvc/llvm-project?rev=323052=rev
Log:
[analyzer] Provide a check name when MallocChecker enables CStringChecker

Fix an assertion failure caused by a missing CheckName. The malloc checker
enables "basic" support in the CStringChecker, which causes some CString
bounds checks to be enabled. In this case, make sure that we have a
valid CheckName for the BugType.

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
cfe/trunk/test/Analysis/Inputs/system-header-simulator.h
cfe/trunk/test/Analysis/malloc.c

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp?rev=323052=323051=323052=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp Sat Jan 20 
15:11:17 2018
@@ -309,9 +309,19 @@ ProgramStateRef CStringChecker::CheckLoc
 if (!N)
   return nullptr;
 
+CheckName Name;
+// These checks are either enabled by the CString out-of-bounds checker
+// explicitly or the "basic" CStringNullArg checker support that Malloc
+// checker enables.
+assert(Filter.CheckCStringOutOfBounds || Filter.CheckCStringNullArg);
+if (Filter.CheckCStringOutOfBounds)
+  Name = Filter.CheckNameCStringOutOfBounds;
+else
+  Name = Filter.CheckNameCStringNullArg;
+
 if (!BT_Bounds) {
   BT_Bounds.reset(new BuiltinBug(
-  Filter.CheckNameCStringOutOfBounds, "Out-of-bound array access",
+  Name, "Out-of-bound array access",
   "Byte string function accesses out-of-bound array element"));
 }
 BuiltinBug *BT = static_cast(BT_Bounds.get());

Modified: cfe/trunk/test/Analysis/Inputs/system-header-simulator.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/Inputs/system-header-simulator.h?rev=323052=323051=323052=diff
==
--- cfe/trunk/test/Analysis/Inputs/system-header-simulator.h (original)
+++ cfe/trunk/test/Analysis/Inputs/system-header-simulator.h Sat Jan 20 
15:11:17 2018
@@ -32,6 +32,7 @@ typedef __typeof(sizeof(int)) size_t;
 size_t strlen(const char *);
 
 char *strcpy(char *restrict, const char *restrict);
+char *strncpy(char *dst, const char *src, size_t n);
 void *memcpy(void *dst, const void *src, size_t n);
 
 typedef unsigned long __darwin_pthread_key_t;

Modified: cfe/trunk/test/Analysis/malloc.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc.c?rev=323052=323051=323052=diff
==
--- cfe/trunk/test/Analysis/malloc.c (original)
+++ cfe/trunk/test/Analysis/malloc.c Sat Jan 20 15:11:17 2018
@@ -1777,6 +1777,15 @@ void freeFunctionPtr() {
   free((void *)fnptr); // expected-warning {{Argument to free() is a function 
pointer}}
 }
 
+// Enabling the malloc checker enables some of the buffer-checking portions
+// of the C-string checker.
+void cstringchecker_bounds_nocrash() {
+  char *p = malloc(2);
+  strncpy(p, "AAA", sizeof("AAA")); // expected-warning {{Size argument is 
greater than the length of the destination buffer}}
+
+  free(p);
+}
+
 // 
 // False negatives.
 


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r319638 - [analyzer] Don't treat lambda-captures float constexprs as undefined

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Sun Dec  3 20:46:47 2017
New Revision: 319638

URL: http://llvm.org/viewvc/llvm-project?rev=319638=rev
Log:
[analyzer] Don't treat lambda-captures float constexprs as undefined

RegionStore has special logic to evaluate captured constexpr variables.
However, if the constexpr initializer cannot be evaluated as an integer, the
value is treated as undefined. This leads to false positives when, for example,
a constexpr float is captured by a lambda.

To fix this, treat a constexpr capture that cannot be evaluated as unknown
rather than undefined.

rdar://problem/35784662

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
cfe/trunk/test/Analysis/lambdas.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=319638=319637=319638=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Sun Dec  3 20:46:47 2017
@@ -1864,11 +1864,18 @@ SVal RegionStoreManager::getBindingForVa
 return svalBuilder.getRegionValueSymbolVal(R);
 
   // Is 'VD' declared constant?  If so, retrieve the constant value.
-  if (VD->getType().isConstQualified())
-if (const Expr *Init = VD->getInit())
+  if (VD->getType().isConstQualified()) {
+if (const Expr *Init = VD->getInit()) {
   if (Optional V = svalBuilder.getConstantVal(Init))
 return *V;
 
+  // If the variable is const qualified and has an initializer but
+  // we couldn't evaluate initializer to a value, treat the value as
+  // unknown.
+  return UnknownVal();
+}
+  }
+
   // This must come after the check for constants because closure-captured
   // constant variables may appear in UnknownSpaceRegion.
   if (isa(MS))

Modified: cfe/trunk/test/Analysis/lambdas.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/lambdas.cpp?rev=319638=319637=319638=diff
==
--- cfe/trunk/test/Analysis/lambdas.cpp (original)
+++ cfe/trunk/test/Analysis/lambdas.cpp Sun Dec  3 20:46:47 2017
@@ -337,6 +337,16 @@ void captureByReference() {
   lambda2();
 }
 
+void testCapturedConstExprFloat() {
+  constexpr float localConstant = 4.0;
+  auto lambda = []{
+// Don't treat localConstant as containing a garbage value
+float copy = localConstant; // no-warning
+(void)copy;
+  };
+
+  lambda();
+}
 
 // CHECK: [B2 (ENTRY)]
 // CHECK:   Succs (1): B1


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r319333 - [analyzer] Fix unreachable creating PathDiagnosticLocation with widen-loops=true

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Wed Nov 29 10:25:37 2017
New Revision: 319333

URL: http://llvm.org/viewvc/llvm-project?rev=319333=rev
Log:
[analyzer] Fix unreachable creating PathDiagnosticLocation with widen-loops=true

In the original design of the analyzer, it was assumed that a BlockEntrance
doesn't create a new binding on the Store, but this assumption isn't true when
'widen-loops' is set to true. Fix this by finding an appropriate location
BlockEntrace program points.

Patch by Henry Wong!

Differential Revision: https://reviews.llvm.org/D37187

Added:
cfe/trunk/test/Analysis/loop-widening-notes.cpp
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp?rev=319333=319332=319333=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp Wed Nov 29 10:25:37 
2017
@@ -690,6 +690,15 @@ PathDiagnosticLocation::create(const Pro
 return getLocationForCaller(CEE->getCalleeContext(),
 CEE->getLocationContext(),
 SMng);
+  } else if (Optional BE = P.getAs()) {
+CFGElement BlockFront = BE->getBlock()->front();
+if (auto StmtElt = BlockFront.getAs()) {
+  return PathDiagnosticLocation(StmtElt->getStmt()->getLocStart(), SMng);
+} else if (auto NewAllocElt = BlockFront.getAs()) {
+  return PathDiagnosticLocation(
+  NewAllocElt->getAllocatorExpr()->getLocStart(), SMng);
+}
+llvm_unreachable("Unexpected CFG element at front of block");
   } else {
 llvm_unreachable("Unexpected ProgramPoint");
   }

Added: cfe/trunk/test/Analysis/loop-widening-notes.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/loop-widening-notes.cpp?rev=319333=auto
==
--- cfe/trunk/test/Analysis/loop-widening-notes.cpp (added)
+++ cfe/trunk/test/Analysis/loop-widening-notes.cpp Wed Nov 29 10:25:37 2017
@@ -0,0 +1,72 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha -analyzer-max-loop 2 
-analyzer-config widen-loops=true -analyzer-output=text -verify %s
+
+int *p_a;
+int bar();
+int flag_a;
+int test_for_bug_25609() {
+  if (p_a == 0) // expected-note {{Assuming 'p_a' is equal to null}} 
+// expected-note@-1 {{Taking true branch}}
+bar();
+  for (int i = 0;  // expected-note {{Loop condition is true.  Entering loop 
body}}
+   // expected-note@-1 {{Loop condition is false. Execution 
continues on line 16}}
+   ++i,// expected-note {{Value assigned to 'p_a'}} 
+   i < flag_a;
+   ++i) {}
+  
+  *p_a = 25609; // no-crash expected-warning {{Dereference of null pointer 
(loaded from variable 'p_a')}}
+// expected-note@-1 {{Dereference of null pointer (loaded from 
variable 'p_a')}}
+  return *p_a;
+}
+
+int flag_b;
+int while_analyzer_output() {
+  flag_b = 100;
+  int num = 10;
+  while (flag_b-- > 0) { // expected-note {{Loop condition is true.  Entering 
loop body}} 
+ // expected-note@-1 {{Value assigned to 'num'}} 
+ // expected-note@-2 {{Loop condition is false. 
Execution continues on line 30}}
+num = flag_b;
+  }
+  if (num < 0) // expected-note {{Assuming 'num' is >= 0}} 
+   // expected-note@-1 {{Taking false branch}}
+flag_b = 0;
+  else if (num >= 1) // expected-note {{Assuming 'num' is < 1}} 
+ // expected-note@-1 {{Taking false branch}}
+flag_b = 50;
+  else
+flag_b = 100;
+  return flag_b / num; // no-crash expected-warning {{Division by zero}} 
+   // expected-note@-1 {{Division by zero}}
+}
+
+int flag_c;
+int do_while_analyzer_output() {
+  int num = 10;
+  do {   // expected-note {{Loop condition is true. Execution continues on 
line 47}} 
+ // expected-note@-1 {{Loop condition is false.  Exiting loop}}
+num--;
+  } while (flag_c-- > 0); //expected-note {{Value assigned to 'num'}}
+  int local = 0;
+  if (num == 0)   // expected-note {{Assuming 'num' is equal to 0}} 
+  // expected-note@-1 {{Taking true branch}}
+local = 10 / num; // no-crash expected-warning {{Division by zero}}
+  // expected-note@-1 {{Division by zero}}
+  return local;
+}
+
+int flag_d;
+int test_for_loop() {
+  int num = 10;
+  for (int i = 0;// expected-note {{Loop condition is true.  Entering loop 
body}} 
+ // expected-note@-1 {{Loop condition is false. Execution 
continues on line 67}}
+   new int(10),  // expected-note {{Value assigned to 'num'}}
+   i < flag_d;
+   ++i) { 
+++num;
+  

r318979 - [analyzer] Teach RetainCountChecker about CoreMedia APIs

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Sat Nov 25 06:57:42 2017
New Revision: 318979

URL: http://llvm.org/viewvc/llvm-project?rev=318979=rev
Log:
[analyzer] Teach RetainCountChecker about CoreMedia APIs

Teach the retain-count checker that CoreMedia reference types use
CoreFoundation-style reference counting. This enables the checker
to catch leaks and over releases of those types.

rdar://problem/33599757

Modified:
cfe/trunk/lib/Analysis/CocoaConventions.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
cfe/trunk/test/Analysis/retain-release.m

Modified: cfe/trunk/lib/Analysis/CocoaConventions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CocoaConventions.cpp?rev=318979=318978=318979=diff
==
--- cfe/trunk/lib/Analysis/CocoaConventions.cpp (original)
+++ cfe/trunk/lib/Analysis/CocoaConventions.cpp Sat Nov 25 06:57:42 2017
@@ -47,12 +47,19 @@ bool cocoa::isRefType(QualType RetTy, St
   return Name.startswith(Prefix);
 }
 
+/// Returns true when the passed-in type is a CF-style reference-counted
+/// type from the DiskArbitration framework.
+static bool isDiskArbitrationAPIRefType(QualType T) {
+  return cocoa::isRefType(T, "DADisk") ||
+  cocoa::isRefType(T, "DADissenter") ||
+  cocoa::isRefType(T, "DASessionRef");
+}
+
 bool coreFoundation::isCFObjectRef(QualType T) {
   return cocoa::isRefType(T, "CF") || // Core Foundation.
  cocoa::isRefType(T, "CG") || // Core Graphics.
- cocoa::isRefType(T, "DADisk") || // Disk Arbitration API.
- cocoa::isRefType(T, "DADissenter") ||
- cocoa::isRefType(T, "DASessionRef");
+ cocoa::isRefType(T, "CM") || // Core Media.
+ isDiskArbitrationAPIRefType(T);
 }
 
 

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp?rev=318979=318978=318979=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp Sat Nov 25 
06:57:42 2017
@@ -1201,10 +1201,10 @@ RetainSummaryManager::getFunctionSummary
 break;
   }
 
-  // For the Disk Arbitration API (DiskArbitration/DADisk.h)
-  if (cocoa::isRefType(RetTy, "DADisk") ||
-  cocoa::isRefType(RetTy, "DADissenter") ||
-  cocoa::isRefType(RetTy, "DASessionRef")) {
+  // For all other CF-style types, use the Create/Get
+  // rule for summaries but don't support Retain functions
+  // with framework-specific prefixes.
+  if (coreFoundation::isCFObjectRef(RetTy)) {
 S = getCFCreateGetRuleSummary(FD);
 break;
   }

Modified: cfe/trunk/test/Analysis/retain-release.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/retain-release.m?rev=318979=318978=318979=diff
==
--- cfe/trunk/test/Analysis/retain-release.m (original)
+++ cfe/trunk/test/Analysis/retain-release.m Sat Nov 25 06:57:42 2017
@@ -450,6 +450,51 @@ void f10(io_service_t media, DADiskRef d
   if (session) NSLog(@"ok");
 }
 
+
+// Handle CoreMedia API
+
+struct CMFoo;
+typedef struct CMFoo *CMFooRef;
+
+CMFooRef CMCreateFooRef();
+CMFooRef CMGetFooRef();
+
+typedef signed long SInt32;
+typedef SInt32  OSStatus;
+OSStatus CMCreateFooAndReturnViaOutParameter(CMFooRef * CF_RETURNS_RETAINED 
fooOut);
+
+void testLeakCoreMediaReferenceType() {
+  CMFooRef f = CMCreateFooRef(); // expected-warning{{leak}}
+}
+
+void testOverReleaseMediaReferenceType() {
+  CMFooRef f = CMGetFooRef();
+  CFRelease(f); // expected-warning{{Incorrect decrement of the reference 
count}}
+}
+
+void testOkToReleaseReturnsRetainedOutParameter() {
+  CMFooRef foo = 0;
+  OSStatus status = CMCreateFooAndReturnViaOutParameter();
+
+  if (status != 0)
+return;
+
+  CFRelease(foo); // no-warning
+}
+
+void testLeakWithReturnsRetainedOutParameter() {
+  CMFooRef foo = 0;
+  OSStatus status = CMCreateFooAndReturnViaOutParameter();
+
+  if (status != 0)
+return;
+
+  // FIXME: Ideally we would report a leak here since it is the caller's
+  // responsibility to release 'foo'. However, we don't currently have
+  // a mechanism in this checker to only require a release when a successful
+  // status is returned.
+}
+
 // Test retain/release checker with CFString and CFMutableArray.
 void f11() {
   // Create the array.


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r311063 - [analyzer] Add support for reference counting of parameters on the callee side

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Wed Aug 16 21:19:07 2017
New Revision: 311063

URL: http://llvm.org/viewvc/llvm-project?rev=311063=rev
Log:
[analyzer] Add support for reference counting of parameters on the callee side

This commit adds the functionality of performing reference counting on the
callee side for Integer Set Library (ISL) to Clang Static Analyzer's
RetainCountChecker.

Reference counting on the callee side can be extensively used to perform
debugging within a function (For example: Finding leaks on error paths).

Patch by Malhar Thakkar!

Differential Revision: https://reviews.llvm.org/D36441

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
cfe/trunk/test/Analysis/retain-release-inline.m

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp?rev=311063=311062=311063=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp Wed Aug 16 
21:19:07 2017
@@ -462,6 +462,7 @@ private:
   ArgEffect getDefaultArgEffect() const { return DefaultArgEffect; }
 
   friend class RetainSummaryManager;
+  friend class RetainCountChecker;
 };
 } // end anonymous namespace
 
@@ -1319,6 +1320,13 @@ static bool isTrustedReferenceCountImple
   return hasRCAnnotation(FD, "rc_ownership_trusted_implementation");
 }
 
+static bool isGeneralizedObjectRef(QualType Ty) {
+  if (Ty.getAsString().substr(0, 4) == "isl_")
+return true;
+  else
+return false;
+}
+
 
//===--===//
 // Summary creation for Selectors.
 
//===--===//
@@ -1848,6 +1856,15 @@ namespace {
 
   class CFRefLeakReport : public CFRefReport {
 const MemRegion* AllocBinding;
+const Stmt *AllocStmt;
+
+// Finds the function declaration where a leak warning for the parameter 
'sym' should be raised.
+void deriveParamLocation(CheckerContext , SymbolRef sym);
+// Finds the location where a leak warning for 'sym' should be raised.
+void deriveAllocLocation(CheckerContext , SymbolRef sym);
+// Produces description of a leak warning which is printed on the console.
+void createDescription(CheckerContext , bool GCEnabled, bool 
IncludeAllocationLine);
+
   public:
 CFRefLeakReport(CFRefBug , const LangOptions , bool GCEnabled,
 const SummaryLogTy , ExplodedNode *n, SymbolRef sym,
@@ -2427,13 +2444,25 @@ CFRefLeakReportVisitor::getEndPath(BugRe
   return llvm::make_unique(L, os.str());
 }
 
-CFRefLeakReport::CFRefLeakReport(CFRefBug , const LangOptions ,
- bool GCEnabled, const SummaryLogTy ,
- ExplodedNode *n, SymbolRef sym,
- CheckerContext ,
- bool IncludeAllocationLine)
-  : CFRefReport(D, LOpts, GCEnabled, Log, n, sym, false) {
+void CFRefLeakReport::deriveParamLocation(CheckerContext , SymbolRef sym) {
+  const SourceManager& SMgr = Ctx.getSourceManager();
+
+  if (!sym->getOriginRegion())
+return;
 
+  auto *Region = dyn_cast(sym->getOriginRegion());
+  if (Region) {
+const Decl *PDecl = Region->getDecl();
+if (PDecl && isa(PDecl)) {
+  PathDiagnosticLocation ParamLocation = 
PathDiagnosticLocation::create(PDecl, SMgr);
+  Location = ParamLocation;
+  UniqueingLocation = ParamLocation;
+  UniqueingDecl = Ctx.getLocationContext()->getDecl();
+}
+  }
+}
+
+void CFRefLeakReport::deriveAllocLocation(CheckerContext ,SymbolRef sym) {
   // Most bug reports are cached at the location where they occurred.
   // With leaks, we want to unique them by the location where they were
   // allocated, and only report a single path.  To do this, we need to find
@@ -2457,8 +2486,12 @@ CFRefLeakReport::CFRefLeakReport(CFRefBu
   // FIXME: This will crash the analyzer if an allocation comes from an
   // implicit call (ex: a destructor call).
   // (Currently there are no such allocations in Cocoa, though.)
-  const Stmt *AllocStmt = PathDiagnosticLocation::getStmt(AllocNode);
-  assert(AllocStmt && "Cannot find allocation statement");
+  AllocStmt = PathDiagnosticLocation::getStmt(AllocNode);
+
+  if (!AllocStmt) {
+AllocBinding = nullptr;
+return;
+  }
 
   PathDiagnosticLocation AllocLocation =
 PathDiagnosticLocation::createBegin(AllocStmt, SMgr,
@@ -2469,8 +2502,10 @@ CFRefLeakReport::CFRefLeakReport(CFRefBu
   // leaks should be uniqued on the allocation site.
   UniqueingLocation = AllocLocation;
   UniqueingDecl = AllocNode->getLocationContext()->getDecl();
+}
 
-  // Fill in the description of the bug.
+void CFRefLeakReport::createDescription(CheckerContext , bool GCEnabled, 
bool 

r309968 - [Analyzer] Add support for displaying cross-file diagnostic paths in HTML output

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Thu Aug  3 11:12:22 2017
New Revision: 309968

URL: http://llvm.org/viewvc/llvm-project?rev=309968=rev
Log:
[Analyzer] Add support for displaying cross-file diagnostic paths in HTML output

This change adds support for cross-file diagnostic paths in html output. If the
diagnostic path is not cross-file, there is no change in the output.

Patch by Vlad Tsyrklevich!

Differential Revision: https://reviews.llvm.org/D30406

Added:
cfe/trunk/test/Analysis/html-diag-singlefile.c
cfe/trunk/test/Analysis/html-diag-singlefile.h
  - copied, changed from r309960, 
cfe/trunk/test/Analysis/diagnostics/diag-cross-file-boundaries.h
cfe/trunk/test/Analysis/html-diags-analyze-headers.c
cfe/trunk/test/Analysis/html-diags-analyze-headers.h
cfe/trunk/test/Coverage/html-multifile-diagnostics.c
  - copied, changed from r309960, cfe/trunk/test/Coverage/html-diagnostics.c
cfe/trunk/test/Coverage/html-multifile-diagnostics.h
Removed:
cfe/trunk/test/Analysis/diagnostics/diag-cross-file-boundaries.c
cfe/trunk/test/Analysis/diagnostics/diag-cross-file-boundaries.h
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/Analyses.def
cfe/trunk/lib/Rewrite/HTMLRewrite.cpp
cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
cfe/trunk/test/Analysis/html-diags-multifile.c
cfe/trunk/test/Analysis/html-diags.c
cfe/trunk/test/Coverage/html-diagnostics.c
cfe/trunk/www/analyzer/open_projects.html

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/Analyses.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/Analyses.def?rev=309968=309967=309968=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/Analyses.def (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/Analyses.def Thu Aug  3 
11:12:22 2017
@@ -28,9 +28,10 @@ ANALYSIS_CONSTRAINTS(Z3Constraints, "z3"
 #define ANALYSIS_DIAGNOSTICS(NAME, CMDFLAG, DESC, CREATEFN)
 #endif
 
-ANALYSIS_DIAGNOSTICS(HTML,  "html",  "Output analysis results using HTML",   
createHTMLDiagnosticConsumer)
+ANALYSIS_DIAGNOSTICS(HTML, "html", "Output analysis results using HTML", 
createHTMLDiagnosticConsumer)
+ANALYSIS_DIAGNOSTICS(HTML_SINGLE_FILE, "html-single-file", "Output analysis 
results using HTML (not allowing for multi-file bugs)", 
createHTMLSingleFileDiagnosticConsumer)
 ANALYSIS_DIAGNOSTICS(PLIST, "plist", "Output analysis results using Plists", 
createPlistDiagnosticConsumer)
-ANALYSIS_DIAGNOSTICS(PLIST_MULTI_FILE, "plist-multi-file", "Output analysis 
results using Plists (allowing for mult-file bugs)", 
createPlistMultiFileDiagnosticConsumer)
+ANALYSIS_DIAGNOSTICS(PLIST_MULTI_FILE, "plist-multi-file", "Output analysis 
results using Plists (allowing for multi-file bugs)", 
createPlistMultiFileDiagnosticConsumer)
 ANALYSIS_DIAGNOSTICS(PLIST_HTML, "plist-html", "Output analysis results using 
HTML wrapped with Plists", createPlistHTMLDiagnosticConsumer)
 ANALYSIS_DIAGNOSTICS(TEXT, "text", "Text output of analysis results", 
createTextPathDiagnosticConsumer)
 

Modified: cfe/trunk/lib/Rewrite/HTMLRewrite.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Rewrite/HTMLRewrite.cpp?rev=309968=309967=309968=diff
==
--- cfe/trunk/lib/Rewrite/HTMLRewrite.cpp (original)
+++ cfe/trunk/lib/Rewrite/HTMLRewrite.cpp Thu Aug  3 11:12:22 2017
@@ -289,6 +289,11 @@ void html::AddHeaderFooterInternalBuilti
   " body { color:#00; background-color:#ff }\n"
   " body { font-family:Helvetica, sans-serif; font-size:10pt }\n"
   " h1 { font-size:14pt }\n"
+  " .FileName { margin-top: 5px; margin-bottom: 5px; display: inline; }\n"
+  " .FileNav { margin-left: 5px; margin-right: 5px; display: inline; }\n"
+  " .FileNav a { text-decoration:none; font-size: larger; }\n"
+  " .divider { margin-top: 30px; margin-bottom: 30px; height: 15px; }\n"
+  " .divider { background-color: gray; }\n"
   " .code { border-collapse:collapse; width:100%; }\n"
   " .code { font-family: \"Monospace\", monospace; font-size:10pt }\n"
   " .code { line-height: 1.2em }\n"

Modified: cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp?rev=309968=309967=309968=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp Thu Aug  3 11:12:22 
2017
@@ -44,8 +44,12 @@ class HTMLDiagnostics : public PathDiagn
   bool createdDir, noDir;
   const Preprocessor 
   AnalyzerOptions 
+  const bool SupportsCrossFileDiagnostics;
 public:
-  HTMLDiagnostics(AnalyzerOptions , const std::string& prefix, 
const Preprocessor );
+  HTMLDiagnostics(AnalyzerOptions ,
+  

r308990 - [analyzer] Add diagnostic text for generalized refcount annotations.

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Tue Jul 25 10:17:09 2017
New Revision: 308990

URL: http://llvm.org/viewvc/llvm-project?rev=308990=rev
Log:
[analyzer] Add diagnostic text for generalized refcount annotations.

Add a 'Generalized' object kind to the retain-count checker and suitable
generic diagnostic text for retain-count diagnostics involving those objects.

For now the object kind is introduced in summaries by 'annotate' attributes.
Once we have more experience with these annotations we will propose explicit
attributes.

Patch by Malhar Thakkar!

Differential Revision: https://reviews.llvm.org/D35613

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
cfe/trunk/test/Analysis/retain-release-inline.m
cfe/trunk/test/Analysis/retain-release.m

Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h?rev=308990=308989=308990=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/ObjCRetainCount.h Tue Jul 
25 10:17:09 2017
@@ -145,9 +145,11 @@ public:
 /// Indicates that the tracked object is an Objective-C object.
 ObjC,
 /// Indicates that the tracked object could be a CF or Objective-C object.
-AnyObj
+AnyObj,
+/// Indicates that the tracked object is a generalized object.
+Generalized
   };
-  
+
 private:
   Kind K;
   ObjKind O;

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp?rev=308990=308989=308990=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp Tue Jul 25 
10:17:09 2017
@@ -1340,6 +1340,8 @@ RetainSummaryManager::getRetEffectFromAn
 
   if (D->hasAttr())
 return RetEffect::MakeOwned(RetEffect::CF);
+  else if (hasRCAnnotation(D, "rc_ownership_returns_retained"))
+return RetEffect::MakeOwned(RetEffect::Generalized);
 
   if (D->hasAttr())
 return RetEffect::MakeNotOwned(RetEffect::CF);
@@ -1363,9 +1365,11 @@ RetainSummaryManager::updateSummaryFromA
 const ParmVarDecl *pd = *pi;
 if (pd->hasAttr())
   Template->addArg(AF, parm_idx, DecRefMsg);
-else if (pd->hasAttr())
+else if (pd->hasAttr() ||
+ hasRCAnnotation(pd, "rc_ownership_consumed"))
   Template->addArg(AF, parm_idx, DecRef);
-else if (pd->hasAttr()) {
+else if (pd->hasAttr() ||
+ hasRCAnnotation(pd, "rc_ownership_returns_retained")) {
   QualType PointeeTy = pd->getType()->getPointeeType();
   if (!PointeeTy.isNull())
 if (coreFoundation::isCFObjectRef(PointeeTy))
@@ -1999,17 +2003,15 @@ CFRefReportVisitor::VisitNode(const Expl
   }
 
   if (CurrV.getObjKind() == RetEffect::CF) {
-if (Sym->getType().isNull()) {
-  os << " returns a Core Foundation object with a ";
-} else {
-  os << " returns a Core Foundation object of type "
- << Sym->getType().getAsString() << " with a ";
-}
-  }
-  else {
+os << " returns a Core Foundation object of type "
+   << Sym->getType().getAsString() << " with a ";
+  } else if (CurrV.getObjKind() == RetEffect::Generalized) {
+os << " returns an object of type " << Sym->getType().getAsString()
+   << " with a ";
+  } else {
 assert (CurrV.getObjKind() == RetEffect::ObjC);
 QualType T = Sym->getType();
-if (T.isNull() || !isa(T)) {
+if (!isa(T)) {
   os << " returns an Objective-C object with a ";
 } else {
   const ObjCObjectPointerType *PT = cast(T);

Modified: cfe/trunk/test/Analysis/retain-release-inline.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/retain-release-inline.m?rev=308990=308989=308990=diff
==
--- cfe/trunk/test/Analysis/retain-release-inline.m (original)
+++ cfe/trunk/test/Analysis/retain-release-inline.m Tue Jul 25 10:17:09 2017
@@ -299,7 +299,7 @@ void test_neg() {
   bar(s);
 }
 
-__attribute__((cf_returns_retained)) isl_basic_map 
*isl_basic_map_cow(__attribute__((cf_consumed)) isl_basic_map *bmap);
+__attribute__((annotate("rc_ownership_returns_retained"))) isl_basic_map 
*isl_basic_map_cow(__attribute__((annotate("rc_ownership_consumed"))) 
isl_basic_map *bmap);
 void free(void *);
 
 // As 'isl_basic_map_free' is annotated with 
'rc_ownership_trusted_implementation', RetainCountChecker trusts its
@@ -307,7 +307,7 @@ void free(void *);
 // a leak 

r308416 - [analyzer] Add annotation attribute to trust retain count implementation

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Tue Jul 18 21:10:44 2017
New Revision: 308416

URL: http://llvm.org/viewvc/llvm-project?rev=308416=rev
Log:
[analyzer] Add annotation attribute to trust retain count implementation

Add support to the retain-count checker for an annotation indicating that a
function's implementation should be trusted by the retain count checker.
Functions with these attributes will not be inlined and the arguments will
be treating as escaping.

Adding this annotation avoids spurious diagnostics when the implementation of
a reference counting operation is visible but the analyzer can't reason
precisely about the ref count.

Patch by Malhar Thakkar!

Differential Revision: https://reviews.llvm.org/D34937

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
cfe/trunk/test/Analysis/retain-release-inline.m

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp?rev=308416=308415=308416=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp Tue Jul 18 
21:10:44 2017
@@ -1304,6 +1304,21 @@ RetainSummaryManager::getCFSummaryGetRul
   DoNothing, DoNothing);
 }
 
+/// Returns true if the declaration 'D' is annotated with 'rcAnnotation'.
+static bool hasRCAnnotation(const Decl *D, StringRef rcAnnotation) {
+  for (const auto *Ann : D->specific_attrs()) {
+if (Ann->getAnnotation() == rcAnnotation)
+  return true;
+  }
+  return false;
+}
+
+/// Returns true if the function declaration 'FD' contains
+/// 'rc_ownership_trusted_implementation' annotate attribute.
+static bool isTrustedReferenceCountImplementation(const FunctionDecl *FD) {
+  return hasRCAnnotation(FD, "rc_ownership_trusted_implementation");
+}
+
 
//===--===//
 // Summary creation for Selectors.
 
//===--===//
@@ -3380,6 +3395,9 @@ bool RetainCountChecker::evalCall(const
 
   // See if it's one of the specific functions we know how to eval.
   bool canEval = false;
+  // See if the function has 'rc_ownership_trusted_implementation'
+  // annotate attribute. If it does, we will not inline it.
+  bool hasTrustedImplementationAnnotation = false;
 
   QualType ResultTy = CE->getCallReturnType(C.getASTContext());
   if (ResultTy->isObjCIdType()) {
@@ -3395,6 +3413,11 @@ bool RetainCountChecker::evalCall(const
 cocoa::isRefType(ResultTy, "CV", FName)) {
   canEval = isRetain(FD, FName) || isAutorelease(FD, FName) ||
 isMakeCollectable(FD, FName);
+} else {
+  if (FD->getDefinition()) {
+canEval = isTrustedReferenceCountImplementation(FD->getDefinition());
+hasTrustedImplementationAnnotation = canEval;
+  }
 }
   }
 
@@ -3404,8 +3427,11 @@ bool RetainCountChecker::evalCall(const
   // Bind the return value.
   const LocationContext *LCtx = C.getLocationContext();
   SVal RetVal = state->getSVal(CE->getArg(0), LCtx);
-  if (RetVal.isUnknown()) {
-// If the receiver is unknown, conjure a return value.
+  if (RetVal.isUnknown() ||
+  (hasTrustedImplementationAnnotation && !ResultTy.isNull())) {
+// If the receiver is unknown or the function has
+// 'rc_ownership_trusted_implementation' annotate attribute, conjure a
+// return value.
 SValBuilder  = C.getSValBuilder();
 RetVal = SVB.conjureSymbolVal(nullptr, CE, LCtx, ResultTy, C.blockCount());
   }
@@ -3421,8 +3447,9 @@ bool RetainCountChecker::evalCall(const
   Binding = getRefBinding(state, Sym);
 
 // Invalidate the argument region.
-state = state->invalidateRegions(ArgRegion, CE, C.blockCount(), LCtx,
- /*CausesPointerEscape*/ false);
+state = state->invalidateRegions(
+ArgRegion, CE, C.blockCount(), LCtx,
+/*CausesPointerEscape*/ hasTrustedImplementationAnnotation);
 
 // Restore the refcount status of the argument.
 if (Binding)

Modified: cfe/trunk/test/Analysis/retain-release-inline.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/retain-release-inline.m?rev=308416=308415=308416=diff
==
--- cfe/trunk/test/Analysis/retain-release-inline.m (original)
+++ cfe/trunk/test/Analysis/retain-release-inline.m Tue Jul 18 21:10:44 2017
@@ -12,7 +12,7 @@
 //
 // It includes the basic definitions for the test cases below.
 
//===--===//
-
+#define NULL 0
 typedef unsigned int __darwin_natural_t;
 typedef unsigned long uintptr_t;
 typedef unsigned int uint32_t;
@@ -267,6 +267,10 @@ typedef NSUInteger 

r308242 - [analyzer] Add missing documentation for static analyzer checkers

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Mon Jul 17 17:34:57 2017
New Revision: 308242

URL: http://llvm.org/viewvc/llvm-project?rev=308242=rev
Log:
[analyzer] Add missing documentation for static analyzer checkers

Some checks did not have documentation in the www/analyzer/ folder and also
some alpha checks became non-alpha.

Patch by Dominik Szabó!

Differential Revision: https://reviews.llvm.org/D33645

Modified:
cfe/trunk/www/analyzer/alpha_checks.html
cfe/trunk/www/analyzer/available_checks.html
cfe/trunk/www/analyzer/implicit_checks.html

Modified: cfe/trunk/www/analyzer/alpha_checks.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/analyzer/alpha_checks.html?rev=308242=308241=308242=diff
==
--- cfe/trunk/www/analyzer/alpha_checks.html (original)
+++ cfe/trunk/www/analyzer/alpha_checks.html Mon Jul 17 17:34:57 2017
@@ -24,6 +24,7 @@ keep them from being on by default. They
 Bug reports are welcome but will likely not be investigated for some time.
 Patches welcome!
 
+Clone Alpha Checkers
 Core Alpha Checkers
 C++ Alpha Checkers
 Variable Argument Alpha Checkers
@@ -33,6 +34,38 @@ Patches welcome!
 Unix Alpha Checkers
 
 
+
+
+Clone Alpha Checkers
+
+
+Name, DescriptionExample
+
+
+
+alpha.clone.CloneChecker
+(C, C++, ObjC)
+Reports similar pieces of code.
+
+
+void log();
+
+int max(int a, int b) { // warn
+  log();
+  if (a > b)
+return a;
+  return b;
+}
+
+int maxClone(int x, int y) { // similar code here
+  log();
+  if (x > y)
+return x;
+  return y;
+}
+
+
+
 
 Core Alpha Checkers
 
@@ -53,6 +86,28 @@ void test() {
 
 
 
+alpha.core.CallAndMessageUnInitRefArg
+(C, C++)
+Check for uninitialized arguments in function calls and Objective-C 
+message expressions.
+
+
+void test(void) {
+  int t;
+  int  = t;
+  int  = p;
+  int  = s;
+  foo(q); // warn
+}
+
+
+void test(void) {
+  int x;
+  foo(); // warn
+}
+
+
+
 alpha.core.CastSize
 (C)
 Check when casting a malloc'ed type T, whether the size is a multiple of the 
@@ -91,6 +146,47 @@ void test(int *p) {
 
 
 
+alpha.core.Conversion
+(C, C++, ObjC)
+Loss of sign or precision in implicit conversions
+
+
+void test(unsigned U, signed S) {
+  if (S > 10) {
+if (U < S) {
+}
+  }
+  if (S < -10) {
+if (U < S) { // warn (loss of sign)
+}
+  }
+}
+
+
+void test() {
+  long long A = 1LL << 60;
+  short X = A; // warn (loss of precision)
+}
+
+
+
+
+alpha.core.DynamicTypeChecker
+(ObjC)
+Check for cases where the dynamic and the static type of an 
+object are unrelated.
+
+
+id date = [NSDate date];
+
+// Warning: Object has a dynamic type 'NSDate *' which is 
+// incompatible with static type 'NSNumber *'"
+NSNumber *number = date;
+[number doubleValue];
+
+
+
+
 alpha.core.FixedAddr
 (C)
 Check for assignment of a fixed address to a pointer.
@@ -178,6 +274,21 @@ int test(struct s *p) {
 }
 
 
+
+
+alpha.core.TestAfterDivZero
+(C, C++, ObjC)
+Check for division by variable that is later compared against 0. 
+Either the comparison is useless or there is division by zero.
+
+
+
+void test(int x) {
+  var = 77 / x;
+  if (x == 0) { } // warn 
+}
+
+
 
 
 
@@ -188,19 +299,6 @@ int test(struct s *p) {
 
 
 
-alpha.cplusplus.NewDeleteLeaks
-(C++)
-Check for memory leaks. Traces memory managed by new/
-delete.
-
-
-void test() {
-  int *p = new int;
-} // warn
-
-
-
-
 alpha.cplusplus.VirtualCall
 (C++)
 Check virtual member function calls during construction or 
@@ -345,66 +443,6 @@ void test(id x) {
 
 
 
-alpha.osx.cocoa.Dealloc
-(ObjC)
-Warn about Objective-C classes that lack a correct implementation 
-of -dealloc.
-
-
-
-@interface MyObject : NSObject {
-  id _myproperty;  
-}
-@end
-
-@implementation MyObject // warn: lacks 'dealloc'
-@end
-
-
-@interface MyObject : NSObject {}
-@property(assign) id myproperty;
-@end
-
-@implementation MyObject // warn: does not send 'dealloc' to super
-- (void)dealloc {
-  self.myproperty = 0;
-}
-@end
-
-
-@interface MyObject : NSObject {
-  id _myproperty;
-}
-@property(retain) id myproperty;
-@end
-
-@implementation MyObject
-@synthesize myproperty = _myproperty;
-  // warn: var was retained but wasn't released
-- (void)dealloc {
-  [super dealloc];
-}
-@end
-
-
-@interface MyObject : NSObject {
-  id _myproperty;
-}
-@property(assign) id myproperty;
-@end
-
-@implementation MyObject
-@synthesize myproperty = _myproperty;
-  // warn: var wasn't retained but was released
-- (void)dealloc {
-  [_myproperty release];
-  [super dealloc];
-}
-@end
-
-
-
-
 alpha.osx.cocoa.DirectIvarAssignment
 (ObjC)
 Check that Objective C properties follow the following rule: the property 
@@ -501,6 +539,32 @@ invalidatable instance variables.<
 @end
 
 
+
+
+alpha.osx.cocoa.localizability.PluralMisuseChecker
+(ObjC)
+Warns against using one vs. many plural pattern in code 
+when generating localized strings.
+
+
+
+NSString *reminderText = 
+  NSLocalizedString(@"None", @"Indicates no reminders");
+if (reminderCount == 1) {
+  // 

r296646 - [analyzer] pr32088: Don't destroy the temporary if its initializer causes return.

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Wed Mar  1 11:48:39 2017
New Revision: 296646

URL: http://llvm.org/viewvc/llvm-project?rev=296646=rev
Log:
[analyzer] pr32088: Don't destroy the temporary if its initializer causes 
return.

In the following code involving GNU statement-expression extension:
  struct S {
~S();
  };

  void foo() {
const S  = ({ return; S(); });
  }
function 'foo()' returns before reference x is initialized. We shouldn't call
the destructor for the temporary object lifetime-extended by 'x' in this case,
because the object never gets constructed in the first place.

The real problem is probably in the CFG somewhere, so this is a quick-and-dirty
hotfix rather than the perfect solution.

A patch by Artem Dergachev!

rdar://problem/30759076

Differential Revision: https://reviews.llvm.org/D30499

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/test/Analysis/temporaries.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=296646=296645=296646=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Wed Mar  1 11:48:39 2017
@@ -615,7 +615,15 @@ void ExprEngine::ProcessAutomaticObjDtor
   const MemRegion *Region = dest.castAs().getRegion();
 
   if (varType->isReferenceType()) {
-Region = state->getSVal(Region).getAsRegion()->getBaseRegion();
+const MemRegion *ValueRegion = state->getSVal(Region).getAsRegion();
+if (!ValueRegion) {
+  // FIXME: This should not happen. The language guarantees a presence
+  // of a valid initializer here, so the reference shall not be undefined.
+  // It seems that we're calling destructors over variables that
+  // were not initialized yet.
+  return;
+}
+Region = ValueRegion->getBaseRegion();
 varType = cast(Region)->getValueType();
   }
 

Modified: cfe/trunk/test/Analysis/temporaries.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/temporaries.cpp?rev=296646=296645=296646=diff
==
--- cfe/trunk/test/Analysis/temporaries.cpp (original)
+++ cfe/trunk/test/Analysis/temporaries.cpp Wed Mar  1 11:48:39 2017
@@ -493,3 +493,13 @@ namespace PR16629 {
 clang_analyzer_eval(x == 47); // expected-warning{{TRUE}}
   }
 }
+
+namespace PR32088 {
+  void testReturnFromStmtExprInitializer() {
+// We shouldn't try to destroy the object pointed to by `obj' upon return.
+const NonTrivial  = ({
+  return; // no-crash
+  NonTrivial(42);
+});
+  }
+}


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r296562 - [Analyzer] Fix crash in ObjCPropertyChecker on protocol property

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Tue Feb 28 19:47:37 2017
New Revision: 296562

URL: http://llvm.org/viewvc/llvm-project?rev=296562=rev
Log:
[Analyzer] Fix crash in ObjCPropertyChecker on protocol property

Fix a crash in the ObjCPropertyChecker when analyzing a 'copy' property of an
NSMutable* type in a protocol.

rdar://problem/30766684

Differential Revision: https://reviews.llvm.org/D30482

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCPropertyChecker.cpp
cfe/trunk/test/Analysis/ObjCPropertiesSyntaxChecks.m

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCPropertyChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCPropertyChecker.cpp?rev=296562=296561=296562=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCPropertyChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCPropertyChecker.cpp Tue Feb 28 
19:47:37 2017
@@ -58,8 +58,7 @@ void ObjCPropertyChecker::checkCopyMutab
   if (const ObjCInterfaceDecl *IntD =
   dyn_cast(D->getDeclContext())) {
 ImplD = IntD->getImplementation();
-  } else {
-const ObjCCategoryDecl *CatD = cast(D->getDeclContext());
+  } else if (auto *CatD = dyn_cast(D->getDeclContext())) {
 ImplD = CatD->getClassInterface()->getImplementation();
   }
 

Modified: cfe/trunk/test/Analysis/ObjCPropertiesSyntaxChecks.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/ObjCPropertiesSyntaxChecks.m?rev=296562=296561=296562=diff
==
--- cfe/trunk/test/Analysis/ObjCPropertiesSyntaxChecks.m (original)
+++ cfe/trunk/test/Analysis/ObjCPropertiesSyntaxChecks.m Tue Feb 28 19:47:37 
2017
@@ -59,3 +59,10 @@
 @interface IWithoutImpl : NSObject {}
 @property(copy) NSMutableString *mutableStr; // no-warning
 @end
+
+@protocol SomeProtocol
+// Don't warn on protocol properties because it is possible to
+// conform to them correctly; it is only synthesized setters that
+// that are definitely incorrect.
+@property (copy) NSMutableString *myProp; // no-crash // no-warning
+@end


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r292800 - [analyzer] Fix memory space of static locals seen from nested blocks.

[Devin Coughlin via cfe-commits]

FYI, I reverted r292800 from trunk in r292874. It was causing our internal 
validation bots to have false positives whenever a static local was 
dereferenced/passed to a nonnull function in a block evaluated at the top level.

Devin


> On Jan 23, 2017, at 4:19 PM, Hans Wennborg  wrote:
> 
> Merged in r292858.
> 
> Thanks,
> Hans
> 
> On Mon, Jan 23, 2017 at 4:15 PM, Anna Zaks  wrote:
>> Yes, ok to merge!
>> Thank you.
>> 
>> Sent from my iPhone
>> 
>>> On Jan 23, 2017, at 1:50 PM, Hans Wennborg  wrote:
>>> 
>>> Sounds good to me.
>>> 
>>> Anna, you're the code owner here. Ok to merge this?
>>> 
>>> Thanks,
>>> Hans
>>> 
 On Mon, Jan 23, 2017 at 10:37 AM, Artem Dergachev  
 wrote:
 Hans,
 
 Could we merge this one into the 4.0.0 release branch? It's a recent bugfix
 for the analyzer.
 
 Thanks,
 Artem.
 
 
 
> On 1/23/17 7:57 PM, Artem Dergachev via cfe-commits wrote:
> 
> Author: dergachev
> Date: Mon Jan 23 10:57:11 2017
> New Revision: 292800
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=292800=rev
> Log:
> [analyzer] Fix memory space of static locals seen from nested blocks.
> 
> When a block within a function accesses a function's static local
> variable,
> this local is captured by reference rather than copied to the heap.
> 
> Therefore this variable's memory space is known: StaticGlobalSpaceRegion.
> Used to be UnknownSpaceRegion, same as for stack locals.
> 
> Fixes a false positive in MacOSXAPIChecker.
> 
> rdar://problem/30105546
> Differential revision: https://reviews.llvm.org/D28946
> 
> Modified:
>cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
>cfe/trunk/test/Analysis/dispatch-once.m
> 
> Modified: cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp?rev=292800=292799=292800=diff
> 
> ==
> --- cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp Mon Jan 23 10:57:11
> 2017
> @@ -776,6 +776,22 @@ getStackOrCaptureRegionForDeclContext(co
>   return (const StackFrameContext *)nullptr;
> }
> +static CanQualType getBlockPointerType(const BlockDecl *BD, ASTContext
> ) {
> +  // FIXME: The fallback type here is totally bogus -- though it should
> +  // never be queried, it will prevent uniquing with the real
> +  // BlockCodeRegion. Ideally we'd fix the AST so that we always had a
> +  // signature.
> +  QualType T;
> +  if (const TypeSourceInfo *TSI = BD->getSignatureAsWritten())
> +T = TSI->getType();
> +  if (T.isNull())
> +T = C.VoidTy;
> +  if (!T->getAs())
> +T = C.getFunctionNoProtoType(T);
> +  T = C.getBlockPointerType(T);
> +  return C.getCanonicalType(T);
> +}
> +
> const VarRegion* MemRegionManager::getVarRegion(const VarDecl *D,
> const LocationContext
> *LC) {
>   const MemRegion *sReg = nullptr;
> @@ -803,7 +819,7 @@ const VarRegion* MemRegionManager::getVa
> sReg = getGlobalsRegion();
> }
> -  // Finally handle static locals.
> +  // Finally handle locals.
>   } else {
> // FIXME: Once we implement scope handling, we will need to properly
> lookup
> // 'D' to the proper LocationContext.
> @@ -816,9 +832,22 @@ const VarRegion* MemRegionManager::getVa
>   const StackFrameContext *STC = V.get();
> -if (!STC)
> -  sReg = getUnknownRegion();
> -else {
> +if (!STC) {
> +  if (D->isStaticLocal()) {
> +const CodeTextRegion *fReg = nullptr;
> +if (const auto *ND = dyn_cast(DC))
> +  fReg = getFunctionCodeRegion(ND);
> +else if (const auto *BD = dyn_cast(DC))
> +  fReg = getBlockCodeRegion(BD, getBlockPointerType(BD,
> getContext()),
> +LC->getAnalysisDeclContext());
> +assert(fReg && "Unable to determine code region for a static
> local!");
> +sReg = getGlobalsRegion(MemRegion::StaticGlobalSpaceRegionKind,
> fReg);
> +  } else {
> +// We're looking at a block-captured local variable, which may be
> either
> +// still local, or already moved to the heap. So we're not sure.
> +sReg = getUnknownRegion();
> +  }
> +} else {
>   if (D->hasLocalStorage()) {
> sReg = isa(D) || isa(D)
>? static_cast MemRegion*>(getStackArgumentsRegion(STC))
> @@ -831,22 +860,9 @@ const VarRegion* MemRegionManager::getVa
>  

r292874 - Revert "[analyzer] Fix memory space of static locals seen from nested blocks."

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Mon Jan 23 20:10:59 2017
New Revision: 292874

URL: http://llvm.org/viewvc/llvm-project?rev=292874=rev
Log:
Revert "[analyzer] Fix memory space of static locals seen from nested blocks."

This reverts commit r292800.

It is causing null pointer dereference false positives when a block that
captures a static local is evaluated at the top level.

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
cfe/trunk/test/Analysis/dispatch-once.m

Modified: cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp?rev=292874=292873=292874=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/MemRegion.cpp Mon Jan 23 20:10:59 2017
@@ -776,22 +776,6 @@ getStackOrCaptureRegionForDeclContext(co
   return (const StackFrameContext *)nullptr;
 }
 
-static CanQualType getBlockPointerType(const BlockDecl *BD, ASTContext ) {
-  // FIXME: The fallback type here is totally bogus -- though it should
-  // never be queried, it will prevent uniquing with the real
-  // BlockCodeRegion. Ideally we'd fix the AST so that we always had a
-  // signature.
-  QualType T;
-  if (const TypeSourceInfo *TSI = BD->getSignatureAsWritten())
-T = TSI->getType();
-  if (T.isNull())
-T = C.VoidTy;
-  if (!T->getAs())
-T = C.getFunctionNoProtoType(T);
-  T = C.getBlockPointerType(T);
-  return C.getCanonicalType(T);
-}
-
 const VarRegion* MemRegionManager::getVarRegion(const VarDecl *D,
 const LocationContext *LC) {
   const MemRegion *sReg = nullptr;
@@ -819,7 +803,7 @@ const VarRegion* MemRegionManager::getVa
 sReg = getGlobalsRegion();
 }
 
-  // Finally handle locals.
+  // Finally handle static locals.
   } else {
 // FIXME: Once we implement scope handling, we will need to properly lookup
 // 'D' to the proper LocationContext.
@@ -832,22 +816,9 @@ const VarRegion* MemRegionManager::getVa
 
 const StackFrameContext *STC = V.get();
 
-if (!STC) {
-  if (D->isStaticLocal()) {
-const CodeTextRegion *fReg = nullptr;
-if (const auto *ND = dyn_cast(DC))
-  fReg = getFunctionCodeRegion(ND);
-else if (const auto *BD = dyn_cast(DC))
-  fReg = getBlockCodeRegion(BD, getBlockPointerType(BD, getContext()),
-LC->getAnalysisDeclContext());
-assert(fReg && "Unable to determine code region for a static local!");
-sReg = getGlobalsRegion(MemRegion::StaticGlobalSpaceRegionKind, fReg);
-  } else {
-// We're looking at a block-captured local variable, which may be 
either
-// still local, or already moved to the heap. So we're not sure.
-sReg = getUnknownRegion();
-  }
-} else {
+if (!STC)
+  sReg = getUnknownRegion();
+else {
   if (D->hasLocalStorage()) {
 sReg = isa(D) || isa(D)
? static_cast(getStackArgumentsRegion(STC))
@@ -860,9 +831,22 @@ const VarRegion* MemRegionManager::getVa
   sReg = getGlobalsRegion(MemRegion::StaticGlobalSpaceRegionKind,
   
getFunctionCodeRegion(cast(STCD)));
 else if (const BlockDecl *BD = dyn_cast(STCD)) {
+  // FIXME: The fallback type here is totally bogus -- though it should
+  // never be queried, it will prevent uniquing with the real
+  // BlockCodeRegion. Ideally we'd fix the AST so that we always had a
+  // signature.
+  QualType T;
+  if (const TypeSourceInfo *TSI = BD->getSignatureAsWritten())
+T = TSI->getType();
+  if (T.isNull())
+T = getContext().VoidTy;
+  if (!T->getAs())
+T = getContext().getFunctionNoProtoType(T);
+  T = getContext().getBlockPointerType(T);
+
   const BlockCodeRegion *BTR =
-  getBlockCodeRegion(BD, getBlockPointerType(BD, getContext()),
- STC->getAnalysisDeclContext());
+getBlockCodeRegion(BD, C.getCanonicalType(T),
+   STC->getAnalysisDeclContext());
   sReg = getGlobalsRegion(MemRegion::StaticGlobalSpaceRegionKind,
   BTR);
 }

Modified: cfe/trunk/test/Analysis/dispatch-once.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dispatch-once.m?rev=292874=292873=292874=diff
==
--- cfe/trunk/test/Analysis/dispatch-once.m (original)
+++ cfe/trunk/test/Analysis/dispatch-once.m Mon Jan 23 20:10:59 2017
@@ -107,10 +107,3 @@ void test_block_var_from_outside_block()
   };
   dispatch_once(, ^{}); // expected-warning{{Call to 'dispatch_once' uses 
the block variable 'once' for the predicate value.}}
 }
-
-void 

r291635 - [analyzer] Fix crash in body farm for getter without implicit self.

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Tue Jan 10 19:02:34 2017
New Revision: 291635

URL: http://llvm.org/viewvc/llvm-project?rev=291635=rev
Log:
[analyzer] Fix crash in body farm for getter without implicit self.

Fix a crash in body farm when synthesizing a getter for a property
synthesized for a property declared in a protocol on a class extension
that shadows a declaration of the property in a category.

In this case, Sema doesn't fill in the implicit 'self' parameter for the getter
in the category, which leads to a crash when trying to synthesize the getter
for it.

To avoid the crash, skip getter synthesis in body farm if the self parameter is
not filled int.

rdar://problem/29938138

Modified:
cfe/trunk/lib/Analysis/BodyFarm.cpp
cfe/trunk/test/Analysis/properties.m

Modified: cfe/trunk/lib/Analysis/BodyFarm.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/BodyFarm.cpp?rev=291635=291634=291635=diff
==
--- cfe/trunk/lib/Analysis/BodyFarm.cpp (original)
+++ cfe/trunk/lib/Analysis/BodyFarm.cpp Tue Jan 10 19:02:34 2017
@@ -467,6 +467,8 @@ static Stmt *createObjCPropertyGetter(AS
   ASTMaker M(Ctx);
 
   const VarDecl *selfVar = Prop->getGetterMethodDecl()->getSelfDecl();
+  if (!selfVar)
+return nullptr;
 
   Expr *loadedIVar =
 M.makeObjCIvarRef(

Modified: cfe/trunk/test/Analysis/properties.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/properties.m?rev=291635=291634=291635=diff
==
--- cfe/trunk/test/Analysis/properties.m (original)
+++ cfe/trunk/test/Analysis/properties.m Tue Jan 10 19:02:34 2017
@@ -315,6 +315,32 @@ void testConsistencyAssign(Person *p) {
 }
 @end
 
+__attribute__((objc_root_class))
+@interface 
ClassWithPrivatePropertyInClassExtensionWithProtocolShadowingCategory
+@end
+
+@protocol HasStuff
+@property (nonatomic, readonly) int stuffProperty;
+@end
+
+@interface 
ClassWithPrivatePropertyInClassExtensionWithProtocolShadowingCategory (Private)
+@property (nonatomic, readonly) int stuffProperty;
+@end
+
+@interface 
ClassWithPrivatePropertyInClassExtensionWithProtocolShadowingCategory 
(Internal) 
+@end
+
+@interface 
ClassWithPrivatePropertyInClassExtensionWithProtocolShadowingCategory() 

+@end
+
+@implementation 
ClassWithPrivatePropertyInClassExtensionWithProtocolShadowingCategory
+@synthesize stuffProperty = _stuffProperty;
+
+-(void)foo {
+  (void)self.stuffProperty;
+}
+@end
+
 //--
 // Setter ivar invalidation.
 //--


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r291581 - [analyzer] Treat pointers to static member functions as function pointers

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Tue Jan 10 12:49:27 2017
New Revision: 291581

URL: http://llvm.org/viewvc/llvm-project?rev=291581=rev
Log:
[analyzer] Treat pointers to static member functions as function pointers

Sema treats pointers to static member functions as having function pointer
type, so treat treat them as function pointer values in the analyzer as well.
This prevents an assertion failure in SValBuilder::evalBinOp caused by code
that expects function pointers to be Locs (in contrast, PointerToMember values
are nonlocs).

Differential Revision: https://reviews.llvm.org/D28033

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
cfe/trunk/test/Analysis/pointer-to-member.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp?rev=291581=291580=291581=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp Tue Jan 10 12:49:27 2017
@@ -218,6 +218,18 @@ SValBuilder::getDerivedRegionValueSymbol
 }
 
 DefinedSVal SValBuilder::getMemberPointer(const DeclaratorDecl* DD) {
+  assert(!DD || isa(DD) || isa(DD));
+
+  if (auto *MD = dyn_cast_or_null(DD)) {
+// Sema treats pointers to static member functions as have function pointer
+// type, so return a function pointer for the method.
+// We don't need to play a similar trick for static member fields
+// because these are represented as plain VarDecls and not FieldDecls
+// in the AST.
+if (MD->isStatic())
+  return getFunctionPointer(MD);
+  }
+
   return nonloc::PointerToMember(DD);
 }
 

Modified: cfe/trunk/test/Analysis/pointer-to-member.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/pointer-to-member.cpp?rev=291581=291580=291581=diff
==
--- cfe/trunk/test/Analysis/pointer-to-member.cpp (original)
+++ cfe/trunk/test/Analysis/pointer-to-member.cpp Tue Jan 10 12:49:27 2017
@@ -77,7 +77,8 @@ bool testDereferencing() {
 namespace testPointerToMemberFunction {
   struct A {
 virtual int foo() { return 1; }
-int bar() { return 2;  }
+int bar() { return 2; }
+int static staticMemberFunction(int p) { return p + 1; };
   };
 
   struct B : public A {
@@ -111,11 +112,19 @@ namespace testPointerToMemberFunction {
 
 clang_analyzer_eval((APtr->*AFP)() == 3); // expected-warning{{TRUE}}
   }
+
+  void testPointerToStaticMemberCall() {
+int (*fPtr)(int) = ::staticMemberFunction;
+if (fPtr != 0) { // no-crash
+  clang_analyzer_eval(fPtr(2) == 3); // expected-warning{{TRUE}}
+}
+  }
 } // end of testPointerToMemberFunction namespace
 
 namespace testPointerToMemberData {
   struct A {
 int i;
+static int j;
   };
 
   void testPointerToMemberData() {
@@ -126,6 +135,13 @@ namespace testPointerToMemberData {
 a.*AMdPointer += 1;
 
 clang_analyzer_eval(a.i == 43); // expected-warning{{TRUE}}
+
+int *ptrToStaticField = ::j;
+if (ptrToStaticField != 0) {
+  *ptrToStaticField = 7;
+  clang_analyzer_eval(*ptrToStaticField == 7); // expected-warning{{TRUE}}
+  clang_analyzer_eval(A::j == 7); // expected-warning{{TRUE}}
+}
   }
 } // end of testPointerToMemberData namespace
 


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r290352 - [analyzer] Update GTestChecker to tighten API detection

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Thu Dec 22 11:52:57 2016
New Revision: 290352

URL: http://llvm.org/viewvc/llvm-project?rev=290352=rev
Log:
[analyzer] Update GTestChecker to tighten API detection

Update the GTestChecker to tighten up the API detection and make it
cleaner in response to post-commit feedback. Also add tests for when
temporary destructors are enabled to make sure we get the expected behavior
when inlining constructors for temporaries.

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp
cfe/trunk/test/Analysis/gtest.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp?rev=290352=290351=290352=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp Thu Dec 22 11:52:57 
2016
@@ -7,7 +7,7 @@
 //
 
//===--===//
 //
-// This checker models the behavior of un-inlined APIs from the the gtest
+// This checker models the behavior of un-inlined APIs from the gtest
 // unit-testing library to avoid false positives when using assertions from
 // that library.
 //
@@ -85,7 +85,7 @@ using namespace ento;
 // does not inline these since it does not yet reliably call temporary
 // destructors).
 //
-// This checker compensates for the missing inlining by propgagating the
+// This checker compensates for the missing inlining by propagating the
 // _success value across the bool and copy constructors so the assertion 
behaves
 // as expected.
 
@@ -102,7 +102,7 @@ public:
 
 private:
   void modelAssertionResultBoolConstructor(const CXXConstructorCall *Call,
-   CheckerContext ) const;
+   bool IsRef, CheckerContext ) 
const;
 
   void modelAssertionResultCopyConstructor(const CXXConstructorCall *Call,
CheckerContext ) const;
@@ -122,35 +122,25 @@ private:
 
 GTestChecker::GTestChecker() : AssertionResultII(nullptr), SuccessII(nullptr) 
{}
 
-/// Model a call to an un-inlined AssertionResult(boolean expression).
+/// Model a call to an un-inlined AssertionResult(bool) or
+/// AssertionResult(bool &, ...).
 /// To do so, constrain the value of the newly-constructed instance's 
'success_'
 /// field to be equal to the passed-in boolean value.
+///
+/// \param IsRef Whether the boolean parameter is a reference or not.
 void GTestChecker::modelAssertionResultBoolConstructor(
-const CXXConstructorCall *Call, CheckerContext ) const {
-  assert(Call->getNumArgs() > 0);
-
-  // Depending on the version of gtest the constructor can be either of:
-  //
-  // v1.7 and earlier:
-  //  AssertionResult(bool success)
-  //
-  // v1.8 and greater:
-  //  template 
-  //  AssertionResult(const T& success,
-  //  typename internal::EnableIf<
-  //  !internal::ImplicitlyConvertible::value>::type*)
+const CXXConstructorCall *Call, bool IsRef, CheckerContext ) const {
+  assert(Call->getNumArgs() >= 1 && Call->getNumArgs() <= 2);
 
+  ProgramStateRef State = C.getState();
   SVal BooleanArgVal = Call->getArgSVal(0);
-  if (Call->getDecl()->getParamDecl(0)->getType()->getAs()) {
-// We have v1.8+, so load the value from the reference.
+  if (IsRef) {
+// The argument is a reference, so load from it to get the boolean value.
 if (!BooleanArgVal.getAs())
   return;
 BooleanArgVal = C.getState()->getSVal(BooleanArgVal.castAs());
   }
 
-  ProgramStateRef State = C.getState();
-
   SVal ThisVal = Call->getCXXThisVal();
 
   SVal ThisSuccess = getAssertionResultSuccessFieldValue(
@@ -169,7 +159,7 @@ void GTestChecker::modelAssertionResultB
 /// 'success_' field.
 void GTestChecker::modelAssertionResultCopyConstructor(
 const CXXConstructorCall *Call, CheckerContext ) const {
-  assert(Call->getNumArgs() > 0);
+  assert(Call->getNumArgs() == 1);
 
   // The first parameter of the the copy constructor must be the other
   // instance to initialize this instances fields from.
@@ -206,22 +196,44 @@ void GTestChecker::checkPostCall(const C
   if (CtorParent->getIdentifier() != AssertionResultII)
 return;
 
-  if (CtorDecl->getNumParams() == 0)
-return;
-
+  unsigned ParamCount = CtorDecl->getNumParams();
 
-  // Call the appropriate modeling method based on the type of the first
-  // constructor parameter.
-  const ParmVarDecl *ParamDecl = CtorDecl->getParamDecl(0);
-  QualType ParamType = ParamDecl->getType();
-  if (CtorDecl->getNumParams() <= 2 &&
-  ParamType.getNonReferenceType()->getCanonicalTypeUnqualified() ==
-  C.getASTContext().BoolTy) {
-// The first parameter is either a boolean or reference to a 

r290143 - [analyzer] Add checker modeling gtest APIs.

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Mon Dec 19 16:50:31 2016
New Revision: 290143

URL: http://llvm.org/viewvc/llvm-project?rev=290143=rev
Log:
[analyzer] Add checker modeling gtest APIs.

gtest is a widely-used unit-testing API. It provides macros for unit test
assertions:

  ASSERT_TRUE(p != nullptr);

that expand into an if statement that constructs an object representing
the result of the assertion and returns when the assertion is false:

  if (AssertionResult gtest_ar_ = AssertionResult(p == nullptr))
  ;
  else
return ...;

Unfortunately, the analyzer does not model the effect of the constructor
precisely because (1) the copy constructor implementation is missing from the
the header (so it can't be inlined) and (2) the boolean-argument constructor
is constructed into a temporary (so the analyzer decides not to inline it since
it doesn't reliably call temporary destructors right now).

This results in false positives because the analyzer does not realize that the
the assertion must hold along the non-return path.

This commit addresses the false positives by explicitly modeling the effects
of the two un-inlined constructors on the AssertionResult state.

I've added a new package, "apiModeling", for these kinds of checkers that
model APIs but don't emit any diagnostics. I envision all the checkers in
this package always being on by default.

This addresses the false positives reported in PR30936.

Differential Revision: https://reviews.llvm.org/D27773

rdar://problem/22705813

Added:
cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp
cfe/trunk/test/Analysis/gtest.cpp
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/lib/Driver/Tools.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
cfe/trunk/test/Driver/analyzer-target-enabled-checkers.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td?rev=290143=290142=290143=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td Mon Dec 19 
16:50:31 2016
@@ -79,6 +79,13 @@ def LocalizabilityOptIn : Package<"local
 def MPI : Package<"mpi">, InPackage;
 
 def LLVM : Package<"llvm">;
+
+// The APIModeling package is for checkers that model APIs and don't perform
+// any diagnostics. These checkers are always turned on; this package is
+// intended for API modeling that is not controlled by the the target triple.
+def APIModeling : Package<"apiModeling">, Hidden;
+def GoogleAPIModeling : Package<"google">, InPackage;
+
 def Debug : Package<"debug">;
 
 def CloneDetectionAlpha : Package<"clone">, InPackage, Hidden;
@@ -643,6 +650,17 @@ def LLVMConventionsChecker : Checker<"Co
   HelpText<"Check code for LLVM codebase conventions">,
   DescFile<"LLVMConventionsChecker.cpp">;
 
+
+
+//===--===//
+// Checkers modeling Google APIs.
+//===--===//
+
+def GTestChecker : Checker<"GTest">,
+  InPackage,
+  HelpText<"Model gtest assertion APIs">,
+  DescFile<"GTestChecker.cpp">;
+
 
//===--===//
 // Debugging checkers (for analyzer development).
 
//===--===//

Modified: cfe/trunk/lib/Driver/Tools.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=290143=290142=290143=diff
==
--- cfe/trunk/lib/Driver/Tools.cpp (original)
+++ cfe/trunk/lib/Driver/Tools.cpp Mon Dec 19 16:50:31 2016
@@ -4263,6 +4263,7 @@ void Clang::ConstructJob(Compilation ,
 // Add default argument set.
 if (!Args.hasArg(options::OPT__analyzer_no_default_checks)) {
   CmdArgs.push_back("-analyzer-checker=core");
+  CmdArgs.push_back("-analyzer-checker=apiModeling");
 
 if (!IsWindowsMSVC) {
   CmdArgs.push_back("-analyzer-checker=unix");

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt?rev=290143=290142=290143=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt Mon Dec 19 16:50:31 
2016
@@ -37,6 +37,7 @@ add_clang_library(clangStaticAnalyzerChe
   ExprInspectionChecker.cpp
   FixedAddressChecker.cpp
   GenericTaintChecker.cpp
+  GTestChecker.cpp
   IdenticalExprChecker.cpp
   IvarInvalidationChecker.cpp
   LLVMConventionsChecker.cpp

Added: cfe/trunk/lib/StaticAnalyzer/Checkers/GTestChecker.cpp
URL: 

r290140 - [analyzer] Add sink after construction of temporary with no-return destructor.

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Mon Dec 19 16:23:22 2016
New Revision: 290140

URL: http://llvm.org/viewvc/llvm-project?rev=290140=rev
Log:
[analyzer] Add sink after construction of temporary with no-return destructor.

The analyzer's CFG currently doesn't have nodes for calls to temporary
destructors. This causes the analyzer to explore infeasible paths in which
a no-return destructor would have stopped exploration and so results in false
positives when no-return destructors are used to implement assertions.

To mitigate these false positives, this patch stops generates a sink after
evaluating a constructor on a temporary object that has a no-return destructor.
This results in a loss of coverage because the time at which the destructor is
called may be after the time of construction (especially for lifetime-extended
temporaries).

This addresses PR15599.

rdar://problem/29131566

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
cfe/trunk/test/Analysis/temporaries.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=290140=290139=290140=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Mon Dec 19 16:23:22 2016
@@ -346,6 +346,30 @@ void ExprEngine::VisitCXXConstructExpr(c
   defaultEvalCall(Bldr, *I, *Call);
   }
 
+  // If the CFG was contructed without elements for temporary destructors
+  // and the just-called constructor created a temporary object then
+  // stop exploration if the temporary object has a noreturn constructor.
+  // This can lose coverage because the destructor, if it were present
+  // in the CFG, would be called at the end of the full expression or
+  // later (for life-time extended temporaries) -- but avoids infeasible
+  // paths when no-return temporary destructors are used for assertions.
+  const AnalysisDeclContext *ADC = LCtx->getAnalysisDeclContext();
+  if (!ADC->getCFGBuildOptions().AddTemporaryDtors) {
+  const MemRegion *Target = Call->getCXXThisVal().getAsRegion();
+  if (Target && isa(Target) &&
+  Call->getDecl()->getParent()->isAnyDestructorNoReturn()) {
+
+  for (ExplodedNode *N : DstEvaluated) {
+Bldr.generateSink(CE, N, N->getState());
+  }
+
+  // There is no need to run the PostCall and PostStmtchecker
+  // callbacks because we just generated sinks on all nodes in th
+  // frontier.
+  return;
+}
+ }
+
   ExplodedNodeSet DstPostCall;
   getCheckerManager().runCheckersForPostCall(DstPostCall, DstEvaluated,
  *Call, *this);

Modified: cfe/trunk/test/Analysis/temporaries.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/temporaries.cpp?rev=290140=290139=290140=diff
==
--- cfe/trunk/test/Analysis/temporaries.cpp (original)
+++ cfe/trunk/test/Analysis/temporaries.cpp Mon Dec 19 16:23:22 2016
@@ -413,6 +413,32 @@ namespace destructors {
   value ? DefaultParam(42) : DefaultParam(42);
 }
   }
+#else // !TEMPORARY_DTORS
+
+// Test for fallback logic that conservatively stops exploration after
+// executing a temporary constructor for a class with a no-return destructor
+// when temporary destructors are not enabled in the CFG.
+
+  struct CtorWithNoReturnDtor {
+CtorWithNoReturnDtor() = default;
+
+~CtorWithNoReturnDtor() __attribute__((noreturn));
+  };
+
+  void testDefaultContructorWithNoReturnDtor() {
+CtorWithNoReturnDtor();
+clang_analyzer_warnIfReached();  // no-warning
+  }
+
+  void testLifeExtensionWithNoReturnDtor() {
+const CtorWithNoReturnDtor  = CtorWithNoReturnDtor();
+
+// This represents an (expected) loss of coverage, since the destructor
+// of the lifetime-exended temporary is executed at at the end of
+// scope.
+clang_analyzer_warnIfReached();  // no-warning
+  }
+
 #endif // TEMPORARY_DTORS
 }
 


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r290023 - [analyzer] UnixAPIChecker: Don't diagnose for functions in C++ namespaces

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Fri Dec 16 19:08:17 2016
New Revision: 290023

URL: http://llvm.org/viewvc/llvm-project?rev=290023=rev
Log:
[analyzer] UnixAPIChecker: Don't diagnose for functions in C++ namespaces

Update the UnixAPIChecker to not diagnose for calls to functions that
are declared in C++ namespaces. This avoids false positives when a
namespaced function has the same name as a Unix API.

This address PR28331.

Added:
cfe/trunk/test/Analysis/unix-api.cpp
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp?rev=290023=290022=290023=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp Fri Dec 16 
19:08:17 2016
@@ -427,6 +427,12 @@ void UnixAPIChecker::checkPreStmt(const
   if (!FD || FD->getKind() != Decl::Function)
 return;
 
+  // Don't treat functions in namespaces with the same name a Unix function
+  // as a call to the Unix function.
+  const DeclContext *NamespaceCtx = FD->getEnclosingNamespaceContext();
+  if (NamespaceCtx && isa(NamespaceCtx))
+return;
+
   StringRef FName = C.getCalleeName(FD);
   if (FName.empty())
 return;

Added: cfe/trunk/test/Analysis/unix-api.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/unix-api.cpp?rev=290023=auto
==
--- cfe/trunk/test/Analysis/unix-api.cpp (added)
+++ cfe/trunk/test/Analysis/unix-api.cpp Fri Dec 16 19:08:17 2016
@@ -0,0 +1,62 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.API -verify %s
+extern "C" {
+#ifndef O_RDONLY
+#define O_RDONLY 0
+#endif
+
+#ifndef NULL
+#define NULL ((void*) 0)
+#endif
+
+int open(const char *, int, ...);
+int close(int fildes);
+
+} // extern "C"
+
+namespace MyNameSpace {
+int open(const char *a, int b, int c, int d);
+}
+
+void unix_open(const char *path) {
+  int fd;
+  fd = open(path, O_RDONLY); // no-warning
+  if (fd > -1)
+close(fd);
+}
+
+void unix_open_misuse(const char *path) {
+  int fd;
+  int mode = 0x0;
+  fd = open(path, O_RDONLY, mode, NULL); // expected-warning{{Call to 'open' 
with more than 3 arguments}}
+  if (fd > -1)
+close(fd);
+}
+
+// Don't treat open() in namespaces as the POSIX open()
+void namespaced_open(const char *path) {
+  MyNameSpace::open("Hi", 2, 3, 4); // no-warning
+
+  using namespace MyNameSpace;
+
+  open("Hi", 2, 3, 4); // no-warning
+
+  int fd;
+  int mode = 0x0;
+  fd = ::open(path, O_RDONLY, mode, NULL); // expected-warning{{Call to 'open' 
with more than 3 arguments}}
+  if (fd > -1)
+close(fd);
+}
+
+class MyClass {
+public:
+  static int open(const char *a, int b, int c, int d);
+
+  int open(int a, int, int c, int d);
+};
+
+void class_qualified_open() {
+  MyClass::open("Hi", 2, 3, 4); // no-warning
+
+  MyClass mc;
+  mc.open(1, 2, 3, 4); // no-warning
+}


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r289970 - [analyzer] Fix crash in MallocChecker.

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Fri Dec 16 12:41:40 2016
New Revision: 289970

URL: http://llvm.org/viewvc/llvm-project?rev=289970=rev
Log:
[analyzer] Fix crash in MallocChecker.

Fix a crash in the MallocChecker when the extent size for the argument
to new[] is not known.

A patch by Abramo Bagnara and Dániel Krupp!

https://reviews.llvm.org/D27849

Differential Revision: https://reviews.llvm.org/D27849

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
cfe/trunk/test/Analysis/out-of-bounds-new.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=289970=289969=289970=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Fri Dec 16 12:41:40 
2016
@@ -1026,8 +1026,7 @@ ProgramStateRef MallocChecker::addExtent
   ASTContext  = C.getASTContext();
   CharUnits TypeSize = AstContext.getTypeSizeInChars(ElementType);
 
-  if (Optional DefinedSize =
-  ElementCount.getAs()) {
+  if (ElementCount.getAs()) {
 DefinedOrUnknownSVal Extent = Region->getExtent(svalBuilder);
 // size in Bytes = ElementCount*TypeSize
 SVal SizeInBytes = svalBuilder.evalBinOpNN(

Modified: cfe/trunk/test/Analysis/out-of-bounds-new.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/out-of-bounds-new.cpp?rev=289970=289969=289970=diff
==
--- cfe/trunk/test/Analysis/out-of-bounds-new.cpp (original)
+++ cfe/trunk/test/Analysis/out-of-bounds-new.cpp Fri Dec 16 12:41:40 2016
@@ -148,3 +148,9 @@ void test_dynamic_size(int s) {
   int *buf = new int[s];
   buf[0] = 1; // no-warning
 }
+//Tests complex arithmetic
+//in new expression
+void test_dynamic_size2(unsigned m,unsigned n){
+  unsigned *U = nullptr;
+  U = new unsigned[m + n + 1];
+}


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r289873 - [analyzer] Add a new SVal to support pointer-to-member operations.

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Thu Dec 15 15:27:06 2016
New Revision: 289873

URL: http://llvm.org/viewvc/llvm-project?rev=289873=rev
Log:
[analyzer] Add a new SVal to support pointer-to-member operations.

Add a new type of NonLoc SVal for C++ pointer-to-member operations. This SVal
supports both pointers to member functions and pointers to member data.

A patch by Kirill Romanenkov!

Differential Revision: https://reviews.llvm.org/D25475

Modified:

cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.def
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h
cfe/trunk/lib/StaticAnalyzer/Core/BasicValueFactory.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
cfe/trunk/lib/StaticAnalyzer/Core/SValBuilder.cpp
cfe/trunk/lib/StaticAnalyzer/Core/SVals.cpp
cfe/trunk/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
cfe/trunk/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
cfe/trunk/test/Analysis/pointer-to-member.cpp

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h?rev=289873=289872=289873=diff
==
--- 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h 
(original)
+++ 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h 
Thu Dec 15 15:27:06 2016
@@ -59,6 +59,29 @@ public:
   void Profile(llvm::FoldingSetNodeID& ID) { Profile(ID, store, region); }
 };
 
+class PointerToMemberData: public llvm::FoldingSetNode {
+  const DeclaratorDecl *D;
+  llvm::ImmutableList L;
+
+public:
+  PointerToMemberData(const DeclaratorDecl *D,
+  llvm::ImmutableList L)
+: D(D), L(L) {}
+
+  typedef llvm::ImmutableList::iterator iterator;
+  iterator begin() const { return L.begin(); }
+  iterator end() const { return L.end(); }
+
+  static void Profile(llvm::FoldingSetNodeID& ID, const DeclaratorDecl *D,
+  llvm::ImmutableList L);
+
+  void Profile(llvm::FoldingSetNodeID& ID) { Profile(ID, D, L); }
+  const DeclaratorDecl *getDeclaratorDecl() const {return D;}
+  llvm::ImmutableList getCXXBaseList() const {
+return L;
+  }
+};
+
 class BasicValueFactory {
   typedef llvm::FoldingSet
   APSIntSetTy;
@@ -71,8 +94,10 @@ class BasicValueFactory {
   void *PersistentSValPairs;
 
   llvm::ImmutableList::Factory SValListFactory;
+  llvm::ImmutableList::Factory CXXBaseListFactory;
   llvm::FoldingSet  CompoundValDataSet;
   llvm::FoldingSet LazyCompoundValDataSet;
+  llvm::FoldingSet PointerToMemberDataSet;
 
   // This is private because external clients should use the factory
   // method that takes a QualType.
@@ -81,7 +106,8 @@ class BasicValueFactory {
 public:
   BasicValueFactory(ASTContext , llvm::BumpPtrAllocator )
 : Ctx(ctx), BPAlloc(Alloc), PersistentSVals(nullptr),
-  PersistentSValPairs(nullptr), SValListFactory(Alloc) {}
+  PersistentSValPairs(nullptr), SValListFactory(Alloc),
+  CXXBaseListFactory(Alloc) {}
 
   ~BasicValueFactory();
 
@@ -172,14 +198,32 @@ public:
   const LazyCompoundValData *getLazyCompoundValData(const StoreRef ,
 const TypedValueRegion *region);
 
+  const PointerToMemberData *getPointerToMemberData(
+  const DeclaratorDecl *DD,
+  llvm::ImmutableList L);
+
   llvm::ImmutableList getEmptySValList() {
 return SValListFactory.getEmptyList();
   }
 
-  llvm::ImmutableList consVals(SVal X, llvm::ImmutableList L) {
+  llvm::ImmutableList prependSVal(SVal X, llvm::ImmutableList L) {
 return SValListFactory.add(X, L);
   }
 
+  llvm::ImmutableList getEmptyCXXBaseList() {
+return CXXBaseListFactory.getEmptyList();
+  }
+
+  llvm::ImmutableList prependCXXBase(
+  const CXXBaseSpecifier *CBS,
+  llvm::ImmutableList L) {
+return CXXBaseListFactory.add(CBS, L);
+  }
+
+  const clang::ento::PointerToMemberData *accumCXXBase(
+  llvm::iterator_range PathRange,
+  const nonloc::PointerToMember );
+
   const llvm::APSInt* evalAPSInt(BinaryOperator::Opcode Op,
  const llvm::APSInt& V1,
  const llvm::APSInt& V2);

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h?rev=289873=289872=289873=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h 

r289685 - [Driver] Add tests for enabled static analyzer checkers.

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Wed Dec 14 12:46:01 2016
New Revision: 289685

URL: http://llvm.org/viewvc/llvm-project?rev=289685=rev
Log:
[Driver] Add tests for enabled static analyzer checkers.

The driver passes flags to cc1 that enable various checkers based on
the target triple. This commit adds tests for these flags on Darwin, Linux,
and Windows.

This is a test-only change.

Added:
cfe/trunk/test/Driver/analyzer-target-enabled-checkers.cpp

Added: cfe/trunk/test/Driver/analyzer-target-enabled-checkers.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/analyzer-target-enabled-checkers.cpp?rev=289685=auto
==
--- cfe/trunk/test/Driver/analyzer-target-enabled-checkers.cpp (added)
+++ cfe/trunk/test/Driver/analyzer-target-enabled-checkers.cpp Wed Dec 14 
12:46:01 2016
@@ -0,0 +1,57 @@
+// Tests for static analyzer checkers that the driver enables by default based
+// on the target triple.
+
+// RUN: %clang -### -target x86_64-apple-darwin10 --analyze %s 2>&1 | 
FileCheck --check-prefix=CHECK-DARWIN %s
+
+// CHECK-DARWIN: "-analyzer-checker=core"
+// CHECK-DARWIN-SAME: "-analyzer-checker=unix"
+// CHECK-DARWIN-SAME: "-analyzer-checker=osx"
+// CHECK-DARWIN-SAME: "-analyzer-checker=deadcode"
+// CHECK-DARWIN-SAME: "-analyzer-checker=cplusplus"
+// CHECK-DARWIN-SAME: "-analyzer-checker=security.insecureAPI.UncheckedReturn"
+// CHECK-DARWIN-SAME: "-analyzer-checker=security.insecureAPI.getpw"
+// CHECK-DARWIN-SAME: "-analyzer-checker=security.insecureAPI.gets"
+// CHECK-DARWIN-SAME: "-analyzer-checker=security.insecureAPI.mktemp"
+// CHECK-DARWIN-SAME: "-analyzer-checker=security.insecureAPI.mkstemp"
+// CHECK-DARWIN-SAME: "-analyzer-checker=security.insecureAPI.vfork"
+// CHECK-DARWIN-SAME: "-analyzer-checker=nullability.NullPassedToNonnull"
+// CHECK-DARWIN-SAME: "-analyzer-checker=nullability.NullReturnedFromNonnull"
+
+
+// RUN: %clang -### -target x86_64-unknown-linux --analyze %s 2>&1 | FileCheck 
--check-prefix=CHECK-LINUX %s
+
+// CHECK-LINUX: "-analyzer-checker=core"
+// CHECK-LINUX-SAME: "-analyzer-checker=unix"
+// CHECK-LINUX-NOT:  "-analyzer-checker=osx"
+// CHECK-LINUX-SAME: "-analyzer-checker=deadcode"
+// CHECK-LINUX-SAME: "-analyzer-checker=cplusplus"
+// CHECK-LINUX-SAME: "-analyzer-checker=security.insecureAPI.UncheckedReturn"
+// CHECK-LINUX-SAME: "-analyzer-checker=security.insecureAPI.getpw"
+// CHECK-LINUX-SAME: "-analyzer-checker=security.insecureAPI.gets"
+// CHECK-LINUX-SAME: "-analyzer-checker=security.insecureAPI.mktemp"
+// CHECK-LINUX-SAME: "-analyzer-checker=security.insecureAPI.mkstemp"
+// CHECK-LINUX-SAME: "-analyzer-checker=security.insecureAPI.vfork"
+// CHECK-LINUX-SAME: "-analyzer-checker=nullability.NullPassedToNonnull"
+// CHECK-LINUX-SAME: "-analyzer-checker=nullability.NullReturnedFromNonnull"
+
+
+// RUN: %clang -### -target x86_64-windows --analyze %s 2>&1 | FileCheck 
--check-prefix=CHECK-WINDOWS %s
+
+// CHECK-WINDOWS: "-analyzer-checker=core"
+// CHECK-WINDOWS-SAME: "-analyzer-checker=unix.API"
+// CHECK-WINDOWS-SAME: "-analyzer-checker=unix.Malloc"
+// CHECK-WINDOWS-SAME: "-analyzer-checker=unix.MallocSizeof"
+// CHECK-WINDOWS-SAME: "-analyzer-checker=unix.MismatchedDeallocator"
+// CHECK-WINDOWS-SAME: "-analyzer-checker=unix.cstring.BadSizeArg"
+// CHECK-WINDOWS-SAME: "-analyzer-checker=unix.cstring.NullArg"
+// CHECK-WINDOWS-NOT:  "-analyzer-checker=osx"
+// CHECK-WINDOWS-SAME: "-analyzer-checker=deadcode"
+// CHECK-WINDOWS-SAME: "-analyzer-checker=cplusplus"
+// CHECK-WINDOWS-SAME: "-analyzer-checker=security.insecureAPI.UncheckedReturn"
+// CHECK-WINDOWS-SAME: "-analyzer-checker=security.insecureAPI.getpw"
+// CHECK-WINDOWS-SAME: "-analyzer-checker=security.insecureAPI.gets"
+// CHECK-WINDOWS-SAME: "-analyzer-checker=security.insecureAPI.mktemp"
+// CHECK-WINDOWS-SAME: "-analyzer-checker=security.insecureAPI.mkstemp"
+// CHECK-WINDOWS-SAME: "-analyzer-checker=security.insecureAPI.vfork"
+// CHECK-WINDOWS-SAME: "-analyzer-checker=nullability.NullPassedToNonnull"
+// CHECK-WINDOWS-SAME: "-analyzer-checker=nullability.NullReturnedFromNonnull"


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r289309 - [analyzer] Improve VirtualCallChecker diagnostics and move into optin package.

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Fri Dec  9 19:16:09 2016
New Revision: 289309

URL: http://llvm.org/viewvc/llvm-project?rev=289309=rev
Log:
[analyzer] Improve VirtualCallChecker diagnostics and move into optin package.

The VirtualCallChecker is in alpha because its interprocedural diagnostics
represent the call path textually in the diagnostic message rather than with a
path sensitive diagnostic.

This patch turns off the AST-based interprocedural analysis in the checker so
that no call path is needed and improves with diagnostic text. With these
changes, the checker is ready to be moved into the optin package.

Ultimately the right fix is to rewrite this checker to be path sensitive -- but
there is still value in enabling the checker for intraprocedural analysis only
The interprocedural mode can be re-enabled with an -analyzer-config flag.

Differential Revision: https://reviews.llvm.org/D26768

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
cfe/trunk/test/Analysis/virtualcall.cpp
cfe/trunk/test/Analysis/virtualcall.h

Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td?rev=289309=289308=289309=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td Fri Dec  9 
19:16:09 2016
@@ -42,6 +42,7 @@ def Nullability : Package<"nullability">
 
 def Cplusplus : Package<"cplusplus">;
 def CplusplusAlpha : Package<"cplusplus">, InPackage, Hidden;
+def CplusplusOptIn : Package<"cplusplus">, InPackage;
 
 def Valist : Package<"valist">;
 def ValistAlpha : Package<"valist">, InPackage, Hidden;
@@ -262,13 +263,13 @@ def CXXSelfAssignmentChecker : Checker<"
 
 } // end: "cplusplus"
 
-let ParentPackage = CplusplusAlpha in {
+let ParentPackage = CplusplusOptIn in {
 
 def VirtualCallChecker : Checker<"VirtualCall">,
   HelpText<"Check virtual function calls during construction or destruction">,
   DescFile<"VirtualCallChecker.cpp">;
 
-} // end: "alpha.cplusplus"
+} // end: "optin.cplusplus"
 
 
 
//===--===//

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp?rev=289309=289308=289309=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp Fri Dec  9 
19:16:09 2016
@@ -32,6 +32,18 @@ class WalkAST : public StmtVisitor DFSWorkList;
 
@@ -59,9 +71,16 @@ class WalkAST : public StmtVisitor(rootMethod) ||
+   isa(rootMethod));
+  }
 
   bool hasWork() const { return !WList.empty(); }
 
@@ -132,7 +151,8 @@ void WalkAST::VisitChildren(Stmt *S) {
 
 void WalkAST::VisitCallExpr(CallExpr *CE) {
   VisitChildren(CE);
-  Enqueue(CE);
+  if (IsInterprocedural)
+Enqueue(CE);
 }
 
 void WalkAST::VisitCXXMemberCallExpr(CallExpr *CE) {
@@ -164,51 +184,64 @@ void WalkAST::VisitCXXMemberCallExpr(Cal
   !MD->getParent()->hasAttr())
 ReportVirtualCall(CE, MD->isPure());
 
-  Enqueue(CE);
+  if (IsInterprocedural)
+Enqueue(CE);
 }
 
 void WalkAST::ReportVirtualCall(const CallExpr *CE, bool isPure) {
+  if (ReportPureOnly && !isPure)
+return;
+
   SmallString<100> buf;
   llvm::raw_svector_ostream os(buf);
 
-  os << "Call Path : ";
-  // Name of current visiting CallExpr.
-  os << *CE->getDirectCallee();
-
-  // Name of the CallExpr whose body is current walking.
-  if (visitingCallExpr)
-os << " <-- " << *visitingCallExpr->getDirectCallee();
-  // Names of FunctionDecls in worklist with state PostVisited.
-  for (SmallVectorImpl::iterator I = WList.end(),
+  // FIXME: The interprocedural diagnostic experience here is not good.
+  // Ultimately this checker should be re-written to be path sensitive.
+  // For now, only diagnose intraprocedurally, by default.
+  if (IsInterprocedural) {
+os << "Call Path : ";
+// Name of current visiting CallExpr.
+os << *CE->getDirectCallee();
+
+// Name of the CallExpr whose body is current being walked.
+if (visitingCallExpr)
+  os << " <-- " << *visitingCallExpr->getDirectCallee();
+// Names of FunctionDecls in worklist with state PostVisited.
+for (SmallVectorImpl::iterator I = WList.end(),
  E = WList.begin(); I != E; --I) {
-const FunctionDecl *FD = (*(I-1))->getDirectCallee();
-assert(FD);
-if (VisitedFunctions[FD] == PostVisited)
-  os << " <-- " << *FD;
+  const FunctionDecl *FD = (*(I-1))->getDirectCallee();
+  assert(FD);
+  if (VisitedFunctions[FD] == PostVisited)
+

r288922 - [analyzer] Fix typo in nullability checker diagnostic

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Wed Dec  7 11:36:27 2016
New Revision: 288922

URL: http://llvm.org/viewvc/llvm-project?rev=288922=rev
Log:
[analyzer] Fix typo in nullability checker diagnostic

'infered' --> 'inferred'

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp?rev=288922=288921=288922=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp Wed Dec  7 
11:36:27 2016
@@ -333,7 +333,7 @@ PathDiagnosticPiece *NullabilityChecker:
 
   std::string InfoText =
   (llvm::Twine("Nullability '") +
-   getNullabilityString(TrackedNullab->getValue()) + "' is infered")
+   getNullabilityString(TrackedNullab->getValue()) + "' is inferred")
   .str();
 
   // Generate the extra diagnostic.


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: Crash in MallocChecker

[Devin Coughlin via cfe-commits]

+ Artem and Daniel,

Thanks for the patch! This fix seems reasonable to me, although it would good 
to add the reproducer as test case! (tests/Analysis/malloc.cpp would be a fine 
place for it).

Devin

Index: lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/MallocChecker.cpp   (revisione 285953)
+++ lib/StaticAnalyzer/Checkers/MallocChecker.cpp   (copia locale)
@@ -1026,8 +1026,7 @@
   ASTContext  = C.getASTContext();
   CharUnits TypeSize = AstContext.getTypeSizeInChars(ElementType);
 
-  if (Optional DefinedSize =
-  ElementCount.getAs()) {
+  if (ElementCount.getAs()) {
 DefinedOrUnknownSVal Extent = Region->getExtent(svalBuilder);
 // size in Bytes = ElementCount*TypeSize
 SVal SizeInBytes = svalBuilder.evalBinOpNN(


> On Nov 30, 2016, at 4:10 PM, Abramo Bagnara  wrote:
> 
> Please consider to review and apply the attached patch.
> 
> This is how to reproduce the bug:
> 
> abramo@tester:~$ cat bug.cpp
> void f(int a, int b)
> {
>new char[a * b];
> }
> abramo@tester:~$ ~/llvm-build/bin/clang -cc1 -analyze
> -analyzer-checker=cplusplus.NewDeleteLeaks bug.cpp
> clang:
> /home/abramo/llvm/tools/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h:76:
> T clang::ento::SVal::castAs() const [with T = clang::ento::NonLoc]:
> Assertion `T::isKind(*this)' failed.
> #0 0x03689a0f llvm::sys::PrintStackTrace(llvm::raw_ostream&)
> /home/abramo/llvm/lib/Support/Unix/Signals.inc:402:0
> #1 0x03689d6a PrintStackTraceSignalHandler(void*)
> /home/abramo/llvm/lib/Support/Unix/Signals.inc:466:0
> #2 0x03687f30 llvm::sys::RunSignalHandlers()
> /home/abramo/llvm/lib/Support/Signals.cpp:44:0
> #3 0x036893a1 SignalHandler(int)
> /home/abramo/llvm/lib/Support/Unix/Signals.inc:256:0
> #4 0x7f7833b31330 __restore_rt
> (/lib/x86_64-linux-gnu/libpthread.so.0+0x10330)
> #5 0x7f783291dc37 gsignal
> /build/eglibc-oGUzwX/eglibc-2.19/signal/../nptl/sysdeps/unix/sysv/linux/raise.c:56:0
> #6 0x7f7832921028 abort
> /build/eglibc-oGUzwX/eglibc-2.19/stdlib/abort.c:91:0
> #7 0x7f7832916bf6 __assert_fail_base
> /build/eglibc-oGUzwX/eglibc-2.19/assert/assert.c:92:0
> #8 0x7f7832916ca2 (/lib/x86_64-linux-gnu/libc.so.6+0x2fca2)
> #9 0x05b1769d clang::ento::NonLoc
> clang::ento::SVal::castAs() const
> /home/abramo/llvm/tools/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h:77:0
> #10 0x05bf5a20 (anonymous
> namespace)::MallocChecker::addExtentSize(clang::ento::CheckerContext&,
> clang::CXXNewExpr const*,
> llvm::IntrusiveRefCntPtr)
> /home/abramo/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1036:0
> #11 0x05bf5601 (anonymous
> namespace)::MallocChecker::checkPostStmt(clang::CXXNewExpr const*,
> clang::ento::CheckerContext&) const
> /home/abramo/llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:991:0
> #12 0x05c0aa29 void
> clang::ento::check::PostStmt::_checkStmt<(anonymous
> namespace)::MallocChecker>(void*, clang::Stmt const*,
> clang::ento::CheckerContext&)
> /home/abramo/llvm/tools/clang/include/clang/StaticAnalyzer/Core/Checker.h:105:0
> #13 0x05f0d9a8 clang::ento::CheckerFn clang::ento::CheckerContext&)>::operator()(clang::Stmt const*,
> clang::ento::CheckerContext&) const
> /home/abramo/llvm/tools/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h:60:0
> #14 0x05f08002 (anonymous
> namespace)::CheckStmtContext::runChecker(clang::ento::CheckerFn (clang::Stmt const*, clang::ento::CheckerContext&)>,
> clang::ento::NodeBuilder&, clang::ento::ExplodedNode*)
> /home/abramo/llvm/tools/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp:161:0
> #15 0x05f0a761 void expandGraphWithCheckers<(anonymous
> namespace)::CheckStmtContext>((anonymous namespace)::CheckStmtContext,
> clang::ento::ExplodedNodeSet&, clang::ento::ExplodedNodeSet const&)
> /home/abramo/llvm/tools/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp:121:0
> #16 0x05f080b2
> clang::ento::CheckerManager::runCheckersForStmt(bool,
> clang::ento::ExplodedNodeSet&, clang::ento::ExplodedNodeSet const&,
> clang::Stmt const*, clang::ento::ExprEngine&, bool)
> /home/abramo/llvm/tools/clang/lib/StaticAnalyzer/Core/CheckerManager.cpp:175:0
> #17 0x05f40184
> clang::ento::CheckerManager::runCheckersForPostStmt(clang::ento::ExplodedNodeSet&,
> clang::ento::ExplodedNodeSet const&, clang::Stmt const*,
> clang::ento::ExprEngine&, bool)
> /home/abramo/llvm/tools/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h:206:0
> #18 0x05f3770a clang::ento::ExprEngine::Visit(clang::Stmt
> const*, clang::ento::ExplodedNode*, clang::ento::ExplodedNodeSet&)
> /home/abramo/llvm/tools/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:1151:0
> #19 0x05f341e4
> clang::ento::ExprEngine::ProcessStmt(clang::CFGStmt,
> clang::ento::ExplodedNode*)
> 

[PATCH] D26762: Add a method to obtain this SVal of a method that created given StackFrameCtx

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

Thanks for the patch!

How does this differ from `getCXXThisVal()` on `CXXInstanceCall` and its 
subclasses in CallEvent.h? Can that be used instead? Or are there places where 
you need access to this without a CallEvent?

Also: it seems like there are a lot of places in the analyzer codebase that do 
this dance (get the 'this' pointer, load from it). Can they be safely replaced 
with calls to this helper? If so, that would be awesome.




Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h:769
+  return None;
+Loc ThisLoc = getStateManager().getSValBuilder().getCXXThis(RD, SFC);
+return getSVal(ThisLoc);

Why this overload of getCXXThis() and not the one taking a CXXMethodDecl?


https://reviews.llvm.org/D26762



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26768: [analyzer] Improve VirtualCallChecker diagnostics and move out of alpha

[Devin Coughlin via cfe-commits]

dcoughlin created this revision.
dcoughlin added reviewers: zaks.anna, NoQ.
dcoughlin added subscribers: cfe-commits, alexfh.

The VirtualCallChecker is in alpha because its interprocedural diagnostics 
represent the call path textually in the diagnostic message rather than with a 
path sensitive diagnostic.

This patch turns off the AST-based interprocedural analysis in the checker so 
that no call path is needed and improves with diagnostic text. With these 
changes, the checker is ready to be evaluated to move out of alpha.

Ultimately the right fix is to rewrite this checker to be path sensitive -- but 
there is still value in enabling the checker by default for intraprocedural 
analysis only. The interprocedural mode can be re-enabled with an 
-analyzer-config flag.


https://reviews.llvm.org/D26768

Files:
  include/clang/StaticAnalyzer/Checkers/Checkers.td
  lib/StaticAnalyzer/Checkers/VirtualCallChecker.cpp
  test/Analysis/dtor.cpp
  test/Analysis/virtualcall.cpp
  test/Analysis/virtualcall.h

Index: test/Analysis/virtualcall.h
===
--- test/Analysis/virtualcall.h
+++ test/Analysis/virtualcall.h
@@ -18,7 +18,12 @@
   class A {
   public:
 A() {
-  foo(); // expected-warning{{Call virtual functions during construction or destruction will never go to a more derived class}}
+  foo();
+#if INTERPROCEDURAL
+  // expected-warning-re@-2 ^}}Call Path : fooCall to virtual function during construction or destruction will not dispatch to derived class}}
+#else
+  // expected-warning-re@-4 ^}}Call to virtual function during construction or destruction will not dispatch to derived class}}
+#endif
 }
 
 virtual int foo();
Index: test/Analysis/virtualcall.cpp
===
--- test/Analysis/virtualcall.cpp
+++ test/Analysis/virtualcall.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.cplusplus.VirtualCall -analyzer-store region -verify -std=c++11 %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=cplusplus.VirtualCall -analyzer-store region -verify -std=c++11 %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=cplusplus.VirtualCall -analyzer-store region -analyzer-config cplusplus.VirtualCall:Interprocedural=true -DINTERPROCEDURAL=1 -verify -std=c++11 %s
 
 class A {
 public:
@@ -8,19 +9,30 @@
   virtual int foo() = 0;
   virtual void bar() = 0;
   void f() {
-foo(); // expected-warning{{Call pure virtual functions during construction or destruction may leads undefined behaviour}}
+foo();
+#if INTERPROCEDURAL
+// expected-warning-re@-2 ^}}Call Path : foo <-- fCall to pure virtual function during construction or destruction has undefined behavior}}
+#endif
   }
 };
 
 class B : public A {
 public:
   B() {
-foo(); // expected-warning{{Call virtual functions during construction or destruction will never go to a more derived class}}
+foo();
+#if INTERPROCEDURAL
+// expected-warning-re@-2 ^}}Call Path : fooCall to virtual function during construction or destruction will not dispatch to derived class}}
+#else
+// expected-warning-re@-4 ^}}Call to virtual function during construction or destruction will not dispatch to derived class}}
+#endif
   }
   ~B();
   
   virtual int foo();
-  virtual void bar() { foo(); }  // expected-warning{{Call virtual functions during construction or destruction will never go to a more derived class}}
+  virtual void bar() { foo(); }
+#if INTERPROCEDURAL
+  // expected-warning-re@-2 ^}}Call Path : foo <-- barCall to virtual function during construction or destruction will not dispatch to derived class}}
+#endif
 };
 
 A::A() {
@@ -30,7 +42,13 @@
 B::~B() {
   this->B::foo(); // no-warning
   this->B::bar();
-  this->foo(); // expected-warning{{Call virtual functions during construction or destruction will never go to a more derived class}}
+  this->foo();
+#if INTERPROCEDURAL
+  // expected-warning-re@-2 ^}}Call Path : fooCall to virtual function during construction or destruction will not dispatch to derived class}}
+#else
+  // expected-warning-re@-4 ^}}Call to virtual function during construction or destruction will not dispatch to derived class}}
+#endif
+
 }
 
 class C : public B {
@@ -43,7 +61,12 @@
 };
 
 C::C() {
-  f(foo()); // expected-warning{{Call virtual functions during construction or destruction will never go to a more derived class}}
+  f(foo());
+#if INTERPROCEDURAL
+  // expected-warning-re@-2 ^}}Call Path : fooCall to virtual function during construction or destruction will not dispatch to derived class}}
+#else
+  // expected-warning-re@-4 ^}}Call to virtual function during construction or destruction will not dispatch to derived class}}
+#endif
 }
 
 class D : public B {
Index: test/Analysis/dtor.cpp
===
--- test/Analysis/dtor.cpp
+++ 

r287105 - [www] Fix spelling error in checker release notes.

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Wed Nov 16 08:23:41 2016
New Revision: 287105

URL: http://llvm.org/viewvc/llvm-project?rev=287105=rev
Log:
[www] Fix spelling error in checker release notes.

Modified:
cfe/trunk/www/analyzer/release_notes.html

Modified: cfe/trunk/www/analyzer/release_notes.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/analyzer/release_notes.html?rev=287105=287104=287105=diff
==
--- cfe/trunk/www/analyzer/release_notes.html (original)
+++ cfe/trunk/www/analyzer/release_notes.html Wed Nov 16 08:23:41 2016
@@ -22,7 +22,7 @@
 The analyzer includes new checks for:
   
 Improper instance cleanup up in Objective-C -dealloc methods under 
manual retain/release.
-Inadvertant comparisons of NSNumber, CFNumberRef, and other number 
object pointers against scalar values.
+Inadvertent comparisons of NSNumber, CFNumberRef, and other number 
object pointers against scalar values.
 Unsafe usage of dispatch_once_t predicates stored in Objective-C 
instance variables and other heap-allocated memory.
 Issues resulting from self-assignment in C++.
 Incorrect usage of MPI APIs in C and C++. This check can be 
enabled by passing the following command to scan-build: 


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r287063 - [www] Update analyzer website for release of checker-279

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Tue Nov 15 18:47:56 2016
New Revision: 287063

URL: http://llvm.org/viewvc/llvm-project?rev=287063=rev
Log:
[www] Update analyzer website for release of checker-279

Modified:
cfe/trunk/www/analyzer/index.html
cfe/trunk/www/analyzer/latest_checker.html.incl
cfe/trunk/www/analyzer/release_notes.html

Modified: cfe/trunk/www/analyzer/index.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/analyzer/index.html?rev=287063=287062=287063=diff
==
--- cfe/trunk/www/analyzer/index.html (original)
+++ cfe/trunk/www/analyzer/index.html Tue Nov 15 18:47:56 2016
@@ -95,7 +95,7 @@ applications.
   
Mac OS X

-Latest build (10.7+):
+Latest build (10.8+):
  
 
 Release notes

Modified: cfe/trunk/www/analyzer/latest_checker.html.incl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/analyzer/latest_checker.html.incl?rev=287063=287062=287063=diff
==
--- cfe/trunk/www/analyzer/latest_checker.html.incl (original)
+++ cfe/trunk/www/analyzer/latest_checker.html.incl Tue Nov 15 18:47:56 2016
@@ -1 +1 @@
-checker-278.tar.bz2 (built 
February 5, 2016)
+checker-279.tar.bz2 (built 
November 14, 2016)

Modified: cfe/trunk/www/analyzer/release_notes.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/analyzer/release_notes.html?rev=287063=287062=287063=diff
==
--- cfe/trunk/www/analyzer/release_notes.html (original)
+++ cfe/trunk/www/analyzer/release_notes.html Tue Nov 15 18:47:56 2016
@@ -14,6 +14,25 @@
 
 
 Release notes for checker-XXX builds
+checker-279
+built: November 14, 2016
+   download: checker-279.tar.bz2
+   highlights:
+   
+The analyzer includes new checks for:
+  
+Improper instance cleanup up in Objective-C -dealloc methods under 
manual retain/release.
+Inadvertant comparisons of NSNumber, CFNumberRef, and other number 
object pointers against scalar values.
+Unsafe usage of dispatch_once_t predicates stored in Objective-C 
instance variables and other heap-allocated memory.
+Issues resulting from self-assignment in C++.
+Incorrect usage of MPI APIs in C and C++. This check can be 
enabled by passing the following command to scan-build: 
+  -enable-checker optin.mpi.MPI-Checker
+
+The scan-build tool now supports a --force-analyze-debug-code 
flag that forces projects to analyze in debug mode. This flag leaves in 
assertions and so typically results in fewer false positives.
+Additional miscellaneous improvements.
+Now requires macOS 10.8 or later.
+   
+
 checker-278
 built: February 5, 2016
download: checker-278.tar.bz2


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r287001 - [analyzer] Add check for when block is called with too few arguments.

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Tue Nov 15 12:40:46 2016
New Revision: 287001

URL: http://llvm.org/viewvc/llvm-project?rev=287001=rev
Log:
[analyzer] Add check for when block is called with too few arguments.

The CallAndMessageChecker has an existing check for when a function pointer
is called with too few arguments. Extend this logic to handle the block
case, as well. While we're at it, do a drive-by grammar correction
("less" --> "fewer") on the diagnostic text.

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
cfe/trunk/test/Analysis/blocks.m
cfe/trunk/test/Analysis/inline.c
cfe/trunk/test/Analysis/inline.cpp
cfe/trunk/test/Analysis/nullability.c

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp?rev=287001=287000=287001=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CallAndMessageChecker.cpp Tue Nov 15 
12:40:46 2016
@@ -356,7 +356,6 @@ void CallAndMessageChecker::checkPreStmt
   }
 }
 
-
 void CallAndMessageChecker::checkPreCall(const CallEvent ,
  CheckerContext ) const {
   ProgramStateRef State = C.getState();
@@ -389,11 +388,10 @@ void CallAndMessageChecker::checkPreCall
   }
 
   const Decl *D = Call.getDecl();
-  const FunctionDecl *FD = dyn_cast_or_null(D);
-  if (FD) {
-// If we have a declaration, we can make sure we pass enough parameters to
-// the function.
-unsigned Params = FD->getNumParams();
+  if (D && (isa(D) || isa(D))) {
+// If we have a function or block declaration, we can make sure we pass
+// enough parameters.
+unsigned Params = Call.parameters().size();
 if (Call.getNumArgs() < Params) {
   ExplodedNode *N = C.generateErrorNode();
   if (!N)
@@ -403,8 +401,14 @@ void CallAndMessageChecker::checkPreCall
 
   SmallString<512> Str;
   llvm::raw_svector_ostream os(Str);
-  os << "Function taking " << Params << " argument"
- << (Params == 1 ? "" : "s") << " is called with less ("
+  if (isa(D)) {
+os << "Function ";
+  } else {
+assert(isa(D));
+os << "Block ";
+  }
+  os << "taking " << Params << " argument"
+ << (Params == 1 ? "" : "s") << " is called with fewer ("
  << Call.getNumArgs() << ")";
 
   C.emitReport(
@@ -425,6 +429,7 @@ void CallAndMessageChecker::checkPreCall
   else
 BT = _call_arg;
 
+  const FunctionDecl *FD = dyn_cast_or_null(D);
   for (unsigned i = 0, e = Call.getNumArgs(); i != e; ++i) {
 const ParmVarDecl *ParamDecl = nullptr;
 if(FD && i < FD->getNumParams())

Modified: cfe/trunk/test/Analysis/blocks.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/blocks.m?rev=287001=287000=287001=diff
==
--- cfe/trunk/test/Analysis/blocks.m (original)
+++ cfe/trunk/test/Analysis/blocks.m Tue Nov 15 12:40:46 2016
@@ -232,3 +232,12 @@ __attribute__((objc_root_class))
   });
 }
 @end
+
+// The incorrect block variable initialization below is a hard compile-time
+// error in C++.
+#if !defined(__cplusplus)
+void call_block_with_fewer_arguments() {
+  void (^b)() = ^(int a) { };
+  b(); // expected-warning {{Block taking 1 argument is called with fewer (0)}}
+}
+#endif

Modified: cfe/trunk/test/Analysis/inline.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inline.c?rev=287001=287000=287001=diff
==
--- cfe/trunk/test/Analysis/inline.c (original)
+++ cfe/trunk/test/Analysis/inline.c Tue Nov 15 12:40:46 2016
@@ -114,5 +114,5 @@ void never_called_by_anyone() {
 void knr_one_argument(a) int a; { }
 
 void call_with_less_arguments() {
-  knr_one_argument(); // expected-warning{{too few arguments}} 
expected-warning{{Function taking 1 argument}}
+  knr_one_argument(); // expected-warning{{too few arguments}} 
expected-warning{{Function taking 1 argument is called with fewer (0)}}
 }

Modified: cfe/trunk/test/Analysis/inline.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inline.cpp?rev=287001=287000=287001=diff
==
--- cfe/trunk/test/Analysis/inline.cpp (original)
+++ cfe/trunk/test/Analysis/inline.cpp Tue Nov 15 12:40:46 2016
@@ -441,6 +441,6 @@ namespace rdar12409977  {
 namespace bug16307 {
   void one_argument(int a) { }
   void call_with_less() {
-reinterpret_cast(one_argument)(); // 
expected-warning{{Function taking 1 argument}}
+reinterpret_cast(one_argument)(); // 
expected-warning{{Function taking 1 argument is called with fewer (0)}}
   }
 }

Modified: cfe/trunk/test/Analysis/nullability.c

[PATCH] D26644: [analyzer] Rename assumeWithinInclusiveRange*()

[Devin Coughlin via cfe-commits]

dcoughlin accepted this revision.
dcoughlin added a comment.
This revision is now accepted and ready to land.

LGTM, other then some indentation issues for arguments and parameters after the 
rename. Please fix those and commit! And thanks for splitting this up.




Comment at: 
include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h:103
+  virtual ProgramStateRef assumeInclusiveRange(ProgramStateRef State,
  NonLoc Value,
  const llvm::APSInt ,

Nit: The indentation here (and elsewhere) is off after the rename.


https://reviews.llvm.org/D26644



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r286901 - [analyzer] Fix crash in NullabilityChecker calling block with too few arguments

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Mon Nov 14 16:46:02 2016
New Revision: 286901

URL: http://llvm.org/viewvc/llvm-project?rev=286901=rev
Log:
[analyzer] Fix crash in NullabilityChecker calling block with too few arguments

Fix a crash when checking parameter nullability on a block invocation
with fewer arguments than the block declaration requires.

rdar://problem/29237566

Added:
cfe/trunk/test/Analysis/nullability.c
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp?rev=286901=286900=286901=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp Mon Nov 14 
16:46:02 2016
@@ -679,9 +679,10 @@ void NullabilityChecker::checkPreCall(co
 if (Param->isParameterPack())
   break;
 
-const Expr *ArgExpr = nullptr;
-if (Idx < Call.getNumArgs())
-  ArgExpr = Call.getArgExpr(Idx);
+if (Idx >= Call.getNumArgs())
+  break;
+
+const Expr *ArgExpr = Call.getArgExpr(Idx);
 auto ArgSVal = Call.getArgSVal(Idx++).getAs();
 if (!ArgSVal)
   continue;

Added: cfe/trunk/test/Analysis/nullability.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/nullability.c?rev=286901=auto
==
--- cfe/trunk/test/Analysis/nullability.c (added)
+++ cfe/trunk/test/Analysis/nullability.c Mon Nov 14 16:46:02 2016
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -fblocks -analyze -analyzer-checker=core,nullability 
-verify %s
+
+void it_takes_two(int a, int b);
+void function_pointer_arity_mismatch() {
+  void(*fptr)() = it_takes_two;
+  fptr(1); // no-crash expected-warning {{Function taking 2 arguments is 
called with less (1)}}
+}
+
+void block_arity_mismatch() {
+  void(^b)() = ^(int a, int b) { }; // no-crash
+  b(1);
+}


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r286700 - [analyzer] Update 'Automated' to 'Automatic' from r286694.

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Fri Nov 11 19:50:04 2016
New Revision: 286700

URL: http://llvm.org/viewvc/llvm-project?rev=286700=rev
Log:
[analyzer] Update 'Automated' to 'Automatic' from r286694.

ARC is 'Automatic Reference Counting' and not 'Automated Reference Counting'.

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
cfe/trunk/test/Analysis/retain-release-arc.m

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp?rev=286700=286699=286700=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp Fri Nov 11 
19:50:04 2016
@@ -2368,7 +2368,7 @@ CFRefLeakReportVisitor::getEndPath(BugRe
 else {
   if (const ObjCMethodDecl *MD = dyn_cast(D)) {
 if (BRC.getASTContext().getLangOpts().ObjCAutoRefCount) {
-  os << "managed by Automated Reference Counting";
+  os << "managed by Automatic Reference Counting";
 } else {
   os << "whose name ('" << MD->getSelector().getAsString()
  << "') does not start with "

Modified: cfe/trunk/test/Analysis/retain-release-arc.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/retain-release-arc.m?rev=286700=286699=286700=diff
==
--- cfe/trunk/test/Analysis/retain-release-arc.m (original)
+++ cfe/trunk/test/Analysis/retain-release-arc.m Fri Nov 11 19:50:04 2016
@@ -59,7 +59,7 @@ typedef struct _NSZone NSZone;
 #if HAS_ARC
   // expected-warning@-2 {{Potential leak of an object stored into 
'testDict'}}
   // expected-note@-3 {{Object returned to caller as an owning reference 
(single retain count transferred to caller)}}
-  // expected-note@-4 {{Object leaked: object allocated and stored into 
'testDict' is returned from a method managed by Automated Reference Counting}}
+  // expected-note@-4 {{Object leaked: object allocated and stored into 
'testDict' is returned from a method managed by Automatic Reference Counting}}
 #endif
 }
 


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r286697 - [analyzer] Fix copy-pasta in NullableReturnedFromNonnullChecker checker name.

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Fri Nov 11 19:23:01 2016
New Revision: 286697

URL: http://llvm.org/viewvc/llvm-project?rev=286697=rev
Log:
[analyzer] Fix copy-pasta in NullableReturnedFromNonnullChecker checker name.

The name of the NullableReturnedFromNonnullChecker in Checkers.td
was accidentally "NullablePassedToNonnull", which made it impossible
to explicitly turn the checker on.

rdar://problem/28354459

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
cfe/trunk/test/Analysis/nullability.mm

Modified: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td?rev=286697=286696=286697=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td Fri Nov 11 
19:23:01 2016
@@ -192,7 +192,7 @@ def NullablePassedToNonnullChecker : Che
   HelpText<"Warns when a nullable pointer is passed to a pointer which has a 
_Nonnull type.">,
   DescFile<"NullabilityChecker.cpp">;
 
-def NullableReturnedFromNonnullChecker : Checker<"NullablePassedToNonnull">,
+def NullableReturnedFromNonnullChecker : 
Checker<"NullableReturnedFromNonnull">,
   HelpText<"Warns when a nullable pointer is returned from a function that has 
_Nonnull return type.">,
   DescFile<"NullabilityChecker.cpp">;
 

Modified: cfe/trunk/test/Analysis/nullability.mm
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/nullability.mm?rev=286697=286696=286697=diff
==
--- cfe/trunk/test/Analysis/nullability.mm (original)
+++ cfe/trunk/test/Analysis/nullability.mm Fri Nov 11 19:23:01 2016
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fblocks -analyze -analyzer-checker=core,nullability 
-DNOSYSTEMHEADERS=0 -verify %s
-// RUN: %clang_cc1 -fblocks -analyze -analyzer-checker=core,nullability 
-analyzer-config nullability:NoDiagnoseCallsToSystemHeaders=true 
-DNOSYSTEMHEADERS=1 -verify %s
+// RUN: %clang_cc1 -fblocks -analyze 
-analyzer-checker=core,nullability.NullPassedToNonnull,nullability.NullReturnedFromNonnull,nullability.NullablePassedToNonnull,nullability.NullableReturnedFromNonnull,nullability.NullableDereferenced
 -DNOSYSTEMHEADERS=0 -verify %s
+// RUN: %clang_cc1 -fblocks -analyze 
-analyzer-checker=core,nullability.NullPassedToNonnull,nullability.NullReturnedFromNonnull,nullability.NullablePassedToNonnull,nullability.NullableReturnedFromNonnull,nullability.NullableDereferenced
 -analyzer-config nullability:NoDiagnoseCallsToSystemHeaders=true 
-DNOSYSTEMHEADERS=1 -verify %s
 
 #include "Inputs/system-header-simulator-for-nullability.h"
 


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r286694 - [analyzer] Improve misleading RetainCountChcker diagnostic under ARC

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Fri Nov 11 19:03:06 2016
New Revision: 286694

URL: http://llvm.org/viewvc/llvm-project?rev=286694=rev
Log:
[analyzer] Improve misleading RetainCountChcker diagnostic under ARC

Under automated reference counting the analyzer treats a methods -- even those
starting with  'copy' and friends -- as returning an unowned value. This is
because ownership of CoreFoundation objects must be transferred to ARC
with __bridge_transfer or CFBridgingRelease() before being returned as
ARC-managed bridged objects.

Unfortunately this could lead to a poor diagnostic inside copy methods under
ARC where the analyzer would complain about a leak of a returned CF value inside
a method "whose name does not start with 'copy'" -- even though the name did
start with 'copy'.

This commit improves the diagnostic under ARC to say inside a method "returned
from a method managed by Automated Reference Counting".

rdar://problem/28849667

Added:
cfe/trunk/test/Analysis/retain-release-arc.m
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp?rev=286694=286693=286694=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp Fri Nov 11 
19:03:06 2016
@@ -2367,10 +2367,15 @@ CFRefLeakReportVisitor::getEndPath(BugRe
   os << "that is annotated as NS_RETURNS_NOT_RETAINED";
 else {
   if (const ObjCMethodDecl *MD = dyn_cast(D)) {
-os << "whose name ('" << MD->getSelector().getAsString()
-   << "') does not start with 'copy', 'mutableCopy', 'alloc' or 'new'."
-  "  This violates the naming convention rules"
-  " given in the Memory Management Guide for Cocoa";
+if (BRC.getASTContext().getLangOpts().ObjCAutoRefCount) {
+  os << "managed by Automated Reference Counting";
+} else {
+  os << "whose name ('" << MD->getSelector().getAsString()
+ << "') does not start with "
+"'copy', 'mutableCopy', 'alloc' or 'new'."
+"  This violates the naming convention rules"
+" given in the Memory Management Guide for Cocoa";
+}
   }
   else {
 const FunctionDecl *FD = cast(D);

Added: cfe/trunk/test/Analysis/retain-release-arc.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/retain-release-arc.m?rev=286694=auto
==
--- cfe/trunk/test/Analysis/retain-release-arc.m (added)
+++ cfe/trunk/test/Analysis/retain-release-arc.m Fri Nov 11 19:03:06 2016
@@ -0,0 +1,86 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -analyze 
-analyzer-checker=core,osx.coreFoundation.CFRetainRelease,osx.cocoa.ClassRelease,osx.cocoa.RetainCount
 -fobjc-arc -fblocks -verify -Wno-objc-root-class %s -analyzer-output=text
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -analyze 
-analyzer-checker=core,osx.coreFoundation.CFRetainRelease,osx.cocoa.ClassRelease,osx.cocoa.RetainCount
 -fblocks -verify -Wno-objc-root-class %s -analyzer-output=text
+
+#define HAS_ARC __has_feature(objc_arc)
+
+typedef unsigned long long CFOptionFlags;
+typedef signed long long CFIndex;
+
+typedef CFIndex CFPropertyListFormat; enum {
+kCFPropertyListOpenStepFormat = 1,
+kCFPropertyListXMLFormat_v1_0 = 100,
+kCFPropertyListBinaryFormat_v1_0 = 200
+};
+
+typedef const struct __CFAllocator * CFAllocatorRef;
+extern const CFAllocatorRef kCFAllocatorDefault;
+typedef struct __CFDictionary * CFDictionaryRef;
+typedef struct __CFError * CFErrorRef;
+typedef struct __CFDataRef * CFDataRef;
+typedef void * CFPropertyListRef;
+
+CFPropertyListRef CFPropertyListCreateWithData(CFAllocatorRef allocator, 
CFDataRef data, CFOptionFlags options, CFPropertyListFormat *format, CFErrorRef 
*error);
+
+typedef signed char BOOL;
+typedef struct _NSZone NSZone;
+@class NSDictionary;
+@class NSData;
+@class NSString;
+
+@protocol NSObject
+- (BOOL)isEqual:(id)object;
+- (id)retain;
+- (oneway void)release;
+- (id)autorelease;
+- (NSString *)description;
+- (id)init;
+@end
+@interface NSObject  {}
++ (id)allocWithZone:(NSZone *)zone;
++ (id)alloc;
++ (id)new;
+- (void)dealloc;
+@end
+
+@interface NSDictionary : NSObject
+@end
+
+@interface SomeClass
+@end
+
+@implementation SomeClass
+- (NSDictionary *)copyTestWithBridgeReturningRetainable:(NSData *)plistData {
+  CFErrorRef error;
+  CFDictionaryRef testDict = CFPropertyListCreateWithData(kCFAllocatorDefault, 
(__bridge CFDataRef)plistData, 0, 0, );
+#if HAS_ARC
+  // expected-note@-2 {{Call to function 'CFPropertyListCreateWithData' 
returns a Core Foundation object with a +1 retain count}}
+#endif
+  return (__bridge 

r286633 - [analyzer] Teach RetainCountChecker about VTCompressionSessionEncodeFrame()

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Fri Nov 11 15:31:38 2016
New Revision: 286633

URL: http://llvm.org/viewvc/llvm-project?rev=286633=rev
Log:
[analyzer] Teach RetainCountChecker about VTCompressionSessionEncodeFrame()

The context argument passed to VideoToolbox's
VTCompressionSessionEncodeFrame() function is ultimately passed to a callback
supplied when creating the compression session and so may be freed by that
callback.  To suppress false positives in this case, teach the retain count
checker to stop tracking that argument.

This isn't suppressed by the usual callback context mechanism because the call
to VTCompressionSessionEncodeFrame() doesn't include the callback itself.

rdar://problem/27685213

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
cfe/trunk/test/Analysis/retain-release.m

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp?rev=286633=286632=286633=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp Fri Nov 11 
15:31:38 2016
@@ -1126,6 +1126,14 @@ RetainSummaryManager::getFunctionSummary
   // correctly.
   ScratchArgs = AF.add(ScratchArgs, 12, StopTracking);
   S = getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, DoNothing);
+} else if (FName == "VTCompressionSessionEncodeFrame") {
+  // The context argument passed to VTCompressionSessionEncodeFrame()
+  // is passed to the callback specified when creating the session
+  // (e.g. with VTCompressionSessionCreate()) which can release it.
+  // To account for this possibility, conservatively stop tracking
+  // the context.
+  ScratchArgs = AF.add(ScratchArgs, 5, StopTracking);
+  S = getPersistentSummary(RetEffect::MakeNoRet(), DoNothing, DoNothing);
 } else if (FName == "dispatch_set_context" ||
FName == "xpc_connection_set_context") {
   //  - The analyzer currently doesn't have

Modified: cfe/trunk/test/Analysis/retain-release.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/retain-release.m?rev=286633=286632=286633=diff
==
--- cfe/trunk/test/Analysis/retain-release.m (original)
+++ cfe/trunk/test/Analysis/retain-release.m Fri Nov 11 15:31:38 2016
@@ -1267,6 +1267,88 @@ void testCVPrefixRetain(CMSampleBufferRe
   CVBufferRelease(pixelBufAlias); // no-warning
 }
 
+typedef signed long SInt32;
+typedef SInt32  OSStatus;
+typedef FourCharCode CMVideoCodecType;
+
+
+typedef UInt32 VTEncodeInfoFlags; enum {
+ kVTEncodeInfo_Asynchronous = 1UL << 0,
+ kVTEncodeInfo_FrameDropped = 1UL << 1,
+};
+typedef struct
+{
+  int ignore;
+} CMTime;
+
+
+typedef void (*VTCompressionOutputCallback)(
+void * _Nullable outputCallbackRefCon,
+void * _Nullable sourceFrameRefCon,
+OSStatus status,
+VTEncodeInfoFlags infoFlags,
+_Nullable CMSampleBufferRef sampleBuffer );
+
+typedef struct OpaqueVTCompressionSession*  VTCompressionSessionRef;
+
+extern OSStatus
+VTCompressionSessionCreate(_Nullable CFAllocatorRef allocator,
+int32_t width,
+int32_t height,
+CMVideoCodecType codecType,
+_Nullable CFDictionaryRef encoderSpecification,
+_Nullable CFDictionaryRef sourceImageBufferAttributes,
+_Nullable CFAllocatorRef compressedDataAllocator,
+_Nullable VTCompressionOutputCallback outputCallback,
+void * _Nullable outputCallbackRefCon,
+CF_RETURNS_RETAINED _Nullable VTCompressionSessionRef * _Nonnull 
compressionSessionOut);
+
+extern OSStatus
+VTCompressionSessionEncodeFrame(
+_Nonnull VTCompressionSessionRef session,
+_Nonnull CVImageBufferRef imageBuffer,
+CMTime presentationTimeStamp,
+CMTime duration,
+_Nullable CFDictionaryRef frameProperties,
+void * _Nullable sourceFrameRefCon,
+VTEncodeInfoFlags * _Nullable infoFlagsOut);
+
+OSStatus test_VTCompressionSessionCreateAndEncode_CallbackReleases(
+_Nullable CFAllocatorRef allocator,
+int32_t width,
+int32_t height,
+CMVideoCodecType codecType,
+_Nullable CFDictionaryRef encoderSpecification,
+_Nullable CFDictionaryRef sourceImageBufferAttributes,
+_Nullable CFAllocatorRef compressedDataAllocator,
+_Nullable VTCompressionOutputCallback outputCallback,
+
+_Nonnull CVImageBufferRef imageBuffer,
+CMTime presentationTimeStamp,
+CMTime duration,
+_Nullable CFDictionaryRef frameProperties
+) {
+
+  // The outputCallback is passed both contexts and so can release either.
+  NSNumber *contextForCreate = [[NSNumber alloc] initWithInt:5]; // no-warning
+  NSNumber *contextForEncode = [[NSNumber alloc] initWithInt:6]; // no-warning
+
+  VTCompressionSessionRef session = 0;
+  OSStatus status = 

[PATCH] D26340: [analyzer] Add SpinLockChecker for the Magenta kernel

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

In https://reviews.llvm.org/D26340#590882, @khazem wrote:

> Devin, based on Artem's review of the other checker that I have posted [1] I 
> am wondering about merging both this SpinLockChecker and the MutexChecker 
> into PthreadLockChecker. Do you think it is still worth landing this 
> SpinLockChecker, or do you suppose that abandoning this patch and working on 
> merging all the checkers would be a good idea? I've noted how the checkers 
> might be merged in my reply to Artem: [2].


Merging seems reasonable to me. It is also a good way to make it less likely 
that the Magenta checkers accidentally regress since the main logic of the 
checker will be exercised when checking a much more common API.


https://reviews.llvm.org/D26340



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26340: [analyzer] Add SpinLockChecker for the Magenta kernel

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

Thanks for adding the path notes and adopting CallDescription. I've added some 
additional comments inline, which are mostly minor nits.

Two additional important changes -- and I should have noted these in the 
initial review -- is that it would be good to remove a MemRegion mapping from 
the program state when a MemRegion escapes and also when it becomes dead. These 
should both be small changes.

**Escaping**
Removing the mapping when the lock escapes will avoid false positives when the 
lock is passed to a function that the analyzer doesn't inline (for example, if 
it is in another translation unit) that unlocks it:

  void foo(lock_t *l) {
spin_lock(l);
...
do_stuff_and_unlock(l);
..
spin_lock();
  }

If `do_stuff_and_unlock()` is in another translation unit the analyzer won't 
see the unlock and so will report a false positive if you don't remove the 
mapping when the lock escapes.

If you remove the mapping when the lock escapes, the analyzer will 
optimistically assume the best-case scenario the next time it sees a lock or 
unlock operation. There is an example of how to do this in 
`ObjCContainersChecker::checkPointerEscape()`

**Removing Dead Symbols**
It is also important for performance to remove mappings that are no longer 
relevant. There is a cost to keep around mappings in the program state: both 
memory (because the analyzer keeps some states around for use in the bug 
reporter visitor) and time (because the analyzer has to iterate over the map 
when determining whether two states are the same).  There is an example of how 
to do this in `ObjCLoopChecker::checkDeadSymbols`.




Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:728
+//===--===//
+// Magenta checkers.
+//===--===//

Nit: I think it would good to add a little bit more context to disambiguate so 
that future maintainers will be able to do a web search to learn about what 
these checkers are for. For example, maybe something like "Checkers for the 
Magenta kernel"?



Comment at: lib/StaticAnalyzer/Checkers/SpinLockChecker.cpp:208
+  *DoubleUnlockBugType,
+  "Execution path found where spinlock is unlocked twice in a row", Node);
+  Report->markInteresting(SpinLockLocation);

Nit: it is enough to have the message be "Spinlock is unlocked twice in a row". 
I don't think it is necessary to explicitly mention the execution path 
(especially now that you have a path note for the first unlock).



Comment at: lib/StaticAnalyzer/Checkers/SpinLockChecker.cpp:224
+  *DoubleLockBugType,
+  "Execution path found where spinlock is locked twice in a row", Node);
+  Report->addRange(Call.getSourceRange());

Nit: Same here.



Comment at: test/Analysis/spinlock_correct.c:1
+// RUN: %clang_cc1 -analyze -analyzer-checker=magenta.SpinLock -verify %s
+// expected-no-diagnostics

Nit: Typically in the analyzer codebase we try to keep the number of test files 
to a minimum. Can these three test files all be combined into one?



Comment at: test/Analysis/spinlock_correct.c:1
+// RUN: %clang_cc1 -analyze -analyzer-checker=magenta.SpinLock -verify %s
+// expected-no-diagnostics

dcoughlin wrote:
> Nit: Typically in the analyzer codebase we try to keep the number of test 
> files to a minimum. Can these three test files all be combined into one?
Another nit: The analyzer relies on always having core checkers enabled because 
they model important aspects of the language. You should change this to `... 
-analyzer-checker=core,magenta.SpinLock ...`


https://reviews.llvm.org/D26340



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [clang-tools-extra] r286222 - [clang-tidy] clang-analyzer-alpha* checks are not registered, so there's no need to disable them

[Devin Coughlin via cfe-commits]

+ Anna, Alexander, and Artem.

> On Nov 9, 2016, at 10:50 AM, Devin Coughlin via cfe-commits 
> <cfe-commits@lists.llvm.org> wrote:
> 
> 
>> On Nov 8, 2016, at 9:44 AM, Malcolm Parsons <malcolm.pars...@gmail.com> 
>> wrote:
>> 
>> On 8 November 2016 at 16:59, Alexander Kornienko <ale...@google.com> wrote:
>>> On Nov 8, 2016 2:11 AM, "Malcolm Parsons" <malcolm.pars...@gmail.com> wrote:
>>>> Oh, I was using clang-analyzer-alpha.cplusplus.VirtualCall.
>>>> 
>>>> Should clang-tidy have an option to enable experimental checks?
>>> 
>>> I'd instead ask Static Analyzer folks if they can graduate this check. 
> 
> We agree that this is a valuable checker and are committed to getting it out 
> of alpha. This check is in alpha because:
> 
> a) The diagnostic experience is not very good. It reports a call path 
> directly in the diagnostic message (for example “Call path: foo <— bar” for a 
> call to foo() in bar()) rather than as a path diagnostic.
> 
> b) The lack of path-sensitive reasoning may result in false positives when a 
> called function uses a member variable flag to track whether initialization 
> is complete and does not call the virtual member function during 
> initialization.
> 
> c) The check issues a warning for both calls to pure virtual functions (which 
> is always an error) and non-pure virtual functions (which is more of a code 
> smell and may be a false positive).
> 
> From our perspective, the diagnostic experience is the primary blocker to 
> moving this checker out of alpha and thus turning it on by default.
> 
> Here is our plan to graduate the checker:
> 
> Step 1) Modify the existing AST-based checker to only report on virtual calls 
> in the constructor/destructor itself. This will result in false negatives but 
> avoid the poor diagnostic experience in a). We’ll move the checker out of 
> alpha, which will turn it on by default for all users, and add an 
> -analyzer-config flag to to recover the old behavior.
> 
> Step 2) Rewrite the checker to be path-sensitive. This will improve the 
> diagnostic experience so that we can turn the inter-procedural case back on 
> and also limit false positives from b).
> 
> I’ll commit to doing Step 1) in the immediate future and Step 2) eventually. 
> Once the checker is on by default we’ll need to assess whether the false 
> positive rate from c) is too high — if so, we’ll need to turn the 
> non-pure-virtual case off by default.
> 
> Devin
> 
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [clang-tools-extra] r286222 - [clang-tidy] clang-analyzer-alpha* checks are not registered, so there's no need to disable them

[Devin Coughlin via cfe-commits]

> On Nov 8, 2016, at 9:44 AM, Malcolm Parsons  wrote:
> 
> On 8 November 2016 at 16:59, Alexander Kornienko  wrote:
>> On Nov 8, 2016 2:11 AM, "Malcolm Parsons"  wrote:
>>> Oh, I was using clang-analyzer-alpha.cplusplus.VirtualCall.
>>> 
>>> Should clang-tidy have an option to enable experimental checks?
>> 
>> I'd instead ask Static Analyzer folks if they can graduate this check. 

We agree that this is a valuable checker and are committed to getting it out of 
alpha. This check is in alpha because:

a) The diagnostic experience is not very good. It reports a call path directly 
in the diagnostic message (for example “Call path: foo <— bar” for a call to 
foo() in bar()) rather than as a path diagnostic.

b) The lack of path-sensitive reasoning may result in false positives when a 
called function uses a member variable flag to track whether initialization is 
complete and does not call the virtual member function during initialization.

c) The check issues a warning for both calls to pure virtual functions (which 
is always an error) and non-pure virtual functions (which is more of a code 
smell and may be a false positive).

From our perspective, the diagnostic experience is the primary blocker to 
moving this checker out of alpha and thus turning it on by default.

Here is our plan to graduate the checker:

Step 1) Modify the existing AST-based checker to only report on virtual calls 
in the constructor/destructor itself. This will result in false negatives but 
avoid the poor diagnostic experience in a). We’ll move the checker out of 
alpha, which will turn it on by default for all users, and add an 
-analyzer-config flag to to recover the old behavior.

Step 2) Rewrite the checker to be path-sensitive. This will improve the 
diagnostic experience so that we can turn the inter-procedural case back on and 
also limit false positives from b).

I’ll commit to doing Step 1) in the immediate future and Step 2) eventually. 
Once the checker is on by default we’ll need to assess whether the false 
positive rate from c) is too high — if so, we’ll need to turn the 
non-pure-virtual case off by default.

Devin

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26373: [analyzer] Provide Contains() on ImmutableMap program state partial trait.

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

In https://reviews.llvm.org/D26373#589614, @ddcc wrote:

> Even though there isn't a performance difference, I think it is semantically 
> clearer since it is explicit that the value is unneeded.


Makes sense to me!


https://reviews.llvm.org/D26373



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26373: [analyzer] Provide Contains() on ImmutableMap program state partial trait.

[Devin Coughlin via cfe-commits]

dcoughlin accepted this revision.
dcoughlin added a comment.
This revision is now accepted and ready to land.

This seems reasonable and it looks good to me (as long as there is a later 
patch coming that calls the new method).

However: is there a situation where `Contains()` is preferable to `Lookup()`? 
It seems to me that in some cases you would want the looked-up value -- and in 
those where you don't there is no cost to return a pointer to it. As far as I 
can tell both ultimately end up as a call to `find()` on the AVL tree.


https://reviews.llvm.org/D26373



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26340: [analyzer] Add SpinLockChecker for the Magenta kernel

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

Thanks for upstreaming this! (And it was great to meet you at the developer 
conference.)




Comment at: lib/StaticAnalyzer/Checkers/SpinLockChecker.cpp:61
+
+const ErrorCategoryStr LockInfo::LockErrCategory("Lock Error");
+const FunctionNameStr LockInfo::SpinLockFuncName("spin_lock");

In the LLVM/clang codebase we try very hard to avoid running constructors for 
globals, as this can have adverse effects on startup time. Typically we try to 
lazily initialize things like these or have them refer to constant data that 
the linker will handle automatically (such as a c string constant).

Can these be a C string constant instead? Or can they be instance members of 
the  SpinLockChecker class? (See the note about `CallDescription` below, which 
may be helpful.)



Comment at: lib/StaticAnalyzer/Checkers/SpinLockChecker.cpp:118
+
+  FunctionNameStr CalleeName = Call.getCalleeIdentifier()->getName();
+  if (CalleeName != LockInfo::getSpinLockFuncName() &&

In the analyzer codebase we try to use IdentifierInfo pointers to compare 
identifiers. Since these are interned it lets us perform pointer comparisons 
rather than expensive string comparisons.

There is a `CallDescription` class in `CallEvent.h` that encapsulates this 
logic. You can see an example of how it is used in SimpleStreamChecker.cpp.




Comment at: lib/StaticAnalyzer/Checkers/SpinLockChecker.cpp:182
+  Report->markInteresting(SpinLockLocation);
+  C.emitReport(std::move(Report));
+}

For these kinds of diagnostics it is often very helpful to emit a path note 
indicating where the first unlock was.

To do this, you can add a custom bug report visitor that will get called on 
each node in the path to a diagnostic and emit a note, if appropriate. In this 
case you would look for nodes with calls to unlock taking the same region as 
the one you are now reporting for.

There is an example of how to implement a bug report visitor in 
'SuperDeallocBRVisitor'.



Comment at: lib/StaticAnalyzer/Checkers/SpinLockChecker.cpp:192
+
+  auto Report = llvm::make_unique(
+  *DoubleLockBugType,

It would be good to emit a path note for the first lock, too.


https://reviews.llvm.org/D26340



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D18073: Add memory allocating functions

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

Thanks for iterating on the patch! Some comments in-line.




Comment at: llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:569
+  // allocating functions initialized to nullptr, which will never equal a
+  //non-null IdentifierInfo*, and never trigger on a non-Windows platform.
+  if (Ctx.getTargetInfo().getTriple().isWindowsMSVCEnvironment()) {

Please add a space here to align with the rest of the comment.



Comment at: llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:622
+  if (FD->getKind() == Decl::Function) {
+if(isStandardNewDelete(FD, C))
+  return false;

The identifier info is null for other operators as well (e.g., +). 
If you want to bail early when the identifier for a function is null, you 
should do so directly instead of trying to enumerate all cases.

For example:
```
if (!FunI)
  return false;
```
This will be future-proof in case additional additional kinds of function 
declarations are added without identifiers.



Comment at: llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:648
+ }
+}
+

The indentation seems off here.



Comment at: llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:664
+  && FunI == II_win_alloca)) {
+assert(!isStandardNewDelete(FD, C) && "We should not reach this point"
+  "with a C++ operator.");

You should remove this assert.

Since you have guaranteed that you are targeting windows before checking and 
you initially II_win_alloca to a non-null value when targeting windows it can 
never be the case that II_win_alloca will be null here.



Comment at: llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:814
+initIdentifierInfo(C.getASTContext());
+IdentifierInfo *FunI = FD->getIdentifier();
+

Do you want to do the same early bailout for an operator here also?



Comment at: llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:850
+   && FunI == II_realloc_dbg) &&
+   !isStandardNewDelete(FD, C.getASTContext())) {
+  State = ReallocMem(C, CE, false, State);

I don't think the `!isStandardNewDelete()` is needed here.

Also, this is triggering -Wlogical-op-parentheses for me when building with 
clang.



Comment at: llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:859
+   FunI == II_calloc_dbg)) &&
+   !isStandardNewDelete(FD, C.getASTContext())) {
+  State = CallocMem(C, CE, State);

Same here.



Comment at: llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:866
+   FunI == II_free_dbg)) &&
+   !isStandardNewDelete(FD, C.getASTContext())) {
+  State = FreeMemAux(C, CE, State, 0, false, ReleasedAllocatedMemory);

Same here.



Comment at: llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:874
+   FunI == II_win_tempnam_dbg || FunI == II_wtempnam_dbg)) &&
+   !isStandardNewDelete(FD, C.getASTContext())) {
+  State = MallocUpdateRefState(C, CE, State);

Same here.



Comment at: llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:881
+   FunI == II_win_alloca) &&
+   !isStandardNewDelete(FD, C.getASTContext())) {
+  if (CE->getNumArgs() < 1)

Same here.



Comment at: llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1183
+
+  //const FunctionDecl *FD = C.getCalleeDecl(CE);
+  //const IdentifierInfo *FI = FD->getIdentifier();

You should remove the commented-out code.



Comment at: llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1185
+  //const IdentifierInfo *FI = FD->getIdentifier();
+  assert(Att->getModule() != nullptr && "Only C++ operators should have a null"
+"IdentifierInfo. We should not reach "

The explanation in the assert is not correct. This invariant holds because Sema 
guarantees that the module is not null when checking the attribute. See 
`handleOwnershipAttr()` for details.



Comment at: llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1191
+  (Att->getModule() != II_malloc_dbg) &&
+ !isStandardNewDelete(C.getCalleeDecl(CE), C.getASTContext()))
+return nullptr;

The '!isStandardNewDelete` check doesn't make any sense here. The code is 
dealing with the identifier in specified attribute and not the name of the 
called function.



Comment at: llvm/tools/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1285
+
+  assert(Att->getModule() != nullptr && 

r285759 - [analyzer] Fix capitalization in ObjCSuperDealloc checker diagnostic.

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Tue Nov  1 17:16:39 2016
New Revision: 285759

URL: http://llvm.org/viewvc/llvm-project?rev=285759=rev
Log:
[analyzer] Fix capitalization in ObjCSuperDealloc checker diagnostic.

Change "use of 'self'..." to "Use of 'self'...". The convention is to
start diagnostics with a capital letter.

rdar://problem/28322494

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp
cfe/trunk/test/Analysis/DeallocUseAfterFreeErrors.m

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp?rev=285759=285758=285759=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCSuperDeallocChecker.cpp Tue Nov  
1 17:16:39 2016
@@ -191,7 +191,7 @@ void ObjCSuperDeallocChecker::reportUseA
 return;
 
   if (Desc.empty())
-Desc = "use of 'self' after it has been deallocated";
+Desc = "Use of 'self' after it has been deallocated";
 
   // Generate the report.
   std::unique_ptr BR(

Modified: cfe/trunk/test/Analysis/DeallocUseAfterFreeErrors.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/DeallocUseAfterFreeErrors.m?rev=285759=285758=285759=diff
==
--- cfe/trunk/test/Analysis/DeallocUseAfterFreeErrors.m (original)
+++ cfe/trunk/test/Analysis/DeallocUseAfterFreeErrors.m Tue Nov  1 17:16:39 2016
@@ -125,8 +125,8 @@ struct SomeStruct {
 }
 - (void)dealloc {
   [super dealloc]; // expected-note {{[super dealloc] called here}}
-  self.ivar = nil; // expected-warning {{use of 'self' after it has been 
deallocated}}
-  // expected-note@-1 {{use of 'self' after it has been deallocated}}
+  self.ivar = nil; // expected-warning {{Use of 'self' after it has been 
deallocated}}
+  // expected-note@-1 {{Use of 'self' after it has been deallocated}}
 }
 @end
 
@@ -144,8 +144,8 @@ struct SomeStruct {
 }
 - (void)dealloc {
   [super dealloc]; // expected-note {{[super dealloc] called here}}
-  self.delegate = nil; // expected-warning {{use of 'self' after it has been 
deallocated}}
-  // expected-note@-1 {{use of 'self' after it has been deallocated}}
+  self.delegate = nil; // expected-warning {{Use of 'self' after it has been 
deallocated}}
+  // expected-note@-1 {{Use of 'self' after it has been deallocated}}
 }
 @end
 
@@ -158,8 +158,8 @@ struct SomeStruct {
 }
 - (void)dealloc {
   [super dealloc]; // expected-note {{[super dealloc] called here}}
-  [self _invalidate]; // expected-warning {{use of 'self' after it has been 
deallocated}}
-  // expected-note@-1 {{use of 'self' after it has been deallocated}}
+  [self _invalidate]; // expected-warning {{Use of 'self' after it has been 
deallocated}}
+  // expected-note@-1 {{Use of 'self' after it has been deallocated}}
 }
 @end
 
@@ -173,8 +173,8 @@ static void _invalidate(NSObject *object
 @implementation SuperDeallocThenCallNonObjectiveCMethodClass
 - (void)dealloc {
   [super dealloc]; // expected-note {{[super dealloc] called here}}
-  _invalidate(self); // expected-warning {{use of 'self' after it has been 
deallocated}}
-  // expected-note@-1 {{use of 'self' after it has been deallocated}}
+  _invalidate(self); // expected-warning {{Use of 'self' after it has been 
deallocated}}
+  // expected-note@-1 {{Use of 'self' after it has been deallocated}}
 }
 @end
 
@@ -187,8 +187,8 @@ static void _invalidate(NSObject *object
 
 - (void)dealloc {
   [super dealloc]; // expected-note {{[super dealloc] called here}}
-  [SuperDeallocThenCallObjectiveClassMethodClass invalidate:self]; // 
expected-warning {{use of 'self' after it has been deallocated}}
-  // expected-note@-1 {{use of 'self' after it has been deallocated}}
+  [SuperDeallocThenCallObjectiveClassMethodClass invalidate:self]; // 
expected-warning {{Use of 'self' after it has been deallocated}}
+  // expected-note@-1 {{Use of 'self' after it has been deallocated}}
 }
 @end
 
@@ -208,8 +208,8 @@ static void _invalidate(NSObject *object
 return;
   }
   [super dealloc];// expected-note {{[super dealloc] called here}}
-  [self _invalidate]; // expected-warning {{use of 'self' after it has been 
deallocated}}
-  // expected-note@-1 {{use of 'self' after it has been deallocated}}
+  [self _invalidate]; // expected-warning {{Use of 'self' after it has been 
deallocated}}
+  // expected-note@-1 {{Use of 'self' after it has been deallocated}}
 }
 @end
 
@@ -366,8 +366,8 @@ static void _invalidate(NSObject *object
 
 - (void)dealloc; {
   [super dealloc]; // expected-note {{[super dealloc] called here}}
-  [self anotherMethod]; // expected-warning {{use of 'self' after it has been 
deallocated}}
-  // expected-note@-1 {{use of 'self' after it has been 

[PATCH] D26061: [analyzer] Refactor and simplify SimpleConstraintManager

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

Thanks for the patch!

Would it be possible to split this up into several patches? I think it is 
important to separate the interface layering changes from the formatting 
changes, renaming changes, and minor optimization changes. This will make the 
patches easier to review.

Also: is this motivated by a different implementation of range constraints? Is 
this something you plan on upstreaming?

In https://reviews.llvm.org/D26061#581778, @ddcc wrote:

> To summarize, here is a list of changes:
>
> - General
>   - Fixed some issues with formatting (`clang-format`)
>   - Fixed inconsistent capitalization following camel case style guidelines
> - `ConstraintManager.h`
>   - Renamed `assumeWithinInclusiveRange*()` to `assumeInclusiveRange*()`, 
> since the range is not necessarily contained within unless `Assumption` is 
> true, to match `assume()`
> - `RangedConstraintManager.h` (inherits `SimpleConstraintManager`)
>   - Moved `assumeSym*` and `canReasonAbout()` from `SimpleConstraintManager` 
> to here
>   - Renamed 
> `assumeSymbolWithinInclusiveRange`/`assumeSymbolOutOfInclusiveRange` to 
> `assumeSymWithinInclusiveRange`/`assumeSymOutsideInclusiveRange` to match the 
> above, and moved to here
>   - This is now inherited by `RangeConstraintManager`, and essentially 
> provides the current interface in `SimpleConstraintManager`


What do you view as the distinction between RangeConstraintMananger and 
RangedConstraintManager? In other words, if I'm a developer adding new 
functionality how would you suggest I decide which to add it to? It would be 
good to document this in the code! Also: the closeness in the names of these 
classes could be potentially confusing. Are there other, more distinctive, 
names that still communicate the difference you intend?

> 
> 
> - `SimpleConstraintManager.h` (inherits `ConstraintManager`)
>   - Implemented a new interface that internally converts the `DefinedSVal` in 
> `assume(...)` from `ConstraintManager.h` to `NonLoc` to `SymbolRef`. 
> Subclasses only need to override `ProgramStateRef assumeSym(ProgramStateRef 
> State, SymbolRef Sym, bool Assumption)`
>   - For inclusive ranges, implemented a new interface that internally 
> converts from `NonLoc` to `SymbolRef`. Subclasses only need to override 
> `ProgramStateRef assumeSymInclusiveRange(ProgramStateRef State, SymbolRef 
> Sym, const llvm::APSInt , const llvm::APSInt , bool InRange)`
>   - Implemented a new interface that internally handles expressions that fail 
> `canReasonAbout()` by adding them directly to the constraint manager state. 
> Subclasses only need to expose `ProgramStateRef assumeSymRel(ProgramStateRef 
> State, SymbolRef Sym, BinaryOperator::Opcode op, const llvm::APSInt )`
> - `RangeConstraintManager.cpp`
>   - Minor optimization to avoid updating the state if nothing is pruned in 
> `removeDeadBindings()`


https://reviews.llvm.org/D26061



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26159: [analyzer] MacOSXAPIChecker: Improve warning messages for __block vars in dispatch_once().

[Devin Coughlin via cfe-commits]

dcoughlin accepted this revision.
dcoughlin added a comment.
This revision is now accepted and ready to land.

LGTM.

We should probably be warning any time the address of a block variable is taken 
since the address is not stable -- but that's a job for a different checker or 
possibly even Sema.


https://reviews.llvm.org/D26159



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25731: [analyzer] NumberObjectConversion: Support OSNumber and CFNumberRef.

[Devin Coughlin via cfe-commits]

dcoughlin accepted this revision.
dcoughlin added a comment.

LGTM.




Comment at: test/Analysis/number-object-conversion.m:98
+
+#define NULL_INSIDE_MACRO NULL
+void test_NULL_inside_macro(NSNumber *p) {

This is great! And a good catch.


https://reviews.llvm.org/D25731



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25940: [analyzer] LibraryFunctions: Fix errors due to different integral types and typedefs on different architectures.

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

Are the parameter types actually needed? I think in general the rest of the 
analyzer uses arity alone.

Is the idea to allow for overloads in C++? If so, then I think this 
equivalent-up-to-size-and-sign approach will disallow those overloads.




Comment at: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:393
 
+bool StdLibraryFunctionsChecker::FunctionSummaryTy::typesAreEqual(
+QualType Lhs, QualType Rhs, ASTContext ) {

Maybe this should be named something like "typesMatch" since it is not actually 
return whether the types are equal?



Comment at: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:418
+
+  return ACtx.getTypeSize(Lhs) == ACtx.getTypeSize(Rhs);
+}

If the problem is only for ssize_t, would it make sense to only do this if both 
the lhs and rhs are signed?

Another possibility is to have an lhs of what we think is the ssize_t type 
match any signed type.


https://reviews.llvm.org/D25940



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25909: [analyzer] MacOSXApiChecker: Disallow dispatch_once predicates on heap and in ivars.

[Devin Coughlin via cfe-commits]

dcoughlin added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp:94
+  else if (isa(RS)) {
+// FIXME: Presence of an IVar region has priority over this branch, because
+// ObjC objects are on the heap even if the core doesn't realize this.

NoQ wrote:
> dcoughlin wrote:
> > It is not clear to me that this FIXME is a good idea. I would remove it so 
> > someone doesn't spend a lot of time trying to address it.
> > 
> > Objective-C objects don't have the strong dis-aliasing guarantee that the 
> > analyzer assumes for heap base regions. In other words, two calls to [[Foo 
> > alloc] init] may yield exactly the same instance.  This is because, unlike 
> > malloc() and C++ global new, ObjC initializers can (and frequently do) 
> > return instances other than the passed-in, freshly-allocated self.
> Hmm, that seems to be exactly the thing i'm looking for: heap-based regions 
> that may alias.
> 
> The property of a region's staying on the heap has little to do with the 
> property of being able to alias.
> 
> I've a feeling that we should have avoided using C++ inheritance in the 
> memregion hierarchy, and instead went for a bunch of constraints. Eg., memory 
> space is essentially a constraint (it may be unknown or get known later 
> through exploring aliasing), region's value type is essentially a constraint 
> (as seen during dynamic type propagation, it may be unknown, it may be 
> partially known, we may get to know it better during the analysis by 
> observing successful dynamic casts), extent is essentially a constraint (that 
> we currently impose on SymbolExtent), offset of a symbolic region inside its 
> true parent region is a constraint, and so on.
> 
> But that's too vague. I've no well-defined idea how to make this better at 
> the moment.
If you feel strongly about this, I would suggest putting the FIXME in the core, 
perhaps in SimpleSValBuilder where the dis-aliasing assumption is introduced or 
perhaps in the declaration of HeapSpaceRegion. This will make it clear to 
future maintainers that it is not a defect in the checker.



Comment at: lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp:70
 
   // Check if the first argument is stack allocated.  If so, issue a warning
   // because that's likely to be bad news.

I guess this comment needs to be updated.



Comment at: test/Analysis/dispatch-once.m:26
+  // Use regexps to check that we're NOT suggesting to make this static.
+  dispatch_once(once, ^{}); // expected-warning-re^Call to 'dispatch_once' 
uses heap-allocated memory for the predicate value.  Using such transient 
memory for the predicate is potentially dangerous$
+}

Clever.



Comment at: test/Analysis/dispatch-once.m:62
+- (void)test_ivar_struct_from_inside {
+  dispatch_once(, ^{}); // expected-warning{{Call to 'dispatch_once' 
uses the instance variable 's' for the predicate value.}}
+}

Interesting. Have you seen this pattern in the wild?

I think this diagnostic is a little bit confusing since the ivar itself isn't 
being used for the predicate value.

Maybe "... uses a subfield of the instance variable 's' for the predicate 
value"? 






https://reviews.llvm.org/D25909



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25909: [analyzer] MacOSXApiChecker: Disallow dispatch_once predicates on heap and in ivars.

[Devin Coughlin via cfe-commits]

dcoughlin added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp:92
+  else if (isa(RS))
+os << " heap allocated memory";
+  else if (isa(RS)) {

"heap allocated" --> "heap-allocated"



Comment at: lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp:94
+  else if (isa(RS)) {
+// FIXME: Presence of an IVar region has priority over this branch, because
+// ObjC objects are on the heap even if the core doesn't realize this.

It is not clear to me that this FIXME is a good idea. I would remove it so 
someone doesn't spend a lot of time trying to address it.

Objective-C objects don't have the strong dis-aliasing guarantee that the 
analyzer assumes for heap base regions. In other words, two calls to [[Foo 
alloc] init] may yield exactly the same instance.  This is because, unlike 
malloc() and C++ global new, ObjC initializers can (and frequently do) return 
instances other than the passed-in, freshly-allocated self.



Comment at: test/Analysis/dispatch-once.m:13
+
+void test_stack() {
+  dispatch_once_t once;

Should the tests for dispatch_once in unix-fns.c be moved here?



Comment at: test/Analysis/dispatch-once.m:15
+  dispatch_once_t once;
+  dispatch_once(, ^{}); // expected-warning{{Call to 'dispatch_once' uses 
the local variable 'once' for the predicate value.}}
+}

I think it would be good to include the full diagnostic text for the stack 
local variable case because there is control flow in its construction.


https://reviews.llvm.org/D25909



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25475: [analyzer] Add a new SVal to support pointer-to-member operations.

[Devin Coughlin via cfe-commits]

dcoughlin added inline comments.



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:462
+  case CK_ReinterpretMemberPointer: {
+const Expr *UOExpr = CastE->getSubExpr()->IgnoreParenCasts();
+assert(isa(UOExpr) &&

kromanenkov wrote:
> dcoughlin wrote:
> > dcoughlin wrote:
> > > dcoughlin wrote:
> > > > I don't think pattern matching on the sub expression to find the 
> > > > referred-to declaration is the right thing to do here. It isn't always 
> > > > the case that the casted expression will be a unary pointer to member 
> > > > operation. For example, this is perfectly fine and triggers an 
> > > > assertion failure on your patch:
> > > > 
> > > > ```
> > > > struct B {
> > > >   int f;
> > > > };
> > > > 
> > > > struct D : public B {
> > > >   int g;
> > > > };
> > > > 
> > > > void foo() {
> > > >   D d;
> > > >   d.f = 7;
> > > > 
> > > >   int B::* pfb = ::f;
> > > >   int D::* pfd = pfb;
> > > >   int v = d.*pfd;
> > > > }
> > > > ```
> > > > Note that you can't just propagate the value already computed for the 
> > > > subexpression. Here is a particularly annoying example from the C++ 
> > > > spec:
> > > > 
> > > > ```
> > > > struct B {
> > > >   int f;
> > > > };
> > > > struct L : public B { };
> > > > struct R : public B { };
> > > > struct D : public L, R { };
> > > > 
> > > > void foo() {
> > > >   D d;
> > > > 
> > > >   int B::* pb = ::f;
> > > >   int L::* pl = pb;
> > > >   int R::* pr = pb;
> > > > 
> > > >   int D::* pdl = pl;
> > > >   int D::* pdr = pr;
> > > > 
> > > >   clang_analyzer_eval(pdl == pdr); // FALSE
> > > >   clang_analyzer_eval(pb == pl); // TRUE
> > > > }
> > > > ```
> > > > My guess is this will require accumulating CXXBasePath s or something 
> > > > similar for each cast. I don't know how to do this efficiently. 
> > > > I don't know how to do this efficiently.
> > > 
> > > Jordan suggested storing this in a bump-pointer allocated object with a 
> > > lifetime of the AnalysisDeclContext. The common case is no multiple 
> > > inheritance, so that should be the fast case.
> > > 
> > > Maybe the Data could be a pointer union between a DeclaratorDecl and an 
> > > immutable linked list (with sharing) of CXXBaseSpecifiers from the 
> > > CastExprs in the AST. The storage for this could be managed with a new 
> > > manager in SValBuilder.
> > (The bump pointer-allocated thing would have to have the Decl as well.)
> > 
> > This could also probably live in BasicValueFactory. The extra data would be 
> > similar to LazyCompoundValData.
> My understanding is that PointerToMember SVal should be represented similar 
> to LazyCompoundVal SVal. If I got you right, PointerToMember SVal should be 
> constructed  in VisitUnaryOperator and  VisitCast 
> (CK_BaseToDerivedMemberPointer and so on) just add CXXBaseSpecifiers to this 
> SVal?
> What do you mean by sharing of immutable linked list?
SVal has very strict size limitations -- you can only store a single pointer 
for its Data. Since you'll need to have multiple CXXBaseSpecifiers in a single 
SVal (for example, consider a multiple inheritance hierarchy with a double 
diamond), the storage for this container will have to live elsewhere.

One way to do this is to bump-pointer allocate linked list nodes for the "path" 
of casts that the value has taken. If you do this then you can share the memory 
for the nodes for a shared path prefix for two paths with a common prefix.


https://reviews.llvm.org/D25475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25876: [analyzer] Report CFNumberGetValue API misuse

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

LGTM!


https://reviews.llvm.org/D25876



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25876: [analyzer] Report CFNumberGetValue API misuse

[Devin Coughlin via cfe-commits]

dcoughlin added inline comments.



Comment at: test/Analysis/CFNumber.c:39
+  unsigned char scalar = 0;
+  CFNumberGetValue(x, kCFNumberSInt16Type, );  // expected-warning{{A 
CFNumber object that represents a 16 bit integer is used to initialize an 8 bit 
integer. 8 bits of the CFNumber value will be lost}}
+  return scalar;

I feel like this diagnostic is not dire enough. The problem is not that the 
bits will be lost -- but rather that they will be overwritten on top of 
something else!

How about something like "The remaining 8 bits will overwrite adjacent 
storage."?



Comment at: test/Analysis/CFNumber.c:45
+  unsigned short scalar = 0;
+  CFNumberGetValue(x, kCFNumberSInt8Type, );  // expected-warning{{A 
CFNumber object that represents an 8 bit integer is used to initialize a 16 bit 
integer. 8 bits of the integer value will be garbage}}
+  return scalar;

Some grammar/style nits. (I realize these also hold for the pre-existing 
checks):

- There should be a dash between the number and 'bit'. That is, it should be 
"16-bit" and "8-bit".
- It is generally considered bad style to start a sentence with a number that 
is not written out (except for dates). How about starting the second sentence 
with "The remaining 8 bits ..."?


https://reviews.llvm.org/D25876



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D20811: [analyzer] Model some library functions

[Devin Coughlin via cfe-commits]

dcoughlin accepted this revision.
dcoughlin added a comment.

This looks great!




Comment at: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:694
+INVALIDATION_APPROACH(EvalCallAsPure))
+  CASE // Is certainly uppercase.
+ARGUMENT_CONDITION(ARG_NO(0), WithinRange)

I think think this comment should say 'lowercase'.



Comment at: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:702
+  END_CASE
+  CASE // Is ascii but not uppercase.
+ARGUMENT_CONDITION(ARG_NO(0), WithinRange)

Same here.


https://reviews.llvm.org/D20811



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D20811: [analyzer] Model some library functions

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

In https://reviews.llvm.org/D20811#575521, @NoQ wrote:

> I thought to give it a pause to take a fresh look at how to arrange the 
> macro-hints in the summaries.
>
> Maybe something like that:
>
>   CASE
> ARGUMENT_CONDITION(ARG_NO(0), OutOfRange)
>   RANGE('0', '9')
>   RANGE('A', 'Z')
>   RANGE('a', 'z')
>   RANGE(128, 255)
> END_ARGUMENT_CONDITION
> RETURN_VALUE_CONDITION(WithinRange)
>   SINGLE_VALUE(0)
> END_RETURN_VALUE_CONDITION
>   END_CASE
>   


Looks great to me!


https://reviews.llvm.org/D20811



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25475: [analyzer] Add a new SVal to support pointer-to-member operations.

[Devin Coughlin via cfe-commits]

dcoughlin added inline comments.



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:462
+  case CK_ReinterpretMemberPointer: {
+const Expr *UOExpr = CastE->getSubExpr()->IgnoreParenCasts();
+assert(isa(UOExpr) &&

dcoughlin wrote:
> dcoughlin wrote:
> > I don't think pattern matching on the sub expression to find the 
> > referred-to declaration is the right thing to do here. It isn't always the 
> > case that the casted expression will be a unary pointer to member 
> > operation. For example, this is perfectly fine and triggers an assertion 
> > failure on your patch:
> > 
> > ```
> > struct B {
> >   int f;
> > };
> > 
> > struct D : public B {
> >   int g;
> > };
> > 
> > void foo() {
> >   D d;
> >   d.f = 7;
> > 
> >   int B::* pfb = ::f;
> >   int D::* pfd = pfb;
> >   int v = d.*pfd;
> > }
> > ```
> > Note that you can't just propagate the value already computed for the 
> > subexpression. Here is a particularly annoying example from the C++ spec:
> > 
> > ```
> > struct B {
> >   int f;
> > };
> > struct L : public B { };
> > struct R : public B { };
> > struct D : public L, R { };
> > 
> > void foo() {
> >   D d;
> > 
> >   int B::* pb = ::f;
> >   int L::* pl = pb;
> >   int R::* pr = pb;
> > 
> >   int D::* pdl = pl;
> >   int D::* pdr = pr;
> > 
> >   clang_analyzer_eval(pdl == pdr); // FALSE
> >   clang_analyzer_eval(pb == pl); // TRUE
> > }
> > ```
> > My guess is this will require accumulating CXXBasePath s or something 
> > similar for each cast. I don't know how to do this efficiently. 
> > I don't know how to do this efficiently.
> 
> Jordan suggested storing this in a bump-pointer allocated object with a 
> lifetime of the AnalysisDeclContext. The common case is no multiple 
> inheritance, so that should be the fast case.
> 
> Maybe the Data could be a pointer union between a DeclaratorDecl and an 
> immutable linked list (with sharing) of CXXBaseSpecifiers from the CastExprs 
> in the AST. The storage for this could be managed with a new manager in 
> SValBuilder.
(The bump pointer-allocated thing would have to have the Decl as well.)

This could also probably live in BasicValueFactory. The extra data would be 
similar to LazyCompoundValData.


https://reviews.llvm.org/D25475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25475: [analyzer] Add a new SVal to support pointer-to-member operations.

[Devin Coughlin via cfe-commits]

dcoughlin added inline comments.



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:462
+  case CK_ReinterpretMemberPointer: {
+const Expr *UOExpr = CastE->getSubExpr()->IgnoreParenCasts();
+assert(isa(UOExpr) &&

dcoughlin wrote:
> I don't think pattern matching on the sub expression to find the referred-to 
> declaration is the right thing to do here. It isn't always the case that the 
> casted expression will be a unary pointer to member operation. For example, 
> this is perfectly fine and triggers an assertion failure on your patch:
> 
> ```
> struct B {
>   int f;
> };
> 
> struct D : public B {
>   int g;
> };
> 
> void foo() {
>   D d;
>   d.f = 7;
> 
>   int B::* pfb = ::f;
>   int D::* pfd = pfb;
>   int v = d.*pfd;
> }
> ```
> Note that you can't just propagate the value already computed for the 
> subexpression. Here is a particularly annoying example from the C++ spec:
> 
> ```
> struct B {
>   int f;
> };
> struct L : public B { };
> struct R : public B { };
> struct D : public L, R { };
> 
> void foo() {
>   D d;
> 
>   int B::* pb = ::f;
>   int L::* pl = pb;
>   int R::* pr = pb;
> 
>   int D::* pdl = pl;
>   int D::* pdr = pr;
> 
>   clang_analyzer_eval(pdl == pdr); // FALSE
>   clang_analyzer_eval(pb == pl); // TRUE
> }
> ```
> My guess is this will require accumulating CXXBasePath s or something similar 
> for each cast. I don't know how to do this efficiently. 
> I don't know how to do this efficiently.

Jordan suggested storing this in a bump-pointer allocated object with a 
lifetime of the AnalysisDeclContext. The common case is no multiple 
inheritance, so that should be the fast case.

Maybe the Data could be a pointer union between a DeclaratorDecl and an 
immutable linked list (with sharing) of CXXBaseSpecifiers from the CastExprs in 
the AST. The storage for this could be managed with a new manager in 
SValBuilder.


https://reviews.llvm.org/D25475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25475: [analyzer] Add a new SVal to support pointer-to-member operations.

[Devin Coughlin via cfe-commits]

dcoughlin added inline comments.



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:462
+  case CK_ReinterpretMemberPointer: {
+const Expr *UOExpr = CastE->getSubExpr()->IgnoreParenCasts();
+assert(isa(UOExpr) &&

I don't think pattern matching on the sub expression to find the referred-to 
declaration is the right thing to do here. It isn't always the case that the 
casted expression will be a unary pointer to member operation. For example, 
this is perfectly fine and triggers an assertion failure on your patch:

```
struct B {
  int f;
};

struct D : public B {
  int g;
};

void foo() {
  D d;
  d.f = 7;

  int B::* pfb = ::f;
  int D::* pfd = pfb;
  int v = d.*pfd;
}
```
Note that you can't just propagate the value already computed for the 
subexpression. Here is a particularly annoying example from the C++ spec:

```
struct B {
  int f;
};
struct L : public B { };
struct R : public B { };
struct D : public L, R { };

void foo() {
  D d;

  int B::* pb = ::f;
  int L::* pl = pb;
  int R::* pr = pb;

  int D::* pdl = pl;
  int D::* pdr = pr;

  clang_analyzer_eval(pdl == pdr); // FALSE
  clang_analyzer_eval(pb == pl); // TRUE
}
```
My guess is this will require accumulating CXXBasePath s or something similar 
for each cast. I don't know how to do this efficiently. 


https://reviews.llvm.org/D25475



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r284351 - Revert "Revert "[analyzer] Make MallocChecker more robust against custom redeclarations""

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Sun Oct 16 17:19:03 2016
New Revision: 284351

URL: http://llvm.org/viewvc/llvm-project?rev=284351=rev
Log:
Revert "Revert "[analyzer] Make MallocChecker more robust against custom 
redeclarations""

This reverts commit r284340 to reapply r284335. The bot breakage was due to
an unrelated change in the polybench test suite.

Added:
cfe/trunk/test/Analysis/malloc-custom.c
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=284351=284350=284351=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Sun Oct 16 17:19:03 
2016
@@ -778,6 +778,8 @@ void MallocChecker::checkPostStmt(const
   State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
   }
 } else if (FunI == II_kmalloc) {
+  if (CE->getNumArgs() < 1)
+return;
   llvm::Optional MaybeState =
 performKernelMalloc(CE, C, State);
   if (MaybeState.hasValue())
@@ -807,6 +809,8 @@ void MallocChecker::checkPostStmt(const
 } else if (FunI == II_strndup) {
   State = MallocUpdateRefState(C, CE, State);
 } else if (FunI == II_alloca || FunI == II_win_alloca) {
+  if (CE->getNumArgs() < 1)
+return;
   State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State,
AF_Alloca);
   State = ProcessZeroAllocation(C, CE, 0, State);

Added: cfe/trunk/test/Analysis/malloc-custom.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc-custom.c?rev=284351=auto
==
--- cfe/trunk/test/Analysis/malloc-custom.c (added)
+++ cfe/trunk/test/Analysis/malloc-custom.c Sun Oct 16 17:19:03 2016
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc 
-Wno-incompatible-library-redeclaration -verify %s
+
+// Various tests to make the the analyzer is robust against custom
+// redeclarations of memory routines.
+//
+// You wouldn't expect to see much of this in normal code, but, for example,
+// CMake tests can generate these.
+
+// expected-no-diagnostics
+
+char alloca();
+char malloc();
+char realloc();
+char kmalloc();
+char valloc();
+char calloc();
+
+char free();
+char kfree();
+
+void testCustomArgumentlessAllocation() {
+  alloca(); // no-crash
+  malloc(); // no-crash
+  realloc(); // no-crash
+  kmalloc(); // no-crash
+  valloc(); // no-crash
+  calloc(); // no-crash
+
+  free(); // no-crash
+  kfree(); // no-crash
+}
+


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r284335 - [analyzer] Make MallocChecker more robust against custom redeclarations

[Devin Coughlin via cfe-commits]

> On Oct 16, 2016, at 12:53 PM, Renato Golin  wrote:
> 
> On 16 October 2016 at 20:36, Devin Coughlin  wrote:
>> Reverted in r284340.
> 
> Hi Devin,
> 
> Sometimes Clang patches break on stage 2 or test-suite, but this is
> not the case. I dug deeper and found that there was another commit,
> not showing on that range (it should have, but that's nothing to do
> with this patch), that broke the bot.
> 
> Please, re-commit your patch, and sorry for the noise.

Will do. Thanks for looking into it!

Devin

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r284335 - [analyzer] Make MallocChecker more robust against custom redeclarations

[Devin Coughlin via cfe-commits]

> On Oct 16, 2016, at 12:12 PM, Devin Coughlin via cfe-commits 
> <cfe-commits@lists.llvm.org> wrote:
> 
>> 
>> On Oct 16, 2016, at 12:04 PM, Renato Golin <renato.go...@linaro.org> wrote:
>> 
>> On 16 October 2016 at 18:26, Devin Coughlin via cfe-commits
>> <cfe-commits@lists.llvm.org> wrote:
>>> Author: dcoughlin
>>> Date: Sun Oct 16 12:26:06 2016
>>> New Revision: 284335
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=284335=rev
>>> Log:
>>> [analyzer] Make MallocChecker more robust against custom redeclarations
>>> 
>>> Add additional checking to MallocChecker to avoid crashing when memory
>>> routines have unexpected numbers of arguments. You wouldn't expect to see 
>>> much
>>> of this in normal code (-Wincompatible-library-redeclaration warns on this),
>>> but, for example, CMake tests can generate these.
>> 
>> Hi Devin,
>> 
>> Sounds like yours:
>> 
>> http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/11121
> 
> I’ll speculatively revert. But I don’t understand how a static analyzer-only 
> change could affect either the execution time or the compile time of a 
> test-suite benchmark. There’s something very funny here.

Reverted in r284340.

Devin

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r284340 - Revert "[analyzer] Make MallocChecker more robust against custom redeclarations"

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Sun Oct 16 14:26:07 2016
New Revision: 284340

URL: http://llvm.org/viewvc/llvm-project?rev=284340=rev
Log:
Revert "[analyzer] Make MallocChecker more robust against custom redeclarations"

This reverts commit r284335.

It appears to be causing test-suite compile-time and execution-time
performance measurements to take longer than expected on several bots.
This is surprising, because r284335 is a static-analyzer-only change.

Removed:
cfe/trunk/test/Analysis/malloc-custom.c
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=284340=284339=284340=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Sun Oct 16 14:26:07 
2016
@@ -778,8 +778,6 @@ void MallocChecker::checkPostStmt(const
   State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
   }
 } else if (FunI == II_kmalloc) {
-  if (CE->getNumArgs() < 1)
-return;
   llvm::Optional MaybeState =
 performKernelMalloc(CE, C, State);
   if (MaybeState.hasValue())
@@ -809,8 +807,6 @@ void MallocChecker::checkPostStmt(const
 } else if (FunI == II_strndup) {
   State = MallocUpdateRefState(C, CE, State);
 } else if (FunI == II_alloca || FunI == II_win_alloca) {
-  if (CE->getNumArgs() < 1)
-return;
   State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State,
AF_Alloca);
   State = ProcessZeroAllocation(C, CE, 0, State);

Removed: cfe/trunk/test/Analysis/malloc-custom.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc-custom.c?rev=284339=auto
==
--- cfe/trunk/test/Analysis/malloc-custom.c (original)
+++ cfe/trunk/test/Analysis/malloc-custom.c (removed)
@@ -1,32 +0,0 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc 
-Wno-incompatible-library-redeclaration -verify %s
-
-// Various tests to make the the analyzer is robust against custom
-// redeclarations of memory routines.
-//
-// You wouldn't expect to see much of this in normal code, but, for example,
-// CMake tests can generate these.
-
-// expected-no-diagnostics
-
-char alloca();
-char malloc();
-char realloc();
-char kmalloc();
-char valloc();
-char calloc();
-
-char free();
-char kfree();
-
-void testCustomArgumentlessAllocation() {
-  alloca(); // no-crash
-  malloc(); // no-crash
-  realloc(); // no-crash
-  kmalloc(); // no-crash
-  valloc(); // no-crash
-  calloc(); // no-crash
-
-  free(); // no-crash
-  kfree(); // no-crash
-}
-


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r284335 - [analyzer] Make MallocChecker more robust against custom redeclarations

[Devin Coughlin via cfe-commits]

> On Oct 16, 2016, at 12:04 PM, Renato Golin <renato.go...@linaro.org> wrote:
> 
> On 16 October 2016 at 18:26, Devin Coughlin via cfe-commits
> <cfe-commits@lists.llvm.org> wrote:
>> Author: dcoughlin
>> Date: Sun Oct 16 12:26:06 2016
>> New Revision: 284335
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=284335=rev
>> Log:
>> [analyzer] Make MallocChecker more robust against custom redeclarations
>> 
>> Add additional checking to MallocChecker to avoid crashing when memory
>> routines have unexpected numbers of arguments. You wouldn't expect to see 
>> much
>> of this in normal code (-Wincompatible-library-redeclaration warns on this),
>> but, for example, CMake tests can generate these.
> 
> Hi Devin,
> 
> Sounds like yours:
> 
> http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/11121

I’ll speculatively revert. But I don’t understand how a static analyzer-only 
change could affect either the execution time or the compile time of a 
test-suite benchmark. There’s something very funny here.

Devin

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r284335 - [analyzer] Make MallocChecker more robust against custom redeclarations

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Sun Oct 16 12:26:06 2016
New Revision: 284335

URL: http://llvm.org/viewvc/llvm-project?rev=284335=rev
Log:
[analyzer] Make MallocChecker more robust against custom redeclarations

Add additional checking to MallocChecker to avoid crashing when memory
routines have unexpected numbers of arguments. You wouldn't expect to see much
of this in normal code (-Wincompatible-library-redeclaration warns on this),
but, for example, CMake tests can generate these.

This is PR30616.

rdar://problem/28631974

Added:
cfe/trunk/test/Analysis/malloc-custom.c
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp?rev=284335=284334=284335=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/MallocChecker.cpp Sun Oct 16 12:26:06 
2016
@@ -778,6 +778,8 @@ void MallocChecker::checkPostStmt(const
   State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State);
   }
 } else if (FunI == II_kmalloc) {
+  if (CE->getNumArgs() < 1)
+return;
   llvm::Optional MaybeState =
 performKernelMalloc(CE, C, State);
   if (MaybeState.hasValue())
@@ -807,6 +809,8 @@ void MallocChecker::checkPostStmt(const
 } else if (FunI == II_strndup) {
   State = MallocUpdateRefState(C, CE, State);
 } else if (FunI == II_alloca || FunI == II_win_alloca) {
+  if (CE->getNumArgs() < 1)
+return;
   State = MallocMemAux(C, CE, CE->getArg(0), UndefinedVal(), State,
AF_Alloca);
   State = ProcessZeroAllocation(C, CE, 0, State);

Added: cfe/trunk/test/Analysis/malloc-custom.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/malloc-custom.c?rev=284335=auto
==
--- cfe/trunk/test/Analysis/malloc-custom.c (added)
+++ cfe/trunk/test/Analysis/malloc-custom.c Sun Oct 16 12:26:06 2016
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc 
-Wno-incompatible-library-redeclaration -verify %s
+
+// Various tests to make the the analyzer is robust against custom
+// redeclarations of memory routines.
+//
+// You wouldn't expect to see much of this in normal code, but, for example,
+// CMake tests can generate these.
+
+// expected-no-diagnostics
+
+char alloca();
+char malloc();
+char realloc();
+char kmalloc();
+char valloc();
+char calloc();
+
+char free();
+char kfree();
+
+void testCustomArgumentlessAllocation() {
+  alloca(); // no-crash
+  malloc(); // no-crash
+  realloc(); // no-crash
+  kmalloc(); // no-crash
+  valloc(); // no-crash
+  calloc(); // no-crash
+
+  free(); // no-crash
+  kfree(); // no-crash
+}
+


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r284317 - Revert "[analyzer] Re-apply r283093 "Add extra notes to ObjCDeallocChecker""

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Sat Oct 15 19:30:08 2016
New Revision: 284317

URL: http://llvm.org/viewvc/llvm-project?rev=284317=rev
Log:
Revert "[analyzer] Re-apply r283093 "Add extra notes to ObjCDeallocChecker""

Revert:
r283662: [analyzer] Re-apply r283093 "Add extra notes to ObjCDeallocChecker"
r283660: [analyzer] Fix build error after r283660 - remove constexpr strings.

It was causing an internal build bot to fail. It looks like in some cases
adding an extra note can cause scan-build plist output to drop a diagnostic
altogether.

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
cfe/trunk/test/Analysis/DeallocMissingRelease.m
cfe/trunk/test/Analysis/PR2978.m
cfe/trunk/test/Analysis/properties.m

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp?rev=284317=284316=284317=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp Sat Oct 15 
19:30:08 2016
@@ -108,10 +108,6 @@ class ObjCDeallocChecker
   std::unique_ptr ExtraReleaseBugType;
   std::unique_ptr MistakenDeallocBugType;
 
-  // FIXME: constexpr initialization isn't supported by MSVC2013.
-  static const char *const MsgDeclared;
-  static const char *const MsgSynthesized;
-
 public:
   ObjCDeallocChecker();
 
@@ -133,9 +129,6 @@ public:
   void checkEndFunction(CheckerContext ) const;
 
 private:
-  void addNoteForDecl(std::unique_ptr , StringRef Msg,
-   const Decl *D) const;
-
   void diagnoseMissingReleases(CheckerContext ) const;
 
   bool diagnoseExtraRelease(SymbolRef ReleasedValue, const ObjCMethodCall ,
@@ -187,11 +180,6 @@ private:
 
 typedef llvm::ImmutableSet SymbolSet;
 
-const char *const ObjCDeallocChecker::MsgDeclared =
-"Property is declared here";
-const char *const ObjCDeallocChecker::MsgSynthesized =
-"Property is synthesized here";
-
 /// Maps from the symbol for a class instance to the set of
 /// symbols remaining that must be released in -dealloc.
 REGISTER_MAP_WITH_PROGRAMSTATE(UnreleasedIvarMap, SymbolRef, SymbolSet)
@@ -503,18 +491,6 @@ ProgramStateRef ObjCDeallocChecker::chec
   return State;
 }
 
-/// Add an extra note piece describing a declaration that is important
-/// for understanding the bug report.
-void ObjCDeallocChecker::addNoteForDecl(std::unique_ptr ,
- StringRef Msg,
- const Decl *D) const {
-  ASTContext  = D->getASTContext();
-  SourceManager  = ACtx.getSourceManager();
-  PathDiagnosticLocation Pos = PathDiagnosticLocation::createBegin(D, SM);
-  if (Pos.isValid() && Pos.asLocation().isValid())
-BR->addNote(Msg, Pos, D->getSourceRange());
-}
-
 /// Report any unreleased instance variables for the current instance being
 /// dealloced.
 void ObjCDeallocChecker::diagnoseMissingReleases(CheckerContext ) const {
@@ -612,9 +588,6 @@ void ObjCDeallocChecker::diagnoseMissing
 std::unique_ptr BR(
 new BugReport(*MissingReleaseBugType, OS.str(), ErrNode));
 
-addNoteForDecl(BR, MsgDeclared, PropDecl);
-addNoteForDecl(BR, MsgSynthesized, PropImpl);
-
 C.emitReport(std::move(BR));
   }
 
@@ -718,12 +691,11 @@ bool ObjCDeallocChecker::diagnoseExtraRe
  );
 
   const ObjCImplDecl *Container = 
getContainingObjCImpl(C.getLocationContext());
-  const ObjCIvarDecl *IvarDecl = PropImpl->getPropertyIvarDecl();
-  OS << "The '" << *IvarDecl << "' ivar in '" << *Container;
+  OS << "The '" << *PropImpl->getPropertyIvarDecl()
+ << "' ivar in '" << *Container;
 
-  bool ReleasedByCIFilterDealloc = isReleasedByCIFilterDealloc(PropImpl);
 
-  if (ReleasedByCIFilterDealloc) {
+  if (isReleasedByCIFilterDealloc(PropImpl)) {
 OS << "' will be released by '-[CIFilter dealloc]' but also released here";
   } else {
 OS << "' was synthesized for ";
@@ -740,10 +712,6 @@ bool ObjCDeallocChecker::diagnoseExtraRe
   new BugReport(*ExtraReleaseBugType, OS.str(), ErrNode));
   BR->addRange(M.getOriginExpr()->getSourceRange());
 
-  addNoteForDecl(BR, MsgDeclared, PropDecl);
-  if (!ReleasedByCIFilterDealloc)
-addNoteForDecl(BR, MsgSynthesized, PropImpl);
-
   C.emitReport(std::move(BR));
 
   return true;

Modified: cfe/trunk/test/Analysis/DeallocMissingRelease.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/DeallocMissingRelease.m?rev=284317=284316=284317=diff
==
--- cfe/trunk/test/Analysis/DeallocMissingRelease.m (original)
+++ cfe/trunk/test/Analysis/DeallocMissingRelease.m Sat Oct 15 19:30:08 2016
@@ -81,9 +81,6 @@
 
 @interface MyPropertyClass1 : NSObject
 @property (copy) NSObject *ivar;
-#if NON_ARC
-// expected-note@-2 {{Property is declared here}}

[PATCH] D25326: [StaticAnalyser] Don't merge different returns in ExplodedGraph

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

In https://reviews.llvm.org/D25326#569061, @danielmarjamaki wrote:

> > You could also try to add a canary with clang analyzer eval after the if 
> > statement to force the test to fail if we do add this symbolic reasoning.
>
> sounds good. sorry but I don't see how to do it.


The trick is to not first store the UnknownVal into a local (the analyzer will 
automatically create a symbol for that when read).

Instead you can do something like:

  if (table[i] != 0) { }
  clang_analyzer_eval((table[i] != 0)) // expected-warning {{UNKNOWN}}

This way if the analyzer ever starts generating symbols for a read from 
table[i] then the case split will change the eval to emit TRUE on one path and 
FALSE on the other.


Repository:
  rL LLVM

https://reviews.llvm.org/D25326



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r284084 - [analyzer] DeallocChecker: Don't warn about directly-set IBOutlet ivars on macOS

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Wed Oct 12 18:57:05 2016
New Revision: 284084

URL: http://llvm.org/viewvc/llvm-project?rev=284084=rev
Log:
[analyzer] DeallocChecker: Don't warn about directly-set IBOutlet ivars on macOS

On macOS (but not iOS), if an ObjC property has no setter, the nib-loading code
for an IBOutlet is documented as directly setting the backing ivar without
retaining the value -- even if the property is 'retain'.

This resulted in false positives from the DeallocChecker for code that did not
release such ivars in -dealloc.

To avoid these false positives, treat IBOutlet ivars that back a property
without a setter as having an unknown release requirement in macOS.

rdar://problem/28507353

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
cfe/trunk/test/Analysis/DeallocMissingRelease.m

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp?rev=284084=284083=284084=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp Wed Oct 12 
18:57:05 2016
@@ -34,6 +34,7 @@
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/Basic/LangOptions.h"
+#include "clang/Basic/TargetInfo.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h"
@@ -180,6 +181,7 @@ private:
   bool classHasSeparateTeardown(const ObjCInterfaceDecl *ID) const;
 
   bool isReleasedByCIFilterDealloc(const ObjCPropertyImplDecl *PropImpl) const;
+  bool isNibLoadedIvarWithoutRetain(const ObjCPropertyImplDecl *PropImpl) 
const;
 };
 } // End anonymous namespace.
 
@@ -935,6 +937,9 @@ ReleaseRequirement ObjCDeallocChecker::g
 if (isReleasedByCIFilterDealloc(PropImpl))
   return ReleaseRequirement::MustNotReleaseDirectly;
 
+if (isNibLoadedIvarWithoutRetain(PropImpl))
+  return ReleaseRequirement::Unknown;
+
 return ReleaseRequirement::MustRelease;
 
   case ObjCPropertyDecl::Weak:
@@ -1091,6 +1096,32 @@ bool ObjCDeallocChecker::isReleasedByCIF
   return false;
 }
 
+/// Returns whether the ivar backing the property is an IBOutlet that
+/// has its value set by nib loading code without retaining the value.
+///
+/// On macOS, if there is no setter, the nib-loading code sets the ivar
+/// directly, without retaining the value,
+///
+/// On iOS and its derivatives, the nib-loading code will call
+/// -setValue:forKey:, which retains the value before directly setting the 
ivar.
+bool ObjCDeallocChecker::isNibLoadedIvarWithoutRetain(
+const ObjCPropertyImplDecl *PropImpl) const {
+  const ObjCIvarDecl *IvarDecl = PropImpl->getPropertyIvarDecl();
+  if (!IvarDecl->hasAttr())
+return false;
+
+  const llvm::Triple  =
+  IvarDecl->getASTContext().getTargetInfo().getTriple();
+
+  if (!Target.isMacOSX())
+return false;
+
+  if (PropImpl->getPropertyDecl()->getSetterMethodDecl())
+return false;
+
+  return true;
+}
+
 void ento::registerObjCDeallocChecker(CheckerManager ) {
   const LangOptions  = Mgr.getLangOpts();
   // These checker only makes sense under MRR.

Modified: cfe/trunk/test/Analysis/DeallocMissingRelease.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/DeallocMissingRelease.m?rev=284084=284083=284084=diff
==
--- cfe/trunk/test/Analysis/DeallocMissingRelease.m (original)
+++ cfe/trunk/test/Analysis/DeallocMissingRelease.m Wed Oct 12 18:57:05 2016
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=osx.cocoa.Dealloc -fblocks 
-verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=osx.cocoa.Dealloc -fblocks 
-triple x86_64-apple-ios4.0 -DMACOS=0 -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=osx.cocoa.Dealloc -fblocks 
-triple x86_64-apple-macosx10.6.0 -DMACOS=1 -verify %s
 // RUN: %clang_cc1 -analyze -analyzer-checker=osx.cocoa.Dealloc -fblocks 
-triple x86_64-apple-darwin10 -fobjc-arc -fobjc-runtime-has-weak -verify %s
 
 #include "Inputs/system-header-simulator-for-objc-dealloc.h"
@@ -938,3 +939,71 @@ __attribute__((objc_root_class))
 @implementation NotMissingDeallocCIFilter // no-warning
 @synthesize inputIvar = inputIvar;
 @end
+
+
+@interface ClassWithRetainPropWithIBOutletIvarButNoSetter : NSObject {
+  // On macOS, the nib-loading code will set the ivar directly without
+  // retaining value (unike iOS, where it is retained). This means that
+  // on macOS we should not warn about a missing release for a property backed
+  // by an IBOutlet ivar when that property does not have a setter.
+  IBOutlet NSObject *ivarForOutlet;
+}
+
+@property (readonly, retain) NSObject *ivarForOutlet;
+#if NON_ARC && !MACOS
+// 

[PATCH] D25326: [StaticAnalyser] Don't merge different returns in ExplodedGraph

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

In https://reviews.llvm.org/D25326#565919, @danielmarjamaki wrote:

> In https://reviews.llvm.org/D25326#564584, @zaks.anna wrote:
>
> > Please, fix the style issues before committing.
>
>
> Would it be ok to run clang-format on some files to clean up the formatting? 
> At least some minor changes like removing trailing spaces.


We generally try to avoid large-scale fixes of formatting and spacing issues if 
they are not on lines already modified by a patch. This makes it a bit easier 
to track down the original commit for a line and also reduces merge conflicts.


Repository:
  rL LLVM

https://reviews.llvm.org/D25326



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25326: [StaticAnalyser] Don't merge different returns in ExplodedGraph

[Devin Coughlin via cfe-commits]

dcoughlin added inline comments.



Comment at: test/Analysis/unreachable-code-path.c:201
+static int inlineFunction(const int i) {
+  if (table[i] != 0)
+return 1;

danielmarjamaki wrote:
> NoQ wrote:
> > a.sidorin wrote:
> > > I have a small question. Is it possible to simplify this sample with 
> > > removing of table[] array? Like putting something like `i != 0` into 
> > > condition. As I understand, the problem is not array-related.
> > Any `UnknownVal` in the condition would trigger this issue.
> > Is it possible to simplify this sample with removing of table[] array? 
> 
> I tried to simplify as much as possible. But as NoQ says an UnknownVal is 
> required here for this test.
This is worth a comment in the test then, so that if we ever add symbolic 
reasoning we'll know how to adjust the test.

You could also try to add a canary with clang analyzer eval after the if 
statement to force the test to fail if we do add this symbolic reasoning.


Repository:
  rL LLVM

https://reviews.llvm.org/D25326



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25326: [StaticAnalyser] Don't merge different returns in ExplodedGraph

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

Can you also add a test that tests this more directly (i.e., with 
clang_analyzer_warnIfReached). I don't think it is good to have the only test 
for this core coverage issue to be in tests for an alpha checker. Adding the 
direct test would also make it easier to track down any regression if it 
happens. The 'func.c' test file might be a good place for such a test.


https://reviews.llvm.org/D25326



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D23236: When ARC is enabled, no warning will be generated when a method1. Returns 'nil' in a method that is attributed to return a 'nonnull'2. The return-statement is a ConditionalOperator, wh

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

I left out the example I was describing above for 
'InitializationSequence::Perform()':

  @interface Bar
  - (nonnull Bar *)method;
  @end
  
  @implementation Bar
  - (Bar *)method {
return 0;
  }
  @end


https://reviews.llvm.org/D23236



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D23236: When ARC is enabled, no warning will be generated when a method1. Returns 'nil' in a method that is attributed to return a 'nonnull'2. The return-statement is a ConditionalOperator, wh

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

In https://reviews.llvm.org/D23236#561467, @parallaxe wrote:

> In https://reviews.llvm.org/D23236#557898, @dcoughlin wrote:
>
> > Upon reflection, I don't think this is the right approach.
> >
> > Desugaring any AttributedType in the return type seems like a really, 
> > really big hammer and could be an unexpected surprise for future attributed 
> > types where this is not the right behavior. I think a better approach for 
> > the nullability issue would be to update `lookThroughImplicitCasts()` in 
> > NullabilityChecker.cpp to look though `ExprWithCleanups` expressions just 
> > like it currently does for implicit casts.
> >
> > What do you think?
>
>
> As far as I understand the code, I would leave my change as it is.


You definitely can't just desugar the return type before passing it to 
InitializedEntity::InitializeResult().  Doing so would place a potential 
landmine for someone to trip over in the future for cases where the type 
qualifier has a real semantic meaning. For nullability qualifiers desugaring 
seems mostly harmless (except for the diagnostic regressions) but for an 
arbitrary type qualifier I don't think this is acceptable.

> The part of the code I have changed tries to guess the return-type for the 
> expression inside the return-stmt. At this point, no type could be computed 
> (as RelatedRetType.isNull() is true). Assuming that any annotation that the 
> return-type of the function has, will also apply to the return-type of the 
> expression, might be misleading.

This code doesn't assume that the the return type of the function is the return 
type of the expression. Instead, it uses the return type of the function to 
drive semantic analysis of the expression and apply any conversions needed from 
the type of the returned expression to the return type of the function. (In 
this case a 'NullToPointer' conversion). These conversions are often 
represented as implicit casts, added by Sema, in the AST. See 
InitializationSequence::Perform() for all the kinds of implicit conversions 
that can go on here. It is important to record these casts so that any codegen 
needed to perform the conversions/casts can be emitted at the right place. If 
such a conversion is not allowed, a diagnostic will be emitted.

> The improvements of the warnings in nullability.m are a good sign of this 
> (which would be lost when I would just make a change in the 
> NullabilityChecker).

The diagnostics changes to Sema/nullability.m in this patch would be a 
regression. Those diagnostic tests are there explicitly to test the 
functionality implemented in mergeInterfaceMethodToImpl() in SemaDeclObjC.cpp. 
This code takes a nullability annotation on a return type from a declaration 
and makes sure that the return type in the definition has the same nullability 
even if the programmer did not explicitly write it. This means that, for 
example, the return type of -[NSMerge methodA:] in the `@implementation` is 
supposed to be 'NSFoo * _Nonnull' since in the `@interface` it is annotated 
with 'nonnull'. You can see the merged return type for the method declaration 
by passing `-Xclang -ast-dump` to the driver.


https://reviews.llvm.org/D23236



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D23236: When ARC is enabled, no warning will be generated when a method1. Returns 'nil' in a method that is attributed to return a 'nonnull'2. The return-statement is a ConditionalOperator, wh

[Devin Coughlin via cfe-commits]

dcoughlin requested changes to this revision.
dcoughlin added a comment.
This revision now requires changes to proceed.

Upon reflection, I don't think this is the right approach.

Desugaring any AttributedType in the return type seems like a really, really 
big hammer and could be an unexpected surprise for future attributed types 
where this is not the right behavior. I think a better approach for the 
nullability issue would be to update `lookThroughImplicitCasts()` in 
NullabilityChecker.cpp to look though `ExprWithCleanups` expressions just like 
it currently does for implicit casts.

What do you think?


https://reviews.llvm.org/D23236



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24905: Fix unreachable code false positive, vardecl in switch

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

Sorry, missed this patch.

I think it would good to add a test to make sure we do warn when the var decl 
has an initializer, since that will not be executed.

  void varDecl(int X) {
switch (X) {
  int A = 12; // We do want a warning here, since the variable will be 
uninitialized in C (This is not allowed in C++).
case 1:
  ...
  break;
}
  }


Repository:
  rL LLVM

https://reviews.llvm.org/D24905



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24759: [RFC][StaticAnalyser] fix unreachable code false positives when block numbers mismatch

[Devin Coughlin via cfe-commits]

dcoughlin accepted this revision.
dcoughlin added a comment.
This revision is now accepted and ready to land.

LGTM. Please commit!


https://reviews.llvm.org/D24759



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23236: When ARC is enabled, no warning will be generated when a method1. Returns 'nil' in a method that is attributed to return a 'nonnull'2. The return-statement is a ConditionalOperator

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

@parallaxe Do you need someone to commit this for you?


https://reviews.llvm.org/D23236



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D23236: When ARC is enabled, no warning will be generated when a method1. Returns 'nil' in a method that is attributed to return a 'nonnull'2. The return-statement is a ConditionalOperator

[Devin Coughlin via cfe-commits]

dcoughlin accepted this revision.
dcoughlin added a comment.

LGTM.


https://reviews.llvm.org/D23236



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24759: [RFC][StaticAnalyser] fix unreachable code false positives when block numbers mismatch

[Devin Coughlin via cfe-commits]

dcoughlin requested changes to this revision.
dcoughlin added a reviewer: dcoughlin.
This revision now requires changes to proceed.


Comment at: lib/Analysis/CFG.cpp:2986
@@ -2985,3 +2985,1 @@
 
-if (!KnownVal.isFalse()) {
-  // Add an intermediate block between the BodyBlock and the

You need to keep this check so that the optimized CFG still removes edges that 
are trivially known to be false.


Comment at: lib/Analysis/CFG.cpp:2994
@@ -2993,3 @@
-  Succ = BodyBlock;
-  CFGBlock *LoopBackBlock = createBlock();
-  LoopBackBlock->setLoopTarget(D);

To keep the unoptimized and optimized blocks in sync, this block creation needs 
to be done regardless of whether the branch condition is known to be false. My 
advice would be to hoist the creation (and the FIXME comments)  to above the 
check for whether KnownVal is false.


https://reviews.llvm.org/D24759



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24792: [analyzer] Fix crash in RetainCountChecker::checkEndFunction

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

I suspect I introduced this regression in r264687.


Repository:
  rL LLVM

https://reviews.llvm.org/D24792



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24759: [RFC][StaticAnalyser] fix unreachable code false positives when block numbers mismatch

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

Here is the discussion on cfe-dev. 
http://lists.llvm.org/pipermail/cfe-dev/2016-September/050800.html

What's going on is that the path-sensitive unreachable code checker traverses 
blocks in the unoptimized CFG and emits a diagnostic if a block in that CFG is 
not reachable. But it decides whether a block is reachable by using the 
exploded node graph, which itself refers to the *optimized* CFG.

Normally this isn't a problem because the difference between the unoptimized 
and optimized CFGs is that the latter prunes edges that are trivially false 
(but the two CFGs have the same blocks). Unfortunately, there is a bug in CFG 
construction for do-while loops that also prunes a block. Since the block ids 
are constructed sequentially, the block IDs between the two CFGs don't match up 
and so the unreachable code checker gets confused.

I think the best fix here is to change construction of the optimized CFG to not 
prune the block in VisitDoStmt(). This matches what I think is the expected 
meaning of the 'PruneTriviallyFalseEdges' CFG option.


https://reviews.llvm.org/D24759



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24759: [RFC][StaticAnalyser] fix unreachable code false positives when block numbers mismatch

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

@danielmarjamaki I see what you mean -- thanks for providing the patch.

I don't think this is the right approach. It should be sufficient to reason 
about blocks and not individual statements; further some blocks may be 
non-empty but not have any statements.

Instead, I think the better approach is make sure the optimized and unoptimized 
CFGs match block IDs. It looks like the only situation where there is currently 
a mismatch is `do { } while (0);`.

I sketched out an approach on cfe-dev for hoisting block creation (but not edge 
creation) in CFGBuilder::VisitDoStmt() during CFG construction to match the 
other control flow constructs. That is the approach I would recommend taking.


https://reviews.llvm.org/D24759



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r281880 - [analyzer] SATestBuild.py: Treat '#' as comment in projectMap.csv

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Sun Sep 18 20:36:40 2016
New Revision: 281880

URL: http://llvm.org/viewvc/llvm-project?rev=281880=rev
Log:
[analyzer] SATestBuild.py: Treat '#' as comment in projectMap.csv

Treat lines in projectMap.csv that start with '#' as comments. This enables a
workflow where projects can be temporarily disabled with a comment describing
when they should be turned back on.

Differential Revision: https://reviews.llvm.org/D24709

Modified:
cfe/trunk/utils/analyzer/SATestBuild.py

Modified: cfe/trunk/utils/analyzer/SATestBuild.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/analyzer/SATestBuild.py?rev=281880=281879=281880=diff
==
--- cfe/trunk/utils/analyzer/SATestBuild.py (original)
+++ cfe/trunk/utils/analyzer/SATestBuild.py Sun Sep 18 20:36:40 2016
@@ -660,11 +660,17 @@ def testProject(ID, ProjectBuildMode, Is
 print "Completed tests for project %s (time: %.2f)." % \
   (ID, (time.time()-TBegin))
 
+def isCommentCSVLine(Entries):
+  # Treat CSV lines starting with a '#' as a comment.
+  return len(Entries) > 0 and Entries[0].startswith("#")
+
 def testAll(IsReferenceBuild = False, UpdateSVN = False, Strictness = 0):
 PMapFile = open(getProjectMapPath(), "rb")
 try:
 # Validate the input.
 for I in csv.reader(PMapFile):
+if (isCommentCSVLine(I)):
+continue
 if (len(I) != 2) :
 print "Error: Rows in the ProjectMapFile should have 3 
entries."
 raise Exception()
@@ -682,6 +688,8 @@ def testAll(IsReferenceBuild = False, Up
 # Test the projects.
 PMapFile.seek(0)
 for I in csv.reader(PMapFile):
+if isCommentCSVLine(I):
+  continue;
 testProject(I[0], int(I[1]), IsReferenceBuild, None, Strictness)
 
 # Add reference results to SVN.


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24709: [analyzer] SATestBuild.py: Treat '#' as comment in projectMap.csv

[Devin Coughlin via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281880: [analyzer] SATestBuild.py: Treat '#' as comment in 
projectMap.csv (authored by dcoughlin).

Changed prior to commit:
  https://reviews.llvm.org/D24709?vs=71757=71766#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24709

Files:
  cfe/trunk/utils/analyzer/SATestBuild.py

Index: cfe/trunk/utils/analyzer/SATestBuild.py
===
--- cfe/trunk/utils/analyzer/SATestBuild.py
+++ cfe/trunk/utils/analyzer/SATestBuild.py
@@ -660,11 +660,17 @@
 print "Completed tests for project %s (time: %.2f)." % \
   (ID, (time.time()-TBegin))
 
+def isCommentCSVLine(Entries):
+  # Treat CSV lines starting with a '#' as a comment.
+  return len(Entries) > 0 and Entries[0].startswith("#")
+
 def testAll(IsReferenceBuild = False, UpdateSVN = False, Strictness = 0):
 PMapFile = open(getProjectMapPath(), "rb")
 try:
 # Validate the input.
 for I in csv.reader(PMapFile):
+if (isCommentCSVLine(I)):
+continue
 if (len(I) != 2) :
 print "Error: Rows in the ProjectMapFile should have 3 
entries."
 raise Exception()
@@ -682,6 +688,8 @@
 # Test the projects.
 PMapFile.seek(0)
 for I in csv.reader(PMapFile):
+if isCommentCSVLine(I):
+  continue;
 testProject(I[0], int(I[1]), IsReferenceBuild, None, Strictness)
 
 # Add reference results to SVN.


Index: cfe/trunk/utils/analyzer/SATestBuild.py
===
--- cfe/trunk/utils/analyzer/SATestBuild.py
+++ cfe/trunk/utils/analyzer/SATestBuild.py
@@ -660,11 +660,17 @@
 print "Completed tests for project %s (time: %.2f)." % \
   (ID, (time.time()-TBegin))
 
+def isCommentCSVLine(Entries):
+  # Treat CSV lines starting with a '#' as a comment.
+  return len(Entries) > 0 and Entries[0].startswith("#")
+
 def testAll(IsReferenceBuild = False, UpdateSVN = False, Strictness = 0):
 PMapFile = open(getProjectMapPath(), "rb")
 try:
 # Validate the input.
 for I in csv.reader(PMapFile):
+if (isCommentCSVLine(I)):
+continue
 if (len(I) != 2) :
 print "Error: Rows in the ProjectMapFile should have 3 entries."
 raise Exception()
@@ -682,6 +688,8 @@
 # Test the projects.
 PMapFile.seek(0)
 for I in csv.reader(PMapFile):
+if isCommentCSVLine(I):
+  continue;
 testProject(I[0], int(I[1]), IsReferenceBuild, None, Strictness)
 
 # Add reference results to SVN.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24709: [analyzer] SATestBuild.py: Treat '#' as comment in projectMap.csv

[Devin Coughlin via cfe-commits]

dcoughlin created this revision.
dcoughlin added reviewers: zaks.anna, NoQ.
dcoughlin added a subscriber: cfe-commits.

Treat lines in projectMap.csv that start with '#' as comments.

This enables a workflow where projects can be temporarily disabled with a 
comment describing when they should be turned back on.

https://reviews.llvm.org/D24709

Files:
  utils/analyzer/SATestBuild.py

Index: utils/analyzer/SATestBuild.py
===
--- utils/analyzer/SATestBuild.py
+++ utils/analyzer/SATestBuild.py
@@ -660,11 +660,17 @@
 print "Completed tests for project %s (time: %.2f)." % \
   (ID, (time.time()-TBegin))
 
+def isCommentCSVLine(Entries):
+  # Treat CSV lines starting with a '#' as a comment.
+  return len(Entries) > 0 and Entries[0].startswith("#")
+
 def testAll(IsReferenceBuild = False, UpdateSVN = False, Strictness = 0):
 PMapFile = open(getProjectMapPath(), "rb")
 try:
 # Validate the input.
 for I in csv.reader(PMapFile):
+if (isCommentCSVLine(I)):
+continue
 if (len(I) != 2) :
 print "Error: Rows in the ProjectMapFile should have 3 
entries."
 raise Exception()
@@ -682,6 +688,8 @@
 # Test the projects.
 PMapFile.seek(0)
 for I in csv.reader(PMapFile):
+if isCommentCSVLine(I):
+  continue;
 testProject(I[0], int(I[1]), IsReferenceBuild, None, Strictness)
 
 # Add reference results to SVN.


Index: utils/analyzer/SATestBuild.py
===
--- utils/analyzer/SATestBuild.py
+++ utils/analyzer/SATestBuild.py
@@ -660,11 +660,17 @@
 print "Completed tests for project %s (time: %.2f)." % \
   (ID, (time.time()-TBegin))
 
+def isCommentCSVLine(Entries):
+  # Treat CSV lines starting with a '#' as a comment.
+  return len(Entries) > 0 and Entries[0].startswith("#")
+
 def testAll(IsReferenceBuild = False, UpdateSVN = False, Strictness = 0):
 PMapFile = open(getProjectMapPath(), "rb")
 try:
 # Validate the input.
 for I in csv.reader(PMapFile):
+if (isCommentCSVLine(I)):
+continue
 if (len(I) != 2) :
 print "Error: Rows in the ProjectMapFile should have 3 entries."
 raise Exception()
@@ -682,6 +688,8 @@
 # Test the projects.
 PMapFile.seek(0)
 for I in csv.reader(PMapFile):
+if isCommentCSVLine(I):
+  continue;
 testProject(I[0], int(I[1]), IsReferenceBuild, None, Strictness)
 
 # Add reference results to SVN.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24278: [analyzer] Extend bug reports with extra notes.

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

This is awesome! I have some minor comments on the dealloc notes. It is also 
fine to remove them entirely; we can add them in a later commit.



Comment at: lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp:604
@@ -588,1 +603,3 @@
 
+addExtraNoteForDecl(BR, "The property is declared here", PropDecl);
+

To be consistent with the rest of clang, I think it is better to remove the 
"The". That is: "Property is declared here" and "Property is synthesized here".


Comment at: lib/StaticAnalyzer/Checkers/CheckObjCDealloc.cpp:733
@@ -712,1 +732,3 @@
 
+  addExtraNoteForDecl(BR, "The property is declared here", PropDecl);
+

Is it possible to factor this out so we only have the hard-coded note text for 
each of "declared"/"synthesized" in one spot? This will make it easier to 
update the text if we decide to change it.


https://reviews.llvm.org/D24278



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20811: [analyzer] Model some library functions

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

In https://reviews.llvm.org/D20811#544981, @NoQ wrote:

> Hmm, what about
>
>   CONSTRAIN
> ARGUMENT_VALUE(0, WithinRange)
>   RANGE('0', '9')
>   RANGE('A', 'Z')
>   RANGE('a', 'z')
> END_ARGUMENT_VALUE
> RETURN_VALUE(OutOfRange)
>   VALUE(0)
> END_RETURN_VALUE
>   END_CONSTRAIN
>   
>
> ?


"CONSTRAIN" is a verb. What is the direct object here? It seems to me that the 
thing being constrained is the return value, so it seems odd to have 
'CONSTRAIN' around the conditions on the arguments.

> Something i don't like here is that the word "value" is overloaded. Maybe 
> rename the inner `VALUE` back to `POINT`?


I don't think the geometric metaphor of 'POINT' makes sense here, especially 
with 'RANGE' (which I think is very good). What is the analog of a range that 
has only a single element?



Comment at: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:547
@@ +546,3 @@
+RANGE {
+  RET_VAL, RANGE_KIND(OutOfRange),
+  SET { POINT(0) }

NoQ wrote:
> dcoughlin wrote:
> > Is it ever the case that this final 'RANGE" constrains anything other than 
> > the return value? If not, can 'RET_VAL' be elided?
> Some summaries only have pre-conditions: "for this argument constraint, any 
> return value is possible". We should also be able to support void functions, 
> which have no return values.
What does a postcondition on a void function mean in this context? Can you 
refer to argument values? Such as "If the the function terminates then it must 
have been the case that the first argument was in the rangy x..z even though we 
didn't know that going in? Is this useful?


https://reviews.llvm.org/D20811



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20811: [analyzer] Model some library functions

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

I think this is much clearer! That said, now that I look at it with 
'POSTCONDITION' alone I don't think it is clear that the provided value 
describes the return value. What do you think about renaming it 'RETURN_VALUE'? 
Or adding back the RET_VAL I asked you about removing before? :-)

Also: do you think CONDITION_KIND is needed? in PRECONDITION? Or can the bare 
kind be used like in POSTCONDITION?


https://reviews.llvm.org/D20811



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20811: [analyzer] Model some library functions

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

Thanks for adding the macros. I've provided some feedback inline.

I think a good rule of thumb for readability is: suppose you are a maintainer 
and need to add a summary for a new function. Can you copy the the summary for 
an existing function and figure out what each component means so you can change 
it for the new function?



Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:419
@@ -418,1 +418,3 @@
 
+def StdLibraryFunctionsChecker : Checker<"StdLibraryFunctions">,
+  HelpText<"Improve modeling of standard library functions">,

I know you and Gábor already discussed this -- but shouldn't this be 
CStdLibraryFunctionsChecker or 'StdCLibraryFunctionsChecker'? Or is is your 
intent that both C and C++ standard libraries would be modeled by this checker?


Comment at: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:11
@@ +10,3 @@
+// This checker improves modeling of a few simple library functions.
+// It does not throw warnings.
+//

"throw" --> "generate"


Comment at: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:534
@@ +533,3 @@
+// The isascii() family of functions.
+SPEC {
+  FOR_FUNCTION(isalnum),

Is "specification" the right term here? Or is this really a "summary"?


Comment at: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:536
@@ +535,3 @@
+  FOR_FUNCTION(isalnum),
+  SPEC_DATA {
+ARGUMENT_TYPES { IntTy },

'SPEC_DATA' doesn't seem to add much in terms of readability. Is it needed?


Comment at: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:537
@@ +536,3 @@
+  SPEC_DATA {
+ARGUMENT_TYPES { IntTy },
+RETURN_TYPE(IntTy),

The argument and return types seem like more a property of the function than 
than the summary. Why are they here and not with the function name?


Comment at: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:540
@@ +539,3 @@
+INVALIDATION_APPROACH(EvalCallAsPure),
+BRANCHES {
+  BRANCH { // Boils down to isupper() or islower() or isdigit()

"Cases" seems more appropriate than "branches" (branching is an implementation 
detail).


Comment at: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:542
@@ +541,3 @@
+  BRANCH { // Boils down to isupper() or islower() or isdigit()
+RANGE {
+  ARG_NO(0), RANGE_KIND(WithinRange),

If I understand correctly, the first "argument" to branch describes the 
constraints on the function arguments and the second (if present) describes the 
resulting constraint on the return value when the argument constraint holds. Is 
there a way to make this apparent in the spelling of the summary?

As a straw proposal, what about renaming the first 'RANGE' to 
'ARGUMENT_CONSTRAINT' and the second the 'RETURN_VALUE_CONSTRAINT'?

Or, more jargony, "PRECONDITION" and "POSTCONDITION"?


Comment at: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:547
@@ +546,3 @@
+RANGE {
+  RET_VAL, RANGE_KIND(OutOfRange),
+  SET { POINT(0) }

Is it ever the case that this final 'RANGE" constrains anything other than the 
return value? If not, can 'RET_VAL' be elided?


Comment at: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:554
@@ +553,3 @@
+  ARG_NO(0), RANGE_KIND(WithinRange),
+  SET { SEG(128, 255) }
+}

What is the motivation behind the use of geometric terms here (i.e., "SEG", 
"POINT")? Why not "INTERVAL" and "EXACT_VALUE"?


Comment at: lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:560
@@ +559,3 @@
+  ARG_NO(0), RANGE_KIND(OutOfRange),
+  SET { SEG('0', '9') U SEG('A', 'Z')
+  U SEG('a', 'z') U SEG(128, 255)}

Would you be opposed to 'UNION' instead of 'U'?


https://reviews.llvm.org/D20811



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [cfe-dev] [Static Analyzer] Retain count checker does not warn about parameters that might leak

[Devin Coughlin via cfe-commits]

- cfe-dev, + cfe-commits

Hi Tobias,

This is a great start. Thanks for the patch!

> Not sure I know how to use the analyzer config flags yet. My patch
> currently also still causes a segfault on Analysis/retain-release.m

The segfault is because the diagnostics machinery expects a declaration with 
body for the uniquing declaration (See below).

> and
> changes a test case, where I am not sure it should.
> 
> I probably need some time to get this tested and get more confidence in
> what it is doing. I will then put it up for review and we can see how to
> add an analyzer-config flag.


You can take a look at the MallocChecker and the malloc-annotations.c test to 
see how analyzer-config flags work.

Also, in the future, it is probably best to submit patches on Phabricator: 
> This makes it easier for 
multiple people to comment on the same patch.

> On Sep 8, 2016, at 2:15 AM, Tobias Grosser  wrote:
> 
> And here a slightly more cleaned up version of my patch, just for
> reference.
> 
> Best,
> Tobias
> <0001-RetainCountChecker-Also-check-function-parameters-fo.patch>

> From 19654ebf904c11e3fa5d7f8c938d4a0bb863fbf0 Mon Sep 17 00:00:00 2001
> From: Tobias Grosser 
> Date: Thu, 8 Sep 2016 09:38:36 +0200
> Subject: [PATCH] RetainCountChecker: Also check function parameters for leaks
> 
> ---
>  lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp | 87 
> +++---
>  test/Analysis/retain-release-function-parameters.m | 26 +++
>  test/Analysis/retain-release-gc-only.m |  2 +-
>  3 files changed, 105 insertions(+), 10 deletions(-)
>  create mode 100644 test/Analysis/retain-release-function-parameters.m
> 
> diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp 
> b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
> index d445e91..71035a0 100644
> --- a/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
> +++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp
> @@ -1823,6 +1823,13 @@ namespace {
>  
>class CFRefLeakReport : public CFRefReport {
>  const MemRegion* AllocBinding;
> +const Stmt *AllocStmt;

It looks like neither of these instance variables are referred to after object 
construction. Do they really need to be ivars?

> +
> +void deriveParamLocation(CheckerContext , SymbolRef sym);
> +void deriveAllocLocation(CheckerContext , SymbolRef sym);
> +void createDescription(CheckerContext ,
> +   bool GCEnabled, bool IncludeAllocationLine);
> +
>public:
>  CFRefLeakReport(CFRefBug , const LangOptions , bool GCEnabled,
>  const SummaryLogTy , ExplodedNode *n, SymbolRef sym,
> @@ -2388,13 +2395,25 @@ CFRefLeakReportVisitor::getEndPath(BugReporterContext 
> ,
>return llvm::make_unique(L, os.str());
>  }
>  
> -CFRefLeakReport::CFRefLeakReport(CFRefBug , const LangOptions ,
> - bool GCEnabled, const SummaryLogTy ,
> - ExplodedNode *n, SymbolRef sym,
> - CheckerContext ,
> - bool IncludeAllocationLine)
> -  : CFRefReport(D, LOpts, GCEnabled, Log, n, sym, false) {
> +void CFRefLeakReport::deriveParamLocation(CheckerContext , SymbolRef 
> sym) {
> +  const SourceManager& SMgr = Ctx.getSourceManager();
> +
> +  if (!sym->getOriginRegion())
> +return;
>  

Some style notes: In LLVM style we typically use ‘auto *’ for pointers to make 
it clear there is not an
expensive copy. For example: 'auto *PDecl = …’

We also try to avoided nested indentation where possible. So, for example:
  if (!Region)
return;

> +  auto Region = dyn_cast(sym->getOriginRegion());
> +  if (Region) {
> +auto PDecl = Region->getDecl();
> +if (PDecl && isa(PDecl)) {

I think it is worth adding a comment here explaining that the location
of the leak is the location of the parameter annotated as consumed.

> +  auto ParamLocation = PathDiagnosticLocation::create(PDecl, SMgr);
> +  Location = ParamLocation;
> +  UniqueingLocation = ParamLocation;
> +  UniqueingDecl = PDecl;

Rather than using PDecl here, you can use the declaration for the containing 
function:
UniqueingDecl = Ctx.getLocationContext()->getDecl();

This will avoid the crash when flushing diagnostics.

> +}
> +  }
> +}
> +
> +void CFRefLeakReport::deriveAllocLocation(CheckerContext ,SymbolRef sym) 
> {
>// Most bug reports are cached at the location where they occurred.
>// With leaks, we want to unique them by the location where they were
>// allocated, and only report a single path.  To do this, we need to find
> @@ -2418,8 +2437,12 @@ CFRefLeakReport::CFRefLeakReport(CFRefBug , const 
> LangOptions ,
>// FIXME: This will crash the analyzer if an allocation comes from an
>// implicit call (ex: a destructor call).
>// (Currently there are no such allocations in Cocoa, though.)
> 

Re: [PATCH] D24470: [analyzer] scan-build-py: Remove relative path hack for SATestsBuild.py

[Devin Coughlin via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL281516: [analyzer] scan-build-py: Remove relative path hack 
for SATestsBuild.py (authored by dcoughlin).

Changed prior to commit:
  https://reviews.llvm.org/D24470?vs=71046=71401#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24470

Files:
  cfe/trunk/tools/scan-build-py/libscanbuild/runner.py
  cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py

Index: cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py
===
--- cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py
+++ cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py
@@ -219,20 +219,6 @@
 self.assertEqual(['-DNDEBUG', '-UNDEBUG'], test(['-DNDEBUG']))
 self.assertEqual(['-DSomething', '-UNDEBUG'], test(['-DSomething']))
 
-def test_set_file_relative_path(self):
-def test(expected, input):
-spy = Spy()
-self.assertEqual(spy.success,
- sut.set_file_path_relative(input, spy.call))
-self.assertEqual(expected, spy.arg['file'])
-
-test('source.c',
- {'file': '/home/me/source.c', 'directory': '/home/me'})
-test('me/source.c',
- {'file': '/home/me/source.c', 'directory': '/home'})
-test('../home/me/source.c',
- {'file': '/home/me/source.c', 'directory': '/tmp'})
-
 def test_set_language_fall_through(self):
 def language(expected, input):
 spy = Spy()
Index: cfe/trunk/tools/scan-build-py/libscanbuild/runner.py
===
--- cfe/trunk/tools/scan-build-py/libscanbuild/runner.py
+++ cfe/trunk/tools/scan-build-py/libscanbuild/runner.py
@@ -205,19 +205,8 @@
 return continuation(opts)
 
 
-@require(['file', 'directory'])
-def set_file_path_relative(opts, continuation=filter_debug_flags):
-""" Set source file path to relative to the working directory.
-
-The only purpose of this function is to pass the SATestBuild.py tests. """
-
-opts.update({'file': os.path.relpath(opts['file'], opts['directory'])})
-
-return continuation(opts)
-
-
 @require(['language', 'compiler', 'file', 'flags'])
-def language_check(opts, continuation=set_file_path_relative):
+def language_check(opts, continuation=filter_debug_flags):
 """ Find out the language from command line parameters or file name
 extension. The decision also influenced by the compiler invocation. """
 


Index: cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py
===
--- cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py
+++ cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py
@@ -219,20 +219,6 @@
 self.assertEqual(['-DNDEBUG', '-UNDEBUG'], test(['-DNDEBUG']))
 self.assertEqual(['-DSomething', '-UNDEBUG'], test(['-DSomething']))
 
-def test_set_file_relative_path(self):
-def test(expected, input):
-spy = Spy()
-self.assertEqual(spy.success,
- sut.set_file_path_relative(input, spy.call))
-self.assertEqual(expected, spy.arg['file'])
-
-test('source.c',
- {'file': '/home/me/source.c', 'directory': '/home/me'})
-test('me/source.c',
- {'file': '/home/me/source.c', 'directory': '/home'})
-test('../home/me/source.c',
- {'file': '/home/me/source.c', 'directory': '/tmp'})
-
 def test_set_language_fall_through(self):
 def language(expected, input):
 spy = Spy()
Index: cfe/trunk/tools/scan-build-py/libscanbuild/runner.py
===
--- cfe/trunk/tools/scan-build-py/libscanbuild/runner.py
+++ cfe/trunk/tools/scan-build-py/libscanbuild/runner.py
@@ -205,19 +205,8 @@
 return continuation(opts)
 
 
-@require(['file', 'directory'])
-def set_file_path_relative(opts, continuation=filter_debug_flags):
-""" Set source file path to relative to the working directory.
-
-The only purpose of this function is to pass the SATestBuild.py tests. """
-
-opts.update({'file': os.path.relpath(opts['file'], opts['directory'])})
-
-return continuation(opts)
-
-
 @require(['language', 'compiler', 'file', 'flags'])
-def language_check(opts, continuation=set_file_path_relative):
+def language_check(opts, continuation=filter_debug_flags):
 """ Find out the language from command line parameters or file name
 extension. The decision also influenced by the compiler invocation. """
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r281516 - [analyzer] scan-build-py: Remove relative path hack for SATestsBuild.py

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Wed Sep 14 13:14:11 2016
New Revision: 281516

URL: http://llvm.org/viewvc/llvm-project?rev=281516=rev
Log:
[analyzer] scan-build-py: Remove relative path hack for SATestsBuild.py

Remove the relative path hack in scan-build-py that converts a fully qualified
directory name and a fully qualified file path to a relative path before running
the analyzer on a file.

This hack is not needed: the bad interaction with SATestsBuild.py it was
intended to address is actually the same underlying problem that r280768 fixed.
Further, because the hack would always relativize paths, it caused
SATestBuild.py to be unable to properly line up issues when the build system
changed directory and then built a source file in a child directory but used a
fully-qualified path for the source file.

Differential Revision: https://reviews.llvm.org/D24470

Modified:
cfe/trunk/tools/scan-build-py/libscanbuild/runner.py
cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py

Modified: cfe/trunk/tools/scan-build-py/libscanbuild/runner.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-build-py/libscanbuild/runner.py?rev=281516=281515=281516=diff
==
--- cfe/trunk/tools/scan-build-py/libscanbuild/runner.py (original)
+++ cfe/trunk/tools/scan-build-py/libscanbuild/runner.py Wed Sep 14 13:14:11 
2016
@@ -205,19 +205,8 @@ def filter_debug_flags(opts, continuatio
 return continuation(opts)
 
 
-@require(['file', 'directory'])
-def set_file_path_relative(opts, continuation=filter_debug_flags):
-""" Set source file path to relative to the working directory.
-
-The only purpose of this function is to pass the SATestBuild.py tests. """
-
-opts.update({'file': os.path.relpath(opts['file'], opts['directory'])})
-
-return continuation(opts)
-
-
 @require(['language', 'compiler', 'file', 'flags'])
-def language_check(opts, continuation=set_file_path_relative):
+def language_check(opts, continuation=filter_debug_flags):
 """ Find out the language from command line parameters or file name
 extension. The decision also influenced by the compiler invocation. """
 

Modified: cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py?rev=281516=281515=281516=diff
==
--- cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py (original)
+++ cfe/trunk/tools/scan-build-py/tests/unit/test_runner.py Wed Sep 14 13:14:11 
2016
@@ -219,20 +219,6 @@ class AnalyzerTest(unittest.TestCase):
 self.assertEqual(['-DNDEBUG', '-UNDEBUG'], test(['-DNDEBUG']))
 self.assertEqual(['-DSomething', '-UNDEBUG'], test(['-DSomething']))
 
-def test_set_file_relative_path(self):
-def test(expected, input):
-spy = Spy()
-self.assertEqual(spy.success,
- sut.set_file_path_relative(input, spy.call))
-self.assertEqual(expected, spy.arg['file'])
-
-test('source.c',
- {'file': '/home/me/source.c', 'directory': '/home/me'})
-test('me/source.c',
- {'file': '/home/me/source.c', 'directory': '/home'})
-test('../home/me/source.c',
- {'file': '/home/me/source.c', 'directory': '/tmp'})
-
 def test_set_language_fall_through(self):
 def language(expected, input):
 spy = Spy()


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24470: [analyzer] scan-build-py: Remove relative path hack for SATestsBuild.py

[Devin Coughlin via cfe-commits]

dcoughlin created this revision.
dcoughlin added a reviewer: rizsotto.mailinglist.
dcoughlin added a subscriber: cfe-commits.

Remove the relative path hack in scan-build-py that converts a fully qualified 
directory name and a fully qualified file path to a relative path before 
running the analyzer on a file.

This hack is not needed: the bad interaction with SATestsBuild.py it was 
intended to address is actually the same underlying problem that D24163 fixed. 
Further, because the hack would always relativize paths, it caused 
SATestBuild.py to be unable to properly line up issues when the build system 
changed directory and then built a source file in a child directory but used a 
fully-qualified path for the source file.

Laszlo: Are you OK with me removing this hack? This is the last outstanding 
issue in scan-build-py preventing us in using it for our regression suite.

Note: I still think the approach used here (and especially in intercept-build) 
is not ideal. I really think we should be storing both the working directory 
and the actual path argument (even if it is relative) in the compilation 
database. Storing the fully-qualified paths loses information (i.e., which of 
the many possible paths for that file was actually used) and makes replaying 
the build with fidelity impossible. That said, after D24163 I don't have any 
examples where is actually an issue in practice.

https://reviews.llvm.org/D24470

Files:
  tools/scan-build-py/libscanbuild/runner.py
  tools/scan-build-py/tests/unit/test_runner.py

Index: tools/scan-build-py/tests/unit/test_runner.py
===
--- tools/scan-build-py/tests/unit/test_runner.py
+++ tools/scan-build-py/tests/unit/test_runner.py
@@ -219,20 +219,6 @@
 self.assertEqual(['-DNDEBUG', '-UNDEBUG'], test(['-DNDEBUG']))
 self.assertEqual(['-DSomething', '-UNDEBUG'], test(['-DSomething']))
 
-def test_set_file_relative_path(self):
-def test(expected, input):
-spy = Spy()
-self.assertEqual(spy.success,
- sut.set_file_path_relative(input, spy.call))
-self.assertEqual(expected, spy.arg['file'])
-
-test('source.c',
- {'file': '/home/me/source.c', 'directory': '/home/me'})
-test('me/source.c',
- {'file': '/home/me/source.c', 'directory': '/home'})
-test('../home/me/source.c',
- {'file': '/home/me/source.c', 'directory': '/tmp'})
-
 def test_set_language_fall_through(self):
 def language(expected, input):
 spy = Spy()
Index: tools/scan-build-py/libscanbuild/runner.py
===
--- tools/scan-build-py/libscanbuild/runner.py
+++ tools/scan-build-py/libscanbuild/runner.py
@@ -205,19 +205,8 @@
 return continuation(opts)
 
 
-@require(['file', 'directory'])
-def set_file_path_relative(opts, continuation=filter_debug_flags):
-""" Set source file path to relative to the working directory.
-
-The only purpose of this function is to pass the SATestBuild.py tests. """
-
-opts.update({'file': os.path.relpath(opts['file'], opts['directory'])})
-
-return continuation(opts)
-
-
 @require(['language', 'compiler', 'file', 'flags'])
-def language_check(opts, continuation=set_file_path_relative):
+def language_check(opts, continuation=filter_debug_flags):
 """ Find out the language from command line parameters or file name
 extension. The decision also influenced by the compiler invocation. """
 


Index: tools/scan-build-py/tests/unit/test_runner.py
===
--- tools/scan-build-py/tests/unit/test_runner.py
+++ tools/scan-build-py/tests/unit/test_runner.py
@@ -219,20 +219,6 @@
 self.assertEqual(['-DNDEBUG', '-UNDEBUG'], test(['-DNDEBUG']))
 self.assertEqual(['-DSomething', '-UNDEBUG'], test(['-DSomething']))
 
-def test_set_file_relative_path(self):
-def test(expected, input):
-spy = Spy()
-self.assertEqual(spy.success,
- sut.set_file_path_relative(input, spy.call))
-self.assertEqual(expected, spy.arg['file'])
-
-test('source.c',
- {'file': '/home/me/source.c', 'directory': '/home/me'})
-test('me/source.c',
- {'file': '/home/me/source.c', 'directory': '/home'})
-test('../home/me/source.c',
- {'file': '/home/me/source.c', 'directory': '/tmp'})
-
 def test_set_language_fall_through(self):
 def language(expected, input):
 spy = Spy()
Index: tools/scan-build-py/libscanbuild/runner.py
===
--- tools/scan-build-py/libscanbuild/runner.py
+++ tools/scan-build-py/libscanbuild/runner.py
@@ -205,19 +205,8 @@
 return continuation(opts)
 
 
-@require(['file', 'directory'])
-def 

Re: [PATCH] D23963: [analyzer] pr28449 - Move literal rvalue construction away from RegionStore.

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

I agree that it is weird that region store modifies the value it was asked to 
bind and over all this seems like the right approach.

My big concern with this patch is that the logic that looks whether an lval is 
being stored into a non-reference location and dereferences it seems overly 
broad. I'm worried that this could paper over other issues by "fixing up" 
mismatches that are really the result of some other bug. Is there a way to make 
that logic more targeted? For example, if it only applies to initializing char 
arrays with string literals, that is a much more precise check for when to load 
from the lvalue.



Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:536
@@ -535,3 +535,3 @@
   } else {
 // We bound the temp obj region to the CXXConstructExpr. Now recover
 // the lazy compound value when the variable is not a reference.

It seems like this comment ("We bound the temp obj region to the 
CXXConstructorExpr...") is incomplete/misleading (and has been for a while) 
because this code is executed when there is no constructor. Can it be updated 
to something more helpful?


Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:665
@@ +664,3 @@
+// type, as any expression. We need to read this type and make the
+// necessary conversions.
+if (const RecordType *RT =

I am surprised Sema wouldn't have already added an implicit cast with an lvalue 
to rvalue conversion when needed. It seems in many cases it already does. Can 
you be more explicit in your comments about why this is needed and under what 
situations Sema doesn't handle this logic? Is initializing an array with a 
string literal the only time this is needed? If so, can the check be more 
targeted? I'll note that `InitListExpr` has an `isStringLiteralInit()` method 
and that CodeGen has a special case for it.


Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:669
@@ +668,3 @@
+  // For structures, check if the respective field is a reference.
+  // FIXME: What if fields mismatch?
+  const RecordDecl *RD = RT->getDecl();

NoQ wrote:
> Whoops, was a note to myself, didn't mean to leave it here, sorry. Will 
> actually have a look and update.
It is probably also worth looking into what it would take to support unions 
here.


Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:673
@@ +672,3 @@
+  auto FieldI = RD->field_begin(), FieldE = RD->field_end();
+  for (auto InitI = IE->rbegin(), InitE = IE->rend();
+   InitI != InitE; ++InitI) {

If you are going to iterate through the initializer expressions in reverse, you 
also have to iterate through the field decls in reverse. Also, I think it is 
worth commenting why you iterate in reverse order here.


Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:679
@@ +678,3 @@
+SVal V = state->getSVal(E, LCtx);
+if (E->isLValue() && !V.isUnknownOrUndef() &&
+!FieldI->getType()->isReferenceType())

This logic for checking with an expression is an lvalue and whether the type of 
the storage location is reference and loading from the lvalue if not is 
duplicated 3 times in this patch. Can you factor it out and give an meaningful 
name?


Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:696
@@ +695,3 @@
+  V = state->getSVal(V.castAs(), ET);
+vals = getBasicVals().consVals(V, vals);
+  }

I think it would good to add a test with array fillers if we don't already have 
them. For example:

```
int a[2000] = {1, 2, 3};
clang_analyzer_eval(a[0] == 1);
clang_analyzer_eval(a[1999] == 0);
```


Comment at: lib/StaticAnalyzer/Core/ExprEngineC.cpp:699
@@ +698,3 @@
+} else {
+  // Vector values, complex numbers, etc: No lvalues to expect.
+  for (auto InitI = IE->rbegin(), InitE = IE->rend();

Should there be an assert here to make sure we have enumerated all the cases? 
I'm worried about the 'etc'. How will we know if we have not considered a case?


https://reviews.llvm.org/D23963



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits