From 202433cccd87a4e7d1e277f80be6b179165bcb8d Mon Sep 17 00:00:00 2001 From: David Majnemer Date: Tue, 26 Jan 2016 19:30:26 +0000 Subject: [PATCH] [MS ABI] Allow a member pointers' converted type to change Member pointers in the MS ABI are tricky for a variety of reasons. The size of a member pointer is indeterminate until the program reaches a point where the representation is required to be known. However, *pointers* to member pointers may exist without knowing the pointee type's representation. In these cases, we synthesize an opaque LLVM type for the pointee type. However, we can be in a situation where the underlying member pointer's representation became known mid-way through the program. To account for this, we attempted to manicure CodeGen's type-cache so that we can replace the opaque member pointer type with the real deal while leaving the pointer types unperturbed. This, unfortunately, is a problematic approach to take as we will violate CodeGen's invariants. These violations are mostly harmless but let's do the right thing instead: invalidate the type-cache if a member pointer's LLVM representation changes. This fixes PR26313. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@258839 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/ASTConsumer.h | 4 ++++ include/clang/Frontend/MultiplexConsumer.h | 1 + lib/CodeGen/CodeGenAction.cpp | 4 ++++ lib/CodeGen/CodeGenModule.cpp | 5 +++++ lib/CodeGen/CodeGenModule.h | 2 ++ lib/CodeGen/CodeGenTypes.cpp | 22 +++++++++++++++---- lib/CodeGen/CodeGenTypes.h | 6 +++++ lib/CodeGen/ModuleBuilder.cpp | 7 ++++++ lib/Frontend/MultiplexConsumer.cpp | 5 +++++ lib/Sema/SemaDecl.cpp | 2 ++ lib/Sema/SemaDeclAttr.cpp | 5 ++++- lib/Sema/SemaType.cpp | 1 + .../microsoft-abi-member-pointers.cpp | 21 ++++++++++++++---- 13 files changed, 76 insertions(+), 9 deletions(-) diff --git a/include/clang/AST/ASTConsumer.h b/include/clang/AST/ASTConsumer.h index 02f64a66ef..7c0acbc6b3 100644 --- a/include/clang/AST/ASTConsumer.h +++ b/include/clang/AST/ASTConsumer.h @@ -121,6 +121,10 @@ public: /// modified by the introduction of an implicit zero initializer. virtual void CompleteTentativeDefinition(VarDecl *D) {} + /// \brief Callback invoked when an MSInheritanceAttr has been attached to a + /// CXXRecordDecl. + virtual void AssignInheritanceModel(CXXRecordDecl *RD) {} + /// HandleCXXStaticMemberVarInstantiation - Tell the consumer that this // variable has been instantiated. virtual void HandleCXXStaticMemberVarInstantiation(VarDecl *D) {} diff --git a/include/clang/Frontend/MultiplexConsumer.h b/include/clang/Frontend/MultiplexConsumer.h index 9782829338..f92e9bf6c5 100644 --- a/include/clang/Frontend/MultiplexConsumer.h +++ b/include/clang/Frontend/MultiplexConsumer.h @@ -49,6 +49,7 @@ public: llvm::StringRef Value) override; void HandleDependentLibrary(llvm::StringRef Lib) override; void CompleteTentativeDefinition(VarDecl *D) override; + void AssignInheritanceModel(CXXRecordDecl *RD) override; void HandleVTable(CXXRecordDecl *RD) override; ASTMutationListener *GetASTMutationListener() override; ASTDeserializationListener *GetASTDeserializationListener() override; diff --git a/lib/CodeGen/CodeGenAction.cpp b/lib/CodeGen/CodeGenAction.cpp index cdd9d09d04..f8ae031f0f 100644 --- a/lib/CodeGen/CodeGenAction.cpp +++ b/lib/CodeGen/CodeGenAction.cpp @@ -210,6 +210,10 @@ namespace clang { Gen->CompleteTentativeDefinition(D); } + void AssignInheritanceModel(CXXRecordDecl *RD) override { + Gen->AssignInheritanceModel(RD); + } + void HandleVTable(CXXRecordDecl *RD) override { Gen->HandleVTable(RD); } diff --git a/lib/CodeGen/CodeGenModule.cpp b/lib/CodeGen/CodeGenModule.cpp index ae46b12b80..be8171971b 100644 --- a/lib/CodeGen/CodeGenModule.cpp +++ b/lib/CodeGen/CodeGenModule.cpp @@ -489,6 +489,11 @@ void CodeGenModule::UpdateCompletedType(const TagDecl *TD) { Types.UpdateCompletedType(TD); } +void CodeGenModule::RefreshTypeCacheForClass(const CXXRecordDecl *RD) { + // Make sure that this type is translated. + Types.RefreshTypeCacheForClass(RD); +} + llvm::MDNode *CodeGenModule::getTBAAInfo(QualType QTy) { if (!TBAA) return nullptr; diff --git a/lib/CodeGen/CodeGenModule.h b/lib/CodeGen/CodeGenModule.h index 046bc2a9fe..74087be3f6 100644 --- a/lib/CodeGen/CodeGenModule.h +++ b/lib/CodeGen/CodeGenModule.h @@ -994,6 +994,8 @@ public: void EmitVTable(CXXRecordDecl *Class); + void RefreshTypeCacheForClass(const CXXRecordDecl *Class); + /// \brief Appends Opts to the "Linker Options" metadata value. void AppendLinkerOptions(StringRef Opts); diff --git a/lib/CodeGen/CodeGenTypes.cpp b/lib/CodeGen/CodeGenTypes.cpp index 09d9bf17b3..554f9ff640 100644 --- a/lib/CodeGen/CodeGenTypes.cpp +++ b/lib/CodeGen/CodeGenTypes.cpp @@ -272,6 +272,17 @@ void CodeGenTypes::UpdateCompletedType(const TagDecl *TD) { DI->completeType(RD); } +void CodeGenTypes::RefreshTypeCacheForClass(const CXXRecordDecl *RD) { + QualType T = Context.getRecordType(RD); + T = Context.getCanonicalType(T); + + const Type *Ty = T.getTypePtr(); + if (RecordsWithOpaqueMemberPointers.count(Ty)) { + TypeCache.clear(); + RecordsWithOpaqueMemberPointers.clear(); + } +} + static llvm::Type *getTypeForFormat(llvm::LLVMContext &VMContext, const llvm::fltSemantics &format, bool UseNativeHalf = false) { @@ -603,10 +614,13 @@ llvm::Type *CodeGenTypes::ConvertType(QualType T) { } case Type::MemberPointer: { - if (!getCXXABI().isMemberPointerConvertible(cast(Ty))) - return llvm::StructType::create(getLLVMContext()); - ResultType = - getCXXABI().ConvertMemberPointerType(cast(Ty)); + auto *MPTy = cast(Ty); + if (!getCXXABI().isMemberPointerConvertible(MPTy)) { + RecordsWithOpaqueMemberPointers.insert(MPTy->getClass()); + ResultType = llvm::StructType::create(getLLVMContext()); + } else { + ResultType = getCXXABI().ConvertMemberPointerType(MPTy); + } break; } diff --git a/lib/CodeGen/CodeGenTypes.h b/lib/CodeGen/CodeGenTypes.h index a96f23c448..a673fa6681 100644 --- a/lib/CodeGen/CodeGenTypes.h +++ b/lib/CodeGen/CodeGenTypes.h @@ -162,6 +162,8 @@ class CodeGenTypes { /// corresponding llvm::Type. llvm::DenseMap TypeCache; + llvm::SmallSet RecordsWithOpaqueMemberPointers; + public: CodeGenTypes(CodeGenModule &cgm); ~CodeGenTypes(); @@ -214,6 +216,10 @@ public: /// replace the 'opaque' type we previously made for it if applicable. void UpdateCompletedType(const TagDecl *TD); + /// \brief Remove stale types from the type cache when an inheritance model + /// gets assigned to a class. + void RefreshTypeCacheForClass(const CXXRecordDecl *RD); + /// getNullaryFunctionInfo - Get the function info for a void() /// function with standard CC. const CGFunctionInfo &arrangeNullaryFunction(); diff --git a/lib/CodeGen/ModuleBuilder.cpp b/lib/CodeGen/ModuleBuilder.cpp index bce19ab2fd..9428bdf1db 100644 --- a/lib/CodeGen/ModuleBuilder.cpp +++ b/lib/CodeGen/ModuleBuilder.cpp @@ -210,6 +210,13 @@ namespace { Builder->Release(); } + void AssignInheritanceModel(CXXRecordDecl *RD) override { + if (Diags.hasErrorOccurred()) + return; + + Builder->RefreshTypeCacheForClass(RD); + } + void CompleteTentativeDefinition(VarDecl *D) override { if (Diags.hasErrorOccurred()) return; diff --git a/lib/Frontend/MultiplexConsumer.cpp b/lib/Frontend/MultiplexConsumer.cpp index fdeb04f624..15f876b7bd 100644 --- a/lib/Frontend/MultiplexConsumer.cpp +++ b/lib/Frontend/MultiplexConsumer.cpp @@ -337,6 +337,11 @@ void MultiplexConsumer::CompleteTentativeDefinition(VarDecl *D) { Consumer->CompleteTentativeDefinition(D); } +void MultiplexConsumer::AssignInheritanceModel(CXXRecordDecl *RD) { + for (auto &Consumer : Consumers) + Consumer->AssignInheritanceModel(RD); +} + void MultiplexConsumer::HandleVTable(CXXRecordDecl *RD) { for (auto &Consumer : Consumers) Consumer->HandleVTable(RD); diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index 166bd8ca24..da2bec38e8 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -2255,6 +2255,8 @@ static bool mergeDeclAttribute(Sema &S, NamedDecl *D, if (NewAttr) { NewAttr->setInherited(true); D->addAttr(NewAttr); + if (isa(NewAttr)) + S.Consumer.AssignInheritanceModel(cast(D)); return true; } diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp index b0a9a009f0..b2169962a5 100644 --- a/lib/Sema/SemaDeclAttr.cpp +++ b/lib/Sema/SemaDeclAttr.cpp @@ -12,6 +12,7 @@ //===----------------------------------------------------------------------===// #include "clang/Sema/SemaInternal.h" +#include "clang/AST/ASTConsumer.h" #include "clang/AST/ASTContext.h" #include "clang/AST/CXXInheritance.h" #include "clang/AST/DeclCXX.h" @@ -4431,8 +4432,10 @@ static void handleMSInheritanceAttr(Sema &S, Decl *D, const AttributeList &Attr) D, Attr.getRange(), /*BestCase=*/true, Attr.getAttributeSpellingListIndex(), (MSInheritanceAttr::Spelling)Attr.getSemanticSpelling()); - if (IA) + if (IA) { D->addAttr(IA); + S.Consumer.AssignInheritanceModel(cast(D)); + } } static void handleDeclspecThreadAttr(Sema &S, Decl *D, diff --git a/lib/Sema/SemaType.cpp b/lib/Sema/SemaType.cpp index fb8e1e7620..91a26a4371 100644 --- a/lib/Sema/SemaType.cpp +++ b/lib/Sema/SemaType.cpp @@ -6611,6 +6611,7 @@ static void assignInheritanceModel(Sema &S, CXXRecordDecl *RD) { S.ImplicitMSInheritanceAttrLoc.isValid() ? S.ImplicitMSInheritanceAttrLoc : RD->getSourceRange())); + S.Consumer.AssignInheritanceModel(RD); } } diff --git a/test/CodeGenCXX/microsoft-abi-member-pointers.cpp b/test/CodeGenCXX/microsoft-abi-member-pointers.cpp index fd22c00342..a3985ba09c 100644 --- a/test/CodeGenCXX/microsoft-abi-member-pointers.cpp +++ b/test/CodeGenCXX/microsoft-abi-member-pointers.cpp @@ -3,16 +3,29 @@ // RUN: %clang_cc1 -std=c++11 -Wno-uninitialized -fno-rtti -emit-llvm %s -o - -triple=i386-pc-win32 -DINCOMPLETE_VIRTUAL -fms-extensions -verify // RUN: %clang_cc1 -std=c++11 -Wno-uninitialized -fno-rtti -emit-llvm %s -o - -triple=i386-pc-win32 -DINCOMPLETE_VIRTUAL -DMEMFUN -fms-extensions -verify +struct PR26313_Y; +typedef void (PR26313_Y::*PR26313_FUNC)(); +struct PR26313_X { + PR26313_FUNC *ptr; + PR26313_X(); +}; +PR26313_X::PR26313_X() {} +void PR26313_f(PR26313_FUNC *p) { delete p; } + +struct PR26313_Z; +int PR26313_Z::**a = nullptr; +int PR26313_Z::*b = *a; +// CHECK-DAG: @"\01?a@@3PAPQPR26313_Z@@HA" = global %0* null, align 4 +// CHECK-DAG: @"\01?b@@3PQPR26313_Z@@HQ1@" = global { i32, i32, i32 } { i32 0, i32 0, i32 -1 }, align 4 + namespace PR20947 { struct A; int A::**a = nullptr; -// CHECK: %[[opaque0:.*]] = type opaque -// CHECK: %[[opaque1:.*]] = type opaque -// CHECK: @"\01?a@PR20947@@3PAPQA@1@HA" = global %[[opaque0]]* null, align 4 +// CHECK-DAG: @"\01?a@PR20947@@3PAPQA@1@HA" = global %{{.*}}* null, align 4 struct B; int B::*&b = b; -// CHECK: @"\01?b@PR20947@@3AAPQB@1@HA" = global %[[opaque1]]* null, align 4 +// CHECK-DAG: @"\01?b@PR20947@@3AAPQB@1@HA" = global %{{.*}}* null, align 4 } namespace PR20017 { -- 2.40.0