[clang] [ASTImporter] Propagate MSInheritanceAttr onto imported redeclarations (PR #214213)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 5 05:20:01 PDT 2026
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 <guillem.bartrina at sonarsource.com>
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);
+}
More information about the cfe-commits
mailing list