From 5e0fb648420702e47c94de53757928360a106e8c Mon Sep 17 00:00:00 2001
From: Artem Dergachev
Date: Thu, 7 Nov 2019 16:17:39 -0800
Subject: [PATCH] [analyzer] Add test cases for the unsupported C++ constructor
modeling.
Namely, for the following items:
- Handle constructors within new[];
- Handle constructors for default arguments.
Update the open projects page with a link to the newly added tests
and more hints for potential contributors.
Patch by Daniel Krupp!
Differential Revision: https://reviews.llvm.org/D69308
---
...dle_constructors_for_default_arguments.cpp | 116 ++++++++++++++++++
.../handle_constructors_with_new_array.cpp | 86 +++++++++++++
clang/www/analyzer/open_projects.html | 43 ++++++-
3 files changed, 242 insertions(+), 3 deletions(-)
create mode 100644 clang/test/Analysis/handle_constructors_for_default_arguments.cpp
create mode 100644 clang/test/Analysis/handle_constructors_with_new_array.cpp
diff --git a/clang/test/Analysis/handle_constructors_for_default_arguments.cpp b/clang/test/Analysis/handle_constructors_for_default_arguments.cpp
new file mode 100644
index 000000000000..c54d86526ec7
--- /dev/null
+++ b/clang/test/Analysis/handle_constructors_for_default_arguments.cpp
@@ -0,0 +1,116 @@
+// RUN: %clang_cc1 -fsyntax-only -analyze \
+// RUN: -analyzer-checker=core,debug.ExprInspection %s -verify
+
+// These test cases demonstrate lack of Static Analyzer features.
+// The FIXME: tags indicate where we expect different output.
+
+// Handle constructors for default arguments.
+// Default arguments in C++ are recomputed at every call,
+// and are therefore local, and not static, variables.
+void clang_analyzer_eval(bool);
+void clang_analyzer_warnIfReached();
+
+struct init_with_list {
+ int a;
+ init_with_list() : a(1) {}
+};
+
+struct init_in_body {
+ int a;
+ init_in_body() { a = 1; }
+};
+
+struct init_default_member {
+ int a = 1;
+};
+
+struct basic_struct {
+ int a;
+};
+
+// Top-level analyzed functions.
+void top_f(init_with_list l = init_with_list()) {
+ // We expect that the analyzer doesn't assume anything about the parameter.
+ clang_analyzer_eval(l.a == 1); // expected-warning {{TRUE}} expected-warning {{FALSE}}
+}
+
+void top_g(init_in_body l = init_in_body()) {
+ // We expect that the analyzer doesn't assume anything about the parameter.
+ clang_analyzer_eval(l.a == 1); // expected-warning {{TRUE}} expected-warning {{FALSE}}
+}
+
+void top_h(init_default_member l = init_default_member()) {
+ // We expect that the analyzer doesn't assume anything about the parameter.
+ clang_analyzer_eval(l.a == 1); // expected-warning {{TRUE}} expected-warning {{FALSE}}
+}
+
+// Not-top-level analyzed functions.
+int called_f(init_with_list l = init_with_list()) {
+ // We expect that the analyzer assumes the default value
+ // when called from test2().
+ return l.a;
+}
+
+int called_g(init_in_body l = init_in_body()) {
+ // We expect that the analyzer assumes the default value
+ // when called from test3().
+ return l.a;
+}
+
+int called_h(init_default_member l = init_default_member()) {
+ // We expect that the analyzer assumes the default value
+ // when called from test4().
+ return l.a;
+}
+
+int called_i(const init_with_list &l = init_with_list()){
+ // We expect that the analyzer assumes the default value
+ // when called from test5().
+ return l.a;
+}
+
+int called_j(init_with_list &&l = init_with_list()){
+ // We expect that the analyzer assumes the default value
+ // when called from test6().
+ return l.a;
+}
+
+int plain_parameter_passing(basic_struct l) {
+ return l.a;
+}
+
+void test1() {
+ basic_struct b;
+ b.a = 1;
+ clang_analyzer_eval(plain_parameter_passing(b) == 1); //expected-warning {{TRUE}}
+}
+
+void test2() {
+ // We expect that the analyzer assumes the default value.
+ // FIXME: Should be TRUE.
+ clang_analyzer_eval(called_f() == 1); //expected-warning {{TRUE}} expected-warning {{FALSE}}
+}
+
+void test3() {
+ // We expect that the analyzer assumes the default value.
+ // FIXME: Should be TRUE.
+ clang_analyzer_eval(called_g() == 1); //expected-warning {{TRUE}} expected-warning {{FALSE}}
+}
+
+void test4() {
+ // We expect that the analyzer assumes the default value.
+ // FIXME: Should be TRUE.
+ clang_analyzer_eval(called_h() == 1); //expected-warning {{TRUE}} expected-warning {{FALSE}}
+}
+
+void test5() {
+ //We expect that the analyzer assumes the default value.
+ // FIXME: Should be TRUE.
+ clang_analyzer_eval(called_i() == 1); //expected-warning {{TRUE}} expected-warning {{FALSE}}
+}
+
+void test6() {
+ // We expect that the analyzer assumes the default value.
+ // FIXME: Should be TRUE.
+ clang_analyzer_eval(called_j() == 1); //expected-warning {{TRUE}} expected-warning {{FALSE}}
+}
diff --git a/clang/test/Analysis/handle_constructors_with_new_array.cpp b/clang/test/Analysis/handle_constructors_with_new_array.cpp
new file mode 100644
index 000000000000..61637afce8d4
--- /dev/null
+++ b/clang/test/Analysis/handle_constructors_with_new_array.cpp
@@ -0,0 +1,86 @@
+// RUN: %clang_cc1 -fsyntax-only -analyze \
+// RUN: -analyzer-checker=core,debug.ExprInspection %s -verify
+
+// These test cases demonstrate lack of Static Analyzer features.
+// The FIXME: tags indicate where we expect different output.
+
+// Handle constructors within new[].
+
+// When an array of objects is allocated using the operator new[],
+// constructors for all elements of the array are called.
+// We should model (potentially some of) such evaluations,
+// and the same applies for destructors called from operator delete[].
+
+void clang_analyzer_eval(bool);
+
+struct init_with_list {
+ int a;
+ init_with_list() : a(1) {}
+};
+
+struct init_in_body {
+ int a;
+ init_in_body() { a = 1; }
+};
+
+struct init_default_member {
+ int a = 1;
+};
+
+void test_automatic() {
+
+ init_with_list a1;
+ init_in_body a2;
+ init_default_member a3;
+
+ clang_analyzer_eval(a1.a == 1); // expected-warning {{TRUE}}
+ clang_analyzer_eval(a2.a == 1); // expected-warning {{TRUE}}
+ clang_analyzer_eval(a3.a == 1); // expected-warning {{TRUE}}
+}
+
+void test_dynamic() {
+
+ auto *a1 = new init_with_list;
+ auto *a2 = new init_in_body;
+ auto *a3 = new init_default_member;
+
+ clang_analyzer_eval(a1->a == 1); // expected-warning {{TRUE}}
+ clang_analyzer_eval(a2->a == 1); // expected-warning {{TRUE}}
+ clang_analyzer_eval(a3->a == 1); // expected-warning {{TRUE}}
+
+ delete a1;
+ delete a2;
+ delete a3;
+}
+
+void test_automatic_aggregate() {
+
+ init_with_list a1[1];
+ init_in_body a2[1];
+ init_default_member a3[1];
+
+ // FIXME: Should be TRUE, not FALSE.
+ clang_analyzer_eval(a1[0].a == 1); // expected-warning {{TRUE}} expected-warning {{FALSE}}
+ // FIXME: Should be TRUE, not FALSE.
+ clang_analyzer_eval(a2[0].a == 1); // expected-warning {{TRUE}} expected-warning {{FALSE}}
+ // FIXME: Should be TRUE, not FALSE.
+ clang_analyzer_eval(a3[0].a == 1); // expected-warning {{TRUE}} expected-warning {{FALSE}}
+}
+
+void test_dynamic_aggregate() {
+
+ auto *a1 = new init_with_list[1];
+ auto *a2 = new init_in_body[1];
+ auto *a3 = new init_default_member[1];
+
+ // FIXME: Should be TRUE, not FALSE.
+ clang_analyzer_eval(a1[0].a == 1); // expected-warning {{TRUE}} expected-warning {{FALSE}}
+ // FIXME: Should be TRUE, not FALSE.
+ clang_analyzer_eval(a2[0].a == 1); // expected-warning {{TRUE}} expected-warning {{FALSE}}
+ // FIXME: Should be TRUE, not FALSE.
+ clang_analyzer_eval(a3[0].a == 1); // expected-warning {{TRUE}} expected-warning {{FALSE}}
+
+ delete[] a1;
+ delete[] a2;
+ delete[] a3;
+}
diff --git a/clang/www/analyzer/open_projects.html b/clang/www/analyzer/open_projects.html
index 46cc2b5c63f2..1f0c7a5fdac7 100644
--- a/clang/www/analyzer/open_projects.html
+++ b/clang/www/analyzer/open_projects.html
@@ -68,7 +68,7 @@ mailing list to notify other members of the community.
Improve C++ support
- - Handle aggregate construction.
+
- Handle construction as part of aggregate initialization.
Aggregates
are objects that can be brace-initialized without calling a
constructor (that is,
@@ -89,12 +89,31 @@ mailing list to notify other members of the community.
(Difficulty: Medium)
- - Handle constructors within
new[]
- When an array of objects is allocated using the operator new[]
,
+
- Handle array constructors.
+
When an array of objects is allocated (say, using the
+ operator new[]
or defining a stack array),
constructors for all elements of the array are called.
We should model (potentially some of) such evaluations,
and the same applies for destructors called from
operator delete[]
.
+ See tests cases in handle_constructors_with_new_array.cpp.
+
+
+ Constructing an array requires invoking multiple (potentially unknown)
+ amount of constructors with the same construct-expression.
+ Apart from the technical difficulties of juggling program points around
+ correctly to avoid accidentally merging paths together, we'll have to
+ be a judge on when to exit the loop and how to widen it.
+ Given that the constructor is going to be a default constructor,
+ a nice 95% solution might be to execute exactly one constructor and
+ then default-bind the resulting LazyCompoundVal to the whole array;
+ it'll work whenever the default constructor doesn't touch global state
+ but only initializes the object to various default values.
+ But if, say, we're making an array of strings,
+ depending on the implementation you might have to allocate a new buffer
+ for each string, and in this case default-binding won't cut it.
+ We might want to come up with an auxiliary analysis in order to perform
+ widening of these simple loops more precisely.
@@ -116,6 +135,24 @@ mailing list to notify other members of the community.
- Handle constructors for default arguments
Default arguments in C++ are recomputed at every call,
and are therefore local, and not static, variables.
+ See tests cases in handle_constructors_for_default_arguments.cpp.
+
+
+ Default arguments are annoying because the initializer expression is
+ evaluated at the call site but doesn't syntactically belong to the
+ caller's AST; instead it belongs to the ParmVarDecl for the default
+ parameter. This can lead to situations when the same expression has to
+ carry different values simultaneously -
+ when multiple instances of the same function are evaluated as part of the
+ same full-expression without specifying the default arguments.
+ Even simply calling the function twice (not necessarily within the
+ same full-expression) may lead to program points agglutinating because
+ it's the same expression. There are some nasty test cases already
+ in temporaries.cpp (struct DefaultParam and so on). I recommend adding a
+ new LocationContext kind specifically to deal with this problem. It'll
+ also help you figure out the construction context when you evaluate the
+ construct-expression (though you might still need to do some additional
+ CFG work to get construction contexts right).