dkrupp updated this revision to Diff 227714.
dkrupp marked 2 inline comments as done.
dkrupp added a comment.
Thanks for your comments @NoQ
I fixed them. Also added your implementation hints to the open projects page.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69308/new/
https://reviews.llvm.org/D69308
Files:
clang/test/Analysis/clangsa_unsupported_features/handle_constructors_for_default_arguments.cpp
clang/test/Analysis/clangsa_unsupported_features/handle_constructors_with_new_array.cpp
clang/www/analyzer/open_projects.html
Index: clang/www/analyzer/open_projects.html
===================================================================
--- clang/www/analyzer/open_projects.html
+++ clang/www/analyzer/open_projects.html
@@ -95,6 +95,26 @@
We should model (potentially some of) such evaluations,
and the same applies for destructors called from
<code>operator delete[]</code>.
+ See tests cases in <a href="https://github.com/llvm/llvm-project/tree/master/clang/test/Analysis/clangsa_unsupported_features/handle_constructors_with_new_array.cpp";>handle_constructors_with_new_array.cpp</a>.
+ </p>
+ <p>
+ Hints for implementation by Artem Dergachev (NoQ):<br>
+ Operator new[] 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, you'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, you'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.
+ You might want to come up with an auxiliary analysis in order to perform
+ widening of these simple loops more precisely.
+
</p>
</li>
@@ -117,6 +137,25 @@
<p>Default arguments in C++ are recomputed at every call,
and are therefore local, and not static, variables.
</p>
+ See tests cases in <a href="https://github.com/llvm/llvm-project/tree/master/clang/test/Analysis/clangsa_unsupported_features/handle_constructors_for_default_arguments.cpp";>handle_constructors_for_default_arguments.cpp</a>.
+ <p>
+ Hints for implementation by Artem Dergachev (NoQ):<br>
+ 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).
+ </p>
</li>
<li>Enhance the modeling of the standard library.
Index: clang/test/Analysis/clangsa_unsupported_features/handle_constructors_with_new_array.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/clangsa_unsupported_features/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.
+ clang_analyzer_eval(a1[0].a == 1); // expected-warning {{TRUE}} expected-warning {{FALSE}}
+ // FIXME: Should be TRUE.
+ clang_analyzer_eval(a2[0].a == 1); // expected-warning {{TRUE}} expected-warning {{FALSE}}
+ // FIXME: Should be TRUE.
+ 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.
+ clang_analyzer_eval(a1[0].a == 1); // expected-warning {{TRUE}} expected-warning {{FALSE}}
+ // FIXME: Should be TRUE.
+ clang_analyzer_eval(a2[0].a == 1); // expected-warning {{TRUE}} expected-warning {{FALSE}}
+ // FIXME: Should be TRUE.
+ clang_analyzer_eval(a3[0].a == 1); // expected-warning {{TRUE}} expected-warning {{FALSE}}
+
+ delete[] a1;
+ delete[] a2;
+ delete[] a3;
+}
Index: clang/test/Analysis/clangsa_unsupported_features/handle_constructors_for_default_arguments.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/clangsa_unsupported_features/handle_constructors_for_default_arguments.cpp
@@ -0,0 +1,110 @@
+// 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 function
+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 analyze functions
+int called_f(init_with_list l = init_with_list()) {
+ return l.a;
+}
+
+int called_g(init_in_body l = init_in_body()) {
+ //We expect that the analyzer assumes the default value (call site test2)
+ return l.a;
+}
+
+int called_h(init_default_member l = init_default_member()) {
+ //We expect that the analyzer assumes the default value (call site test3)
+ return l.a;
+}
+
+int called_i(const init_with_list &l = init_with_list()){
+ //We expect that the analyzer assumes the default value
+ return l.a;
+}
+
+int called_j(init_with_list &&l = init_with_list()){
+ //We expect that the analyzer assumes the default value
+ 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}}
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
