[PATCH] D46664: Fix null MSInheritanceAttr deref in CXXRecordDecl::getMSInheritanceModel()
adr26 created this revision.
adr26 added a reviewer: rnk.
adr26 added a project: clang.
Herald added a subscriber: cfe-commits.
Ensure latest MPT decl has a MSInheritanceAttr when instantiating templates, to
avoid null MSInheritanceAttr deref in CXXRecordDecl::getMSInheritanceModel().
See PR#37399 for repo / details.
Repository:
rC Clang
https://reviews.llvm.org/D46664
Files:
include/clang/Sema/Sema.h
lib/Sema/SemaTemplate.cpp
lib/Sema/SemaTemplateInstantiateDecl.cpp
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1551,6 +1551,8 @@
LocalInstantiations.perform();
}
+ SemaRef.CheckCXXRecordDecl(Record, PrevDecl);
+
SemaRef.DiagnoseUnusedNestedTypedefs(Record);
return Record;
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -7076,6 +7076,21 @@
<< TemplateParams->getSourceRange();
}
+void Sema::CheckCXXRecordDecl(CXXRecordDecl *Record, CXXRecordDecl *Prev)
+{
+ if (Prev && Prev->hasAttr()) {
+MSInheritanceAttr::Spelling IM = Prev->getMSInheritanceModel();
+Record->addAttr(MSInheritanceAttr::CreateImplicit(
+getASTContext(), IM,
+/*BestCase=*/MSPointerToMemberRepresentationMethod ==
+LangOptions::PPTMK_BestCase,
+ImplicitMSInheritanceAttrLoc.isValid()
+? ImplicitMSInheritanceAttrLoc
+: Record->getSourceRange()));
+Consumer.AssignInheritanceModel(Record);
+ }
+}
+
/// Determine what kind of template specialization the given declaration
/// is.
static TemplateSpecializationKind getTemplateSpecializationKind(Decl *D) {
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -6470,6 +6470,8 @@
bool CheckTemplateDeclScope(Scope *S, TemplateParameterList *TemplateParams);
+ void CheckCXXRecordDecl(CXXRecordDecl *Record, CXXRecordDecl *Prev);
+
/// Called when the parser has parsed a C++ typename
/// specifier, e.g., "typename T::type".
///
Index: lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1551,6 +1551,8 @@
LocalInstantiations.perform();
}
+ SemaRef.CheckCXXRecordDecl(Record, PrevDecl);
+
SemaRef.DiagnoseUnusedNestedTypedefs(Record);
return Record;
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -7076,6 +7076,21 @@
<< TemplateParams->getSourceRange();
}
+void Sema::CheckCXXRecordDecl(CXXRecordDecl *Record, CXXRecordDecl *Prev)
+{
+ if (Prev && Prev->hasAttr()) {
+MSInheritanceAttr::Spelling IM = Prev->getMSInheritanceModel();
+Record->addAttr(MSInheritanceAttr::CreateImplicit(
+getASTContext(), IM,
+/*BestCase=*/MSPointerToMemberRepresentationMethod ==
+LangOptions::PPTMK_BestCase,
+ImplicitMSInheritanceAttrLoc.isValid()
+? ImplicitMSInheritanceAttrLoc
+: Record->getSourceRange()));
+Consumer.AssignInheritanceModel(Record);
+ }
+}
+
/// Determine what kind of template specialization the given declaration
/// is.
static TemplateSpecializationKind getTemplateSpecializationKind(Decl *D) {
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -6470,6 +6470,8 @@
bool CheckTemplateDeclScope(Scope *S, TemplateParameterList *TemplateParams);
+ void CheckCXXRecordDecl(CXXRecordDecl *Record, CXXRecordDecl *Prev);
+
/// Called when the parser has parsed a C++ typename
/// specifier, e.g., "typename T::type".
///
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46664: Fix null MSInheritanceAttr deref in CXXRecordDecl::getMSInheritanceModel()
adr26 updated this revision to Diff 148044.
adr26 added a comment.
Update to change MSInheritanceAttr to always be attached to the latest
non-injected class name decl, as suggested by Reid.
https://reviews.llvm.org/D46664
Files:
include/clang/AST/DeclCXX.h
lib/AST/MicrosoftMangle.cpp
lib/AST/Type.cpp
lib/CodeGen/MicrosoftCXXABI.cpp
lib/Sema/SemaType.cpp
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -7544,7 +7544,7 @@
/// Locks in the inheritance model for the given class and all of its bases.
static void assignInheritanceModel(Sema &S, CXXRecordDecl *RD) {
- RD = RD->getMostRecentDecl();
+ RD = RD->getMostRecentNonInjectedDecl();
if (!RD->hasAttr()) {
MSInheritanceAttr::Spelling IM;
Index: lib/CodeGen/MicrosoftCXXABI.cpp
===
--- lib/CodeGen/MicrosoftCXXABI.cpp
+++ lib/CodeGen/MicrosoftCXXABI.cpp
@@ -2733,7 +2733,7 @@
assert(MD->isInstance() && "Member function must not be static!");
CharUnits NonVirtualBaseAdjustment = CharUnits::Zero();
- const CXXRecordDecl *RD = MD->getParent()->getMostRecentDecl();
+ const CXXRecordDecl *RD = MD->getParent()->getMostRecentNonInjectedDecl();
CodeGenTypes &Types = CGM.getTypes();
unsigned VBTableIndex = 0;
Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -2040,7 +2040,7 @@
return false;
// The inheritance attribute might only be present on the most recent
// CXXRecordDecl, use that one.
-RD = RD->getMostRecentDecl();
+RD = RD->getMostRecentNonInjectedDecl();
// Nothing interesting to do if the inheritance attribute is already set.
if (RD->hasAttr())
return false;
@@ -3936,5 +3936,5 @@
}
CXXRecordDecl *MemberPointerType::getMostRecentCXXRecordDecl() const {
- return getClass()->getAsCXXRecordDecl()->getMostRecentDecl();
+ return getClass()->getAsCXXRecordDecl()->getMostRecentNonInjectedDecl();
}
Index: lib/AST/MicrosoftMangle.cpp
===
--- lib/AST/MicrosoftMangle.cpp
+++ lib/AST/MicrosoftMangle.cpp
@@ -1370,12 +1370,12 @@
const NamedDecl *ND = TA.getAsDecl();
if (isa(ND) || isa(ND)) {
mangleMemberDataPointer(
- cast(ND->getDeclContext())->getMostRecentDecl(),
+
cast(ND->getDeclContext())->getMostRecentNonInjectedDecl(),
cast(ND));
} else if (const FunctionDecl *FD = dyn_cast(ND)) {
const CXXMethodDecl *MD = dyn_cast(FD);
if (MD && MD->isInstance()) {
-mangleMemberFunctionPointer(MD->getParent()->getMostRecentDecl(), MD);
+
mangleMemberFunctionPointer(MD->getParent()->getMostRecentNonInjectedDecl(),
MD);
} else {
Out << "$1?";
mangleName(FD);
Index: include/clang/AST/DeclCXX.h
===
--- include/clang/AST/DeclCXX.h
+++ include/clang/AST/DeclCXX.h
@@ -751,6 +751,21 @@
return const_cast(this)->getMostRecentDecl();
}
+ CXXRecordDecl *getMostRecentNonInjectedDecl() {
+CXXRecordDecl *Recent =
+static_cast(this)->getMostRecentDecl();
+while (Recent->isInjectedClassName()) {
+ // FIXME: Does injected class name need to be in the redeclarations
chain?
+ assert(Recent->getPreviousDecl());
+ Recent = Recent->getPreviousDecl();
+}
+return Recent;
+ }
+
+ const CXXRecordDecl *getMostRecentNonInjectedDecl() const {
+return const_cast(this)->getMostRecentNonInjectedDecl();
+ }
+
CXXRecordDecl *getDefinition() const {
// We only need an update if we don't already know which
// declaration is the definition.
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -7544,7 +7544,7 @@
/// Locks in the inheritance model for the given class and all of its bases.
static void assignInheritanceModel(Sema &S, CXXRecordDecl *RD) {
- RD = RD->getMostRecentDecl();
+ RD = RD->getMostRecentNonInjectedDecl();
if (!RD->hasAttr()) {
MSInheritanceAttr::Spelling IM;
Index: lib/CodeGen/MicrosoftCXXABI.cpp
===
--- lib/CodeGen/MicrosoftCXXABI.cpp
+++ lib/CodeGen/MicrosoftCXXABI.cpp
@@ -2733,7 +2733,7 @@
assert(MD->isInstance() && "Member function must not be static!");
CharUnits NonVirtualBaseAdjustment = CharUnits::Zero();
- const CXXRecordDecl *RD = MD->getParent()->getMostRecentDecl();
+ const CXXRecordDecl *RD = MD->getParent()->getMostRecentNonInjectedDecl();
CodeGenTypes &Types = CGM.getTypes();
unsigned VBTableIndex = 0;
Index: lib/AST/Type.cpp
===
--- lib/AST/Type.cpp
+++ lib/AST/Type.cpp
@@ -2040,7
[PATCH] D46664: Fix null MSInheritanceAttr deref in CXXRecordDecl::getMSInheritanceModel()
adr26 updated this revision to Diff 149300.
adr26 added a comment.
Hi Reid,
I've added testcases matching the issues I hit in
`microsoft-abi-member-pointers.cpp` (no change to the rest of the fix). If you
are happy with this, please feel free to push.
I checked with MSVC, and as per the added testcases, it appears to always lock
in the inheritance model as virutal when there's a member funtion pointer in a
dependant base, irrespective of whether the most-derived class ends up using
single/multiple/virtual inheritance. In fact if, for example, you annotate
DerivedFunctor (in the test I've added) using an MS inheritance keyword - i.e.
as "class __single_inheritance DerviceFunctor" - then MSVC complains that it's
already fixed the representation to be virtual:
C:\Temp\clang\clang>cl
C:\workspaces\llvm\tools\clang\test\CodeGenCXX\microsoft-abi-member-pointers.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.14.26428.1 for x86
Copyright (C) Microsoft Corporation. All rights reserved.
microsoft-abi-member-pointers.cpp
C:\workspaces\llvm\tools\clang\test\CodeGenCXX\microsoft-abi-member-pointers.cpp(81):
error C2286: pointers to members of 'pr37399::DerivedFunctor'
representation is already set to virtual_inheritance - declaration ignored
C:\workspaces\llvm\tools\clang\test\CodeGenCXX\microsoft-abi-member-pointers.cpp(27):
note: see declaration of 'pr37399::DerivedFunctor'
as such, these tests cover the cases I was hitting and appear to have
behavioural parity with MSVC (as is the intention!).
Thanks, Andrew R
https://reviews.llvm.org/D46664
Files:
include/clang/AST/DeclCXX.h
lib/AST/MicrosoftMangle.cpp
lib/AST/Type.cpp
lib/CodeGen/MicrosoftCXXABI.cpp
lib/Sema/SemaType.cpp
test/CodeGenCXX/microsoft-abi-member-pointers.cpp
Index: test/CodeGenCXX/microsoft-abi-member-pointers.cpp
===
--- test/CodeGenCXX/microsoft-abi-member-pointers.cpp
+++ test/CodeGenCXX/microsoft-abi-member-pointers.cpp
@@ -3,6 +3,124 @@
// 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
+namespace pr37399 {
+template
+struct Functor {
+ void (T::*PtrToMemberFunction)();
+};
+// CHECK-DAG: %"struct.pr37399::Functor" = type { i8* }
+
+template
+class SimpleDerivedFunctor;
+template
+class SimpleDerivedFunctor : public Functor> {};
+// CHECK-DAG: %"class.pr37399::SimpleDerivedFunctor" = type { %"struct.pr37399::Functor" }
+
+SimpleDerivedFunctor SimpleFunctor;
+// CHECK-DAG: @"?SimpleFunctor@pr37399@@3V?$SimpleDerivedFunctor@X@1@A" = dso_local global %"class.pr37399::SimpleDerivedFunctor" zeroinitializer, align 4
+
+short Global = 0;
+template
+class DerivedFunctor;
+template
+class DerivedFunctor
+: public Functor> {
+public:
+ void Foo() {
+Global = 42;
+ }
+};
+
+class MultipleBase {
+public:
+ MultipleBase() : Value() {}
+ short Value;
+};
+// CHECK-DAG: %"class.pr37399::MultipleBase" = type { i16 }
+
+template
+class MultiplyDerivedFunctor;
+template
+class MultiplyDerivedFunctor
+: public Functor>,
+ public MultipleBase {
+public:
+ void Foo() {
+MultipleBase::Value = 42*2;
+ }
+};
+
+class VirtualBase {
+public:
+ VirtualBase() : Value() {}
+ short Value;
+};
+// CHECK-DAG: %"class.pr37399::VirtualBase" = type { i16 }
+
+template
+class VirtBaseFunctor
+: public Functor,
+ public virtual VirtualBase{};
+template
+class VirtuallyDerivedFunctor;
+template
+class VirtuallyDerivedFunctor
+: public VirtBaseFunctor>,
+ public virtual VirtualBase {
+public:
+ void Foo() {
+VirtualBase::Value = 42*3;
+ }
+};
+} // namespace pr37399
+
+pr37399::DerivedFunctor BFunctor;
+// CHECK-DAG: @"?BFunctor@@3V?$DerivedFunctor@H@pr37399@@A" = dso_local global %"[[BFUNCTOR:class.pr37399::DerivedFunctor(\.[0-9]+)?]]" zeroinitializer, align 8
+// CHECK-DAG: %"[[BFUNCTOR]]" = type { %"[[BFUNCTORBASE:struct.pr37399::Functor(\.[0-9]+)?]]" }
+// CHECK-DAG: %"[[BFUNCTORBASE]]" = type { { i8*, i32, i32, i32 } }
+pr37399::DerivedFunctor AFunctor;
+// CHECK-DAG: @"?AFunctor@@3V?$DerivedFunctor@X@pr37399@@A" = dso_local global %"[[AFUNCTOR:class.pr37399::DerivedFunctor(\.[0-9]+)?]]" zeroinitializer, align 8
+// CHECK-DAG: %"[[AFUNCTOR]]" = type { %"[[AFUNCTORBASE:struct.pr37399::Functor(\.[0-9]+)?]]" }
+// CHECK-DAG: %"[[AFUNCTORBASE]]" = type { { i8*, i32, i32, i32 } }
+
+pr37399::MultiplyDerivedFunctor DFunctor;
+// CHECK-DAG: @"?DFunctor@@3V?$MultiplyDerivedFunctor@H@pr37399@@A" = dso_local global %"[[DFUNCTOR:class.pr37399::MultiplyDerivedFunctor(\.[0-9]+)?]]" zeroinitializer, align 8
+// CHECK-DAG: %"[[DFUNCTOR]]" = type { %"[[DFUNCTORBASE:struct.pr37399::Functor(\.[0-9]+)?]]", %"class.pr37399::MultipleBase", [6 x i8]
[PATCH] D46664: Fix null MSInheritanceAttr deref in CXXRecordDecl::getMSInheritanceModel()
adr26 added a comment. That's a nice change to avoid duplication in `ClassTemplateSpecializationDecl::getMostRecentDecl()`. Thanks for your help getting this out the door! Repository: rC Clang https://reviews.llvm.org/D46664 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
