from:"Sean Eveson via cfe\-commits"

r322000 - Fix test added in r321992 failing on some buildbots (again), test requires x86.

2018-01-08 Thread Sean Eveson via cfe-commits

Author: seaneveson
Date: Mon Jan  8 07:46:18 2018
New Revision: 322000

URL: http://llvm.org/viewvc/llvm-project?rev=322000=rev
Log:
Fix test added in r321992 failing on some buildbots (again), test requires x86.

Modified:
cfe/trunk/test/CodeGen/stack-size-section.c

Modified: cfe/trunk/test/CodeGen/stack-size-section.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/stack-size-section.c?rev=322000=321999=322000=diff
==
--- cfe/trunk/test/CodeGen/stack-size-section.c (original)
+++ cfe/trunk/test/CodeGen/stack-size-section.c Mon Jan  8 07:46:18 2018
@@ -1,3 +1,5 @@
+// REQUIRES: x86-registered-target
+
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -S -o - | FileCheck %s 
--check-prefix=CHECK-ABSENT
 // CHECK-ABSENT-NOT: section .stack_sizes
 


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

r321995 - Fix test added in r321992 failing on some buildbots.

2018-01-08 Thread Sean Eveson via cfe-commits

Author: seaneveson
Date: Mon Jan  8 06:43:28 2018
New Revision: 321995

URL: http://llvm.org/viewvc/llvm-project?rev=321995=rev
Log:
Fix test added in r321992 failing on some buildbots.

Modified:
cfe/trunk/test/CodeGen/stack-size-section.c

Modified: cfe/trunk/test/CodeGen/stack-size-section.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/stack-size-section.c?rev=321995=321994=321995=diff
==
--- cfe/trunk/test/CodeGen/stack-size-section.c (original)
+++ cfe/trunk/test/CodeGen/stack-size-section.c Mon Jan  8 06:43:28 2018
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -triple x86_64-unknown %s -S -o - | FileCheck %s 
--check-prefix=CHECK-ABSENT
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -S -o - | FileCheck %s 
--check-prefix=CHECK-ABSENT
 // CHECK-ABSENT-NOT: section .stack_sizes
 
-// RUN: %clang_cc1 -triple x86_64-unknown -fstack-size-section %s -S -o - | 
FileCheck %s --check-prefix=CHECK-PRESENT
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fstack-size-section %s -S 
-o - | FileCheck %s --check-prefix=CHECK-PRESENT
 // CHECK-PRESENT: section .stack_sizes
 
 int foo() { return 42; }


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

r321992 - [Driver] Add flag enabling the function stack size section that was added in r319430

2018-01-08 Thread Sean Eveson via cfe-commits

Author: seaneveson
Date: Mon Jan  8 05:42:26 2018
New Revision: 321992

URL: http://llvm.org/viewvc/llvm-project?rev=321992=rev
Log:
[Driver] Add flag enabling the function stack size section that was added in 
r319430

Adds the -fstack-size-section flag to enable the .stack_sizes section. The flag 
defaults to on for the PS4 triple.

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

Added:
cfe/trunk/test/CodeGen/stack-size-section.c
cfe/trunk/test/Driver/stack-size-section.c
Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Frontend/CodeGenOptions.def
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=321992=321991=321992=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Mon Jan  8 05:42:26 2018
@@ -1574,6 +1574,10 @@ def fdata_sections : Flag <["-"], "fdata
  Flags<[CC1Option]>, HelpText<"Place each data in its own section (ELF Only)">;
 def fno_data_sections : Flag <["-"], "fno-data-sections">, Group,
   Flags<[CC1Option]>;
+def fstack_size_section : Flag<["-"], "fstack-size-section">, Group, 
Flags<[CC1Option]>,
+  HelpText<"Emit section containing metadata on function stack sizes">;
+def fno_stack_size_section : Flag<["-"], "fno-stack-size-section">, 
Group, Flags<[CC1Option]>,
+  HelpText<"Don't emit section containing metadata on function stack sizes">;
 
 def funique_section_names : Flag <["-"], "funique-section-names">,
   Group, Flags<[CC1Option]>,

Modified: cfe/trunk/include/clang/Frontend/CodeGenOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CodeGenOptions.def?rev=321992=321991=321992=diff
==
--- cfe/trunk/include/clang/Frontend/CodeGenOptions.def (original)
+++ cfe/trunk/include/clang/Frontend/CodeGenOptions.def Mon Jan  8 05:42:26 2018
@@ -83,6 +83,7 @@ CODEGENOPT(InstrumentFunctionEntryBare ,
 
 CODEGENOPT(XRayInstrumentFunctions , 1, 0) ///< Set when -fxray-instrument is
///< enabled.
+CODEGENOPT(StackSizeSection  , 1, 0) ///< Set when -fstack-size-section is 
enabled.
 
 ///< Set when -fxray-always-emit-customevents is enabled.
 CODEGENOPT(XRayAlwaysEmitCustomEvents , 1, 0)

Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=321992=321991=321992=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Mon Jan  8 05:42:26 2018
@@ -448,6 +448,7 @@ static void initTargetOptions(llvm::Targ
   Options.UniqueSectionNames = CodeGenOpts.UniqueSectionNames;
   Options.EmulatedTLS = CodeGenOpts.EmulatedTLS;
   Options.DebuggerTuning = CodeGenOpts.getDebuggerTuning();
+  Options.EmitStackSizeSection = CodeGenOpts.StackSizeSection;
 
   if (CodeGenOpts.EnableSplitDwarf)
 Options.MCOptions.SplitDwarfFile = CodeGenOpts.SplitDwarfFile;

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=321992=321991=321992=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Mon Jan  8 05:42:26 2018
@@ -3798,6 +3798,10 @@ void Clang::ConstructJob(Compilation ,
 CmdArgs.push_back(A->getValue());
   }
 
+  if (Args.hasFlag(options::OPT_fstack_size_section,
+   options::OPT_fno_stack_size_section, RawTriple.isPS4()))
+CmdArgs.push_back("-fstack-size-section");
+
   CmdArgs.push_back("-ferror-limit");
   if (Arg *A = Args.getLastArg(options::OPT_ferror_limit_EQ))
 CmdArgs.push_back(A->getValue());

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=321992=321991=321992=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Mon Jan  8 05:42:26 2018
@@ -682,6 +682,8 @@ static bool ParseCodeGenArgs(CodeGenOpti
OPT_fno_function_sections, false);
   Opts.DataSections = Args.hasFlag(OPT_fdata_sections,
OPT_fno_data_sections, false);
+  Opts.StackSizeSection =
+  Args.hasFlag(OPT_fstack_size_section, OPT_fno_stack_size_section, false);
   Opts.UniqueSectionNames = Args.hasFlag(OPT_funique_section_names,

Re: r319715 - Fix record-parsing-invocation.c test on Windows

2017-12-05 Thread Sean Eveson via cfe-commits

Hi,

It looks like the test is still failing on one of the Windows build bots:
http://lab.llvm.org:8011/builders/clang-x86-windows-msvc2015

>From looking at the test locally it seems the problem is when you do `//
RUN: not env...`. The env command doesn't get handled by llvm-lit.py,
because it gets passed through to `not.exe`.

In terms of fixing this test, changing from `not env X=Y COMMAND` to `env
X=Y not COMMAND` works on my machine. Otherwise maybe llvm-lit could be
changed to make the handling of env (and other utilities) consistent when
used after a not?

Thanks,

Sean Eveson
SN Systems - Sony Interactive Entertainment

On Mon, Dec 4, 2017 at 11:21 PM, Alex Lorenz via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: arphaman
> Date: Mon Dec  4 15:21:07 2017
> New Revision: 319715
>
> URL: http://llvm.org/viewvc/llvm-project?rev=319715=rev
> Log:
> Fix record-parsing-invocation.c test on Windows
>
> We should not check for the forward slash '/'
>
> Modified:
> cfe/trunk/test/Index/record-parsing-invocation.c
>
> Modified: cfe/trunk/test/Index/record-parsing-invocation.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/
> record-parsing-invocation.c?rev=319715=319714=319715=diff
> 
> ==
> --- cfe/trunk/test/Index/record-parsing-invocation.c (original)
> +++ cfe/trunk/test/Index/record-parsing-invocation.c Mon Dec  4 15:21:07
> 2017
> @@ -18,4 +18,4 @@
>  #  pragma clang __debug parser_crash
>  #endif
>
> -// CHECK: {"toolchain":"{{.*}}","libclang.operation":"parse","
> libclang.opts":1,"args":["clang","-fno-spell-checking","
> {{.*}}/record-parsing-invocation.c","-Xclang","-
> detailed-preprocessing-record","-fallow-editor-placeholders"]}
> +// CHECK: {"toolchain":"{{.*}}","libclang.operation":"parse","
> libclang.opts":1,"args":["clang","-fno-spell-checking","
> {{.*}}record-parsing-invocation.c","-Xclang","-
> detailed-preprocessing-record","-fallow-editor-placeholders"]}
>
>
> ___
> 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

[PATCH] D26837: [analyzer] Litter the SVal/SymExpr/MemRegion class hierarchy with asserts.

2016-11-18 Thread Sean Eveson via cfe-commits

seaneveson added inline comments.



Comment at: lib/StaticAnalyzer/Core/MemRegion.cpp:334
 void BlockCodeRegion::Profile(llvm::FoldingSetNodeID& ID) const {
+  locTy->getTypePtr()->isBlockPointerType();
   BlockCodeRegion::ProfileRegion(ID, BD, locTy, AC, superRegion);

Was this meant to be an assert?


https://reviews.llvm.org/D26837



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

Re: [PATCH] D19866: [Analyzer] Correct stack address escape diagnostic

2016-05-26 Thread Sean Eveson via cfe-commits

seaneveson added a subscriber: seaneveson.
seaneveson closed this revision.
seaneveson added a comment.

Committed: http://reviews.llvm.org/rL270849


http://reviews.llvm.org/D19866



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

r270849 - [Analyzer] Correct stack address escape diagnostic

2016-05-26 Thread Sean Eveson via cfe-commits

Author: seaneveson
Date: Thu May 26 09:02:17 2016
New Revision: 270849

URL: http://llvm.org/viewvc/llvm-project?rev=270849=rev
Log:
[Analyzer] Correct stack address escape diagnostic

Summary:
Leaking a stack address via a static variable refers to it in the diagnostic as 
a 'global'. This patch corrects the diagnostic for static variables.


Patch by Phil Camp, SN Systems

Reviewers: dcoughlin, zaks.anna

Subscribers: xazax.hun, cfe-commits

Differential Revision: http://reviews.llvm.org/D19866

Patch by Phil Camp

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
cfe/trunk/test/Analysis/stackaddrleak.c

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp?rev=270849=270848=270849=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp Thu May 26 
09:02:17 2016
@@ -236,7 +236,12 @@ void StackAddrEscapeChecker::checkEndFun
 SmallString<512> buf;
 llvm::raw_svector_ostream os(buf);
 SourceRange range = genName(os, cb.V[i].second, Ctx.getASTContext());
-os << " is still referred to by the global variable '";
+os << " is still referred to by the ";
+if (isa(cb.V[i].first->getMemorySpace()))
+  os << "static";
+else
+  os << "global";
+os << " variable '";
 const VarRegion *VR = cast(cb.V[i].first->getBaseRegion());
 os << *VR->getDecl()
<< "' upon returning to the caller.  This will be a dangling reference";

Modified: cfe/trunk/test/Analysis/stackaddrleak.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/stackaddrleak.c?rev=270849=270848=270849=diff
==
--- cfe/trunk/test/Analysis/stackaddrleak.c (original)
+++ cfe/trunk/test/Analysis/stackaddrleak.c Thu May 26 09:02:17 2016
@@ -19,7 +19,7 @@ void f2() {
   p = (const char *) __builtin_alloca(12);
 } // expected-warning{{Address of stack memory allocated by call to alloca() 
on line 19 is still referred to by the global variable 'p' upon returning to 
the caller.  This will be a dangling reference}}
 
-// PR 7383 - previosly the stack address checker would crash on this example
+// PR 7383 - previously the stack address checker would crash on this example
 //  because it would attempt to do a direct load from 'pr7383_list'. 
 static int pr7383(__const char *__)
 {
@@ -33,7 +33,7 @@ void test_multi_return() {
   int x;
   a = 
   b = 
-} // expected-warning{{Address of stack memory associated with local variable 
'x' is still referred to by the global variable 'a' upon returning}} 
expected-warning{{Address of stack memory associated with local variable 'x' is 
still referred to by the global variable 'b' upon returning}}
+} // expected-warning{{Address of stack memory associated with local variable 
'x' is still referred to by the static variable 'a' upon returning}} 
expected-warning{{Address of stack memory associated with local variable 'x' is 
still referred to by the static variable 'b' upon returning}}
 
 intptr_t returnAsNonLoc() {
   int x;


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

r256926 - [Analyzer] Change the default SA checkers for PS4

2016-01-06 Thread Sean Eveson via cfe-commits

Author: seaneveson
Date: Wed Jan  6 04:03:58 2016
New Revision: 256926

URL: http://llvm.org/viewvc/llvm-project?rev=256926=rev
Log:
[Analyzer] Change the default SA checkers for PS4

Summary: This patch removes security.*, unix.API and unix.Vfork from the 
default checkers for PS4.

Reviewers: dcoughlin, zaks.anna

Subscribers: cfe-commits

Differential Revision: http://reviews.llvm.org/D15888

Added:
cfe/trunk/test/Driver/ps4-analyzer-defaults.cpp
Modified:
cfe/trunk/lib/Driver/Tools.cpp

Modified: cfe/trunk/lib/Driver/Tools.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=256926=256925=256926=diff
==
--- cfe/trunk/lib/Driver/Tools.cpp (original)
+++ cfe/trunk/lib/Driver/Tools.cpp Wed Jan  6 04:03:58 2016
@@ -3592,6 +3592,12 @@ void Clang::ConstructJob(Compilation ,
   if (!IsWindowsMSVC)
 CmdArgs.push_back("-analyzer-checker=unix");
 
+  // Disable some unix checkers for PS4.
+  if (IsPS4CPU) {
+CmdArgs.push_back("-analyzer-disable-checker=unix.API");
+CmdArgs.push_back("-analyzer-disable-checker=unix.Vfork");
+  }
+
   if (getToolChain().getTriple().getVendor() == llvm::Triple::Apple)
 CmdArgs.push_back("-analyzer-checker=osx");
 
@@ -3600,14 +3606,15 @@ void Clang::ConstructJob(Compilation ,
   if (types::isCXX(Input.getType()))
 CmdArgs.push_back("-analyzer-checker=cplusplus");
 
-  // Enable the following experimental checkers for testing.
-  CmdArgs.push_back(
-  "-analyzer-checker=security.insecureAPI.UncheckedReturn");
-  CmdArgs.push_back("-analyzer-checker=security.insecureAPI.getpw");
-  CmdArgs.push_back("-analyzer-checker=security.insecureAPI.gets");
-  CmdArgs.push_back("-analyzer-checker=security.insecureAPI.mktemp");
-  CmdArgs.push_back("-analyzer-checker=security.insecureAPI.mkstemp");
-  CmdArgs.push_back("-analyzer-checker=security.insecureAPI.vfork");
+  if (!IsPS4CPU) {
+CmdArgs.push_back(
+"-analyzer-checker=security.insecureAPI.UncheckedReturn");
+CmdArgs.push_back("-analyzer-checker=security.insecureAPI.getpw");
+CmdArgs.push_back("-analyzer-checker=security.insecureAPI.gets");
+CmdArgs.push_back("-analyzer-checker=security.insecureAPI.mktemp");
+CmdArgs.push_back("-analyzer-checker=security.insecureAPI.mkstemp");
+CmdArgs.push_back("-analyzer-checker=security.insecureAPI.vfork");
+  }
 
   // Default nullability checks.
   CmdArgs.push_back("-analyzer-checker=nullability.NullPassedToNonnull");

Added: cfe/trunk/test/Driver/ps4-analyzer-defaults.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/ps4-analyzer-defaults.cpp?rev=256926=auto
==
--- cfe/trunk/test/Driver/ps4-analyzer-defaults.cpp (added)
+++ cfe/trunk/test/Driver/ps4-analyzer-defaults.cpp Wed Jan  6 04:03:58 2016
@@ -0,0 +1,33 @@
+// Check that the default analyzer checkers for PS4 are:
+//   core
+//   cplusplus
+//   deadcode
+//   nullability
+//   unix
+// Excluding:
+//   unix.API
+//   unix.Vfork
+
+// Check for expected checkers
+// RUN: %clang -target x86_64-scei-ps4 --analyze %s -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-PS4-POS-CHECKERS
+//
+// Negative check for unexpected checkers
+// RUN: %clang -target x86_64-scei-ps4 --analyze %s -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-PS4-NEG-CHECKERS
+//
+// Check for all unix checkers except API and Vfork
+// RUN: %clang -target x86_64-scei-ps4 --analyze %s -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-PS4-UNIX-CHECKERS
+
+// CHECK-PS4-POS-CHECKERS-DAG: analyzer-checker=core
+// CHECK-PS4-POS-CHECKERS-DAG: analyzer-checker=cplusplus
+// CHECK-PS4-POS-CHECKERS-DAG: analyzer-checker=deadcode
+// CHECK-PS4-POS-CHECKERS-DAG: analyzer-checker=nullability
+//
+// CHECK-PS4-NEG-CHECKERS-NOT: analyzer-checker={{osx|security}}
+//
+// CHECK-PS4-UNIX-CHECKERS: analyzer-checker=unix
+// CHECK-PS4-UNIX-CHECKERS-DAG: analyzer-disable-checker=unix.API
+// CHECK-PS4-UNIX-CHECKERS-DAG: analyzer-disable-checker=unix.Vfork
+// CHECK-PS4-UNIX-CHECKERS-NOT: analyzer-checker=unix.{{API|Vfork}}


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

Re: [PATCH] D15888: [Analyzer] Change the default SA checkers for PS4

2016-01-06 Thread Sean Eveson via cfe-commits

seaneveson updated this revision to Diff 44098.
seaneveson marked an inline comment as done.
seaneveson added a comment.

Removing old "experimental" comment


http://reviews.llvm.org/D15888

Files:
  lib/Driver/Tools.cpp
  test/Driver/ps4-analyzer-defaults.cpp

Index: test/Driver/ps4-analyzer-defaults.cpp
===
--- /dev/null
+++ test/Driver/ps4-analyzer-defaults.cpp
@@ -0,0 +1,33 @@
+// Check that the default analyzer checkers for PS4 are:
+//   core
+//   cplusplus
+//   deadcode
+//   nullability
+//   unix
+// Excluding:
+//   unix.API
+//   unix.Vfork
+
+// Check for expected checkers
+// RUN: %clang -target x86_64-scei-ps4 --analyze %s -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-PS4-POS-CHECKERS
+//
+// Negative check for unexpected checkers
+// RUN: %clang -target x86_64-scei-ps4 --analyze %s -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-PS4-NEG-CHECKERS
+//
+// Check for all unix checkers except API and Vfork
+// RUN: %clang -target x86_64-scei-ps4 --analyze %s -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-PS4-UNIX-CHECKERS
+
+// CHECK-PS4-POS-CHECKERS-DAG: analyzer-checker=core
+// CHECK-PS4-POS-CHECKERS-DAG: analyzer-checker=cplusplus
+// CHECK-PS4-POS-CHECKERS-DAG: analyzer-checker=deadcode
+// CHECK-PS4-POS-CHECKERS-DAG: analyzer-checker=nullability
+//
+// CHECK-PS4-NEG-CHECKERS-NOT: analyzer-checker={{osx|security}}
+//
+// CHECK-PS4-UNIX-CHECKERS: analyzer-checker=unix
+// CHECK-PS4-UNIX-CHECKERS-DAG: analyzer-disable-checker=unix.API
+// CHECK-PS4-UNIX-CHECKERS-DAG: analyzer-disable-checker=unix.Vfork
+// CHECK-PS4-UNIX-CHECKERS-NOT: analyzer-checker=unix.{{API|Vfork}}
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -3592,6 +3592,12 @@
   if (!IsWindowsMSVC)
 CmdArgs.push_back("-analyzer-checker=unix");
 
+  // Disable some unix checkers for PS4.
+  if (IsPS4CPU) {
+CmdArgs.push_back("-analyzer-disable-checker=unix.API");
+CmdArgs.push_back("-analyzer-disable-checker=unix.Vfork");
+  }
+
   if (getToolChain().getTriple().getVendor() == llvm::Triple::Apple)
 CmdArgs.push_back("-analyzer-checker=osx");
 
@@ -3600,14 +3606,15 @@
   if (types::isCXX(Input.getType()))
 CmdArgs.push_back("-analyzer-checker=cplusplus");
 
-  // Enable the following experimental checkers for testing.
-  CmdArgs.push_back(
-  "-analyzer-checker=security.insecureAPI.UncheckedReturn");
-  CmdArgs.push_back("-analyzer-checker=security.insecureAPI.getpw");
-  CmdArgs.push_back("-analyzer-checker=security.insecureAPI.gets");
-  CmdArgs.push_back("-analyzer-checker=security.insecureAPI.mktemp");
-  CmdArgs.push_back("-analyzer-checker=security.insecureAPI.mkstemp");
-  CmdArgs.push_back("-analyzer-checker=security.insecureAPI.vfork");
+  if (!IsPS4CPU) {
+CmdArgs.push_back(
+"-analyzer-checker=security.insecureAPI.UncheckedReturn");
+CmdArgs.push_back("-analyzer-checker=security.insecureAPI.getpw");
+CmdArgs.push_back("-analyzer-checker=security.insecureAPI.gets");
+CmdArgs.push_back("-analyzer-checker=security.insecureAPI.mktemp");
+CmdArgs.push_back("-analyzer-checker=security.insecureAPI.mkstemp");
+CmdArgs.push_back("-analyzer-checker=security.insecureAPI.vfork");
+  }
 
   // Default nullability checks.
   CmdArgs.push_back("-analyzer-checker=nullability.NullPassedToNonnull");


Index: test/Driver/ps4-analyzer-defaults.cpp
===
--- /dev/null
+++ test/Driver/ps4-analyzer-defaults.cpp
@@ -0,0 +1,33 @@
+// Check that the default analyzer checkers for PS4 are:
+//   core
+//   cplusplus
+//   deadcode
+//   nullability
+//   unix
+// Excluding:
+//   unix.API
+//   unix.Vfork
+
+// Check for expected checkers
+// RUN: %clang -target x86_64-scei-ps4 --analyze %s -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-PS4-POS-CHECKERS
+//
+// Negative check for unexpected checkers
+// RUN: %clang -target x86_64-scei-ps4 --analyze %s -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-PS4-NEG-CHECKERS
+//
+// Check for all unix checkers except API and Vfork
+// RUN: %clang -target x86_64-scei-ps4 --analyze %s -### 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=CHECK-PS4-UNIX-CHECKERS
+
+// CHECK-PS4-POS-CHECKERS-DAG: analyzer-checker=core
+// CHECK-PS4-POS-CHECKERS-DAG: analyzer-checker=cplusplus
+// CHECK-PS4-POS-CHECKERS-DAG: analyzer-checker=deadcode
+// CHECK-PS4-POS-CHECKERS-DAG: analyzer-checker=nullability
+//
+// CHECK-PS4-NEG-CHECKERS-NOT: analyzer-checker={{osx|security}}
+//
+// CHECK-PS4-UNIX-CHECKERS: analyzer-checker=unix
+// CHECK-PS4-UNIX-CHECKERS-DAG: analyzer-disable-checker=unix.API
+// CHECK-PS4-UNIX-CHECKERS-DAG: analyzer-disable-checker=unix.Vfork
+//

Re: [PATCH] D14919: Fix IssueHash generation

2015-11-26 Thread Sean Eveson via cfe-commits

seaneveson added a comment.

In http://reviews.llvm.org/D14919#296597, @o.gyorgy wrote:

> I did not create a test checker for the NormalizeLine error in the patch.
>  Should I add a test checker for this?

I was wondering what sort of source code causes the crash, but I do think it 
would be good to have a regression test.

> Do you have any suggestions where to put it if required?

Maybe in bug_hash_test.cpp, although I'm not sure its necessary to check the 
plist output when testing for a crash.

http://reviews.llvm.org/D14919

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

Re: [PATCH] D10305: [Clang Static Analyzer] Bug identification

2015-11-12 Thread Sean Eveson via cfe-commits

seaneveson added a comment.

> If you have multiple users using a bug suppression system, I would design 
> such system using only a single hash version across all users; using a mix 
> seems error prone.. Once all of your users upgrade to a version of the 
> analyzer where a new hash version is available, you upgrade the hash in the 
> database to reflect that.


We want users to be able to take advantage of the new hash when they upgrade to 
a new version. There may be a large amount of time between the first user(s) to 
upgrade and the last. On top of this there is no way to identify when all the 
users have upgraded.

> Let's talk about your example.

>  You have user1 using hash_version_1 and user2 using hash_version_2. If the 
> second user suppresses an issue and hash_version_2 is used to record that, 
> how will user1 be able to identity that issue?


For each suppressed warning we will store all of the produced hashes, then the 
tool can compare any generated hash against the stored hashes.

> Also, as I've mentioned before, the newest issue hash would not necessarily 
> be the best.




> We could have a list of issue hashes and each item in the list containing 
> type_of_hash, hash_version, hash. However, I am not yet convinced that this 
> complexity is justified.


I did not consider that the hashes might be used for something other than 
suppression. In that case, our problem is identifying which hashes to use for 
suppression and knowing which order to use them in. Do you have any suggestions?


http://reviews.llvm.org/D10305



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

Re: [PATCH] D10305: [Clang Static Analyzer] Bug identification

2015-11-11 Thread Sean Eveson via cfe-commits

seaneveson added a comment.

In http://reviews.llvm.org/D10305#286385, @zaks.anna wrote:

> The reason I like names more than the numbers is that we may use different 
> solutions for issue hash generation and some users might prefer one over the 
> other. It is not necessarily clear which one is the best. Numbers would 
> obfuscate the heuristic used to produce the hash and the quality of the hash 
> and would be mainly based on the time when the hash was introduced.

Thanks, I understand where you are coming from now.

> > This makes forwards compatibility difficult, since there is no way to 
> > predict the names of future hashes

> 

> >  (As far as I understand).

> 

> 

> Can you describe what you are trying to achieve?

Just for the sake of explaining, lets say in 3 subsequent Analyzer releases the 
hashes are called “hash_1”, “hash_2” and “hash_3”.

In the first release the suppression tool will record hash_1 to suppress a 
warning. Some developers will upgrade to the second and third release, where 
others may jump straight to the third release. Additionally some developers may 
temporarily downgrade to the second after upgrading to the third release. The 
suppression tool (which may not be upgraded during this period) needs to 
maintain suppressions between these version upgrades/downgrades. Also, some 
developers may upgrade before others, where they all share one repository of 
suppressions.

Say there is an issue with hash_1 and two warnings collide; this is fixed by 
hash_2. When the user upgrades to the 2nd release, the suppression tool will 
record hash_2 on top of hash_1. The tool will then suppress using hash_2 if it 
is present in the plist file, ignoring hash_1. If the user downgrades and 
hash_2 is not in the plist file the tool will suppress using hash_1. For this 
to work the tool needs to know the order in which to look at the hashes.

> We can agree that all issue hashes start with "issue_hash" prefix. If you 
> find an entry with "issue_hash" prefix and unknown suffix, you would know 
> that it's new. It would be the same as a number you have not seen so far.

The tool would not be able to establish the order of multiple produced hashes 
when it was first run.

> > A third alternative would be to have both semantic names (containing hash) 
> > and a number suffix

> 

> >  which indicates the ordering.

> 

> 

> If there is a minor enhancement to the existing issue hash method, appending 
> the version number to it is fine by me. Though, this might be confusing in 
> it's own right..

This would work for us, although I agree it might be a little confusing. Would 
issue_hash__ be better? e.g. "issue_hash_1_content_of 
_line_in_context".

http://reviews.llvm.org/D10305

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

Re: [PATCH] D10305: [Clang Static Analyzer] Bug identification

2015-11-10 Thread Sean Eveson via cfe-commits

seaneveson added a comment.

In http://reviews.llvm.org/D10305#286119, @xazax.hun wrote:

> A third alternative would be to have both semantic names (containing hash) 
> and a number suffix which indicates the ordering.

Yes that would work, but I don't understand the benefit of having the semantic 
names in the plist file.

http://reviews.llvm.org/D10305

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

Re: [PATCH] D14498: [Analyzer] Fix an assertion caused by r250237 (PR25392)

2015-11-10 Thread Sean Eveson via cfe-commits

seaneveson abandoned this revision.
seaneveson added a comment.

Fixed by r252506. Thanks Devin.


http://reviews.llvm.org/D14498



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

r252599 - [Analyzer] Fix comments and formatting. NFC.

2015-11-10 Thread Sean Eveson via cfe-commits

Author: seaneveson
Date: Tue Nov 10 05:48:55 2015
New Revision: 252599

URL: http://llvm.org/viewvc/llvm-project?rev=252599=rev
Log:
[Analyzer] Fix comments and formatting. NFC.

Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h?rev=252599=252598=252599=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h 
Tue Nov 10 05:48:55 2015
@@ -1,4 +1,4 @@
-//===--- LoopWidening.h - Instruction class definition --*- C++ 
-*-===//
+//===--- LoopWidening.h - Widen loops ---*- C++ 
-*-===//
 //
 // The LLVM Compiler Infrastructure
 //
@@ -28,8 +28,7 @@ namespace ento {
 /// by the loop body in any iteration.
 ProgramStateRef getWidenedLoopState(ProgramStateRef PrevState,
 const LocationContext *LCtx,
-unsigned BlockCount,
-const Stmt *LoopStmt);
+unsigned BlockCount, const Stmt *LoopStmt);
 
 } // end namespace ento
 } // end namespace clang

Modified: cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp?rev=252599=252598=252599=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp Tue Nov 10 05:48:55 2015
@@ -420,8 +420,8 @@ const FunctionDecl *CXXInstanceCall::get
   return getSVal(CE->getCallee()).getAsFunctionDecl();
 }
 
-void CXXInstanceCall::getExtraInvalidatedValues(ValueList ,
-RegionAndSymbolInvalidationTraits *ETraits) const {
+void CXXInstanceCall::getExtraInvalidatedValues(
+ValueList , RegionAndSymbolInvalidationTraits *ETraits) const {
   SVal ThisVal = getCXXThisVal();
   Values.push_back(ThisVal);
 
@@ -442,7 +442,7 @@ void CXXInstanceCall::getExtraInvalidate
   return;
 
 ETraits->setTrait(ThisRegion->getBaseRegion(),
-  RegionAndSymbolInvalidationTraits::TK_PreserveContents);
+  RegionAndSymbolInvalidationTraits::TK_PreserveContents);
   }
 }
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp?rev=252599=252598=252599=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp Tue Nov 10 05:48:55 2015
@@ -1,4 +1,4 @@
-//===--- LoopWidening.cpp - Instruction class definition *- C++ 
-*-===//
+//===--- LoopWidening.cpp - Widen loops -*- C++ 
-*-===//
 //
 // The LLVM Compiler Infrastructure
 //


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

Re: [PATCH] D10305: [Clang Static Analyzer] Bug identification

2015-11-10 Thread Sean Eveson via cfe-commits

seaneveson added a comment.

We are working on tools that use the new hash for bug suppression. There seems 
to be no way to predict the names of future hashes. We have products (that will 
use the bug identification) that are on a different release schedule to our 
clang compiler. These tools will not be able to take advantage of new hashes, 
unless they know the future hash names.

Regarding the previous discussions on the hash name:

In http://reviews.llvm.org/D10305#250238, @zaks.anna wrote:

> Maybe we could have "issue_hash", "issue_hash_1"(offset based), and 
> "issue_hash_2"(content of line) and add another field "issue_hash_version" 
> that describes the version "issue_hash" is using?

This would allow tools to discern the order of the hashes from the plist file. 
It would also be possible to identify new hashes and those that are no longer 
supported. These are features we would like in order to make our tools as 
flexible as possible.

In http://reviews.llvm.org/D10305#258162, @zaks.anna wrote:

> I'd suggest to suffix each issue hash field with the description of that 
> hash. For example, we would remove "issue_hash" and replace it with 
> "issue_hash_function_offset" and add "issue_hash_content_of _line_in_context".

This makes forwards compatibility difficult, since there is no way to predict 
the names of future hashes (As far as I understand).

To resolve the forwards compatibility issues, what are peoples opinions on:

- Having a consistent name with an incrementing number (i.e. issue_hash_1)?
- Adding an ordered list of all the hash names to the plist file?

Regards,
Sean Eveson
SN Systems - Sony Computer Entertainment

http://reviews.llvm.org/D10305

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

Re: [PATCH] D14498: [Analyzer] Fix an assertion caused by r250237 (PR25392)

2015-11-09 Thread Sean Eveson via cfe-commits

seaneveson added a comment.

Fix an assertion which occurs when getCXXThisVal() returns an Unknown SVal and 
the Analyzer tries to get the corresponding memory region.

This assertion was reported in PR25392. The test case from this Bugzilla has 
been added.

(Forgot to add cfe-commits when creating the review).


http://reviews.llvm.org/D14498



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

r251697 - Revert r251621 "[Analyzer] Widening loops which do not exit" (bot failure)

2015-10-30 Thread Sean Eveson via cfe-commits

Author: seaneveson
Date: Fri Oct 30 06:13:07 2015
New Revision: 251697

URL: http://llvm.org/viewvc/llvm-project?rev=251697=rev
Log:
Revert r251621 "[Analyzer] Widening loops which do not exit" (bot failure)

Seems to be causing clang-cmake-mips build bot to fail (timeout)

http://lab.llvm.org:8011/builders/clang-cmake-mips/builds/10299

Removed:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp
cfe/trunk/test/Analysis/loop-widening.c
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/test/Analysis/analyzer-config.c
cfe/trunk/test/Analysis/analyzer-config.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h?rev=251697=251696=251697=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h Fri Oct 30 
06:13:07 2015
@@ -262,9 +262,6 @@ private:
   /// \sa shouldInlineLambdas
   Optional InlineLambdas;
 
-  /// \sa shouldWidenLoops
-  Optional WidenLoops;
-
   /// A helper function that retrieves option for a given full-qualified
   /// checker name.
   /// Options for checkers can be specified via 'analyzer-config' command-line
@@ -529,10 +526,6 @@ public:
   /// generated each time a LambdaExpr is visited.
   bool shouldInlineLambdas();
 
-  /// Returns true if the analysis should try to widen loops.
-  /// This is controlled by the 'widen-loops' config option.
-  bool shouldWidenLoops();
-
 public:
   AnalyzerOptions() :
 AnalysisStoreOpt(RegionStoreModel),

Removed: 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h?rev=251696=auto
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h 
(removed)
@@ -1,37 +0,0 @@
-//===--- LoopWidening.h - Instruction class definition --*- C++ 
-*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-///
-/// This header contains the declarations of functions which are used to widen
-/// loops which do not otherwise exit. The widening is done by invalidating
-/// anything which might be modified by the body of the loop.
-///
-//===--===//
-
-#ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_LOOPWIDENING_H
-#define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_LOOPWIDENING_H
-
-#include "clang/Analysis/CFG.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
-
-namespace clang {
-namespace ento {
-
-/// \brief Get the states that result from widening the loop.
-///
-/// Widen the loop by invalidating anything that might be modified
-/// by the loop body in any iteration.
-ProgramStateRef getWidenedLoopState(ProgramStateRef PrevState,
-const LocationContext *LCtx,
-unsigned BlockCount,
-const Stmt *LoopStmt);
-
-} // end namespace ento
-} // end namespace clang
-
-#endif

Modified: cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp?rev=251697=251696=251697=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp Fri Oct 30 06:13:07 
2015
@@ -338,9 +338,3 @@ bool AnalyzerOptions::shouldInlineLambda
 InlineLambdas = getBooleanOption("inline-lambdas", /*Default=*/true);
   return InlineLambdas.getValue();
 }
-
-bool AnalyzerOptions::shouldWidenLoops() {
-  if (!WidenLoops.hasValue())
-WidenLoops = getBooleanOption("widen-loops", /*Default=*/false);
-  return WidenLoops.getValue();
-}

Modified: cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt?rev=251697=251696=251697=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt (original)

r251702 - Reapply r251621 "[Analyzer] Widening loops which do not exit"

2015-10-30 Thread Sean Eveson via cfe-commits

Author: seaneveson
Date: Fri Oct 30 10:23:57 2015
New Revision: 251702

URL: http://llvm.org/viewvc/llvm-project?rev=251702=rev
Log:
Reapply r251621 "[Analyzer] Widening loops which do not exit"

It was not the cause of the build bot failure.

Added:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
  - copied unchanged from r251696, 
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp
  - copied unchanged from r251696, 
cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp
cfe/trunk/test/Analysis/loop-widening.c
  - copied unchanged from r251696, cfe/trunk/test/Analysis/loop-widening.c
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/test/Analysis/analyzer-config.c
cfe/trunk/test/Analysis/analyzer-config.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h?rev=251702=251701=251702=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h Fri Oct 30 
10:23:57 2015
@@ -262,6 +262,9 @@ private:
   /// \sa shouldInlineLambdas
   Optional InlineLambdas;
 
+  /// \sa shouldWidenLoops
+  Optional WidenLoops;
+
   /// A helper function that retrieves option for a given full-qualified
   /// checker name.
   /// Options for checkers can be specified via 'analyzer-config' command-line
@@ -526,6 +529,10 @@ public:
   /// generated each time a LambdaExpr is visited.
   bool shouldInlineLambdas();
 
+  /// Returns true if the analysis should try to widen loops.
+  /// This is controlled by the 'widen-loops' config option.
+  bool shouldWidenLoops();
+
 public:
   AnalyzerOptions() :
 AnalysisStoreOpt(RegionStoreModel),

Modified: cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp?rev=251702=251701=251702=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp Fri Oct 30 10:23:57 
2015
@@ -338,3 +338,9 @@ bool AnalyzerOptions::shouldInlineLambda
 InlineLambdas = getBooleanOption("inline-lambdas", /*Default=*/true);
   return InlineLambdas.getValue();
 }
+
+bool AnalyzerOptions::shouldWidenLoops() {
+  if (!WidenLoops.hasValue())
+WidenLoops = getBooleanOption("widen-loops", /*Default=*/false);
+  return WidenLoops.getValue();
+}

Modified: cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt?rev=251702=251701=251702=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt Fri Oct 30 10:23:57 2015
@@ -28,6 +28,7 @@ add_clang_library(clangStaticAnalyzerCor
   ExprEngineObjC.cpp
   FunctionSummary.cpp
   HTMLDiagnostics.cpp
+  LoopWidening.cpp
   MemRegion.cpp
   PathDiagnostic.cpp
   PlistDiagnostics.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=251702=251701=251702=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Fri Oct 30 10:23:57 2015
@@ -26,6 +26,7 @@
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h"
 #include "llvm/ADT/ImmutableList.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Support/raw_ostream.h"
@@ -1395,8 +1396,25 @@ void ExprEngine::processCFGBlockEntrance
  ExplodedNode *Pred) {
   PrettyStackTraceLocationContext CrashInfo(Pred->getLocationContext());
 
+  // If this block is terminated by a loop and it has already been visited the
+  // maximum number of times, widen the loop.
+  unsigned int BlockCount = nodeBuilder.getContext().blockCount();
+  if (BlockCount == AMgr.options.maxBlockVisitOnPath - 1 &&
+  AMgr.options.shouldWidenLoops()) {
+const Stmt *Term = nodeBuilder.getContext().getBlock()->getTerminator();
+if (!(Term &&
+  (isa(Term) || isa(Term) || isa(Term
+

Re: [PATCH] D12358: [Analyzer] Widening loops which do not exit

2015-10-29 Thread Sean Eveson via cfe-commits

seaneveson updated this revision to Diff 38717.
seaneveson added a comment.

Updated to latest revision


http://reviews.llvm.org/D12358

Files:
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/CMakeLists.txt
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/LoopWidening.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/loop-widening.c

Index: test/Analysis/loop-widening.c
===
--- /dev/null
+++ test/Analysis/loop-widening.c
@@ -0,0 +1,190 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-max-loop 4 -analyzer-config widen-loops=true -verify %s
+
+void clang_analyzer_eval(int);
+void clang_analyzer_warnIfReached();
+
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+void free(void *);
+
+void loop_which_iterates_limit_times_not_widened() {
+  int i;
+  int x = 1;
+  // Check loop isn't widened by checking x isn't invalidated
+  for (i = 0; i < 1; ++i) {}
+  clang_analyzer_eval(x == 1); // expected-warning {{TRUE}}
+  for (i = 0; i < 2; ++i) {}
+  clang_analyzer_eval(x == 1); // expected-warning {{TRUE}}
+  for (i = 0; i < 3; ++i) {}
+  // FIXME loss of precision as a result of evaluating the widened loop body
+  //   *instead* of the last iteration.
+  clang_analyzer_eval(x == 1); // expected-warning {{UNKNOWN}}
+}
+
+int a_global;
+
+void loop_evaluated_before_widening() {
+  int i;
+  a_global = 1;
+  for (i = 0; i < 10; ++i) {
+if (i == 2) {
+  // True before widening then unknown after.
+  clang_analyzer_eval(a_global == 1); // expected-warning{{TRUE}} expected-warning{{UNKNOWN}}
+}
+  }
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void warnings_after_loop() {
+  int i;
+  for (i = 0; i < 10; ++i) {}
+  char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void for_loop_exits() {
+  int i;
+  for (i = 0; i < 10; ++i) {}
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void while_loop_exits() {
+  int i = 0;
+  while (i < 10) {++i;}
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void do_while_loop_exits() {
+  int i = 0;
+  do {++i;} while (i < 10);
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void loop_body_is_widened() {
+  int i = 0;
+  while (i < 100) {
+if (i > 10) {
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+++i;
+  }
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void invariably_infinite_loop() {
+  int i = 0;
+  while (1) { ++i; }
+  clang_analyzer_warnIfReached(); // no-warning
+}
+
+void invariably_infinite_break_loop() {
+  int i = 0;
+  while (1) {
+++i;
+int x = 1;
+if (!x) break;
+  }
+  clang_analyzer_warnIfReached(); // no-warning
+}
+
+void reachable_break_loop() {
+  int i = 0;
+  while (1) {
+++i;
+if (i == 100) break;
+  }
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void condition_constrained_true_in_loop() {
+  int i = 0;
+  while (i < 50) {
+clang_analyzer_eval(i < 50); // expected-warning {{TRUE}}
+++i;
+  }
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void condition_constrained_false_after_loop() {
+  int i = 0;
+  while (i < 50) {
+++i;
+  }
+  clang_analyzer_eval(i >= 50); // expected-warning {{TRUE}}
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void multiple_exit_test() {
+  int x = 0;
+  int i = 0;
+  while (i < 50) {
+if (x) {
+  i = 10;
+  break;
+}
+++i;
+  }
+  // Reachable by 'normal' exit
+  if (i == 50) clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  // Reachable by break point
+  if (i == 10) clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  // Not reachable
+  if (i < 10) clang_analyzer_warnIfReached(); // no-warning
+  if (i > 10 && i < 50) clang_analyzer_warnIfReached(); // no-warning
+}
+
+void pointer_doesnt_leak_from_loop() {
+  int *h_ptr = (int *) malloc(sizeof(int));
+  for (int i = 0; i < 2; ++i) {}
+  for (int i = 0; i < 10; ++i) {} // no-warning
+  free(h_ptr);
+}
+
+int g_global;
+
+void unknown_after_loop(int s_arg) {
+  g_global = 0;
+  s_arg = 1;
+  int s_local = 2;
+  int *h_ptr = malloc(sizeof(int));
+
+  for (int i = 0; i < 10; ++i) {}
+
+  clang_analyzer_eval(g_global); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(s_arg); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(s_local); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(h_ptr == 0); // expected-warning {{UNKNOWN}}
+  free(h_ptr);
+}
+
+void variable_bound_exiting_loops_widened(int x) {
+  int i = 0;
+  int t = 1;
+  while (i < x) {

Re: [PATCH] D12358: [Analyzer] Widening loops which do not exit

2015-10-29 Thread Sean Eveson via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL251621: [Analyzer] Widening loops which do not exit 
(authored by seaneveson).

Changed prior to commit:
  http://reviews.llvm.org/D12358?vs=38717=38718#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D12358

Files:
  cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
  cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt
  cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp
  cfe/trunk/test/Analysis/analyzer-config.c
  cfe/trunk/test/Analysis/analyzer-config.cpp
  cfe/trunk/test/Analysis/loop-widening.c

Index: cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -262,6 +262,9 @@
   /// \sa shouldInlineLambdas
   Optional InlineLambdas;
 
+  /// \sa shouldWidenLoops
+  Optional WidenLoops;
+
   /// A helper function that retrieves option for a given full-qualified
   /// checker name.
   /// Options for checkers can be specified via 'analyzer-config' command-line
@@ -526,6 +529,10 @@
   /// generated each time a LambdaExpr is visited.
   bool shouldInlineLambdas();
 
+  /// Returns true if the analysis should try to widen loops.
+  /// This is controlled by the 'widen-loops' config option.
+  bool shouldWidenLoops();
+
 public:
   AnalyzerOptions() :
 AnalysisStoreOpt(RegionStoreModel),
Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
@@ -0,0 +1,37 @@
+//===--- LoopWidening.h - Instruction class definition --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+///
+/// This header contains the declarations of functions which are used to widen
+/// loops which do not otherwise exit. The widening is done by invalidating
+/// anything which might be modified by the body of the loop.
+///
+//===--===//
+
+#ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_LOOPWIDENING_H
+#define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_LOOPWIDENING_H
+
+#include "clang/Analysis/CFG.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
+
+namespace clang {
+namespace ento {
+
+/// \brief Get the states that result from widening the loop.
+///
+/// Widen the loop by invalidating anything that might be modified
+/// by the loop body in any iteration.
+ProgramStateRef getWidenedLoopState(ProgramStateRef PrevState,
+const LocationContext *LCtx,
+unsigned BlockCount,
+const Stmt *LoopStmt);
+
+} // end namespace ento
+} // end namespace clang
+
+#endif
Index: cfe/trunk/test/Analysis/analyzer-config.c
===
--- cfe/trunk/test/Analysis/analyzer-config.c
+++ cfe/trunk/test/Analysis/analyzer-config.c
@@ -25,6 +25,7 @@
 // CHECK-NEXT: min-cfg-size-treat-functions-as-large = 14
 // CHECK-NEXT: mode = deep
 // CHECK-NEXT: region-store-small-struct-limit = 2
+// CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 14
+// CHECK-NEXT: num-entries = 15
 
Index: cfe/trunk/test/Analysis/loop-widening.c
===
--- cfe/trunk/test/Analysis/loop-widening.c
+++ cfe/trunk/test/Analysis/loop-widening.c
@@ -0,0 +1,190 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-max-loop 4 -analyzer-config widen-loops=true -verify %s
+
+void clang_analyzer_eval(int);
+void clang_analyzer_warnIfReached();
+
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+void free(void *);
+
+void loop_which_iterates_limit_times_not_widened() {
+  int i;
+  int x = 1;
+  // Check loop isn't widened by checking x isn't invalidated
+  for (i = 0; i < 1; ++i) {}
+  clang_analyzer_eval(x == 1); // expected-warning {{TRUE}}
+  for (i = 0; i < 2; ++i) {}
+  clang_analyzer_eval(x == 1); // expected-warning {{TRUE}}
+  for (i = 0; i < 3; ++i) {}
+  // FIXME loss of precision as a result of evaluating the widened loop body
+  //   *instead* of the last iteration.
+  clang_analyzer_eval(x == 1); //

r251621 - [Analyzer] Widening loops which do not exit

2015-10-29 Thread Sean Eveson via cfe-commits

Author: seaneveson
Date: Thu Oct 29 05:04:41 2015
New Revision: 251621

URL: http://llvm.org/viewvc/llvm-project?rev=251621=rev
Log:
[Analyzer] Widening loops which do not exit

Summary:
Dear All,

We have been looking at the following problem, where any code after the 
constant bound loop is not analyzed because of the limit on how many times the 
same block is visited, as described in bugzillas #7638 and #23438. This problem 
is of interest to us because we have identified significant bugs that the 
checkers are not locating. We have been discussing a solution involving ranges 
as a longer term project, but I would like to propose a patch to improve the 
current implementation.

Example issue:
```
for (int i = 0; i < 1000; ++i) {...something...}
int *p = 0;
*p = 0xDEADBEEF;
```

The proposal is to go through the first and last iterations of the loop. The 
patch creates an exploded node for the approximate last iteration of constant 
bound loops, before the max loop limit / block visit limit is reached. It does 
this by identifying the variable in the loop condition and finding the value 
which is “one away” from the loop being false. For example, if the condition is 
(x < 10), then an exploded node is created where the value of x is 9. 
Evaluating the loop body with x = 9 will then result in the analysis continuing 
after the loop, providing x is incremented.

The patch passes all the tests, with some modifications to coverage.c, in order 
to make the ‘function_which_gives_up’ continue to give up, since the changes 
allowed the analysis to progress past the loop.

This patch does introduce possible false positives, as a result of not knowing 
the state of variables which might be modified in the loop. I believe that, as 
a user, I would rather have false positives after loops than do no analysis at 
all. I understand this may not be the common opinion and am interested in 
hearing your views. There are also issues regarding break statements, which are 
not considered. A more advanced implementation of this approach might be able 
to consider other conditions in the loop, which would allow paths leading to 
breaks to be analyzed.

Lastly, I have performed a study on large code bases and I think there is 
little benefit in having “max-loop” default to 4 with the patch. For variable 
bound loops this tends to result in duplicated analysis after the loop, and it 
makes little difference to any constant bound loop which will do more than a 
few iterations. It might be beneficial to lower the default to 2, especially 
for the shallow analysis setting.

Please let me know your opinions on this approach to processing constant bound 
loops and the patch itself.

Regards,

Sean Eveson
SN Systems - Sony Computer Entertainment Group

Reviewers: jordan_rose, krememek, xazax.hun, zaks.anna, dcoughlin

Subscribers: krememek, xazax.hun, cfe-commits

Differential Revision: http://reviews.llvm.org/D12358

Added:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
cfe/trunk/lib/StaticAnalyzer/Core/LoopWidening.cpp
cfe/trunk/test/Analysis/loop-widening.c
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/test/Analysis/analyzer-config.c
cfe/trunk/test/Analysis/analyzer-config.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h?rev=251621=251620=251621=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h Thu Oct 29 
05:04:41 2015
@@ -262,6 +262,9 @@ private:
   /// \sa shouldInlineLambdas
   Optional InlineLambdas;
 
+  /// \sa shouldWidenLoops
+  Optional WidenLoops;
+
   /// A helper function that retrieves option for a given full-qualified
   /// checker name.
   /// Options for checkers can be specified via 'analyzer-config' command-line
@@ -526,6 +529,10 @@ public:
   /// generated each time a LambdaExpr is visited.
   bool shouldInlineLambdas();
 
+  /// Returns true if the analysis should try to widen loops.
+  /// This is controlled by the 'widen-loops' config option.
+  bool shouldWidenLoops();
+
 public:
   AnalyzerOptions() :
 AnalysisStoreOpt(RegionStoreModel),

Added: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h?rev=251621=auto
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h 
(added)
+++

Re: [PATCH] D12358: [Analyzer] Widening loops which do not exit

2015-10-27 Thread Sean Eveson via cfe-commits

seaneveson updated this revision to Diff 38519.
seaneveson added a comment.

Removed checking for already exited loops
Addressed Comments


http://reviews.llvm.org/D12358

Files:
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/CMakeLists.txt
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/LoopWidening.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/loop-widening.c

Index: test/Analysis/loop-widening.c
===
--- test/Analysis/loop-widening.c
+++ test/Analysis/loop-widening.c
@@ -0,0 +1,190 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-max-loop 4 -analyzer-config widen-loops=true -verify %s
+
+void clang_analyzer_eval(int);
+void clang_analyzer_warnIfReached();
+
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+void free(void *);
+
+void loop_which_iterates_limit_times_not_widened() {
+  int i;
+  int x = 1;
+  // Check loop isn't widened by checking x isn't invalidated
+  for (i = 0; i < 1; ++i) {}
+  clang_analyzer_eval(x == 1); // expected-warning {{TRUE}}
+  for (i = 0; i < 2; ++i) {}
+  clang_analyzer_eval(x == 1); // expected-warning {{TRUE}}
+  for (i = 0; i < 3; ++i) {}
+  // FIXME loss of precision as a result of evaluating the widened loop body
+  //   *instead* of the last iteration.
+  clang_analyzer_eval(x == 1); // expected-warning {{UNKNOWN}}
+}
+
+int a_global;
+
+void loop_evaluated_before_widening() {
+  int i;
+  a_global = 1;
+  for (i = 0; i < 10; ++i) {
+if (i == 2) {
+  // True before widening then unknown after.
+  clang_analyzer_eval(a_global == 1); // expected-warning{{TRUE}} expected-warning{{UNKNOWN}}
+}
+  }
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void warnings_after_loop() {
+  int i;
+  for (i = 0; i < 10; ++i) {}
+  char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void for_loop_exits() {
+  int i;
+  for (i = 0; i < 10; ++i) {}
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void while_loop_exits() {
+  int i = 0;
+  while (i < 10) {++i;}
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void do_while_loop_exits() {
+  int i = 0;
+  do {++i;} while (i < 10);
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void loop_body_is_widened() {
+  int i = 0;
+  while (i < 100) {
+if (i > 10) {
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+++i;
+  }
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void invariably_infinite_loop() {
+  int i = 0;
+  while (1) { ++i; }
+  clang_analyzer_warnIfReached(); // no-warning
+}
+
+void invariably_infinite_break_loop() {
+  int i = 0;
+  while (1) {
+++i;
+int x = 1;
+if (!x) break;
+  }
+  clang_analyzer_warnIfReached(); // no-warning
+}
+
+void reachable_break_loop() {
+  int i = 0;
+  while (1) {
+++i;
+if (i == 100) break;
+  }
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void condition_constrained_true_in_loop() {
+  int i = 0;
+  while (i < 50) {
+clang_analyzer_eval(i < 50); // expected-warning {{TRUE}}
+++i;
+  }
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void condition_constrained_false_after_loop() {
+  int i = 0;
+  while (i < 50) {
+++i;
+  }
+  clang_analyzer_eval(i >= 50); // expected-warning {{TRUE}}
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void multiple_exit_test() {
+  int x = 0;
+  int i = 0;
+  while (i < 50) {
+if (x) {
+  i = 10;
+  break;
+}
+++i;
+  }
+  // Reachable by 'normal' exit
+  if (i == 50) clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  // Reachable by break point
+  if (i == 10) clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  // Not reachable
+  if (i < 10) clang_analyzer_warnIfReached(); // no-warning
+  if (i > 10 && i < 50) clang_analyzer_warnIfReached(); // no-warning
+}
+
+void pointer_doesnt_leak_from_loop() {
+  int *h_ptr = (int *) malloc(sizeof(int));
+  for (int i = 0; i < 2; ++i) {}
+  for (int i = 0; i < 10; ++i) {} // no-warning
+  free(h_ptr);
+}
+
+int g_global;
+
+void unknown_after_loop(int s_arg) {
+  g_global = 0;
+  s_arg = 1;
+  int s_local = 2;
+  int *h_ptr = malloc(sizeof(int));
+
+  for (int i = 0; i < 10; ++i) {}
+
+  clang_analyzer_eval(g_global); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(s_arg); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(s_local); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(h_ptr == 0); // expected-warning {{UNKNOWN}}
+  free(h_ptr);
+}
+
+void

Re: [PATCH] D12358: [Analyzer] Widening loops which do not exit

2015-10-27 Thread Sean Eveson via cfe-commits

seaneveson marked 5 inline comments as done.


Comment at: test/Analysis/loop-widening.c:160
@@ +159,3 @@
+void variable_bound_exiting_loops_widened(int x) {
+  int i = 0;
+  int t = 1;

The inner loop will now be widened in the first example test, which 
'coincidentally' widens the outer loop as well. This means this text now passes.


http://reviews.llvm.org/D12358



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

Re: [PATCH] D12358: [Analyzer] Widening loops which do not exit

2015-10-26 Thread Sean Eveson via cfe-commits

seaneveson marked an inline comment as done.
seaneveson added a comment.

Hi Devin,

Sorry it also took me so long to get back to you.



Comment at: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h:266
@@ +265,3 @@
+  /// \sa shouldWidenLoops
+  bool WidenLoops;
+

dcoughlin wrote:
> Is this field used?
No. Thanks I'll fix that.


Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:1407
@@ +1406,3 @@
+const CFGBlock *Following = getBlockAfterLoop(L.getDst());
+if (Following && nodeBuilder.getContext().Eng.blockWasVisited(Following))
+  return;

dcoughlin wrote:
> What is the purpose of checking whether the block has already been visited by 
> some other path?
> 
> If I understand correctly, this will stop the analyzer from widening before 
> the "last" iteration through the loop and so will result in a sink after that 
> iteration. What this means is that we will never explore the code beyond the 
> loop in the widened state -- but isn't this the whole point of the widening?
> 
> So, for example, in your `variable_bound_exiting_loops_not_widened()` test, 
> don't we want the clang_analyzer_eval() statement after the loop to be 
> symbolically executed on 4 separate paths? That is, once when i is 0, once 
> when i is 1, once when i is 2 and once when i is $conj_i$ + 1 where $conj_i$ 
> is the value conjured for i when widening.
> 
> Also, in general, analysis of one path should not depend at all on whether 
> another path has been explored. This is because we want the analyzer to be 
> free choose different strategies about path exploration (e.g., BFS vs. DFS, 
> prioritizing some paths over others, etc.) without changing the issues 
> reported on along any given path. For this reason, I don't think we 
> necessarily want to track and expose `blockWasVisited()` on CoreEngine or use 
> this to determine when to permit a sink.
> 
> 
I was trying to avoid doing unnecessary invalidation, where the variable bound 
loop had already exited. I suppose this won’t be a concern when the 
invalidation is improved. If everyone is happy for loops that have already 
exited to be widened then I will remove the related changes.


Comment at: lib/StaticAnalyzer/Core/LoopWidening.cpp:98
@@ +97,3 @@
+  RegionAndSymbolInvalidationTraits ITraits;
+  for (int RegionIndex = 0; RegionIndex < NumRegions; ++ RegionIndex) {
+ITraits.setTrait(Regions[RegionIndex],

dcoughlin wrote:
> I get a warning here about comparing a signed int (RegionIndex) to an 
> unsigned (NumRegions).
> 
> I think you can avoid this and simplify things by using a range-based for 
> loop:
> ```
>   const MemRegion *Regions[] = {
>   ...
>   };
>   RegionAndSymbolInvalidationTraits ITraits;
>   for (auto *Region : Regions) {
> ...
>   }
> ```
Will do. Thanks.


http://reviews.llvm.org/D12358



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

r250500 - Test commit

2015-10-16 Thread Sean Eveson via cfe-commits

Author: seaneveson
Date: Fri Oct 16 03:54:23 2015
New Revision: 250500

URL: http://llvm.org/viewvc/llvm-project?rev=250500=rev
Log:
Test commit

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp?rev=250500=250499=250500=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp Fri Oct 16 03:54:23 2015
@@ -408,7 +408,7 @@ void CXXInstanceCall::getExtraInvalidate
   SVal ThisVal = getCXXThisVal();
   Values.push_back(ThisVal);
 
-  // Don't invalidate if the method is const and there are no mutable fields
+  // Don't invalidate if the method is const and there are no mutable fields.
   if (const CXXMethodDecl *D = cast_or_null(getDecl())) {
 if (!D->isConst())
   return;


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

Re: [PATCH] D12358: [Analyzer] Widening loops which do not exit

2015-10-15 Thread Sean Eveson via cfe-commits

seaneveson marked 3 inline comments as done.
seaneveson added a comment.

In http://reviews.llvm.org/D12358#266391, @dcoughlin wrote:

> I think this is an acceptable loss of precision because, in general, it is 
> unlikely that a concrete-bounded loop will be executed *exactly* 
> maxBlockVisitOnPath times. You could add special syntactic checks to try to 
> detect this, but I think it is unlikely to make much different in practice.

I agree.

The problem can be solved by executing the widened state after the last 
iteration, rather than instead of it. This can only really be done by changing 
the max block visit approach. This needs to be done in the future in order to 
support nested loops. This precision issue could also be resolved at that time.

Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h:84
@@ +83,3 @@
+  /// The blocks that have been visited during analysis
+  llvm::DenseSet blocksVisited;
+

The BlockCount is specific to the path of analysis, so it can't be used to 
check if a loop has been exited before. I've added this set to store and 
identify previously visited blocks. Is there a better way this can/could be 
done?

Comment at: lib/StaticAnalyzer/Core/LoopWidening.cpp:65
@@ +64,3 @@
+  if (FalseBranch)
+return FalseBranch;
+  // If there isn't a false branch we still have to check for break points

Perfect, thanks.

Perhaps it could be changed back to false when the invalidation is better? Then 
it could catch true positives e.g.
```
int *a = new int[10]
for (int i = 0; i < 100; ++i) {
  if (i > 90) 
++a;
  ...
}
```

Comment at: test/Analysis/loop-widening.c:159
@@ +158,3 @@
+
+void variable_bound_exiting_loops_not_widened(int x) {
+  int i = 0;

Thank you for the example tests.

When an inner loop is widened, any outer loops are widened 'coincidentally' 
since everything is invalidated. This means the second example test passes. 
When an inner loop is not widened the outer loop will not be widened. This is 
because a sink is generated when the max block count is reached the second time 
through the outer loop. i.e.
```
for (i = 0; i < 10; i++) {
  for (j = 0; j < 2; j++) {
// A sink is generated here on the second iteration of the outer loop,
// since this block was already visited twice (when i == 0).
...
  }
}
// Never reach here
```
This means the first example test does not pass. I’ve added the tests with a 
FIXME comment.

http://reviews.llvm.org/D12358

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

Re: [PATCH] D12358: [Analyzer] Widening loops which do not exit

2015-10-15 Thread Sean Eveson via cfe-commits

seaneveson updated this revision to Diff 37462.
seaneveson added a comment.

Set CausedByPointerEscape to true
Check if the loop has already been exited before widening
Changed tests to use void clang_analyze_eval(int)
Added variable bound loop tests
Added nested loop tests


http://reviews.llvm.org/D12358

Files:
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h
  include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/CMakeLists.txt
  lib/StaticAnalyzer/Core/CoreEngine.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/LoopWidening.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/loop-widening.c

Index: test/Analysis/loop-widening.c
===
--- test/Analysis/loop-widening.c
+++ test/Analysis/loop-widening.c
@@ -0,0 +1,202 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-store=region -analyzer-max-loop 4 -analyzer-config widen-loops=true -verify %s
+
+void clang_analyzer_eval(int);
+void clang_analyzer_warnIfReached();
+
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+void free(void *);
+
+void loop_which_iterates_limit_times_not_widened() {
+  int i;
+  int x = 1;
+  // Check loop isn't widened by checking x isn't invalidated
+  for (i = 0; i < 1; ++i) {}
+  clang_analyzer_eval(x == 1); // expected-warning {{TRUE}}
+  for (i = 0; i < 2; ++i) {}
+  clang_analyzer_eval(x == 1); // expected-warning {{TRUE}}
+  for (i = 0; i < 3; ++i) {}
+  // FIXME loss of precision as a result of evaluating the widened loop body
+  //   *instead* of the last iteration.
+  clang_analyzer_eval(x == 1); // expected-warning {{UNKNOWN}}
+}
+
+int a_global;
+
+void loop_evaluated_before_widening() {
+  int i;
+  a_global = 1;
+  for (i = 0; i < 10; ++i) {
+if (i == 2) {
+  // True before widening then unknown after.
+  clang_analyzer_eval(a_global == 1); // expected-warning{{TRUE}} expected-warning{{UNKNOWN}}
+}
+  }
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void warnings_after_loop() {
+  int i;
+  for (i = 0; i < 10; ++i) {}
+  char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void for_loop_exits() {
+  int i;
+  for (i = 0; i < 10; ++i) {}
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void while_loop_exits() {
+  int i = 0;
+  while (i < 10) {++i;}
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void do_while_loop_exits() {
+  int i = 0;
+  do {++i;} while (i < 10);
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void loop_body_is_widened() {
+  int i = 0;
+  while (i < 100) {
+if (i > 10) {
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+++i;
+  }
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void invariably_infinite_loop() {
+  int i = 0;
+  while (1) { ++i; }
+  clang_analyzer_warnIfReached(); // no-warning
+}
+
+void invariably_infinite_break_loop() {
+  int i = 0;
+  while (1) {
+++i;
+int x = 1;
+if (!x) break;
+  }
+  clang_analyzer_warnIfReached(); // no-warning
+}
+
+void reachable_break_loop() {
+  int i = 0;
+  while (1) {
+++i;
+if (i == 100) break;
+  }
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void condition_constrained_true_in_loop() {
+  int i = 0;
+  while (i < 50) {
+clang_analyzer_eval(i < 50); // expected-warning {{TRUE}}
+++i;
+  }
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void condition_constrained_false_after_loop() {
+  int i = 0;
+  while (i < 50) {
+++i;
+  }
+  clang_analyzer_eval(i >= 50); // expected-warning {{TRUE}}
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void multiple_exit_test() {
+  int x = 0;
+  int i = 0;
+  while (i < 50) {
+if (x) {
+  i = 10;
+  break;
+}
+++i;
+  }
+  // Reachable by 'normal' exit
+  if (i == 50) clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  // Reachable by break point
+  if (i == 10) clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  // Not reachable
+  if (i < 10) clang_analyzer_warnIfReached(); // no-warning
+  if (i > 10 && i < 50) clang_analyzer_warnIfReached(); // no-warning
+}
+
+void pointer_doesnt_leak_from_loop() {
+  int *h_ptr = (int *) malloc(sizeof(int));
+  for (int i = 0; i < 2; ++i) {}
+  for (int i = 0; i < 10; ++i) {} // no-warning
+  free(h_ptr);
+}
+
+int g_global;
+
+void unknown_after_loop(int s_arg) {
+  g_global = 0;
+  s_arg = 1;
+  int s_local = 2;
+  int *h_ptr = malloc(sizeof(int));
+  
+  for (int i = 0; i < 10; ++i) {}
+  
+  clang_analyzer_eval(g_global); // expected-warning

Re: [PATCH] D13099: [Analyzer] Don’t invalidate CXXThis when conservatively evaluating const methods (PR 21606)

2015-10-12 Thread Sean Eveson via cfe-commits

seaneveson updated this revision to Diff 37084.
seaneveson added a comment.

Updated to latest trunk
Replaced some code with an existing method: ignoreParenBaseCasts()

Could someone commit this for me as I don't have SVN access?
Thank you.


http://reviews.llvm.org/D13099

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  lib/StaticAnalyzer/Core/CallEvent.cpp
  test/Analysis/const-method-call.cpp

Index: test/Analysis/const-method-call.cpp
===
--- test/Analysis/const-method-call.cpp
+++ test/Analysis/const-method-call.cpp
@@ -0,0 +1,230 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(bool);
+
+struct A {
+  int x;
+  void foo() const;
+  void bar();
+};
+
+struct B {
+  mutable int mut;
+  void foo() const;
+};
+
+struct C {
+  int *p;
+  void foo() const;
+};
+
+struct MutBase {
+  mutable int b_mut;
+};
+
+struct MutDerived : MutBase {
+  void foo() const;
+};
+
+struct PBase {
+  int *p;
+};
+
+struct PDerived : PBase {
+  void foo() const;
+};
+
+struct Inner {
+  int x;
+  int *p;
+  void bar() const;
+};
+
+struct Outer {
+  int x;
+  Inner in;
+  void foo() const;
+};
+
+void checkThatConstMethodWithoutDefinitionDoesNotInvalidateObject() {
+  A t;
+  t.x = 3;
+  t.foo();
+  clang_analyzer_eval(t.x == 3); // expected-warning{{TRUE}}
+  // Test non-const does invalidate
+  t.bar();
+  clang_analyzer_eval(t.x); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidateMutableFields() {
+  B t;
+  t.mut = 4;
+  t.foo();
+  clang_analyzer_eval(t.mut); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidatePointedAtMemory() {
+  int x = 1;
+  C t;
+  t.p = 
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(t.p == ); // expected-warning{{TRUE}}
+}
+
+void checkThatConstMethodDoesInvalidateInheritedMutableFields() {
+  MutDerived t;
+  t.b_mut = 4;
+  t.foo();
+  clang_analyzer_eval(t.b_mut); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidateInheritedPointedAtMemory() {
+  int x = 1;
+  PDerived t;
+  t.p = 
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(t.p == ); // expected-warning{{TRUE}}
+}
+
+void checkThatConstMethodDoesInvalidateContainedPointedAtMemory() {
+  int x = 1;
+  Outer t;
+  t.x = 2;
+  t.in.p = 
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(t.x == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(t.in.p == ); // expected-warning{{TRUE}}
+}
+
+void checkThatContainedConstMethodDoesNotInvalidateObjects() {
+  Outer t;
+  t.x = 1;
+  t.in.x = 2;
+  t.in.bar();
+  clang_analyzer_eval(t.x == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(t.in.x == 2); // expected-warning{{TRUE}}
+}
+
+// --- Versions of the above tests where the const method is inherited --- //
+
+struct B1 {
+  void foo() const;
+};
+
+struct D1 : public B1 {
+  int x;
+};
+
+struct D2 : public B1 {
+  mutable int mut;
+};
+
+struct D3 : public B1 {
+  int *p;
+};
+
+struct DInner : public B1 {
+  int x;
+  int *p;
+};
+
+struct DOuter : public B1 {
+  int x;
+  DInner in;
+};
+
+void checkThatInheritedConstMethodDoesNotInvalidateObject() {
+  D1 t;
+  t.x = 1;
+  t.foo();
+  clang_analyzer_eval(t.x == 1); // expected-warning{{TRUE}}
+}
+
+void checkThatInheritedConstMethodDoesInvalidateMutableFields() {
+  D2 t;
+  t.mut = 1;
+  t.foo();
+  clang_analyzer_eval(t.mut); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatInheritedConstMethodDoesInvalidatePointedAtMemory() {
+  int x = 1;
+  D3 t;
+  t.p = 
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(t.p == ); // expected-warning{{TRUE}}
+}
+
+void checkThatInheritedConstMethodDoesInvalidateContainedPointedAtMemory() {
+  int x = 1;
+  DOuter t;
+  t.x = 2;
+  t.in.x = 3;
+  t.in.p = 
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(t.x == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(t.in.x == 3); // expected-warning{{TRUE}}
+  clang_analyzer_eval(t.in.p == ); // expected-warning{{TRUE}}
+}
+
+void checkThatInheritedContainedConstMethodDoesNotInvalidateObjects() {
+  DOuter t;
+  t.x = 1;
+  t.in.x = 2;
+  t.in.foo();
+  clang_analyzer_eval(t.x == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(t.in.x == 2); // expected-warning{{TRUE}}
+}
+
+// --- PR21606 --- //
+
+struct s1 {
+void g(const int *i) const;
+};
+
+struct s2 {
+void f(int *i) {
+m_i = i;
+m_s.g(m_i);
+if (m_i)
+*i = 42; // no-warning
+}
+
+int *m_i;
+s1 m_s;
+};
+
+void PR21606()
+{
+s2().f(0);
+}
+
+// FIXME
+// When there is a circular reference to an object and a const method is called
+// the object is not invalidated because TK_PreserveContents has already been
+// set.
+struct

Re: [PATCH] D13099: [Analyzer] Don’t invalidate CXXThis when conservatively evaluating const methods (PR 21606)

2015-10-09 Thread Sean Eveson via cfe-commits

seaneveson updated this revision to Diff 36927.
seaneveson added a comment.

Updated to latest trunk.
Added FIXME test for circular reference issue.


http://reviews.llvm.org/D13099

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  lib/StaticAnalyzer/Core/CallEvent.cpp
  test/Analysis/const-method-call.cpp

Index: test/Analysis/const-method-call.cpp
===
--- test/Analysis/const-method-call.cpp
+++ test/Analysis/const-method-call.cpp
@@ -0,0 +1,230 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(bool);
+
+struct A {
+  int x;
+  void foo() const;
+  void bar();
+};
+
+struct B {
+  mutable int mut;
+  void foo() const;
+};
+
+struct C {
+  int *p;
+  void foo() const;
+};
+
+struct MutBase {
+  mutable int b_mut;
+};
+
+struct MutDerived : MutBase {
+  void foo() const;
+};
+
+struct PBase {
+  int *p;
+};
+
+struct PDerived : PBase {
+  void foo() const;
+};
+
+struct Inner {
+  int x;
+  int *p;
+  void bar() const;
+};
+
+struct Outer {
+  int x;
+  Inner in;
+  void foo() const;
+};
+
+void checkThatConstMethodWithoutDefinitionDoesNotInvalidateObject() {
+  A t;
+  t.x = 3;
+  t.foo();
+  clang_analyzer_eval(t.x == 3); // expected-warning{{TRUE}}
+  // Test non-const does invalidate
+  t.bar();
+  clang_analyzer_eval(t.x); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidateMutableFields() {
+  B t;
+  t.mut = 4;
+  t.foo();
+  clang_analyzer_eval(t.mut); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidatePointedAtMemory() {
+  int x = 1;
+  C t;
+  t.p = 
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(t.p == ); // expected-warning{{TRUE}}
+}
+
+void checkThatConstMethodDoesInvalidateInheritedMutableFields() {
+  MutDerived t;
+  t.b_mut = 4;
+  t.foo();
+  clang_analyzer_eval(t.b_mut); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidateInheritedPointedAtMemory() {
+  int x = 1;
+  PDerived t;
+  t.p = 
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(t.p == ); // expected-warning{{TRUE}}
+}
+
+void checkThatConstMethodDoesInvalidateContainedPointedAtMemory() {
+  int x = 1;
+  Outer t;
+  t.x = 2;
+  t.in.p = 
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(t.x == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(t.in.p == ); // expected-warning{{TRUE}}
+}
+
+void checkThatContainedConstMethodDoesNotInvalidateObjects() {
+  Outer t;
+  t.x = 1;
+  t.in.x = 2;
+  t.in.bar();
+  clang_analyzer_eval(t.x == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(t.in.x == 2); // expected-warning{{TRUE}}
+}
+
+// --- Versions of the above tests where the const method is inherited --- //
+
+struct B1 {
+  void foo() const;
+};
+
+struct D1 : public B1 {
+  int x;
+};
+
+struct D2 : public B1 {
+  mutable int mut;
+};
+
+struct D3 : public B1 {
+  int *p;
+};
+
+struct DInner : public B1 {
+  int x;
+  int *p;
+};
+
+struct DOuter : public B1 {
+  int x;
+  DInner in;
+};
+
+void checkThatInheritedConstMethodDoesNotInvalidateObject() {
+  D1 t;
+  t.x = 1;
+  t.foo();
+  clang_analyzer_eval(t.x == 1); // expected-warning{{TRUE}}
+}
+
+void checkThatInheritedConstMethodDoesInvalidateMutableFields() {
+  D2 t;
+  t.mut = 1;
+  t.foo();
+  clang_analyzer_eval(t.mut); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatInheritedConstMethodDoesInvalidatePointedAtMemory() {
+  int x = 1;
+  D3 t;
+  t.p = 
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(t.p == ); // expected-warning{{TRUE}}
+}
+
+void checkThatInheritedConstMethodDoesInvalidateContainedPointedAtMemory() {
+  int x = 1;
+  DOuter t;
+  t.x = 2;
+  t.in.x = 3;
+  t.in.p = 
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(t.x == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(t.in.x == 3); // expected-warning{{TRUE}}
+  clang_analyzer_eval(t.in.p == ); // expected-warning{{TRUE}}
+}
+
+void checkThatInheritedContainedConstMethodDoesNotInvalidateObjects() {
+  DOuter t;
+  t.x = 1;
+  t.in.x = 2;
+  t.in.foo();
+  clang_analyzer_eval(t.x == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(t.in.x == 2); // expected-warning{{TRUE}}
+}
+
+// --- PR21606 --- //
+
+struct s1 {
+void g(const int *i) const;
+};
+
+struct s2 {
+void f(int *i) {
+m_i = i;
+m_s.g(m_i);
+if (m_i)
+*i = 42; // no-warning
+}
+
+int *m_i;
+s1 m_s;
+};
+
+void PR21606()
+{
+s2().f(0);
+}
+
+// FIXME
+// When there is a circular reference to an object and a const method is called
+// the object is not invalidated because TK_PreserveContents has already been
+// set.
+struct Outer2;
+
+struct InnerWithRef {
+  Outer2 *ref;
+};
+
+struct Outer2 {
+  int x;
+

Re: [PATCH] D13099: [Analyzer] Don’t invalidate CXXThis when conservatively evaluating const methods (PR 21606)

2015-10-08 Thread Sean Eveson via cfe-commits

seaneveson updated this revision to Diff 36845.
seaneveson added a comment.

Move PR21606 test into const-method-call.cpp.
Added test for const calls on member objects.
Added tests for inherited const methods.
Changed TK_PreserveContents to be set for the base region. This is so memory is 
still preserved when 'this' is a member of another object.
Added code to get the instance's class, rather than the method's class. This 
solves issues where the method's class has no mutable members, but the class 
which inherits the method does.


http://reviews.llvm.org/D13099

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  lib/StaticAnalyzer/Core/CallEvent.cpp
  test/Analysis/const-method-call.cpp

Index: test/Analysis/const-method-call.cpp
===
--- test/Analysis/const-method-call.cpp
+++ test/Analysis/const-method-call.cpp
@@ -0,0 +1,205 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(bool);
+
+struct A {
+  int x;
+  void foo() const;
+  void bar();
+};
+
+struct B {
+  mutable int mut;
+  void foo() const;
+};
+
+struct C {
+  int *p;
+  void foo() const;
+};
+
+struct MutBase {
+  mutable int b_mut;
+};
+
+struct MutDerived : MutBase {
+  void foo() const;
+};
+
+struct PBase {
+  int *p;
+};
+
+struct PDerived : PBase {
+  void foo() const;
+};
+
+struct Inner {
+  int x;
+  int *p;
+  void bar() const;
+};
+
+struct Outer {
+  int x;
+  Inner in;
+  void foo() const;
+};
+
+void checkThatConstMethodWithoutDefinitionDoesNotInvalidateObject() {
+  A t;
+  t.x = 3;
+  t.foo();
+  clang_analyzer_eval(t.x == 3); // expected-warning{{TRUE}}
+  // Test non-const does invalidate
+  t.bar();
+  clang_analyzer_eval(t.x); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidateMutableFields() {
+  B t;
+  t.mut = 4;
+  t.foo();
+  clang_analyzer_eval(t.mut); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidatePointedAtMemory() {
+  int x = 1;
+  C t;
+  t.p = 
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(t.p == ); // expected-warning{{TRUE}}
+}
+
+void checkThatConstMethodDoesInvalidateInheritedMutableFields() {
+  MutDerived t;
+  t.b_mut = 4;
+  t.foo();
+  clang_analyzer_eval(t.b_mut); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidateInheritedPointedAtMemory() {
+  int x = 1;
+  PDerived t;
+  t.p = 
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(t.p == ); // expected-warning{{TRUE}}
+}
+
+void checkThatConstMethodDoesInvalidateContainedPointedAtMemory() {
+  int x = 1;
+  Outer t;
+  t.x = 2;
+  t.in.p = 
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(t.x == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(t.in.p == ); // expected-warning{{TRUE}}
+}
+
+void checkThatContainedConstMethodDoesNotInvalidateObjects() {
+  Outer t;
+  t.x = 1;
+  t.in.x = 2;
+  t.in.bar();
+  clang_analyzer_eval(t.x == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(t.in.x == 2); // expected-warning{{TRUE}}
+}
+
+// --- Versions of the above tests where the const method is inherited --- //
+
+struct B1 {
+  void foo() const;
+};
+
+struct D1 : public B1 {
+  int x;
+};
+
+struct D2 : public B1 {
+  mutable int mut;
+};
+
+struct D3 : public B1 {
+  int *p;
+};
+
+struct DInner : public B1 {
+  int x;
+  int *p;
+};
+
+struct DOuter : public B1 {
+  int x;
+  DInner in;
+};
+
+void checkThatInheritedConstMethodDoesNotInvalidateObject() {
+  D1 t;
+  t.x = 1;
+  t.foo();
+  clang_analyzer_eval(t.x == 1); // expected-warning{{TRUE}}
+}
+
+void checkThatInheritedConstMethodDoesInvalidateMutableFields() {
+  D2 t;
+  t.mut = 1;
+  t.foo();
+  clang_analyzer_eval(t.mut); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatInheritedConstMethodDoesInvalidatePointedAtMemory() {
+  int x = 1;
+  D3 t;
+  t.p = 
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(t.p == ); // expected-warning{{TRUE}}
+}
+
+void checkThatInheritedConstMethodDoesInvalidateContainedPointedAtMemory() {
+  int x = 1;
+  DOuter t;
+  t.x = 2;
+  t.in.x = 3;
+  t.in.p = 
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(t.x == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(t.in.x == 3); // expected-warning{{TRUE}}
+  clang_analyzer_eval(t.in.p == ); // expected-warning{{TRUE}}
+}
+
+void checkThatInheritedContainedConstMethodDoesNotInvalidateObjects() {
+  DOuter t;
+  t.x = 1;
+  t.in.x = 2;
+  t.in.foo();
+  clang_analyzer_eval(t.x == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(t.in.x == 2); // expected-warning{{TRUE}}
+}
+
+// --- PR21606 --- //
+
+struct s1 {
+void g(const int *i) const;
+};
+
+struct s2 {
+void f(int *i) {
+m_i = i;
+m_s.g(m_i);
+if (m_i)
+

Re: [PATCH] D12358: [Analyzer] Handling constant bound loops

2015-10-07 Thread Sean Eveson via cfe-commits

seaneveson marked 4 inline comments as done.
seaneveson added a comment.

There are some issues which haven't been resolved yet:

- There is a loss of precision for loops that need to be executed exactly 
maxBlockVisitOnPath times, as the loop body is executed with the widened state 
**instead** of the last iteration.
- The invalidation still causes memory leak false positives (see failing test: 
pointer_doesnt_leak_from_loop).


http://reviews.llvm.org/D12358



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

Re: [PATCH] D12358: [Analyzer] Handling constant bound loops

2015-10-07 Thread Sean Eveson via cfe-commits

seaneveson updated this revision to Diff 36739.
seaneveson added a comment.

Updated to latest revision.
Change patch to widen all loops which do not exit.
Changed to widen on block entrance so the guard condition is accounted for 
(thanks for the suggestion).
Updated tests to reflect changes.


http://reviews.llvm.org/D12358

Files:
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/CMakeLists.txt
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/LoopWidening.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/loop-widening.c

Index: test/Analysis/loop-widening.c
===
--- test/Analysis/loop-widening.c
+++ test/Analysis/loop-widening.c
@@ -0,0 +1,159 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-store=region -analyzer-max-loop 4 -analyzer-config widen-loops=true -verify %s
+
+extern void clang_analyzer_eval(_Bool);
+extern void clang_analyzer_warnIfReached();
+
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+void free(void *);
+
+void loop_which_iterates_limit_times_not_widened() {
+  int i;
+  int x = 1;
+  // Check loop isn't widened by checking x isn't invalidated
+  for (i = 0; i < 1; ++i) {}
+  clang_analyzer_eval(x == 1); // expected-warning {{TRUE}}
+  for (i = 0; i < 2; ++i) {}
+  clang_analyzer_eval(x == 1); // expected-warning {{TRUE}}
+  for (i = 0; i < 3; ++i) {}
+  clang_analyzer_eval(x == 1); // expected-warning {{TRUE}} // TODO?
+}
+
+int a_global;
+
+void loop_evaluated_before_widening() {
+  int i;
+  a_global = 1;
+  for (i = 0; i < 10; ++i) {
+if (i == 2) {
+  // True before widening then unknown after
+  clang_analyzer_eval(a_global == 1); // expected-warning{{TRUE}} expected-warning{{UNKNOWN}}
+}
+  }
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void warnings_after_loop() {
+  int i;
+  for (i = 0; i < 10; ++i) {}
+  char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void for_loop_exits() {
+  int i;
+  for (i = 0; i < 10; ++i) {}
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void while_loop_exits() {
+  int i = 0;
+  while (i < 10) {++i;}
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void do_while_loop_exits() {
+  int i = 0;
+  do {++i;} while (i < 10);
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void loop_body_is_widened() {
+  int i = 0;
+  while (i < 100) {
+if (i > 10) {
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+++i;
+  }
+}
+
+void invariably_infinite_loop() {
+  int i = 0;
+  while (1) { ++i; }
+  clang_analyzer_warnIfReached(); // no-warning
+}
+
+void invariably_infinite_break_loop() {
+  int i = 0;
+  while (1) {
+++i;
+int x = 1;
+if (!x) break;
+  }
+  clang_analyzer_warnIfReached(); // no-warning
+}
+
+void reachable_break_loop() {
+  int i = 0;
+  while (1) {
+++i;
+if (i == 100) break;
+  }
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void condition_constrained_true_in_loop() {
+  int i = 0;
+  while (i < 50) {
+if (i >= 50) {
+  clang_analyzer_warnIfReached(); // no-warning
+}
+++i;
+  }
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void condition_constrained_false_after_loop() {
+  int i = 0;
+  while (i < 50) {
+++i;
+  }
+  if (i < 50) {
+clang_analyzer_warnIfReached(); // no-warning
+  }
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+}
+
+void multiple_exit_test() {
+  int x = 0;
+  int i = 0;
+  while (i < 50) {
+if (x) {
+  i = 10;
+  break;
+}
+++i;
+  }
+  // Reachable by 'normal' exit
+  if (i == 50) clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  // Reachable by break point
+  if (i == 10) clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  // Not reachable
+  if (i < 10) clang_analyzer_warnIfReached(); // no-warning
+  if (i > 10 && i < 50) clang_analyzer_warnIfReached(); // no-warning
+}
+
+void pointer_doesnt_leak_from_loop() {
+  int *h_ptr = (int *) malloc(sizeof(int));
+  for (int i = 0; i < 2; ++i) {}
+  for (int i = 0; i < 10; ++i) {} // no-warning // TODO
+  free(h_ptr);
+}
+
+int g_global;
+
+void unknown_after_loop(int s_arg) {
+  g_global = 0;
+  s_arg = 1;
+  int s_local = 2;
+  int *h_ptr = malloc(sizeof(int));
+  *h_ptr = 3;
+  
+  for (int i = 0; i < 10; ++i) {}
+  
+  clang_analyzer_eval(g_global); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(s_arg); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(s_local); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(h_ptr); // expected-warning {{UNKNOWN}}
+

Re: [PATCH] D13099: [Analyzer] Don’t invalidate CXXThis when conservatively evaluating const methods (PR 21606)

2015-10-06 Thread Sean Eveson via cfe-commits

seaneveson marked 6 inline comments as done.
seaneveson added a comment.

There is an issue where pointers to the object (this) should cause it to be 
invalidated, but don't since TK_PreserveContents has been set.

For example:

  class B;
  class A {
B b;
const foo();
  };
  class B {
A *ptr_a;
  }
  
  A a;
  a.b.ptr_a = 
  a.foo();

The method foo might modify 'this' via the object b, but 'this' will not be 
invalidated as foo is const.

Again I'm not sure this is worth checking for, based on the assumption that a 
reasonable const method won't modify the relevant object. What do people think?


http://reviews.llvm.org/D13099



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

Re: [PATCH] D13099: [Analyzer] Don’t invalidate CXXThis when conservatively evaluating const methods (PR 21606)

2015-10-06 Thread Sean Eveson via cfe-commits

seaneveson updated this revision to Diff 36604.
seaneveson added a comment.

Removed mutable field from derived class in inheritance test case.


http://reviews.llvm.org/D13099

Files:
  include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h
  lib/StaticAnalyzer/Core/CallEvent.cpp
  test/Analysis/PR21606.cpp
  test/Analysis/const-method-call.cpp

Index: test/Analysis/const-method-call.cpp
===
--- test/Analysis/const-method-call.cpp
+++ test/Analysis/const-method-call.cpp
@@ -0,0 +1,89 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(bool);
+
+struct A {
+  int x;
+  void foo() const;
+  void bar();
+};
+
+struct B {
+  mutable int mut;
+  void foo() const;
+};
+
+struct C {
+  int *p;
+  void foo() const;
+};
+
+struct Base {
+  mutable int b_mut;
+  int *p;
+};
+
+struct Derived : Base {
+  void foo() const;
+};
+
+struct Inner {
+  int *p;
+};
+
+struct Outer {
+  int x;
+  Inner in;
+  void foo() const;
+};
+
+void checkThatConstMethodWithoutDefinitionDoesNotInvalidateObject() {
+  A t;
+  t.x = 3;
+  t.foo();
+  clang_analyzer_eval(t.x == 3); // expected-warning{{TRUE}}
+  // Test non-const does invalidate
+  t.bar();
+  clang_analyzer_eval(t.x); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidateMutableFields() {
+  B t;
+  t.mut = 4;
+  t.foo();
+  clang_analyzer_eval(t.mut); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidatePointedAtMemory() {
+  int x = 1;
+  C t;
+  t.p = 
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidateInheritedMutableFields() {
+  Derived t;
+  t.b_mut = 4;
+  t.foo();
+  clang_analyzer_eval(t.b_mut); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidateInheritedPointedAtMemory() {
+  int x = 1;
+  Derived t;
+  t.p = 
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidateContainedPointedAtMemory() {
+  int x = 1;
+  Outer t;
+  t.x = 2;
+  t.in.p = 
+  t.foo();
+  clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(t.x == 2); // expected-warning{{TRUE}}
+  clang_analyzer_eval(t.in.p == ); // expected-warning{{TRUE}}
+}
Index: test/Analysis/PR21606.cpp
===
--- test/Analysis/PR21606.cpp
+++ test/Analysis/PR21606.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core
+// PR21606
+
+struct s1 {
+void g(const int *i) const;
+};
+
+struct s2 {
+void f(int *i) {
+m_i = i;
+m_s.g(m_i);
+if (m_i)
+*i = 42; // no-warning
+}
+
+int *m_i;
+s1 m_s;
+};
+
+int main()
+{
+s2().f(0);
+}
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -148,7 +148,7 @@
   SmallVector ValuesToInvalidate;
   RegionAndSymbolInvalidationTraits ETraits;
 
-  getExtraInvalidatedValues(ValuesToInvalidate);
+  getExtraInvalidatedValues(ValuesToInvalidate, );
 
   // Indexes of arguments whose values will be preserved by the call.
   llvm::SmallSet PreserveArgs;
@@ -403,8 +403,20 @@
   return getSVal(CE->getCallee()).getAsFunctionDecl();
 }
 
-void CXXInstanceCall::getExtraInvalidatedValues(ValueList ) const {
-  Values.push_back(getCXXThisVal());
+void CXXInstanceCall::getExtraInvalidatedValues(ValueList ,
+RegionAndSymbolInvalidationTraits *ETraits) const {
+  SVal ThisVal = getCXXThisVal();
+  Values.push_back(ThisVal);
+
+  // Don't invalitate if the method is const and there are no mutable fields
+  if (const CXXMethodDecl *D = cast_or_null(getDecl())) {
+if (D->isConst() && !D->getParent()->hasMutableFields()) {
+  const MemRegion *ThisRegion = ThisVal.getAsRegion();
+  assert(ThisRegion && "ThisValue was not a memory region");
+  ETraits->setTrait(ThisRegion->StripCasts(),
+RegionAndSymbolInvalidationTraits::TK_PreserveContents);
+}
+  }
 }
 
 SVal CXXInstanceCall::getCXXThisVal() const {
@@ -550,7 +562,8 @@
   return D->parameters();
 }
 
-void BlockCall::getExtraInvalidatedValues(ValueList ) const {
+void BlockCall::getExtraInvalidatedValues(ValueList ,
+  RegionAndSymbolInvalidationTraits *ETraits) const {
   // FIXME: This also needs to invalidate captured globals.
   if (const MemRegion *R = getBlockRegion())
 Values.push_back(loc::MemRegionVal(R));
@@ -571,7 +584,8 @@
   return UnknownVal();
 }
 
-void CXXConstructorCall::getExtraInvalidatedValues(ValueList ) const {
+void CXXConstructorCall::getExtraInvalidatedValues(ValueList ,
+   RegionAndSymbolInvalidationTraits *ETraits) const {
   if (Data)

Re: [PATCH] D13099: [Analyzer] Don’t invalidate CXXThis when conservatively evaluating const methods (PR 21606)

2015-10-06 Thread Sean Eveson via cfe-commits

seaneveson marked an inline comment as done.
seaneveson added a comment.

http://reviews.llvm.org/D13099



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

Re: [PATCH] D12993: [analyzer] Add TK_EntireMemSpace invalidation trait.

2015-10-01 Thread Sean Eveson via cfe-commits

seaneveson added a comment.

Could you update this patch to the latest trunk please? There's a conflict with 
http://reviews.llvm.org/rL248516.

Thanks.


http://reviews.llvm.org/D12993



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

Re: [PATCH] D12358: [Analyzer] Handling constant bound loops

2015-09-25 Thread Sean Eveson via cfe-commits

seaneveson added a comment.

My initial approach was for the analyzer to have as much information as 
possible after the loop. This means there are cases where the information is 
incorrect. Future work would be to reduce these cases.

I believe your preferred approach is to have no inaccuracies after the loop, 
which can initially be achieved by providing less information. Further work 
would add more (guaranteed accurate) information. In this way the analyzer is 
naturally conservative when it isn't sure of something. I now agree that this 
is a better approach to take.

What do people think about me creating a new patch based on your feedback? The 
aim would be to widen any non-exiting loops by invalidation. The initial patch 
would be behind a flag and would use the TK_EntireMemSpace trait to 
conservatively invalidate 'everything' when a loop does not exit. It would then 
run through the loop body in the invalidated state, resulting in the analysis 
of all possible paths. The loop would then exit from the (now possibly false) 
loop condition, or a (now reachable) break statement. Loops that never exit 
regardless of any variables would be an exception; see my comment below for 
thoughts on handling infinite loops.

Future improvements would prevent unnecessary invalidation and calculate the 
values of modified variables (after the loop).

Comment at: lib/StaticAnalyzer/Core/LoopWidening.cpp:10
@@ +9,3 @@
+///
+/// This file contains functions which are used to widen constant bound loops.
+/// A loop may be widened to approximate the exit state(s), without analysing

zaks.anna wrote:
> This would allow to widen not just the constant bound loops!
> I think we should widen any loop that we know we did not fully execute.
I agree that the goal should be to widen any loop that isn't fully executed, 
but we need to consider infinite loops, which might be falsely exited after 
widening. The way I see it, assuming the widening will be done by invalidating 
variables/memory, we could either:

  # Only widen loops which definitely exit.
  # Widen all loops unless they are definitely infinite (or widen them in a way 
which minimizes the number of infinite loops which then exit).
  # Come up with a method which decides if a loops is infinite or not (but 
tolerate mistakes either way) and widen when we "think" the loop will exit. 
This is similar to the current approach for constant bound loops.

My current preference would be option 2. This is based on the assumption that 
loops are generally written to exit at some point, and if they aren't, they are 
unlikely to have code after them anyway. When it does not know which branch the 
program will go down, the analyzer's approach is to go down both. Similarly if 
the analyzer does not know whether the loop exits, it should optimistically go 
down the exit branch (IMO).

Comment at: test/Analysis/constant-bound-loops.c:174
@@ +173,3 @@
+
+clang_analyzer_eval(g_global); // expected-warning {{UNKNOWN}}
+clang_analyzer_eval(s_arg); // expected-warning {{UNKNOWN}}

zaks.anna wrote:
> seaneveson wrote:
> > zaks.anna wrote:
> > > I think we should extend this test case.
> > > Ex: what about heap, what about variables touched by the loop variables 
> > > declared in the loop's scope?
> > Good point. I actually encountered a false positive while improving this 
> > case.
> > 
> > 
> > ```
> > int *h_ptr = malloc(sizeof(int));
> > *h_ptr = 3;
> > for (int i = 0; i < 10; ++i) {} // WARNING: Potential leak of memory 
> > pointed to by 'h_ptr'
> > ```
> > 
> > The value of h_ptr is invalidated, but I'm not sure about *h_ptr. Is it 
> > likely this warning is produced because the pointer is invalidated, but not 
> > the memory?
> Could you provide the full example? There is leak in the code segment above. 
Sorry, I meant to say the warning is there regardless of what comes after the 
loop. Here is a full test case:
```
typedef __typeof(sizeof(int)) size_t;
void *malloc(size_t);
void free(void *);

void test() {
int *h_ptr = (int *) malloc(sizeof(int));
for (int i = 0; i < 2; ++i) {} // No warning.
for (int i = 0; i < 10; ++i) {}
free(h_ptr);
}
```
It produces the following for me:
```
clang.exe -cc1 -analyze -analyzer-checker=core,unix.Malloc -analyzer-config 
widen-constant-bound-loops=true test.cpp
test.cpp:8:31: warning: Potential leak of memory pointed to by 'h_ptr'
for (int i = 0; i < 10; ++i) {}
  ^
1 warning generated.
```

http://reviews.llvm.org/D12358

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

Re: [PATCH] D13099: [Analyzer] Don’t invalidate CXXThis when conservatively evaluating const methods (PR 21606)

2015-09-25 Thread Sean Eveson via cfe-commits

seaneveson added a comment.

I've realized that the patch doesn't handle pointers correctly, since a const 
method can modify the memory pointed at by a member. While pointer members 
should not be invalidated by const methods (if they are not mutable), the 
memory they point to should still be invalidated. I'll address this in the next 
version.

Regarding ConstPointerEscape:

In http://reviews.llvm.org/D13099#253111, @zaks.anna wrote:

> The analyzer has a notion of ConstPointerEscape, see checkConstPointerEscape 
> callback.
>  All the pointers to const parameters are escaped this way. The 
> implementation for that is in CallEvent::invalidateRegions, right below the 
> code you've added:
>
>   ...
>
>
> I think we should const escape all non-mutable fields as well as 'this'.
>
> (A motivation behind this callback is that one can call delete on pointers of 
> const *void type.)

I think I understand, but to clarify:
The fields that shouldn't be invalidated should still be added to 
ValuesToInvalidate, but with 
RegionAndSymbolInvalidationTraits::TK_PreserveContents set. This will result in 
checkConstPointerEscape being called properly.

Is that correct?

http://reviews.llvm.org/D13099

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

Re: [PATCH] D13099: [Analyzer] Don’t invalidate CXXThis when conservatively evaluating const methods (PR 21606)

2015-09-24 Thread Sean Eveson via cfe-commits

seaneveson added a comment.

Thank you for looking at the patch and all your comments.

In http://reviews.llvm.org/D13099#252492, @xazax.hun wrote:

> One more note. Do we want to support const_cast for this? A possible way to 
> do that is to invalidate this, when a const cast appears in the body of the 
> function. (However the body might not be available. It is only my opinion, 
> but I would be ok to accept this patch without const cast support, but it is 
> something that we might want to document somewhere as a caveat.


IMO it is reasonable to assume that a const method wont modify non-mutable 
members. While it is possible to use casts or other pointers to make changes, 
I'm not sure it is worth checking for these cases.



Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:409
@@ +408,3 @@
+  if (const CXXMethodDecl *D = cast_or_null(getDecl())) {
+if(D->getCanonicalDecl()->isConst()) {
+  // Check if the containing class/struct has mutable members

xazax.hun wrote:
> Do we need to get the cannonical decl here? When one declaration is const, 
> all of them supposed to be const.
I was getting a strange case where the PR21606 test didn't work without 
getCanonical, but when testing it now it seems to be fine. Perhaps something 
was fixed elsewhere? Regardless I'll remove the call. Thanks.


Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:412
@@ +411,3 @@
+  const MemRegion *ThisRegion = getCXXThisVal().getAsRegion();
+  if (ThisRegion) {
+MemRegionManager *MemMgr = ThisRegion->getMemRegionManager();

xazax.hun wrote:
> Is it possible to fail to get ThisRegion? Should this be an assert?
I put the test in because getCXXThisVal can return an UnknownVal, but on closer 
inspection I don't think this will ever happen for a CXXMemberCall, so I will 
change this to an assert. Thanks.


Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:415
@@ +414,3 @@
+const CXXRecordDecl *Parent = D->getParent();
+for (const auto *I : Parent->fields()) {
+  if (I->isMutable()) {

xazax.hun wrote:
> What about the mutable fields of base classes? Are they covered here?
Good point, I don't think they are so I'll work on that.


http://reviews.llvm.org/D13099



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

Re: [PATCH] D13099: [Analyzer] Don’t invalidate CXXThis when conservatively evaluating const methods (PR 21606)

2015-09-24 Thread Sean Eveson via cfe-commits

seaneveson updated this revision to Diff 35617.
seaneveson added a comment.

Removed unnecessary call to getCanonical.
Changed if statement checking getting ThisRegion to an assert.
Added code to handle mutable members of base classes.


http://reviews.llvm.org/D13099

Files:
  lib/StaticAnalyzer/Core/CallEvent.cpp
  test/Analysis/PR21606.cpp
  test/Analysis/method-call.cpp

Index: test/Analysis/method-call.cpp
===
--- test/Analysis/method-call.cpp
+++ test/Analysis/method-call.cpp
@@ -13,6 +13,25 @@
   int x;
 };
 
+struct C {
+  int x;
+  void foo() const;
+  void bar();
+};
+
+struct D {
+  mutable int x;
+  void foo() const;
+};
+
+struct Base {
+  mutable int x;
+};
+
+struct Derived : Base {
+  void foo() const;
+};
+
 void testNullObject(A *a) {
   clang_analyzer_eval(a); // expected-warning{{UNKNOWN}}
   (void)a->getx(); // assume we know what we're doing
@@ -45,3 +64,27 @@
   B t2(t);
   clang_analyzer_eval(t.x == 0); // expected-warning{{TRUE}}
 }
+
+void checkThatConstMethodWithoutDefinitionDoesNotInvalidateObject() {
+  C t;
+  t.x = 3;
+  t.foo();
+  clang_analyzer_eval(t.x == 3); // expected-warning{{TRUE}}
+  // Test non-const does invalidate
+  t.bar();
+  clang_analyzer_eval(t.x); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidateMutableFields() {
+  D t;
+  t.x = 3;
+  t.foo();
+  clang_analyzer_eval(t.x); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidateInheritedMutableFields() {
+  Derived t;
+  t.x = 3;
+  t.foo();
+  clang_analyzer_eval(t.x); // expected-warning{{UNKNOWN}}
+}
Index: test/Analysis/PR21606.cpp
===
--- test/Analysis/PR21606.cpp
+++ test/Analysis/PR21606.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core
+// PR21606
+
+struct s1 {
+void g(const int *i) const;
+};
+
+struct s2 {
+void f(int *i) {
+m_i = i;
+m_s.g(m_i);
+if (m_i)
+*i = 42; // no-warning
+}
+
+int *m_i;
+s1 m_s;
+};
+
+int main()
+{
+s2().f(0);
+}
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -403,7 +403,40 @@
   return getSVal(CE->getCallee()).getAsFunctionDecl();
 }
 
+/// Get all mutable fields of Record and its base classes.
+static void getMutableFields(const CXXRecordDecl *Record,
+ SmallVector ) {
+  if (Record == nullptr)
+return;
+  for (auto F : Record->fields()) {
+if (F->isMutable())
+  MutFields.push_back(F);
+  }
+  for (auto C : Record->bases()) {
+getMutableFields(C.getType()->getAsCXXRecordDecl(), MutFields);
+  }
+}
+
 void CXXInstanceCall::getExtraInvalidatedValues(ValueList ) const {
+  // Check if this is a call to a const method.
+  if (const CXXMethodDecl *D = cast_or_null(getDecl())) {
+if(D->isConst()) {
+  // Get any mutable members and invalidate them.
+  SmallVector MutableFields;
+  getMutableFields(D->getParent(), MutableFields);
+  if (!MutableFields.empty()) {
+const MemRegion *ThisRegion = getCXXThisVal().getAsRegion();
+assert(ThisRegion && "CXXThisVal was not a region");
+MemRegionManager *MemMgr = ThisRegion->getMemRegionManager();
+for (auto it = MutableFields.begin(), end = MutableFields.end();
+ it != end; ++it) {
+  const FieldRegion *FR = MemMgr->getFieldRegion(*it, ThisRegion);
+  Values.push_back(loc::MemRegionVal(FR));
+}
+  }
+  return;
+}
+  }
   Values.push_back(getCXXThisVal());
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12358: [Analyzer] Handling constant bound loops

2015-09-23 Thread Sean Eveson via cfe-commits

seaneveson added a comment.

Thank you for your comments.

@zaks.anna wrote:

> What do you mean by "approximate"?

I think @dcoughlin gave a perfect example in the following comment:

@dcoughlin wrote:

> This doesn't seem quite right. Consider:
>
>   int i;
>   for (i = 0; i < 21; i += 3) {}
>   clang_analyzer_eval(i == 23);
>
>
> The value of i should be 21 after the loop, but this code sets it to 23. And 
> what happens if i starts at 1 instead of 0?

The patch only examines the loop condition to avoid having to consider any of 
the loop body. It approximates an exit value for the variable in the condition 
by evaluating an iteration with the variable at its maximum/minimum value. This 
is loosely based on the widening and narrowing described in this paper: 
http://dl.acm.org/citation.cfm?id=512973

@zaks.anna wrote:

> Note that people rely on the analyzer to track the exact values of variables. 
> They expect it to know what the value is. It can be very confusing if the 
> analyzer states that it knows the value of the variable and it is the wrong 
> value.
>
> I am concerned with changing the value of a single variable of the loop based 
> on a syntactic check. Also, I am not sure there is a strong need for 
> preserving the value of the index variable either.

Since the goal of this patch is to get the analyzer to move past loops, I see 
no problem with invalidating the index variable. That said, I still think it is 
worth doing the approximation, as it identifies infinite loops.

Further work could then be done to set the value of the index variable after 
'simple' loops, where the analyzer can be sure of the value. In addition to 
this, the detection of infinite loops could be improved to work for any loop. 
This would allow the widening to be applied to any non-infinite, non-exiting 
loop.

Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:1616
@@ +1615,3 @@
+builder.isFeasible(false) && !StFalse && 
+BldCtx.blockCount() == AMgr.options.maxBlockVisitOnPath - 1) {
+

zaks.anna wrote:
> Do we loose precision for loops that need to be executed exactly 
> maxBlockVisitOnPath times?
Yes.

To be precise, the loss is for loops that need to be executed 
(maxBlockVisitOnPath - 1) times, because processCFGBlockEntrance generates a 
sink if blockCount >= maxBlockVisitOnPath.

With the default value for maxBlockVisitOnPath, a loop that iterated three 
times would be fully analyzed with widen=false, but would be unnecessarily 
widened if widen=true.

This could be fixed by (effectively) incrementing the value of 
maxBlockVisitOnPath, when the widening was enabled.

e.g. replacing the following line from processCFGBlockEntrance:

```
if (nodeBuilder.getContext().blockCount() >= AMgr.options.maxBlockVisitOnPath) {
...
```
With something like:

```
int blockLimit = AMgr.options.maxBlockVisitOnPath + 
 (AMgr.options.shouldWidenConstantBoundLoops() ? 1 : 0);
if (nodeBuilder.getContext().blockCount() >= blockLimit) {
...
```

Does that seem resonable?

Comment at: test/Analysis/constant-bound-loops.c:174
@@ +173,3 @@
+
+clang_analyzer_eval(g_global); // expected-warning {{UNKNOWN}}
+clang_analyzer_eval(s_arg); // expected-warning {{UNKNOWN}}

zaks.anna wrote:
> I think we should extend this test case.
> Ex: what about heap, what about variables touched by the loop variables 
> declared in the loop's scope?
Good point. I actually encountered a false positive while improving this case.

```
int *h_ptr = malloc(sizeof(int));
*h_ptr = 3;
for (int i = 0; i < 10; ++i) {} // WARNING: Potential leak of memory pointed to 
by 'h_ptr'
```

The value of h_ptr is invalidated, but I'm not sure about *h_ptr. Is it likely 
this warning is produced because the pointer is invalidated, but not the memory?

http://reviews.llvm.org/D12358

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

[PATCH] D13099: [Analyzer] Don’t invalidate CXXThis when conservatively evaluating const methods (PR 21606)

2015-09-23 Thread Sean Eveson via cfe-commits

seaneveson created this revision.
seaneveson added a subscriber: cfe-commits.

Dear All,

I would like to propose a patch that prevents the invalidation of ‘this’ when a 
method is const; fixing the test case given below, taken from PR 21606 
(https://llvm.org/bugs/show_bug.cgi?id=21606). 

```
struct s1 {
void g(const int *i) const;
};

struct s2 {
void f(int *i) {
m_i = i;
m_s.g(m_i); // Diagnostic goes away if you remove this line.
if (m_i)
*i = 42;
}
int *m_i;
s1 m_s;
};

int main() {
s2().f(0);
}
```

Mutable members of the object are a special case and are still invalidated; if 
a mutable member is invalidated the entire object will be invalidated because 
invalidateRegions invalidates the base region.

Whilst the patch fixes the test case from PR 21606, the same false-positive 
occurs when the method ‘s1::g’ isn’t const;  i.e. when ‘s2::f’ is called, 
subsequently calling ‘s1::g’, the memory region for the instance of s1 is 
(correctly) invalidated.  However, the containing memory region (the instance 
of s2) is also invalidated, which I think is overly conservative.  

Why is the base region (in this case: S2) invalidated? Would it be acceptable 
to change invalidation to modify the given region and not the base region when
  # invalidating only the mutable members for a const method call? 
  # invalidating an object as a result of conservative ‘method call’ 
evaluations?

Regards,

Sean Eveson
SN Systems - Sony Computer Entertainment Group

http://reviews.llvm.org/D13099

Files:
  lib/StaticAnalyzer/Core/CallEvent.cpp
  test/Analysis/PR21606.cpp
  test/Analysis/method-call.cpp

Index: test/Analysis/method-call.cpp
===
--- test/Analysis/method-call.cpp
+++ test/Analysis/method-call.cpp
@@ -13,6 +13,17 @@
   int x;
 };
 
+struct C {
+  int x;
+  void foo() const;
+  void bar();
+};
+
+struct D {
+  mutable int x;
+  void foo() const;
+};
+
 void testNullObject(A *a) {
   clang_analyzer_eval(a); // expected-warning{{UNKNOWN}}
   (void)a->getx(); // assume we know what we're doing
@@ -45,3 +56,21 @@
   B t2(t);
   clang_analyzer_eval(t.x == 0); // expected-warning{{TRUE}}
 }
+
+void checkThatConstMethodWithoutDefinitionDoesNotInvalidateObject() {
+  C t;
+  t.x = 3;
+  t.foo();
+  clang_analyzer_eval(t.x == 3); // expected-warning{{TRUE}}
+  // Test non-const does invalidate
+  t.bar();
+  clang_analyzer_eval(t.x); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidateMutableFields() {
+  D t;
+  t.x = 3;
+  t.foo();
+  clang_analyzer_eval(t.x); // expected-warning{{UNKNOWN}}
+}
+
Index: test/Analysis/PR21606.cpp
===
--- test/Analysis/PR21606.cpp
+++ test/Analysis/PR21606.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core
+// PR21606
+
+struct s1 {
+void g(const int *i) const;
+};
+
+struct s2 {
+void f(int *i) {
+m_i = i;
+m_s.g(m_i);
+if (m_i)
+*i = 42; // no-warning
+}
+
+int *m_i;
+s1 m_s;
+};
+
+int main()
+{
+s2().f(0);
+}
\ No newline at end of file
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -404,6 +404,24 @@
 }
 
 void CXXInstanceCall::getExtraInvalidatedValues(ValueList ) const {
+  // Check if this is a call to a const method.
+  if (const CXXMethodDecl *D = cast_or_null(getDecl())) {
+if(D->getCanonicalDecl()->isConst()) {
+  // Check if the containing class/struct has mutable members
+  const MemRegion *ThisRegion = getCXXThisVal().getAsRegion();
+  if (ThisRegion) {
+MemRegionManager *MemMgr = ThisRegion->getMemRegionManager();
+const CXXRecordDecl *Parent = D->getParent();
+for (const auto *I : Parent->fields()) {
+  if (I->isMutable()) {
+const FieldRegion *FR = MemMgr->getFieldRegion(I, ThisRegion);
+Values.push_back(loc::MemRegionVal(FR));
+  }
+}
+  }
+  return;
+}
+  }
   Values.push_back(getCXXThisVal());
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D12358: [Analyzer] Handling constant bound loops

2015-09-21 Thread Sean Eveson via cfe-commits

seaneveson updated this revision to Diff 35216.
seaneveson added a comment.

The TK_EntireMemSpace trait is now used when invalidating. The trait was added 
in http://reviews.llvm.org/D12993, thank you Devin for that patch.
Updated to the latest trunk.


http://reviews.llvm.org/D12358

Files:
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/CMakeLists.txt
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/LoopWidening.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/constant-bound-loops.c

Index: test/Analysis/constant-bound-loops.c
===
--- test/Analysis/constant-bound-loops.c
+++ test/Analysis/constant-bound-loops.c
@@ -0,0 +1,178 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-store=region -analyzer-max-loop 4 -analyzer-config widen-constant-bound-loops=true -verify %s
+
+extern void clang_analyzer_eval(_Bool);
+
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+
+void incr_for_loop() {
+int i;
+for (i = 0; i < 10; ++i) {}
+clang_analyzer_eval(i == 10); // expected-warning {{TRUE}}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void decr_for_loop() {
+int i;
+for (i = 10; i > 0; --i) {}
+clang_analyzer_eval(i == 0); // expected-warning {{TRUE}}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void incr_while_loop() {
+int i = 0;
+while (i < 10) {++i;}
+clang_analyzer_eval(i == 10); // expected-warning {{TRUE}}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void decr_while_loop() {
+int i = 10;
+while (i > 0) {--i;}
+clang_analyzer_eval(i == 0); // expected-warning {{TRUE}}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void incr_do_while_loop() {
+int i = 0;
+do {++i;} while (i < 10);
+clang_analyzer_eval(i == 10); // expected-warning {{TRUE}}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void decr_do_while_loop() {
+int i = 10;
+do {--i;} while (i > 0);
+clang_analyzer_eval(i == 0); // expected-warning {{TRUE}}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void negative_incr_for_loop() {
+int i;
+for (i = -10; i < 5 - 10; ++i) {}
+clang_analyzer_eval(i == -5); // expected-warning {{TRUE}}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void negative_decr_for_loop() {
+int i;
+for (i = 5 - 10; i > -20; --i) {}
+clang_analyzer_eval(i == -20); // expected-warning {{TRUE}}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void larger_incr_for_loop() {
+int i;
+for (i = 0; i < 20; i += 3) {}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void larger_decr_for_loop() {
+int i;
+for (i = 20; i > 0; i -= 3) {}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void unsigned_incr_for_loop() {
+unsigned i;
+for (i = 0; i < 10; ++i) {}
+clang_analyzer_eval(i == 10); // expected-warning {{TRUE}}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void unsigned_decr_for_loop() {
+unsigned i;
+for (i = 10; i > 0; --i) {}
+clang_analyzer_eval(i == 0); // expected-warning {{TRUE}}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void short_for_loop() {
+short i;
+for (i = 0; i < 10; ++i) {}
+clang_analyzer_eval(i == 10); // expected-warning {{TRUE}}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void lte_for_loop() {
+int i;
+for (i = 0; i <= 10; ++i) {}
+clang_analyzer_eval(i == 11); // expected-warning {{TRUE}}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void gte_for_loop() {
+int i;
+for (i = 10; i >= 0; --i) {}
+clang_analyzer_eval(i == -1); // expected-warning {{TRUE}}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void incr_ne_for_loop() {
+int i;
+for (i = 0; i != 10; ++i) {}
+clang_analyzer_eval(i == 10); // expected-warning {{TRUE}}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+

Re: [PATCH] D12358: [Analyzer] Handling constant bound loops

2015-09-11 Thread Sean Eveson via cfe-commits

seaneveson added a comment.

In http://reviews.llvm.org/D12358#241631, @cfe-commits wrote:

> Hi Sean,
>
> Ted provided more details off-list. He suspects that the problem is that we 
> likely don't add MemSpaceRegions to the worklist because every region is a 
> subregion of a MemSpaceRegion, and thus we would invalidate, by default, all 
> regions that were in the same MemSpace as the regions we were invalidating.  
> He thinks we want to not change that behavior, but also provide a way of 
> invalidating an entire MemSpace if so desired.  That's probably just a slight 
> tweak to the algorithm.
>
> I’ll take a look at your updated patch to reproduce what you are seeing and 
> investigate to see if that is what is going on.
>
> Thanks,
> Devin


Hi Devin,

That sounds good, thanks for looking at this.  Can you let me know when you 
will be able to look at this?  I am doing some work that builds on this patch's 
functionality and having this patch accepted would help.

Thanks,
Sean


http://reviews.llvm.org/D12358



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

Re: [PATCH] D12406: [Analyzer] Add -analyzer-config option for function size the inliner considers as large

2015-09-08 Thread Sean Eveson via cfe-commits

seaneveson updated this revision to Diff 34196.
seaneveson added a comment.

I changed the name to min-cfg-size-treat-functions-as-large, thanks for the 
suggestion.

If the patch is acceptable can someone commit it for me?


http://reviews.llvm.org/D12406

Files:
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp

Index: test/Analysis/analyzer-config.cpp
===
--- test/Analysis/analyzer-config.cpp
+++ test/Analysis/analyzer-config.cpp
@@ -1,8 +1,14 @@
-// RUN: %clang -target x86_64-apple-darwin10 --analyze %s -o /dev/null -Xclang -analyzer-checker=debug.ConfigDumper > %t 2>&1
+// RUN: %clang -target x86_64-apple-darwin10 --analyze %s -o /dev/null -Xclang -analyzer-checker=debug.ConfigDumper -Xclang -analyzer-max-loop -Xclang 34 > %t 2>&1
 // RUN: FileCheck --input-file=%t %s
 
 void bar() {}
-void foo() { bar(); }
+void foo() { 
+  // Call bar 33 times so max-times-inline-large is met and
+  // min-blocks-for-inline-large is checked
+  for (int i = 0; i < 34; ++i) {
+bar();
+  }
+}
 
 class Foo {
 public:
@@ -26,7 +32,8 @@
 // CHECK-NEXT: max-inlinable-size = 50
 // CHECK-NEXT: max-nodes = 15
 // CHECK-NEXT: max-times-inline-large = 32
+// CHECK-NEXT: min-cfg-size-treat-functions-as-large = 14
 // CHECK-NEXT: mode = deep
 // CHECK-NEXT: region-store-small-struct-limit = 2
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 17
+// CHECK-NEXT: num-entries = 18
Index: test/Analysis/analyzer-config.c
===
--- test/Analysis/analyzer-config.c
+++ test/Analysis/analyzer-config.c
@@ -1,8 +1,14 @@
-// RUN: %clang -target x86_64-apple-darwin10 --analyze %s -o /dev/null -Xclang -analyzer-checker=debug.ConfigDumper > %t 2>&1
+// RUN: %clang -target x86_64-apple-darwin10 --analyze %s -o /dev/null -Xclang -analyzer-checker=debug.ConfigDumper -Xclang -analyzer-max-loop -Xclang 34 > %t 2>&1
 // RUN: FileCheck --input-file=%t %s
 
 void bar() {}
-void foo() { bar(); }
+void foo() { 
+  // Call bar 33 times so max-times-inline-large is met and
+  // min-blocks-for-inline-large is checked
+  for (int i = 0; i < 34; ++i) {
+bar();
+  }
+}
 
 // CHECK: [config]
 // CHECK-NEXT: cfg-conditional-static-initializers = true
@@ -15,8 +21,9 @@
 // CHECK-NEXT: max-inlinable-size = 50
 // CHECK-NEXT: max-nodes = 15
 // CHECK-NEXT: max-times-inline-large = 32
+// CHECK-NEXT: min-cfg-size-treat-functions-as-large = 14
 // CHECK-NEXT: mode = deep
 // CHECK-NEXT: region-store-small-struct-limit = 2
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 12
+// CHECK-NEXT: num-entries = 13
 
Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -870,7 +870,8 @@
   // Do not inline large functions too many times.
   if ((Engine.FunctionSummaries->getNumTimesInlined(D) >
Opts.getMaxTimesInlineLarge()) &&
-  CalleeCFG->getNumBlockIDs() > 13) {
+   CalleeCFG->getNumBlockIDs() >=
+   Opts.getMinCFGSizeTreatFunctionsAsLarge()) {
 NumReachedInlineCountMax++;
 return false;
   }
Index: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===
--- lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -295,6 +295,13 @@
   return MaxTimesInlineLarge.getValue();
 }
 
+unsigned AnalyzerOptions::getMinCFGSizeTreatFunctionsAsLarge() {
+  if (!MinCFGSizeTreatFunctionsAsLarge.hasValue())
+MinCFGSizeTreatFunctionsAsLarge = getOptionAsInteger(
+  "min-cfg-size-treat-functions-as-large", 14);
+  return MinCFGSizeTreatFunctionsAsLarge.getValue();
+}
+
 unsigned AnalyzerOptions::getMaxNodesPerTopLevelFunction() {
   if (!MaxNodesPerTopLevelFunction.hasValue()) {
 int DefaultValue = 0;
Index: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
===
--- include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -253,6 +253,9 @@
   /// \sa getMaxTimesInlineLarge
   Optional MaxTimesInlineLarge;
 
+  /// \sa getMinCFGSizeTreatFunctionsAsLarge
+  Optional MinCFGSizeTreatFunctionsAsLarge;
+
   /// \sa getMaxNodesPerTopLevelFunction
   Optional MaxNodesPerTopLevelFunction;
 
@@ -502,6 +505,13 @@
   /// This is controlled by the 'max-times-inline-large' config option.
   unsigned getMaxTimesInlineLarge();
 
+  /// Returns the number of basic blocks a function needs to have to be
+  /// considered large for the 'max-times-inline-large' config option.
+  ///
+  /// This is controlled by the 'min-cfg-size-treat-functions-as-large'

Re: [PATCH] D12406: [Analyzer] Add -analyzer-config option for function size the inliner considers as large

2015-09-07 Thread Sean Eveson via cfe-commits

seaneveson added a comment.

Ping


http://reviews.llvm.org/D12406



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

Re: [PATCH] D12358: [Analyzer] Handling constant bound loops

2015-09-03 Thread Sean Eveson via cfe-commits

seaneveson updated this revision to Diff 33906.
seaneveson added a comment.

Refactored into a new file: LoopWidening.cpp (and LoopWidening.h).
Added an analyzer-config option, which defaults to false: 
widen-constant-bound-loops=false
Modified analyzer-config tests to check for the new option.
Added code to invalidate the stack locals, stack arguments and global regions 
(WIP).
Added a test to check variables are unknown after a widened loop, which 
currently fails.


http://reviews.llvm.org/D12358

Files:
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/CMakeLists.txt
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  lib/StaticAnalyzer/Core/LoopWidening.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp
  test/Analysis/constant-bound-loops.c

Index: test/Analysis/constant-bound-loops.c
===
--- test/Analysis/constant-bound-loops.c
+++ test/Analysis/constant-bound-loops.c
@@ -0,0 +1,178 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-store=region -analyzer-max-loop 4 -analyzer-config widen-constant-bound-loops=true -verify %s
+
+extern void clang_analyzer_eval(_Bool);
+
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+
+void incr_for_loop() {
+int i;
+for (i = 0; i < 10; ++i) {}
+clang_analyzer_eval(i == 10); // expected-warning {{TRUE}}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void decr_for_loop() {
+int i;
+for (i = 10; i > 0; --i) {}
+clang_analyzer_eval(i == 0); // expected-warning {{TRUE}}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void incr_while_loop() {
+int i = 0;
+while (i < 10) {++i;}
+clang_analyzer_eval(i == 10); // expected-warning {{TRUE}}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void decr_while_loop() {
+int i = 10;
+while (i > 0) {--i;}
+clang_analyzer_eval(i == 0); // expected-warning {{TRUE}}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void incr_do_while_loop() {
+int i = 0;
+do {++i;} while (i < 10);
+clang_analyzer_eval(i == 10); // expected-warning {{TRUE}}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void decr_do_while_loop() {
+int i = 10;
+do {--i;} while (i > 0);
+clang_analyzer_eval(i == 0); // expected-warning {{TRUE}}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void negative_incr_for_loop() {
+int i;
+for (i = -10; i < 5 - 10; ++i) {}
+clang_analyzer_eval(i == -5); // expected-warning {{TRUE}}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void negative_decr_for_loop() {
+int i;
+for (i = 5 - 10; i > -20; --i) {}
+clang_analyzer_eval(i == -20); // expected-warning {{TRUE}}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void larger_incr_for_loop() {
+int i;
+for (i = 0; i < 20; i += 3) {}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void larger_decr_for_loop() {
+int i;
+for (i = 20; i > 0; i -= 3) {}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void unsigned_incr_for_loop() {
+unsigned i;
+for (i = 0; i < 10; ++i) {}
+clang_analyzer_eval(i == 10); // expected-warning {{TRUE}}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void unsigned_decr_for_loop() {
+unsigned i;
+for (i = 10; i > 0; --i) {}
+clang_analyzer_eval(i == 0); // expected-warning {{TRUE}}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void short_for_loop() {
+short i;
+for (i = 0; i < 10; ++i) {}
+clang_analyzer_eval(i == 10); // expected-warning {{TRUE}}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void lte_for_loop() {
+int i;
+for (i = 0; i <= 10; ++i) {}
+clang_analyzer_eval(i == 11); // expected-warning {{TRUE}}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void gte_for_loop() {
+int i;
+for (i = 10; i >= 0; --i) {}
+clang_analyzer_eval(i == -1); // expected-warning {{TRUE}}
+char *m = (char*)malloc(12);
+} // expected-warning {{Potential leak of memory pointed to by 'm'}}
+
+void incr_ne_for_loop() {

Re: [PATCH] D12358: [Analyzer] Handling constant bound loops

2015-09-02 Thread Sean Eveson via cfe-commits

seaneveson added a comment.

In http://reviews.llvm.org/D12358#237099, @krememek wrote:

> To get the conservative invalidation that Anna suggests (actually, a little 
> less conservative), I think you can just pass in a two MemRegions as the 
> input to that method and get a new ProgramState with the appropriate regions 
> invalidated.


Thank you for the suggestion. Unfortunately nothing seems to get invalidated 
when I call invalidateRegions, in the following code:

  const StackFrameContext *STC = LCtx->getCurrentStackFrame();
  MemRegionManager  = svalBuilder.getRegionManager();
  const MemRegion *Regions[] = {
MRMgr.getStackLocalsRegion(STC),
MRMgr.getStackArgumentsRegion(STC),
MRMgr.getGlobalsRegion()
  };
  ProgramStateRef State;
  State = PrevState->invalidateRegions(Regions, Cond, BlockCount, LCtx, false);

Do you have any suggestions on what I have done wrong?

If there is no simple way to invalidate all three regions I will work on 
implementing something like the following:

In http://reviews.llvm.org/D12358#234975, @krememek wrote:

> A general scheme for widening, which can just be invalidation of anything 
> touched within a loop.  I think that can be done by having an algorithm that 
> does an AST walk, and looks at all variables whose lvalues are taken (used to 
> store values or otherwise get their address using '&') or anything 
> passed-by-reference to a function.  That set of VarDecls can be cached in a 
> side table, mapping ForStmt*'s (other loops as well) to the set of VarDecls 
> that are invalidated.  Those could then be passed to something that does the 
> invalidation using the currently invalidation logic we have in place.  The 
> invalidation logic should also handle checker state, which also needs to get 
> invalidated alongside variable values.



http://reviews.llvm.org/D12358



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

[PATCH] D12406: [Analyzer] Add -analyzer-config option for function size the inliner considers as large

2015-08-27 Thread Sean Eveson via cfe-commits

seaneveson created this revision.
seaneveson added a subscriber: cfe-commits.

Dear All,

I would like to propose a small patch to add an option (-analyzer-config 
min-blocks-for-inline-large=14) to control the function size the inliner 
considers as large, in relation to max-times-inline-large. In my patch the 
option defaults to the original hard coded behaviour, which I believe should be 
adjustable with the other inlining settings.

The analyzer-config test has been modified so that the analyzer will reach the 
getMinBlocksForInlineLarge() method and store the result in the ConfigTable, to 
ensure it is dumped by the debug checker.

Regards,

Sean Eveson
SN Systems - Sony Computer Entertainment Group


http://reviews.llvm.org/D12406

Files:
  include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  test/Analysis/analyzer-config.c
  test/Analysis/analyzer-config.cpp

Index: test/Analysis/analyzer-config.cpp
===
--- test/Analysis/analyzer-config.cpp
+++ test/Analysis/analyzer-config.cpp
@@ -1,8 +1,14 @@
-// RUN: %clang -target x86_64-apple-darwin10 --analyze %s -o /dev/null -Xclang -analyzer-checker=debug.ConfigDumper  %t 21
+// RUN: %clang -target x86_64-apple-darwin10 --analyze %s -o /dev/null -Xclang -analyzer-checker=debug.ConfigDumper -Xclang -analyzer-max-loop -Xclang 34  %t 21
 // RUN: FileCheck --input-file=%t %s
 
 void bar() {}
-void foo() { bar(); }
+void foo() { 
+  // Call bar 33 times so max-times-inline-large is met and
+  // min-blocks-for-inline-large is checked
+  for (int i = 0; i  34; ++i) {
+bar();
+  }
+}
 
 class Foo {
 public:
@@ -26,7 +32,8 @@
 // CHECK-NEXT: max-inlinable-size = 50
 // CHECK-NEXT: max-nodes = 15
 // CHECK-NEXT: max-times-inline-large = 32
+// CHECK-NEXT: min-blocks-for-inline-large = 14
 // CHECK-NEXT: mode = deep
 // CHECK-NEXT: region-store-small-struct-limit = 2
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 17
+// CHECK-NEXT: num-entries = 18
Index: test/Analysis/analyzer-config.c
===
--- test/Analysis/analyzer-config.c
+++ test/Analysis/analyzer-config.c
@@ -1,8 +1,14 @@
-// RUN: %clang -target x86_64-apple-darwin10 --analyze %s -o /dev/null -Xclang -analyzer-checker=debug.ConfigDumper  %t 21
+// RUN: %clang -target x86_64-apple-darwin10 --analyze %s -o /dev/null -Xclang -analyzer-checker=debug.ConfigDumper -Xclang -analyzer-max-loop -Xclang 34  %t 21
 // RUN: FileCheck --input-file=%t %s
 
 void bar() {}
-void foo() { bar(); }
+void foo() { 
+  // Call bar 33 times so max-times-inline-large is met and
+  // min-blocks-for-inline-large is checked
+  for (int i = 0; i  34; ++i) {
+bar();
+  }
+}
 
 // CHECK: [config]
 // CHECK-NEXT: cfg-conditional-static-initializers = true
@@ -15,8 +21,9 @@
 // CHECK-NEXT: max-inlinable-size = 50
 // CHECK-NEXT: max-nodes = 15
 // CHECK-NEXT: max-times-inline-large = 32
+// CHECK-NEXT: min-blocks-for-inline-large = 14
 // CHECK-NEXT: mode = deep
 // CHECK-NEXT: region-store-small-struct-limit = 2
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 12
+// CHECK-NEXT: num-entries = 13
 
Index: lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -870,7 +870,7 @@
   // Do not inline large functions too many times.
   if ((Engine.FunctionSummaries-getNumTimesInlined(D) 
Opts.getMaxTimesInlineLarge()) 
-  CalleeCFG-getNumBlockIDs()  13) {
+  CalleeCFG-getNumBlockIDs() = Opts.getMinBlocksForInlineLarge()) {
 NumReachedInlineCountMax++;
 return false;
   }
Index: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
===
--- lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -295,6 +295,13 @@
   return MaxTimesInlineLarge.getValue();
 }
 
+unsigned AnalyzerOptions::getMinBlocksForInlineLarge() {
+  if (!MinBlocksForInlineLarge.hasValue())
+MinBlocksForInlineLarge = getOptionAsInteger(min-blocks-for-inline-large,
+ 14);
+  return MinBlocksForInlineLarge.getValue();
+}
+
 unsigned AnalyzerOptions::getMaxNodesPerTopLevelFunction() {
   if (!MaxNodesPerTopLevelFunction.hasValue()) {
 int DefaultValue = 0;
Index: include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
===
--- include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -253,6 +253,9 @@
   /// \sa getMaxTimesInlineLarge
   Optionalunsigned MaxTimesInlineLarge;
 
+  /// \sa getMinBlocksForInlineLarge
+  Optionalunsigned MinBlocksForInlineLarge;
+
   /// \sa

Re: [PATCH] D12358: [Analyzer] Handling constant bound loops

2015-08-27 Thread Sean Eveson via cfe-commits

seaneveson added a comment.

In http://reviews.llvm.org/D12358#233983, @zaks.anna wrote:

 This will leave us in a state that is wrong and will likely lead to false 
 positives and inconsistencies, avoiding which is extremely important.


I accept that my current patch is not a comprehensive solution to the problem 
and that it may introduce false positives, however I do think it is an 
improvement, where it is preferable to have false positives over doing no 
analysis after the loop.

In http://reviews.llvm.org/D12358#234016, @krememek wrote:

 My recommendation is that we still unroll loops a couple times, getting full 
 precision, and then employ a widening technique like the ones being discussed 
 to allow the last loop iteration to act as the last one, but as a 
 conservative over-approximation.


In the patch, constant bound loops are unrolled with the “max-loop” setting 
(default of 4), (i = 0, 1, 2, 99) rather than (i = 0, 1, 2, 3); as such, we 
analyze the same number of paths through the loop body.

In my experience, constant bound loops are normally used to make simple 
modifications to fixed length collections of data, I think the behaviour of the 
majority of these loops will be represented by the first and last iteration.

In http://reviews.llvm.org/D12358#234016, @krememek wrote:

 Another, more principled hack, would be to look at all DeclRefExprs within a 
 loop and invalidate all memory in the cone-of-influence of those variables 
 (i.e., values they point to, etc.), but that's it.


Could this be done easily with a visitor and the existing invalidate methods? 
In cases where the anaylzer is unsure which values might be modified by the 
loop, it could fall back to invalidating all the values in the state.

In http://reviews.llvm.org/D12358#234016, @krememek wrote:

 Then there is the problem of called functions within the loop, as they won't 
 be analyzed. Those could interfere with the ability of a checker to do its 
 job.

 My recommendation is that we still unroll loops a couple times, getting full 
 precision, and then employ a widening technique like the ones being discussed 
 to allow the last loop iteration to act as the last one, but as a 
 conservative over-approximation.


Wouldn’t widening before the last iteration result in more paths being explored 
than additional unrolling? i.e. as a result of values in conditions being 
unknown.

Regards,

Sean Eveson
SN Systems - Sony Computer Entertainment Group


http://reviews.llvm.org/D12358



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

51 matches

Mail list logo