Author: Richard Smith Date: 2020-01-31T17:06:48-08:00 New Revision: aade5fbbfef3e8555df202082bea905deebc2ca5
URL: https://github.com/llvm/llvm-project/commit/aade5fbbfef3e8555df202082bea905deebc2ca5 DIFF: https://github.com/llvm/llvm-project/commit/aade5fbbfef3e8555df202082bea905deebc2ca5.diff LOG: Fix wrong devirtualization when the final overrider in one base class overrides the final overrider in a different base class. Added: Modified: clang/lib/AST/CXXInheritance.cpp clang/lib/AST/DeclCXX.cpp clang/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp Removed: ################################################################################ diff --git a/clang/lib/AST/CXXInheritance.cpp b/clang/lib/AST/CXXInheritance.cpp index a3a3794b2edd..0377bd324cb6 100644 --- a/clang/lib/AST/CXXInheritance.cpp +++ b/clang/lib/AST/CXXInheritance.cpp @@ -758,6 +758,8 @@ CXXRecordDecl::getFinalOverriders(CXXFinalOverriderMap &FinalOverriders) const { return false; }; + // FIXME: IsHidden reads from Overriding from the middle of a remove_if + // over the same sequence! Is this guaranteed to work? Overriding.erase( std::remove_if(Overriding.begin(), Overriding.end(), IsHidden), Overriding.end()); diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp index 48e310e858b2..227fe80ccab4 100644 --- a/clang/lib/AST/DeclCXX.cpp +++ b/clang/lib/AST/DeclCXX.cpp @@ -2038,17 +2038,36 @@ CXXMethodDecl::getCorrespondingMethodInClass(const CXXRecordDecl *RD, if (auto *MD = getCorrespondingMethodDeclaredInClass(RD, MayBeBase)) return MD; + llvm::SmallVector<CXXMethodDecl*, 4> FinalOverriders; + auto AddFinalOverrider = [&](CXXMethodDecl *D) { + // If this function is overridden by a candidate final overrider, it is not + // a final overrider. + for (CXXMethodDecl *OtherD : FinalOverriders) { + if (declaresSameEntity(D, OtherD) || recursivelyOverrides(OtherD, D)) + return; + } + + // Other candidate final overriders might be overridden by this function. + FinalOverriders.erase( + std::remove_if(FinalOverriders.begin(), FinalOverriders.end(), + [&](CXXMethodDecl *OtherD) { + return recursivelyOverrides(D, OtherD); + }), + FinalOverriders.end()); + + FinalOverriders.push_back(D); + }; + for (const auto &I : RD->bases()) { const RecordType *RT = I.getType()->getAs<RecordType>(); if (!RT) continue; const auto *Base = cast<CXXRecordDecl>(RT->getDecl()); - CXXMethodDecl *T = this->getCorrespondingMethodInClass(Base); - if (T) - return T; + if (CXXMethodDecl *D = this->getCorrespondingMethodInClass(Base)) + AddFinalOverrider(D); } - return nullptr; + return FinalOverriders.size() == 1 ? FinalOverriders.front() : nullptr; } CXXMethodDecl *CXXMethodDecl::Create(ASTContext &C, CXXRecordDecl *RD, @@ -2105,6 +2124,11 @@ CXXMethodDecl *CXXMethodDecl::getDevirtualizedMethod(const Expr *Base, CXXMethodDecl *DevirtualizedMethod = getCorrespondingMethodInClass(BestDynamicDecl); + // If there final overrider in the dynamic type is ambiguous, we can't + // devirtualize this call. + if (!DevirtualizedMethod) + return nullptr; + // If that method is pure virtual, we can't devirtualize. If this code is // reached, the result would be UB, not a direct call to the derived class // function, and we can't assume the derived class function is defined. diff --git a/clang/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp b/clang/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp index 130103de97ae..6f5e844b587e 100644 --- a/clang/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp +++ b/clang/test/CodeGenCXX/devirtualize-virtual-function-calls-final.cpp @@ -255,6 +255,49 @@ namespace Test10 { } } +namespace TestVBase { + struct A { virtual void f(); }; + struct B : virtual A {}; + struct C : virtual A { void f() override; }; + + extern struct BC final : B, C {} &bc; + extern struct BCusingA final : B, C { using A::f; } &bc_using_a; + extern struct BCusingB final : B, C { using B::f; } &bc_using_b; + extern struct BCusingC final : B, C { using C::f; } &bc_using_c; + + extern struct CB final : C, B {} &cb; + extern struct CBusingA final : C, B { using A::f; } &cb_using_a; + extern struct CBusingB final : C, B { using B::f; } &cb_using_b; + extern struct CBusingC final : C, B { using C::f; } &cb_using_c; + + // CHECK-LABEL: @_ZN9TestVBase4testEv( + void test() { + // FIXME: The 'using A' case can be devirtualized to call A's virtual + // adjustment thunk for C::f. + // FIXME: The 'using B' case can be devirtualized, but requires us to emit + // a derived-to-base or base-to-derived conversion as part of + // devirtualization. + + // CHECK: call void @_ZN9TestVBase1C1fEv( + bc.f(); + // CHECK: call void % + bc_using_a.f(); + // CHECK: call void % + bc_using_b.f(); + // CHECK: call void @_ZN9TestVBase1C1fEv( + bc_using_c.f(); + + // CHECK: call void @_ZN9TestVBase1C1fEv( + cb.f(); + // CHECK: call void % + cb_using_a.f(); + // CHECK: call void % + cb_using_b.f(); + // CHECK: call void @_ZN9TestVBase1C1fEv( + cb_using_c.f(); + } +} + namespace Test11 { // Check that the definitions of Derived's operators are emitted. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
