r321992 - [Driver] Add flag enabling the function stack size section that was added in r319430
Author: seaneveson
Date: Mon Jan 8 05:42:26 2018
New Revision: 321992
URL: http://llvm.org/viewvc/llvm-project?rev=321992&view=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&r1=321991&r2=321992&view=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&r1=321991&r2=321992&view=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&r1=321991&r2=321992&view=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&r1=321991&r2=321992&view=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 &C,
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&r1=321991&r2=321992&view=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.Uni
r321995 - Fix test added in r321992 failing on some buildbots.
Author: seaneveson
Date: Mon Jan 8 06:43:28 2018
New Revision: 321995
URL: http://llvm.org/viewvc/llvm-project?rev=321995&view=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&r1=321994&r2=321995&view=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
r322000 - Fix test added in r321992 failing on some buildbots (again), test requires x86.
Author: seaneveson Date: Mon Jan 8 07:46:18 2018 New Revision: 322000 URL: http://llvm.org/viewvc/llvm-project?rev=322000&view=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&r1=321999&r2=322000&view=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
Re: r319715 - Fix record-parsing-invocation.c test on Windows
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&view=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&r1=319714&r2=319715&view=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.
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] D12358: [Analyzer] Handling constant bound loops
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] D12358: [Analyzer] Handling constant bound loops
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'}}
+
+vo
Re: [PATCH] D12993: [analyzer] Add TK_EntireMemSpace invalidation trait.
seaneveson added a comment. Thank you for working on this! I have updated my patch (http://reviews.llvm.org/D12358) to depend on this one. 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
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)
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 &Values) 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] D13099: [Analyzer] Don’t invalidate CXXThis when conservatively evaluating const methods (PR 21606)
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)
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 &MutFields) {
+ 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 &Values) 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] D13099: [Analyzer] Don’t invalidate CXXThis when conservatively evaluating const methods (PR 21606)
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] D12358: [Analyzer] Handling constant bound loops
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] D12993: [analyzer] Add TK_EntireMemSpace invalidation trait.
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] D13099: [Analyzer] Don’t invalidate CXXThis when conservatively evaluating const methods (PR 21606)
seaneveson updated this revision to Diff 36600.
seaneveson added a comment.
Updated to latest revision.
Moved tests into their own file.
Added (RegionAndSymbolInvalidationTraits *ETraits) parameter to
CallEvent::getExtraInvalidatedValues and overrides.
Prevent invalidation by using TK_PreserveContents (Which also fixes the pointer
issue).
Simplified the patch so it falls back on invalidating the entire object if
there is a mutable field, since invalidating a single field invalidated the
entire object anyway.
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,94 @@
+// 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 {
+ mutable int mut;
+ void foo() const;
+};
+
+struct Outer;
+
+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 = &x;
+ t.foo();
+ clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidateInheritedMutableFields() {
+ Derived t;
+ t.b_mut = 4;
+ t.mut = 4;
+ t.foo();
+ clang_analyzer_eval(t.b_mut); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(t.mut); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidateInheritedPointedAtMemory() {
+ int x = 1;
+ Derived t;
+ t.p = &x;
+ t.foo();
+ clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidateContainedPointedAtMemory() {
+ int x = 1;
+ Outer t;
+ t.x = 2;
+ t.in.p = &x;
+ t.foo();
+ clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(t.x == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(t.in.p == &x); // 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, &ETraits);
// 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 &Values) const {
- Values.push_back(getCXXThisVal());
+void CXXInstanceCall::getExtraInvalidatedValues(ValueList &Values,
+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 &Values) const {
+void BlockCall::getExtraInvalidatedValues(ValueList &Values,
+ RegionAndSymbolInval
Re: [PATCH] D13099: [Analyzer] Don’t invalidate CXXThis when conservatively evaluating const methods (PR 21606)
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;
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)
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 = &x;
+ 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 = &x;
+ t.foo();
+ clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+}
+
+void checkThatConstMethodDoesInvalidateContainedPointedAtMemory() {
+ int x = 1;
+ Outer t;
+ t.x = 2;
+ t.in.p = &x;
+ t.foo();
+ clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(t.x == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(t.in.p == &x); // 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, &ETraits);
// 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 &Values) const {
- Values.push_back(getCXXThisVal());
+void CXXInstanceCall::getExtraInvalidatedValues(ValueList &Values,
+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 &Values) const {
+void BlockCall::getExtraInvalidatedValues(ValueList &Values,
+ 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 &Values) const {
+void CXXConstructorCall::getExtraInvalidatedValues(ValueList &Values,
+ RegionAndSymbolInvalidationTraits *ETraits) const {
if (Data)
Values.
Re: [PATCH] D13099: [Analyzer] Don’t invalidate CXXThis when conservatively evaluating const methods (PR 21606)
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] D12358: [Analyzer] Handling constant bound loops
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}}
+ free
Re: [PATCH] D12358: [Analyzer] Handling constant bound loops
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
[PATCH] D12358: [Analyzer] Handling constant bound loops
seaneveson created this revision.
seaneveson added a subscriber: cfe-commits.
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
http://reviews.llvm.org/D12358
Files:
lib/StaticAnalyzer/Core/ExprEngine.cpp
test/Analysis/constant-bound-loops.c
test/Analysis/coverage.c
Index: test/Analysis/coverage.c
===
--- test/Analysis/coverage.c
+++ test/Analysis/coverage.c
@@ -15,13 +15,15 @@
}
static void function_which_gives_up(int *x) {
- for (int i = 0; i < 5; ++i)
+ // i + 1 to prevent the analyzer approximating the last iteration
+ for (int i = 0; i + 1 < 6; ++i)
(*x)++;
}
static void function_which_gives_up_nested(int *x) {
function_which_gives_up(x);
- for (int i = 0; i < 5; ++i)
+ // i + 1 to prevent the analyzer approximating the last iteration
+ for (int i = 0; i + 1 < 6; ++i)
(*x)++;
}
@@ -55,7 +57,8 @@
} // expected-warning {{Potential leak of memory pointed to by 'm'}}
void coverage5(int *x) {
- for (int i = 0; i<7; ++i)
+ // i + 1 to prevent the analyzer approximating the last iteration
+ for (int i = 0; i + 1 < 8; ++i)
function_which_gives_up(x);
// The root function gives up here.
char *m = (char*)malloc(12); // no-warning
@@ -83,7 +86,8 @@
void function_which_gives_up_settonull(int **x) {
*x = 0;
int y = 0;
- for (int i = 0; i < 5; ++i)
+ // i + 1 to prevent the analyzer approximating the last iteration
+ for (int i = 0; i + 1 < 6; ++i)
y++;
}
Index: test/Analysis/constant-bound-loops.c
===
--- test/Analysis/constant-bound-loops.c
+++ test/Analysis/constant-bound-loops.c
@@ -0,0 +1,164 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-store=region -analyzer-max-loop 4 -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
[PATCH] D12406: [Analyzer] Add -analyzer-config option for function size the inliner considers as large
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 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-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 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-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
Optional MaxTimesInlineLarge;
+ /// \sa getMinBlocksForInlineLarge
+ Optional MinBlocksForInlineLarge;
+
/// \
Re: [PATCH] D12358: [Analyzer] Handling constant bound loops
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
Re: [PATCH] D12358: [Analyzer] Handling constant bound loops
seaneveson added a comment. In http://reviews.llvm.org/D12358#234949, @zaks.anna wrote: > We try to avoid false positives as much as possible. They are very painful > for users to deal with. > You should develop this feature behind a flag. This would allow for > incremental development and simplify evaluation. I absolutely want to minimize false positives, but I also want to make a start on just doing something after the loop, where the solution could then be improved upon. Developing behind a flag seems to be an ideal solution to me. In http://reviews.llvm.org/D12358#234975, @krememek wrote: > I think the functionality started here by doing something clever with loops > is complex enough to warrant putting this into a separate .cpp file. Good idea. In http://reviews.llvm.org/D12358#234986, @krememek wrote: > I do wonder if nested loops will cause a problem here Yes the patch only works with the inner most loop. A fix to this should ideally prevent inner loops being analyzed again from an identical state. I haven’t looked into if this will ‘just happen’ as a result of the framework, or how to do it otherwise. In http://reviews.llvm.org/D12358#234977, @krememek wrote: > I think we’d all be quite amendable for this work to go in under a flag, with > further improvements going in on top of that. That way we can all > collectively start hashing this out in trunk instead of feature creeping a > patch. If I refactor this patch in its current state into a separate file and put it behind a flag, would it then be acceptable? I would then like to take some time to look at the invalidation improvements as discussed in this thread. 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
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 &MRMgr = 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
Re: [PATCH] D12358: [Analyzer] Handling constant bound loops
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] D12406: [Analyzer] Add -analyzer-config option for function size the inliner considers as large
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] D12406: [Analyzer] Add -analyzer-config option for function size the inliner considers as large
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' con
Re: [PATCH] D13099: [Analyzer] Don’t invalidate CXXThis when conservatively evaluating const methods (PR 21606)
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 = &x;
+ t.foo();
+ clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(t.p == &x); // 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 = &x;
+ t.foo();
+ clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(t.p == &x); // expected-warning{{TRUE}}
+}
+
+void checkThatConstMethodDoesInvalidateContainedPointedAtMemory() {
+ int x = 1;
+ Outer t;
+ t.x = 2;
+ t.in.p = &x;
+ t.foo();
+ clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(t.x == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(t.in.p == &x); // 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 = &x;
+ t.foo();
+ clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(t.p == &x); // expected-warning{{TRUE}}
+}
+
+void checkThatInheritedConstMethodDoesInvalidateContainedPointedAtMemory() {
+ int x = 1;
+ DOuter t;
+ t.x = 2;
+ t.in.x = 3;
+ t.in.p = &x;
+ 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 == &x); // 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);
+
Re: [PATCH] D13099: [Analyzer] Don’t invalidate CXXThis when conservatively evaluating const methods (PR 21606)
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 = &x;
+ t.foo();
+ clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(t.p == &x); // 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 = &x;
+ t.foo();
+ clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(t.p == &x); // expected-warning{{TRUE}}
+}
+
+void checkThatConstMethodDoesInvalidateContainedPointedAtMemory() {
+ int x = 1;
+ Outer t;
+ t.x = 2;
+ t.in.p = &x;
+ t.foo();
+ clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(t.x == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(t.in.p == &x); // 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 = &x;
+ t.foo();
+ clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(t.p == &x); // expected-warning{{TRUE}}
+}
+
+void checkThatInheritedConstMethodDoesInvalidateContainedPointedAtMemory() {
+ int x = 1;
+ DOuter t;
+ t.x = 2;
+ t.in.x = 3;
+ t.in.p = &x;
+ 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 == &x); // 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 {
Re: [PATCH] D13099: [Analyzer] Don’t invalidate CXXThis when conservatively evaluating const methods (PR 21606)
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 = &x;
+ t.foo();
+ clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(t.p == &x); // 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 = &x;
+ t.foo();
+ clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(t.p == &x); // expected-warning{{TRUE}}
+}
+
+void checkThatConstMethodDoesInvalidateContainedPointedAtMemory() {
+ int x = 1;
+ Outer t;
+ t.x = 2;
+ t.in.p = &x;
+ t.foo();
+ clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(t.x == 2); // expected-warning{{TRUE}}
+ clang_analyzer_eval(t.in.p == &x); // 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 = &x;
+ t.foo();
+ clang_analyzer_eval(x); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(t.p == &x); // expected-warning{{TRUE}}
+}
+
+void checkThatInheritedConstMethodDoesInvalidateContainedPointedAtMemory() {
+ int x = 1;
+ DOuter t;
+ t.x = 2;
+ t.in.x = 3;
+ t.in.p = &x;
+ 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 == &x); // 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 b
Re: [PATCH] D12358: [Analyzer] Widening loops which do not exit
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 {{UNKNO
Re: [PATCH] D12358: [Analyzer] Widening loops which do not exit
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
r250500 - Test commit
Author: seaneveson
Date: Fri Oct 16 03:54:23 2015
New Revision: 250500
URL: http://llvm.org/viewvc/llvm-project?rev=250500&view=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&r1=250499&r2=250500&view=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
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
Re: [PATCH] D12358: [Analyzer] Widening loops which do not exit
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 variable_bound_exiting_loops_widened(int
Re: [PATCH] D12358: [Analyzer] Widening loops which do not exit
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
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
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&id=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); // exp
r251621 - [Analyzer] Widening loops which do not exit
Author: seaneveson
Date: Thu Oct 29 05:04:41 2015
New Revision: 251621
URL: http://llvm.org/viewvc/llvm-project?rev=251621&view=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&r1=251620&r2=251621&view=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&view=auto
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/LoopWidening.h
(added)
+++ cfe/trunk/include/clan
r251697 - Revert r251621 "[Analyzer] Widening loops which do not exit" (bot failure)
Author: seaneveson
Date: Fri Oct 30 06:13:07 2015
New Revision: 251697
URL: http://llvm.org/viewvc/llvm-project?rev=251697&view=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&r1=251696&r2=251697&view=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&view=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&r1=251696&r2=251697&view=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&r1=251696&r2=251697&view=diff
==
--- cfe/trunk/lib/Sta
r251702 - Reapply r251621 "[Analyzer] Widening loops which do not exit"
Author: seaneveson
Date: Fri Oct 30 10:23:57 2015
New Revision: 251702
URL: http://llvm.org/viewvc/llvm-project?rev=251702&view=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&r1=251701&r2=251702&view=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&r1=251701&r2=251702&view=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&r1=251701&r2=251702&view=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&r1=251701&r2=251702&view=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 &&
+ (is
Re: [PATCH] D14498: [Analyzer] Fix an assertion caused by r250237 (PR25392)
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
Re: [PATCH] D14498: [Analyzer] Fix an assertion caused by r250237 (PR25392)
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.
Author: seaneveson
Date: Tue Nov 10 05:48:55 2015
New Revision: 252599
URL: http://llvm.org/viewvc/llvm-project?rev=252599&view=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&r1=252598&r2=252599&view=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&r1=252598&r2=252599&view=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 &Values,
-RegionAndSymbolInvalidationTraits *ETraits) const {
+void CXXInstanceCall::getExtraInvalidatedValues(
+ValueList &Values, 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&r1=252598&r2=252599&view=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
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] D10305: [Clang Static Analyzer] Bug identification
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] D10305: [Clang Static Analyzer] Bug identification
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
r270849 - [Analyzer] Correct stack address escape diagnostic
Author: seaneveson
Date: Thu May 26 09:02:17 2016
New Revision: 270849
URL: http://llvm.org/viewvc/llvm-project?rev=270849&view=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&r1=270848&r2=270849&view=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&r1=270848&r2=270849&view=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 = &x;
b = &x;
-} // 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
Re: [PATCH] D19866: [Analyzer] Correct stack address escape diagnostic
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
[PATCH] D15888: [Analyzer] Change the default SA checkers for PS4
seaneveson created this revision.
seaneveson added reviewers: zaks.anna, dcoughlin.
seaneveson added a subscriber: cfe-commits.
This patch removes security.*, unix.API and unix.Vfork from the default
checkers for PS4.
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");
@@ -3601,13 +3607,15 @@
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=u
Re: [PATCH] D15888: [Analyzer] Change the default SA checkers for PS4
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
+// CHECK
r256926 - [Analyzer] Change the default SA checkers for PS4
Author: seaneveson
Date: Wed Jan 6 04:03:58 2016
New Revision: 256926
URL: http://llvm.org/viewvc/llvm-project?rev=256926&view=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&r1=256925&r2=256926&view=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 &C,
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 &C,
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&view=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] D10305: [Clang Static Analyzer] Bug identification
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] D14919: Fix IssueHash generation
seaneveson added a subscriber: seaneveson. seaneveson added a comment. > If the Decl *D pointer is nullptr the NormalizeLine would crash while getting > the LangOptions. Do you have a reproducible test case for this? 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] D14919: Fix IssueHash generation
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
