diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 7a529d6c98abf2c04c5ce3c4771a746da7656151..219d89f71b972835a7bc12fe4b08d0c779365028 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -1135,6 +1135,10 @@ def err_different_return_type_for_overriding_virtual_function : Error< "than the function it overrides}1,2">; def note_overridden_virtual_function : Note< "overridden virtual function is here">; +def err_conflicting_overriding_cc_attributes : Error< + "virtual function %0 has different calling convention attributes " + "%diff{($) than the function it overrides (which has calling convention $)|" + "than the function it overrides}1,2">; def err_covariant_return_inaccessible_base : Error< "invalid covariant return for virtual function: %1 is a " diff --git a/include/clang/Basic/TargetInfo.h b/include/clang/Basic/TargetInfo.h index c4094a6093b6cc4c3422e9ef582119da15071a5a..fbf2d30e4e28f0214581d9e1bc3607e21a37e6c6 100644 --- a/include/clang/Basic/TargetInfo.h +++ b/include/clang/Basic/TargetInfo.h @@ -742,13 +742,19 @@ public: bool isBigEndian() const { return BigEndian; } + enum CallingConvMethodType { + CCMT_Unknown, + CCMT_Member, + CCMT_NonMember + }; + /// \brief Gets the default calling convention for the given target and /// declaration context. - virtual CallingConv getDefaultCallingConv() const { + virtual CallingConv getDefaultCallingConv(CallingConvMethodType MT) const { // Not all targets will specify an explicit calling convention that we can // express. This will always do the right thing, even though it's not // an explicit calling convention. - return CC_Default; + return CC_C; } enum CallingConvCheckResult { diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index e6b4ac32aac61d8878966e03668809f60d70996e..16360e5e8cd8d183d72fdc922c6c37f4646fb714 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -2291,7 +2291,8 @@ public: void checkUnusedDeclAttributes(Declarator &D); bool CheckRegparmAttr(const AttributeList &attr, unsigned &value); - bool CheckCallingConvAttr(const AttributeList &attr, CallingConv &CC); + bool CheckCallingConvAttr(const AttributeList &attr, CallingConv &CC, + const FunctionDecl *FD = 0); bool CheckNoReturnAttr(const AttributeList &attr); /// \brief Stmt attributes - this routine is the top level dispatcher. @@ -4487,6 +4488,9 @@ public: std::string getAmbiguousPathsDisplayString(CXXBasePaths &Paths); + bool CheckOverridingFunctionAttributes(const CXXMethodDecl *New, + const CXXMethodDecl *Old); + /// CheckOverridingFunctionReturnType - Checks whether the return types are /// covariant, according to C++ [class.virtual]p5. bool CheckOverridingFunctionReturnType(const CXXMethodDecl *New, diff --git a/lib/Basic/Targets.cpp b/lib/Basic/Targets.cpp index 9cfa278824bfa94d9a09a202a3c9b391f3e52985..6d883c8a4b372263b3a20b6af4c044b9507a0ca7 100644 --- a/lib/Basic/Targets.cpp +++ b/lib/Basic/Targets.cpp @@ -1806,8 +1806,8 @@ public: CC == CC_X86Pascal) ? CCCR_OK : CCCR_Warning; } - virtual CallingConv getDefaultCallingConv() const { - return CC_C; + virtual CallingConv getDefaultCallingConv(CallingConvMethodType MT) const { + return MT == CCMT_Member ? CC_X86ThisCall : CC_C; } }; @@ -2896,8 +2896,8 @@ public: return TargetInfo::checkCallingConvention(CC); } - virtual CallingConv getDefaultCallingConv() const { - return CC_Default; + virtual CallingConv getDefaultCallingConv(CallingConvMethodType MT) const { + return CC_C; } }; diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index be300539ebd19c16c91917c9cb026afabaa6d558..aa0b41bc7a6c7ec1ff76603a8183c4218d095743 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -4846,6 +4846,7 @@ bool Sema::AddOverriddenMethods(CXXRecordDecl *DC, CXXMethodDecl *MD) { if (CXXMethodDecl *OldMD = dyn_cast<CXXMethodDecl>(*I)) { MD->addOverriddenMethod(OldMD->getCanonicalDecl()); if (!CheckOverridingFunctionReturnType(MD, OldMD) && + !CheckOverridingFunctionAttributes(MD, OldMD) && !CheckOverridingFunctionExceptionSpec(MD, OldMD) && !CheckIfOverriddenFunctionIsMarkedFinal(MD, OldMD)) { hasDeletedOverridenMethods |= OldMD->isDeleted(); diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp index e36ba2a4578481eebdc691a10822eecb29513570..502b358315a099d0f282b61d9498e056cda7c438 100644 --- a/lib/Sema/SemaDeclAttr.cpp +++ b/lib/Sema/SemaDeclAttr.cpp @@ -3557,10 +3557,11 @@ static void handleGNUInlineAttr(Sema &S, Decl *D, const AttributeList &Attr) { static void handleCallConvAttr(Sema &S, Decl *D, const AttributeList &Attr) { if (hasDeclarator(D)) return; + const FunctionDecl *FD = dyn_cast<FunctionDecl>(D); // Diagnostic is emitted elsewhere: here we store the (valid) Attr // in the Decl node for syntactic reasoning, e.g., pretty-printing. CallingConv CC; - if (S.CheckCallingConvAttr(Attr, CC)) + if (S.CheckCallingConvAttr(Attr, CC, FD)) return; if (!isa<ObjCMethodDecl>(D)) { @@ -3615,7 +3616,8 @@ static void handleOpenCLKernelAttr(Sema &S, Decl *D, const AttributeList &Attr){ D->addAttr(::new (S.Context) OpenCLKernelAttr(Attr.getRange(), S.Context)); } -bool Sema::CheckCallingConvAttr(const AttributeList &attr, CallingConv &CC) { +bool Sema::CheckCallingConvAttr(const AttributeList &attr, CallingConv &CC, + const FunctionDecl *FD) { if (attr.isInvalid()) return true; @@ -3665,7 +3667,12 @@ bool Sema::CheckCallingConvAttr(const AttributeList &attr, CallingConv &CC) { TargetInfo::CallingConvCheckResult A = TI.checkCallingConvention(CC); if (A == TargetInfo::CCCR_Warning) { Diag(attr.getLoc(), diag::warn_cconv_ignored) << attr.getName(); - CC = TI.getDefaultCallingConv(); + + TargetInfo::CallingConvMethodType MT = TargetInfo::CCMT_Unknown; + if (FD) + MT = FD->isCXXInstanceMember() ? TargetInfo::CCMT_Member : + TargetInfo::CCMT_NonMember; + CC = TI.getDefaultCallingConv(MT); } return false; diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index 6f1b489a2d28c6ef98e7ed3f4b7122445430869a..2fae8c9f91bb1811726f747446a5e4988adaa563 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -26,6 +26,7 @@ #include "clang/AST/TypeLoc.h" #include "clang/AST/TypeOrdering.h" #include "clang/Basic/PartialDiagnostic.h" +#include "clang/Basic/TargetInfo.h" #include "clang/Lex/Preprocessor.h" #include "clang/Sema/CXXFieldCollector.h" #include "clang/Sema/DeclSpec.h" @@ -10950,6 +10951,40 @@ void Sema::DiagnoseReturnInConstructorExceptionHandler(CXXTryStmt *TryBlock) { } } +bool Sema::CheckOverridingFunctionAttributes(const CXXMethodDecl *New, + const CXXMethodDecl *Old) { + const FunctionType *NewFT = New->getType()->getAs<FunctionType>(); + const FunctionType *OldFT = Old->getType()->getAs<FunctionType>(); + + CallingConv NewCC = NewFT->getCallConv(), OldCC = OldFT->getCallConv(); + + // If the calling conventions match, everything is fine + if (NewCC == OldCC) + return false; + + // If either of the calling conventions are set to "default", we need to pick + // something more sensible based on the target. This supports code where the + // one method explicitly sets thiscall, and another has no explicit calling + // convention. + CallingConv Default = + Context.getTargetInfo().getDefaultCallingConv(TargetInfo::CCMT_Member); + if (NewCC == CC_Default) + NewCC = Default; + if (OldCC == CC_Default) + OldCC = Default; + + // If the calling conventions still don't match, then report the error + if (NewCC != OldCC) { + Diag(New->getLocation(), + diag::err_conflicting_overriding_cc_attributes) + << New->getDeclName() << New->getType() << Old->getType(); + Diag(Old->getLocation(), diag::note_overridden_virtual_function); + return true; + } + + return false; +} + bool Sema::CheckOverridingFunctionReturnType(const CXXMethodDecl *New, const CXXMethodDecl *Old) { QualType NewTy = New->getType()->getAs<FunctionType>()->getResultType(); diff --git a/test/SemaCXX/virtual-override-x64.cpp b/test/SemaCXX/virtual-override-x64.cpp new file mode 100644 index 0000000000000000000000000000000000000000..8d5aad8889df686a9eae8ad15fe8c2e9f6ae5b2b --- /dev/null +++ b/test/SemaCXX/virtual-override-x64.cpp @@ -0,0 +1,36 @@ +// RUN: %clang_cc1 -triple=x86_64-pc-unknown -fsyntax-only -verify %s + +// Non-x86 targets ignore the calling conventions by default (but will warn +// when one is encountered), so we want to make sure the virtual overrides +// continue to work. +namespace PR14339 { + class A { + public: + virtual void __attribute__((thiscall)) f(); // expected-warning {{calling convention 'thiscall' ignored for this target}} + }; + + class B : public A { + public: + void __attribute__((cdecl)) f(); + }; + + class C : public A { + public: + void __attribute__((thiscall)) f(); // expected-warning {{calling convention 'thiscall' ignored for this target}} + }; + + class D : public A { + public: + void f(); + }; + + class E { + public: + virtual void __attribute__((stdcall)) g(); // expected-warning {{calling convention 'stdcall' ignored for this target}} + }; + + class F : public E { + public: + void g(); + }; +} diff --git a/test/SemaCXX/virtual-override-x86.cpp b/test/SemaCXX/virtual-override-x86.cpp new file mode 100644 index 0000000000000000000000000000000000000000..ad70d3f22437d68962c3f00277a2ea1e85a5c92f --- /dev/null +++ b/test/SemaCXX/virtual-override-x86.cpp @@ -0,0 +1,33 @@ +// RUN: %clang_cc1 -triple=i686-pc-unknown -fsyntax-only -verify %s -std=c++11 + +namespace PR14339 { + class A { + public: + virtual void __attribute__((thiscall)) f(); // expected-note{{overridden virtual function is here}} + }; + + class B : public A { + public: + void __attribute__((cdecl)) f(); // expected-error{{virtual function 'f' has different calling convention attributes ('void () __attribute__((cdecl))') than the function it overrides (which has calling convention 'void () __attribute__((thiscall))'}} + }; + + class C : public A { + public: + void __attribute__((thiscall)) f(); // This override is correct + }; + + class D : public A { + public: + void f(); // This override is correct because thiscall is the default calling convention for class members + }; + + class E { + public: + virtual void __attribute__((stdcall)) g(); // expected-note{{overridden virtual function is here}} + }; + + class F : public E { + public: + void g(); // expected-error{{virtual function 'g' has different calling convention attributes ('void ()') than the function it overrides (which has calling convention 'void () __attribute__((stdcall))'}} + }; +}