teemperor updated this revision to Diff 55189.
teemperor added a comment.
- Moved checks to the UninitializedFieldVisitor
- Also check for dynamic_cast on this during construction
http://reviews.llvm.org/D19312
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaDeclCXX.cpp
test/SemaCXX/ctor-init-with-dynamic-cast.cpp
test/SemaCXX/ctor-init-with-member-call-std.cpp
test/SemaCXX/ctor-init-with-member-call.cpp
Index: test/SemaCXX/ctor-init-with-member-call.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/ctor-init-with-member-call.cpp
@@ -0,0 +1,68 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -Wmember-call-in-ctor-init
+
+// Helper class for the following test cases.
+class A {
+public:
+ A(int i) {}
+};
+
+// Calling member functions before bass class initialized is undefined behavior.
+class B : public A {
+ int member_;
+
+public:
+ B()
+ : A(1 + get_i()) { // expected-warning {{member function call this->get_i() in ctor-initializer for base class 'A' results in undefined behavior}}
+ }
+ B(int var) : A(0), member_(get_i()) {} // no-warning
+
+ int get_i() { return 2; }
+};
+
+// Same as previous test but with explicit this.
+class C : public A {
+public:
+ C()
+ : A(this->get_i() + 1) { // expected-warning {{member function call this->get_i() in ctor-initializer for base class 'A' results in undefined behavior}}
+ }
+
+ int get_i() { return 2; }
+};
+
+// Check that the whole ctor-initializer is checked for member calls.
+class OtherA {
+public:
+ OtherA(int i) {}
+};
+
+class D : public OtherA, public A {
+public:
+ D()
+ : OtherA(this->get_i() + 1), A(this->get_i() + 1) { // expected-warning {{member function call this->get_i() in ctor-initializer for base class 'OtherA' results in undefined behavior}} \
+ // expected-warning {{member function call this->get_i() in ctor-initializer for base class 'A' results in undefined behavior}}
+ }
+
+ int get_i() { return 2; }
+};
+
+// Calling static functions of this class is not undefined behavior.
+class E : public A {
+public:
+ E() : A(this->get_i() + 1) { // no-warning
+ }
+
+ static int get_i() { return 2; }
+};
+
+// Calling other functions of this class is not undefined behavior.
+int other_foo() { return 2; }
+class F : public A {
+public:
+ F() : A(other_foo()) {} // no-warning
+};
+
+// Calling member functions of other classes is not undefined behavior.
+class G : public A {
+public:
+ G(B &b) : A(b.get_i()) {} // no-warning
+};
Index: test/SemaCXX/ctor-init-with-member-call-std.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/ctor-init-with-member-call-std.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -Wmember-call-in-ctor-init
+
+// Test UB in ctor-initializer with the example from [C++11 12.6.2 p13]
+class A {
+public:
+ A(int);
+};
+
+class B : public A {
+ int j;
+
+public:
+ int f();
+ B()
+ : A(f()), // expected-warning {{member function call this->f() in ctor-initializer for base class 'A' results in undefined behavior}}
+ j(f()) {} // no-warning
+};
+
+class C {
+public:
+ C(int);
+};
+class D : public B, C {
+ int i;
+
+public:
+ D()
+ : C(f()), // expected-warning {{member function call this->f() in ctor-initializer for base class 'C' results in undefined behavior}}
+ i(f()) {} // no-warning
+};
Index: test/SemaCXX/ctor-init-with-dynamic-cast.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/ctor-init-with-dynamic-cast.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11 -Wdynamic-cast-in-ctor-init
+
+// Helper class for the following test case.
+class A {
+public:
+ A(A *a) {}
+};
+
+// Calling dynamic cast on this before base class is initialized is undefined
+// behavior.
+class B : public A {
+
+ A *j;
+
+public:
+ B()
+ : A(dynamic_cast<A *>(this) + 1), j(nullptr) { // expected-warning {{dynamic_cast with this as operand in ctor-initializer for base class 'A' results in undefined behavior}}
+ }
+ B(int var) : A(nullptr), j(dynamic_cast<A *>(this)) {} // no-warning
+};
Index: lib/Sema/SemaDeclCXX.cpp
===================================================================
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -2290,6 +2290,9 @@
bool InitList;
FieldDecl *InitListFieldDecl;
llvm::SmallVector<unsigned, 4> InitFieldIndex;
+ // The base class that is initialized by the current CXXCtorInitializer
+ // or 0 if the current CXXCtorInitializer isn't initializing a base class.
+ const Type *CurrentBaseClass;
public:
typedef EvaluatedExprVisitor<UninitializedFieldVisitor> Inherited;
@@ -2488,6 +2491,7 @@
Constructor = FieldConstructor;
InitListExpr *ILE = dyn_cast<InitListExpr>(E);
+ CurrentBaseClass = BaseClass;
if (ILE && Field) {
InitList = true;
@@ -2534,7 +2538,54 @@
Inherited::VisitCXXConstructExpr(E);
}
+ void VisitCXXDynamicCastExpr(CXXDynamicCastExpr *E) {
+ // Calling dynamic_cast on the current object before
+ // all base classes are initialized is undefined behavior.
+ // [C++14/17 12.6.2 p16] [C++11 12.6.2 p13]
+
+ // Check that we are currently initializing a base class (which means
+ // that not all base classes are initialized at the moment).
+ if (CurrentBaseClass) {
+ // Simple check if the operand of the dynamic_cast is a this expression
+ // and therefore results in undefined behavior.
+ if (isa<CXXThisExpr>(E->getSubExpr())) {
+ S.Diag(E->getExprLoc(), diag::warn_dynamic_cast_in_ctor_init)
+ << CurrentBaseClass->getAsCXXRecordDecl();
+ }
+ }
+ Inherited::VisitCXXDynamicCastExpr(E);
+ }
+
void VisitCXXMemberCallExpr(CXXMemberCallExpr *E) {
+ // Calling a member function of the current object before
+ // all base classes are initialized is undefined behavior.
+ // [C++14/17 12.6.2 p16] [C++11 12.6.2 p13]
+
+ // Check that we are currently initializing a base class (which means
+ // that not all base classes are initialized at the moment).
+ if (CurrentBaseClass) {
+
+ // Calling a member function of the current object in this context
+ // is undefined behavior.
+ // As getImplicitObjectArgument() returns 0 for member pointer
+ // calls we use dyn_cast_or_null instead of isa.
+ if (dyn_cast_or_null<CXXThisExpr>(E->getImplicitObjectArgument())) {
+ S.Diag(E->getExprLoc(), diag::warn_member_call_in_ctor_init)
+ << E << CurrentBaseClass->getAsCXXRecordDecl();
+ }
+
+ // Same as above but taking the implicit cast when calling
+ // a member function of a parent class into account.
+ auto ImplicitCast =
+ dyn_cast_or_null<ImplicitCastExpr>(E->getImplicitObjectArgument());
+ if (ImplicitCast) {
+ if (isa<CXXThisExpr>(ImplicitCast->getSubExpr())) {
+ S.Diag(E->getExprLoc(), diag::warn_member_call_in_ctor_init)
+ << E << CurrentBaseClass->getAsCXXRecordDecl();
+ }
+ }
+ }
+
Expr *Callee = E->getCallee();
if (isa<MemberExpr>(Callee)) {
HandleValue(Callee, false /*AddressOf*/);
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -6778,6 +6778,13 @@
"taking address of function is not allowed">;
def err_invalid_conversion_between_vector_and_scalar : Error<
+def warn_member_call_in_ctor_init : Warning<
+ "member function call %0 in ctor-initializer for base class %1 results in undefined behavior">,
+ InGroup<DiagGroup<"member-call-in-ctor-init">>, DefaultIgnore;
+def warn_dynamic_cast_in_ctor_init : Warning<
+ "dynamic_cast with this as operand in ctor-initializer for base class %0 results in undefined behavior">,
+ InGroup<DiagGroup<"dynamic-cast-in-ctor-init">>, DefaultIgnore;
+
"invalid conversion between vector type %0 and scalar type %1">;
// C++ member initializers.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
