[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
David Dean
david_dean at apple.com
Tue Jun 26 11:57:35 PDT 2012
I'm seeing a build failure when doing a bootstrapped debug+asserts build of clang after this commit:
llvm[1]: Building Intrinsics.gen.tmp from Intrinsics.td
make[1]: *** [/Users/buildslave/zorg/buildbot/smooshlab/slave-0.8/build.clang-x86_64-darwin10-DA/clang-build/lib/VMCore/Debug+Asserts/Intrinsics.gen.tmp] Segmentation fault
make: *** [all] Error 1
On 26 Jun 2012, at 10:45 AM, Rafael Espindola <rafael.espindola at gmail.com> wrote:
> 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();
> + }
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
-David
More information about the cfe-commits
mailing list