From 02df2e08720f26f93130c48ec7a14e82e86062fb Mon Sep 17 00:00:00 2001 From: Aaron Ballman Date: Sun, 9 Dec 2012 17:45:41 +0000 Subject: [PATCH] Virtual method overrides can no longer have mismatched calling conventions. This fixes PR14339. llvm-svn: 169705 --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 4 +++ clang/include/clang/Basic/TargetInfo.h | 10 +++++-- clang/include/clang/Sema/Sema.h | 6 +++- clang/lib/Basic/Targets.cpp | 8 +++--- clang/lib/Sema/SemaDecl.cpp | 1 + clang/lib/Sema/SemaDeclAttr.cpp | 13 +++++++-- clang/lib/Sema/SemaDeclCXX.cpp | 35 +++++++++++++++++++++++ clang/test/SemaCXX/virtual-override-x64.cpp | 36 ++++++++++++++++++++++++ clang/test/SemaCXX/virtual-override-x86.cpp | 33 ++++++++++++++++++++++ 9 files changed, 136 insertions(+), 10 deletions(-) create mode 100644 clang/test/SemaCXX/virtual-override-x64.cpp create mode 100644 clang/test/SemaCXX/virtual-override-x86.cpp diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 7a529d6..219d89f 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/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/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h index c4094a6..fbf2d30 100644 --- a/clang/include/clang/Basic/TargetInfo.h +++ b/clang/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/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index e6b4ac3..16360e5 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/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/clang/lib/Basic/Targets.cpp b/clang/lib/Basic/Targets.cpp index 9cfa278..6d883c8 100644 --- a/clang/lib/Basic/Targets.cpp +++ b/clang/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/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index be30053..aa0b41b 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -4846,6 +4846,7 @@ bool Sema::AddOverriddenMethods(CXXRecordDecl *DC, CXXMethodDecl *MD) { if (CXXMethodDecl *OldMD = dyn_cast(*I)) { MD->addOverriddenMethod(OldMD->getCanonicalDecl()); if (!CheckOverridingFunctionReturnType(MD, OldMD) && + !CheckOverridingFunctionAttributes(MD, OldMD) && !CheckOverridingFunctionExceptionSpec(MD, OldMD) && !CheckIfOverriddenFunctionIsMarkedFinal(MD, OldMD)) { hasDeletedOverridenMethods |= OldMD->isDeleted(); diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index e36ba2a..502b358 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/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(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(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/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 6f1b489..2fae8c9 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/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(); + const FunctionType *OldFT = Old->getType()->getAs(); + + 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()->getResultType(); diff --git a/clang/test/SemaCXX/virtual-override-x64.cpp b/clang/test/SemaCXX/virtual-override-x64.cpp new file mode 100644 index 0000000..8d5aad8 --- /dev/null +++ b/clang/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/clang/test/SemaCXX/virtual-override-x86.cpp b/clang/test/SemaCXX/virtual-override-x86.cpp new file mode 100644 index 0000000..ad70d3f --- /dev/null +++ b/clang/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))'}} + }; +} -- 2.7.4