[cfe-commits] r114153 - in /cfe/trunk: lib/CodeGen/CGClass.cpp test/CodeGenCXX/constructor-init.cpp

[John McCall]

Author: rjmccall
Date: Thu Sep 16 21:31:44 2010
New Revision: 114153

URL: http://llvm.org/viewvc/llvm-project?rev=114153&view=rev
Log:
Currently we're initializing the vtable pointers of a class only after
the bases are completely initialized.  This won't work --- base
initializer expressions can rely on the vtables having been set up.
Check for uses of 'this' in the initializers and force a vtable
initialization if found.

This might not be good enough;  we might need to extend this to handle
the possibility of arbitrary code finding an external reference to this
(not yet completely-constructed!) object and accessing through it,
in which case we'll probably find ourselves doing a lot more unnecessary
stores.


Modified:
    cfe/trunk/lib/CodeGen/CGClass.cpp
    cfe/trunk/test/CodeGenCXX/constructor-init.cpp

Modified: cfe/trunk/lib/CodeGen/CGClass.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=114153&r1=114152&r2=114153&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGClass.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGClass.cpp Thu Sep 16 21:31:44 2010
@@ -14,6 +14,7 @@
 #include "CGDebugInfo.h"
 #include "CodeGenFunction.h"
 #include "clang/AST/CXXInheritance.h"
+#include "clang/AST/EvaluatedExprVisitor.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/StmtCXX.h"
 
@@ -334,6 +335,29 @@
       CGF.EmitCXXDestructorCall(D, Dtor_Base, BaseIsVirtual, Addr);
     }
   };
+
+  /// A visitor which checks whether an initializer uses 'this' in a
+  /// way which requires the vtable to be properly set.
+  struct DynamicThisUseChecker : EvaluatedExprVisitor<DynamicThisUseChecker> {
+    typedef EvaluatedExprVisitor<DynamicThisUseChecker> super;
+
+    bool UsesThis;
+
+    DynamicThisUseChecker(ASTContext &C) : super(C), UsesThis(false) {}
+
+    // Black-list all explicit and implicit references to 'this'.
+    //
+    // Do we need to worry about external references to 'this' derived
+    // from arbitrary code?  If so, then anything which runs arbitrary
+    // external code might potentially access the vtable.
+    void VisitCXXThisExpr(CXXThisExpr *E) { UsesThis = true; }
+  };
+}
+
+static bool BaseInitializerUsesThis(ASTContext &C, const Expr *Init) {
+  DynamicThisUseChecker Checker(C);
+  Checker.Visit(const_cast<Expr*>(Init));
+  return Checker.UsesThis;
 }
 
 static void EmitBaseInitializer(CodeGenFunction &CGF, 
@@ -355,6 +379,12 @@
   if (CtorType == Ctor_Base && isBaseVirtual)
     return;
 
+  // If the initializer for the base (other than the constructor
+  // itself) accesses 'this' in any way, we need to initialize the
+  // vtables.
+  if (BaseInitializerUsesThis(CGF.getContext(), BaseInit->getInit()))
+    CGF.InitializeVTablePointers(ClassDecl);
+
   // We can pretend to be a complete class because it only matters for
   // virtual bases, and we only do virtual bases for complete ctors.
   llvm::Value *V = 

Modified: cfe/trunk/test/CodeGenCXX/constructor-init.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/constructor-init.cpp?rev=114153&r1=114152&r2=114153&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/constructor-init.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/constructor-init.cpp Thu Sep 16 21:31:44 2010
@@ -81,6 +81,38 @@
   // CHECK: ret void
 }
 
+// Make sure we initialize the vtable pointer if it's required by a
+// base initializer.
+namespace InitVTable {
+  struct A { A(int); };
+  struct B : A {
+    virtual int foo();
+    B();
+    B(int);
+  };
+
+  // CHECK: define void @_ZN10InitVTable1BC2Ev(
+  // CHECK:      [[T0:%.*]] = bitcast [[B:%.*]]* [[THIS:%.*]] to i8***
+  // CHECK-NEXT: store i8** getelementptr inbounds ([3 x i8*]* @_ZTVN10InitVTable1BE, i64 0, i64 2), i8*** [[T0]]
+  // CHECK:      [[VTBL:%.*]] = load i32 ([[B]]*)*** {{%.*}}
+  // CHECK-NEXT: [[FNP:%.*]] = getelementptr inbounds i32 ([[B]]*)** [[VTBL]], i64 0
+  // CHECK-NEXT: [[FN:%.*]] = load i32 ([[B]]*)** [[FNP]]
+  // CHECK-NEXT: [[ARG:%.*]] = call i32 [[FN]]([[B]]* [[THIS]])
+  // CHECK-NEXT: call void @_ZN10InitVTable1AC2Ei({{.*}}* %1, i32 [[ARG]])
+  // CHECK-NEXT: [[T0:%.*]] = bitcast [[B]]* [[THIS]] to i8***
+  // CHECK-NEXT: store i8** getelementptr inbounds ([3 x i8*]* @_ZTVN10InitVTable1BE, i64 0, i64 2), i8*** [[T0]]
+  // CHECK-NEXT: ret void
+  B::B() : A(foo()) {}
+
+  // CHECK: define void @_ZN10InitVTable1BC2Ei(
+  // CHECK:      [[ARG:%.*]] = add nsw i32 {{%.*}}, 5
+  // CHECK-NEXT: call void @_ZN10InitVTable1AC2Ei({{.*}}* {{%.*}}, i32 [[ARG]])
+  // CHECK-NEXT: [[T0:%.*]] = bitcast [[B]]* {{%.*}} to i8***
+  // CHECK-NEXT: store i8** getelementptr inbounds ([3 x i8*]* @_ZTVN10InitVTable1BE, i64 0, i64 2), i8*** [[T0]]
+  // CHECK-NEXT: ret void
+  B::B(int x) : A(x + 5) {}
+}
+
 template<typename T>
 struct X {
   X(const X &);