[cfe-commits] r159212 - in /cfe/trunk: include/clang/AST/DeclCXX.h include/clang/AST/Expr.h lib/AST/DeclCXX.cpp lib/AST/Expr.cpp lib/CodeGen/CGExprCXX.cpp lib/Sema/SemaExpr.cpp test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp

Rafael Espindola rafael.espindola at gmail.com
Tue Jun 26 10:45:31 PDT 2012


Author: rafael
Date: Tue Jun 26 12:45:31 2012
New Revision: 159212

URL: http://llvm.org/viewvc/llvm-project?rev=159212&view=rev
Log:
During codegen of a virtual call we would extract any casts in the expression
to see if we had an underlying final class or method, but we would then
use the cast type to do the call, resulting in a direct call to the wrong
method.

Modified:
    cfe/trunk/include/clang/AST/DeclCXX.h
    cfe/trunk/include/clang/AST/Expr.h
    cfe/trunk/lib/AST/DeclCXX.cpp
    cfe/trunk/lib/AST/Expr.cpp
    cfe/trunk/lib/CodeGen/CGExprCXX.cpp
    cfe/trunk/lib/Sema/SemaExpr.cpp
    cfe/trunk/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp

Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=159212&r1=159211&r2=159212&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Tue Jun 26 12:45:31 2012
@@ -1630,7 +1630,20 @@
   /// supplied by IR generation to either forward to the function call operator
   /// or clone the function call operator.
   bool isLambdaStaticInvoker() const;
-  
+
+  /// \brief Find the method in RD that corresponds to this one.
+  ///
+  /// Find if RD or one of the classes it inherits from override this method.
+  /// If so, return it. RD is assumed to be a base class of the class defining
+  /// this method (or be the class itself).
+  CXXMethodDecl *
+  getCorrespondingMethodInClass(const CXXRecordDecl *RD);
+
+  const CXXMethodDecl *
+  getCorrespondingMethodInClass(const CXXRecordDecl *RD) const {
+    return const_cast<CXXMethodDecl*>(this)->getCorrespondingMethodInClass(RD);
+  }
+
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Decl *D) { return classofKind(D->getKind()); }
   static bool classof(const CXXMethodDecl *D) { return true; }

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=159212&r1=159211&r2=159212&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Tue Jun 26 12:45:31 2012
@@ -665,6 +665,13 @@
 
   static bool hasAnyTypeDependentArguments(llvm::ArrayRef<Expr *> Exprs);
 
+  /// \brief If we have class type (or pointer to class type), return the
+  /// class decl. Return NULL otherwise.
+  ///
+  /// If this expression is a cast, this method looks through it to find the
+  /// most derived decl that can be infered from the expression.
+  const CXXRecordDecl *getMostDerivedClassDeclForType() const;
+
   static bool classof(const Stmt *T) {
     return T->getStmtClass() >= firstExprConstant &&
            T->getStmtClass() <= lastExprConstant;

Modified: cfe/trunk/lib/AST/DeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=159212&r1=159211&r2=159212&view=diff
==============================================================================
--- cfe/trunk/lib/AST/DeclCXX.cpp (original)
+++ cfe/trunk/lib/AST/DeclCXX.cpp Tue Jun 26 12:45:31 2012
@@ -1269,6 +1269,55 @@
 
 void CXXMethodDecl::anchor() { }
 
+static bool recursivelyOverrides(const CXXMethodDecl *DerivedMD,
+                                 const CXXMethodDecl *BaseMD) {
+  for (CXXMethodDecl::method_iterator I = DerivedMD->begin_overridden_methods(),
+         E = DerivedMD->end_overridden_methods(); I != E; ++I) {
+    const CXXMethodDecl *MD = *I;
+    if (MD->getCanonicalDecl() == BaseMD->getCanonicalDecl())
+      return true;
+    if (recursivelyOverrides(MD, BaseMD))
+      return true;
+  }
+  return false;
+}
+
+CXXMethodDecl *
+CXXMethodDecl::getCorrespondingMethodInClass(const CXXRecordDecl *RD) {
+  if (this->getParent()->getCanonicalDecl() == RD->getCanonicalDecl())
+    return this;
+
+  // Lookup doesn't work for destructors, so handle them separately.
+  if (isa<CXXDestructorDecl>(this)) {
+    CXXMethodDecl *MD = RD->getDestructor();
+    if (recursivelyOverrides(MD, this))
+      return MD;
+    return NULL;
+  }
+
+  lookup_const_result Candidates = RD->lookup(getDeclName());
+  for (NamedDecl * const * I = Candidates.first; I != Candidates.second; ++I) {
+    CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(*I);
+    if (!MD)
+      continue;
+    if (recursivelyOverrides(MD, this))
+      return MD;
+  }
+
+  for (CXXRecordDecl::base_class_const_iterator I = RD->bases_begin(),
+         E = RD->bases_end(); I != E; ++I) {
+    const RecordType *RT = I->getType()->getAs<RecordType>();
+    if (!RT)
+      continue;
+    const CXXRecordDecl *Base = cast<CXXRecordDecl>(RT->getDecl());
+    CXXMethodDecl *T = this->getCorrespondingMethodInClass(Base);
+    if (T)
+      return T;
+  }
+
+  return NULL;
+}
+
 CXXMethodDecl *
 CXXMethodDecl::Create(ASTContext &C, CXXRecordDecl *RD,
                       SourceLocation StartLoc,

Modified: cfe/trunk/lib/AST/Expr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=159212&r1=159211&r2=159212&view=diff
==============================================================================
--- cfe/trunk/lib/AST/Expr.cpp (original)
+++ cfe/trunk/lib/AST/Expr.cpp Tue Jun 26 12:45:31 2012
@@ -33,6 +33,37 @@
 #include <cstring>
 using namespace clang;
 
+const CXXRecordDecl *Expr::getMostDerivedClassDeclForType() const {
+  const Expr *E = this;
+
+  while (true) {
+    E = E->IgnoreParens();
+    if (const CastExpr *CE = dyn_cast<CastExpr>(E)) {
+      if (CE->getCastKind() == CK_DerivedToBase ||
+          CE->getCastKind() == CK_UncheckedDerivedToBase ||
+          CE->getCastKind() == CK_NoOp) {
+        E = CE->getSubExpr();
+        continue;
+      }
+    }
+
+    break;
+  }
+
+  QualType DerivedType = E->getType();
+  if (DerivedType->isDependentType())
+    return NULL;
+  if (const PointerType *PTy = DerivedType->getAs<PointerType>())
+    DerivedType = PTy->getPointeeType();
+
+  const RecordType *Ty = DerivedType->castAs<RecordType>();
+  if (!Ty)
+    return NULL;
+
+  Decl *D = Ty->getDecl();
+  return cast<CXXRecordDecl>(D);
+}
+
 /// isKnownToHaveBooleanValue - Return true if this is an integer expression
 /// that is known to return 0 or 1.  This happens for _Bool/bool expressions
 /// but also int expressions which are produced by things like comparisons in

Modified: cfe/trunk/lib/CodeGen/CGExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprCXX.cpp?rev=159212&r1=159211&r2=159212&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGExprCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp Tue Jun 26 12:45:31 2012
@@ -56,30 +56,6 @@
                   Callee, ReturnValue, Args, MD);
 }
 
-static const CXXRecordDecl *getMostDerivedClassDecl(const Expr *Base) {
-  const Expr *E = Base;
-  
-  while (true) {
-    E = E->IgnoreParens();
-    if (const CastExpr *CE = dyn_cast<CastExpr>(E)) {
-      if (CE->getCastKind() == CK_DerivedToBase || 
-          CE->getCastKind() == CK_UncheckedDerivedToBase ||
-          CE->getCastKind() == CK_NoOp) {
-        E = CE->getSubExpr();
-        continue;
-      }
-    }
-
-    break;
-  }
-
-  QualType DerivedType = E->getType();
-  if (const PointerType *PTy = DerivedType->getAs<PointerType>())
-    DerivedType = PTy->getPointeeType();
-
-  return cast<CXXRecordDecl>(DerivedType->castAs<RecordType>()->getDecl());
-}
-
 // FIXME: Ideally Expr::IgnoreParenNoopCasts should do this, but it doesn't do
 // quite what we want.
 static const Expr *skipNoOpCastsAndParens(const Expr *E) {
@@ -126,7 +102,8 @@
   //   b->f();
   // }
   //
-  const CXXRecordDecl *MostDerivedClassDecl = getMostDerivedClassDecl(Base);
+  const CXXRecordDecl *MostDerivedClassDecl =
+    Base->getMostDerivedClassDeclForType();
   if (MostDerivedClassDecl->hasAttr<FinalAttr>())
     return true;
 
@@ -247,10 +224,13 @@
   //
   // We also don't emit a virtual call if the base expression has a record type
   // because then we know what the type is.
-  bool UseVirtualCall;
-  UseVirtualCall = MD->isVirtual() && !ME->hasQualifier()
-                   && !canDevirtualizeMemberFunctionCalls(getContext(),
-                                                          ME->getBase(), MD);
+  const Expr *Base = ME->getBase();
+  bool UseVirtualCall = MD->isVirtual() && !ME->hasQualifier()
+                        && !canDevirtualizeMemberFunctionCalls(getContext(),
+                                                               Base, MD);
+  const CXXRecordDecl *MostDerivedClassDecl =
+    Base->getMostDerivedClassDeclForType();
+
   llvm::Value *Callee;
   if (const CXXDestructorDecl *Dtor = dyn_cast<CXXDestructorDecl>(MD)) {
     if (UseVirtualCall) {
@@ -260,8 +240,13 @@
           MD->isVirtual() &&
           ME->hasQualifier())
         Callee = BuildAppleKextVirtualCall(MD, ME->getQualifier(), Ty);
-      else
-        Callee = CGM.GetAddrOfFunction(GlobalDecl(Dtor, Dtor_Complete), Ty);
+      else {
+        const CXXMethodDecl *DM =
+          Dtor->getCorrespondingMethodInClass(MostDerivedClassDecl);
+        assert(DM);
+        const CXXDestructorDecl *DDtor = cast<CXXDestructorDecl>(DM);
+        Callee = CGM.GetAddrOfFunction(GlobalDecl(DDtor, Dtor_Complete), Ty);
+      }
     }
   } else if (const CXXConstructorDecl *Ctor =
                dyn_cast<CXXConstructorDecl>(MD)) {
@@ -273,8 +258,12 @@
         MD->isVirtual() &&
         ME->hasQualifier())
       Callee = BuildAppleKextVirtualCall(MD, ME->getQualifier(), Ty);
-    else 
-      Callee = CGM.GetAddrOfFunction(MD, Ty);
+    else {
+      const CXXMethodDecl *DerivedMethod =
+        MD->getCorrespondingMethodInClass(MostDerivedClassDecl);
+      assert(DerivedMethod);
+      Callee = CGM.GetAddrOfFunction(DerivedMethod, Ty);
+    }
   }
 
   return EmitCXXMemberCall(MD, Callee, ReturnValue, This, /*VTT=*/0,

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=159212&r1=159211&r2=159212&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Jun 26 12:45:31 2012
@@ -10844,6 +10844,22 @@
   }
 
   SemaRef.MarkAnyDeclReferenced(Loc, D);
+
+  // If this is a call to a method via a cast, also mark the method in the
+  // derived class used in case codegen can devirtualize the call.
+  const MemberExpr *ME = dyn_cast<MemberExpr>(E);
+  if (!ME)
+    return;
+  CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(ME->getMemberDecl());
+  if (!MD)
+    return;
+  const Expr *Base = ME->getBase();
+  const CXXRecordDecl *MostDerivedClassDecl
+    = Base->getMostDerivedClassDeclForType();
+  if (!MostDerivedClassDecl)
+    return;
+  CXXMethodDecl *DM = MD->getCorrespondingMethodInClass(MostDerivedClassDecl);
+  SemaRef.MarkAnyDeclReferenced(Loc, DM);
 } 
 
 /// \brief Perform reference-marking and odr-use handling for a DeclRefExpr.

Modified: cfe/trunk/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp?rev=159212&r1=159211&r2=159212&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp Tue Jun 26 12:45:31 2012
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 %s -emit-llvm -o - | FileCheck %s
 
 namespace Test1 {
   struct A {
@@ -49,3 +49,61 @@
     return static_cast<B*>(v)->f();
   }
 }
+
+namespace Test4 {
+  struct A {
+    virtual void f();
+  };
+
+  struct B final : A {
+    virtual void f();
+  };
+
+  // CHECK: define void @_ZN5Test41fEPNS_1BE
+  void f(B* d) {
+    // CHECK: call void @_ZN5Test41B1fEv
+    static_cast<A*>(d)->f();
+  }
+}
+
+namespace Test5 {
+  struct A {
+    virtual void f();
+  };
+
+  struct B : A {
+    virtual void f();
+  };
+
+  struct C final : B {
+  };
+
+  // CHECK: define void @_ZN5Test51fEPNS_1CE
+  void f(C* d) {
+    // CHECK: call void @_ZN5Test51B1fEv
+    static_cast<A*>(d)->f();
+  }
+}
+
+namespace Test6 {
+  struct A {
+    virtual ~A();
+  };
+
+  struct B : public A {
+    virtual ~B();
+  };
+
+  struct C {
+    virtual ~C();
+  };
+
+  struct D final : public C, public B {
+  };
+
+  // CHECK: define void @_ZN5Test61fEPNS_1DE
+  void f(D* d) {
+    // CHECK: call void @_ZN5Test61DD1Ev
+    static_cast<A*>(d)->~A();
+  }
+}





More information about the cfe-commits mailing list