[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-commits
Subject:    Re: [PATCH] D19312: Warn about UB at member function calls from base class ctor initializers.
From:       Raphael Isemann via cfe-commits <cfe-commits () lists ! llvm ! org>
Date:       2016-04-30 14:51:24
Message-ID: 2820ae0a2ac3e63bed9512d9f5b50979 () localhost ! localdomain
[Download RAW message or body]

teemperor updated this revision to Diff 55716.
teemperor added a comment.

Merged all tests into one file.


http://reviews.llvm.org/D19312

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDeclCXX.cpp
  test/SemaCXX/warn-undefined-in-ctor.cpp


["D19312.55716.patch" (text/x-patch)]

Index: test/SemaCXX/warn-undefined-in-ctor.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/warn-undefined-in-ctor.cpp
@@ -0,0 +1,174 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s -Wundefined-call-from-ctor
+
+// Warn about member function calls
+namespace MemberFunctionCallsTests {
+
+  // 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(float var)
+        : A(sizeof(get_i())) { // no-warning
+    }
+    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
+  };
+}
+
+
+
+// Warn about member function calls - example code from [C++11 12.6.2 p13]
+namespace MemberFunctionCallsStdTests {
+
+  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 +  };
+
+}
+
+
+
+
+// Warn about dynamic_cast on this
+namespace DynamicCastTests {
+
+  // Helper class for the following test case.
+  class A {
+  public:
+    A(A *a) {}
+    A(unsigned long 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(float var)
+        : A(sizeof(dynamic_cast<A *>(this) + 1)), j(nullptr) { // no-warning
+    }
+    B(int var) : A(nullptr), j(dynamic_cast<A *>(this)) {} // no-warning
+  };
+}
+
+
+
+// Warn about typeid on this
+namespace std {
+  class type_info {
+  };
+}
+
+namespace TypeidTests {
+
+  class A {
+  public:
+    A(const std::type_info& a) {}
+    A(unsigned a) {}
+  };
+
+  // Calling dynamic cast on this before base class is initialized is undefined
+  // behavior.
+  class B : public A {
+
+    const std::type_info& j;
+
+  public:
+    B()
+        : A(typeid(this)), j(typeid(nullptr)) { // expected-warning {{typeid with \
this as operand in ctor-initializer for base class 'A' results in undefined \
behavior}} +    }
+    B(float var)
+        : A(sizeof(typeid(this))), j(typeid(nullptr)) { // no-warning
+    }
+    B(int var) : A(typeid(nullptr)), j(typeid(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,72 @@
       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 VisitCXXTypeidExpr(CXXTypeidExpr *E) {
+      // Calling typeid 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 typeid is a this expression
+        // and therefore results in undefined behavior.
+        if (!E->isTypeOperand() && isa<CXXThisExpr>(E->getExprOperand())) {
+          S.Diag(E->getExprLoc(), diag::warn_typeid_in_ctor_init)
+              << CurrentBaseClass->getAsCXXRecordDecl();
+        }
+      }
+      Inherited::VisitCXXTypeidExpr(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,19 @@
 
 def err_opencl_taking_function_address : Error<
   "taking address of function is not allowed">;
+def warn_member_call_in_ctor_init : Warning<
+  "member function call %0 in ctor-initializer for base class %1 results in "
+  "undefined behavior">,
+  InGroup<UndefinedCallFromCtor>, 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<UndefinedCallFromCtor>, DefaultIgnore;
+def warn_typeid_in_ctor_init : Warning<
+  "typeid with this as operand in ctor-initializer for base class %0 "
+  "results in undefined behavior">,
+  InGroup<UndefinedCallFromCtor>, DefaultIgnore;
+
 
 def err_invalid_conversion_between_vector_and_scalar : Error<
   "invalid conversion between vector type %0 and scalar type %1">;
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -408,6 +408,7 @@
 def InvalidOffsetof : DiagGroup<"invalid-offsetof">;
 def : DiagGroup<"strict-prototypes">;
 def StrictSelector : DiagGroup<"strict-selector-match">;
+def UndefinedCallFromCtor : DiagGroup<"undefined-call-from-ctor">;
 def MethodDuplicate : DiagGroup<"duplicate-method-match">;
 def ObjCCStringFormat : DiagGroup<"cstring-format-directive">;
 def CoveredSwitchDefault : DiagGroup<"covered-switch-default">;


[Attachment #4 (text/plain)]

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


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic