https://github.com/guillem-bartrina-sonarsource created https://github.com/llvm/llvm-project/pull/214213
Under the Microsoft C++ ABI, Sema::assignInheritanceModel() attaches an implicit MSInheritanceAttr to whichever declaration of a class is most recent when a pointer-to-member type first has to be complete, and CXXRecordDecl::getMSInheritanceModel() reads it back off getMostRecentCXXRecordDecl(). Importing a declaration of that class appends it to the redeclaration chain and makes it the most recent one, but the attribute is not inherited along the chain, so a later query for the model -- e.g. laying out a record with a pointer-to-member field -- reaches a declaration without it and asserts, or dereferences null in a release build. Clone it onto the imported declaration instead, searching the whole chain since Sema may have attached it to any node. Add a CTU regression test reproducing the crash. >From 8ebfea30aafbe2ee6ae17b584b51aa38d9be6b23 Mon Sep 17 00:00:00 2001 From: guillem-bartrina-sonarsource <[email protected]> Date: Wed, 5 Aug 2026 13:34:46 +0200 Subject: [PATCH] [ASTImporter] Propagate MSInheritanceAttr onto imported redeclarations Under the Microsoft C++ ABI, Sema::assignInheritanceModel() attaches an implicit MSInheritanceAttr to whichever declaration of a class is most recent when a pointer-to-member type first has to be complete, and CXXRecordDecl::getMSInheritanceModel() reads it back off getMostRecentCXXRecordDecl(). Importing a declaration of that class appends it to the redeclaration chain and makes it the most recent one, but the attribute is not inherited along the chain, so a later query for the model -- e.g. laying out a record with a pointer-to-member field -- reaches a declaration without it and asserts, or dereferences null in a release build. Clone it onto the imported declaration instead, searching the whole chain since Sema may have attached it to any node. Add a CTU regression test reproducing the crash. --- clang/lib/AST/ASTImporter.cpp | 22 ++++ .../msinheritance-member-pointer.cpp | 115 ++++++++++++++++++ 2 files changed, 137 insertions(+) create mode 100644 clang/test/Analysis/ctu/regression/msinheritance-member-pointer.cpp diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 3ad71a223903c..740e857b625cc 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -3386,6 +3386,24 @@ ExpectedDecl ASTNodeImporter::VisitEnumDecl(EnumDecl *D) { return D2; } +/// Carry over the inheritable attributes onto a freshly imported declaration. +static void importInheritableAttrs(ASTContext &Ctx, Decl *To, Decl *Prev) { + if (!To || !Prev || To == Prev) + return; + + if (!isa<CXXRecordDecl>(To) || To->hasAttr<MSInheritanceAttr>()) + return; + + for (Decl *R : Prev->redecls()) { + if (const auto *IA = R->getAttr<MSInheritanceAttr>()) { + auto *Clone = cast<InheritableAttr>(IA->clone(Ctx)); + Clone->setInherited(true); + To->addAttr(Clone); + return; + } + } +} + ExpectedDecl ASTNodeImporter::VisitRecordDecl(RecordDecl *D) { bool IsFriendTemplate = false; if (auto *DCXX = dyn_cast<CXXRecordDecl>(D)) { @@ -3568,6 +3586,8 @@ ExpectedDecl ASTNodeImporter::VisitRecordDecl(RecordDecl *D) { addDeclToContexts(D, D2); } + importInheritableAttrs(Importer.getToContext(), D2, PrevDecl); + if (auto BraceRangeOrErr = import(D->getBraceRange())) D2->setBraceRange(*BraceRangeOrErr); else @@ -6621,6 +6641,8 @@ ExpectedDecl ASTNodeImporter::VisitClassTemplateSpecializationDecl( ClassTemplate->AddSpecialization(D2, InsertPos); } + importInheritableAttrs(Importer.getToContext(), D2, PrevDecl); + D2->setSpecializationKind(D->getSpecializationKind()); // Set the context of this specialization/instantiation. diff --git a/clang/test/Analysis/ctu/regression/msinheritance-member-pointer.cpp b/clang/test/Analysis/ctu/regression/msinheritance-member-pointer.cpp new file mode 100644 index 0000000000000..d639f69e9b552 --- /dev/null +++ b/clang/test/Analysis/ctu/regression/msinheritance-member-pointer.cpp @@ -0,0 +1,115 @@ +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: split-file %s %t + +// Pathological case: under the Microsoft C++ ABI a class's inheritance model is +// assigned lazily. Sema::assignInheritanceModel() attaches an implicit +// MSInheritanceAttr to whichever declaration of the class is most recent at the +// moment a pointer-to-member type first has to be complete, and +// CXXRecordDecl::getMSInheritanceModel() later reads it back off +// getMostRecentCXXRecordDecl(). Importing a declaration of that class appends it +// to the redeclaration chain, so it becomes the most recent one, but the +// attribute stays behind on the declaration Sema picked -- it is not inherited +// along the chain. Any later query then reaches a declaration without the +// attribute and asserts; a release build dereferences null instead. + +// RUN: %clang_cc1 -std=c++17 -triple x86_64-pc-windows \ +// RUN: -fms-compatibility -fms-extensions -fms-compatibility-version=19.40 \ +// RUN: -fdelayed-template-parsing \ +// RUN: -emit-pch -o %t/MavLinkConnectionImpl.cpp.ast %t/MavLinkConnectionImpl.cpp + +// RUN: %clang_extdef_map %t/MavLinkConnectionImpl.cpp -- -std=c++17 \ +// RUN: --target=x86_64-pc-windows -fms-compatibility -fms-extensions \ +// RUN: -fms-compatibility-version=19.40 -fdelayed-template-parsing \ +// RUN: > %t/externalDefMap.tmp.txt +// On windows, absolute paths generated by extdef_map are not recognized, +// so CSA prepends the workdir path to them. Force relative paths to work +// around this issue. The match is anchored and excludes spaces because the USR +// of Holder::Holder contains one (its member-pointer parameter) -- which is what +// the "<length>:" prefix of every defmap entry exists to tolerate. +// RUN: sed -e 's| [^ ]*/MavLinkConnectionImpl\.cpp$| MavLinkConnectionImpl.cpp.ast|' \ +// RUN: %t/externalDefMap.tmp.txt > %t/externalDefMap.txt + +// RUN: %clang_cc1 -std=c++17 -triple x86_64-pc-windows \ +// RUN: -fms-compatibility -fms-extensions -fms-compatibility-version=19.40 \ +// RUN: -fdelayed-template-parsing -analyze \ +// RUN: -analyzer-checker=core \ +// RUN: -analyzer-config experimental-enable-naive-ctu-analysis=true \ +// RUN: -analyzer-config ctu-dir=%t \ +// RUN: -verify %t/main.cpp + +//--- main.cpp + +// expected-no-diagnostics + +namespace impl { +class Inner; +} + +class Outer { + void entry(); + +public: + impl::Inner *pInner; +}; + +// This definition is the one Sema hands the MSInheritanceAttr to, because it is +// the most recent declaration of impl::Inner while this TU is parsed. +namespace impl { +class Inner { +public: + void entry(); + void target(); +}; +} // namespace impl + +void Outer::entry() { + // Importing impl::Inner::entry brings the other TU's forward declaration of + // impl::Inner along, appending it after the definition above and making it the + // most recent declaration -- without the attribute. Inlining the imported body + // then lays out Holder, which asks for the inheritance model again. + pInner->entry(); // no-crash +} + +//--- MavLinkConnectionImpl.cpp + +namespace impl { +// This forward declaration is what ends up appended to the main TU's chain. +class Inner; +} // namespace impl + +class Outer { +public: + impl::Inner *pInner; +}; + +namespace impl { +class Inner { + void entry(); + void target(); +}; +} // namespace impl + +// Holds a pointer-to-member by value, so laying this record out needs the MS +// inheritance model of impl::Inner. It must not be a template: an implicit +// instantiation of a template member never reaches the external definition map, +// so naive CTU could not import it, and an uninlined constructor never triggers +// the layout. +class Holder { + void (impl::Inner::*pm)(); + +public: + Holder(void (impl::Inner::*p)()) : pm(p) {} +}; + +Outer *g; + +void impl::Inner::target() { + // A field access through a pointer, so the analyzer builds the region whose + // getAsOffset() forces the record layout. An empty body does not reproduce. + g->pInner; +} + +void impl::Inner::entry() { + Holder h(&impl::Inner::target); +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
