[clang] [clang][ASTImporter] Propagate MSInheritanceAttr onto imported redeclarations (PR #214213)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 19 01:49:02 PDT 2026
https://github.com/guillem-bartrina-sonarsource updated https://github.com/llvm/llvm-project/pull/214213
>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 1/3] [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);
+}
>From de079430eb78fd48a5ea812e56f1b73771cdc69a Mon Sep 17 00:00:00 2001
From: guillem-bartrina-sonarsource <guillem.bartrina at sonarsource.com>
Date: Wed, 19 Aug 2026 10:40:34 +0200
Subject: [PATCH 2/3] Add unittest test
---
clang/unittests/AST/ASTImporterTest.cpp | 39 +++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/clang/unittests/AST/ASTImporterTest.cpp b/clang/unittests/AST/ASTImporterTest.cpp
index 503f5da8af90f..d08f365f38d6a 100644
--- a/clang/unittests/AST/ASTImporterTest.cpp
+++ b/clang/unittests/AST/ASTImporterTest.cpp
@@ -8260,6 +8260,42 @@ TEST_P(ImportAttributes, ImportC99NoThrowAttr) {
checkImported(FromAttr->getAttrName(), ToAttr->getAttrName());
}
+struct ImportMSInheritanceAttr : ASTImporterOptionSpecificTestBase {
+ std::vector<std::string> getExtraArgs() const override {
+ return {"--target=x86_64-pc-windows-msvc"};
+ }
+};
+
+// The implicit MSInheritanceAttr must be cloned onto an imported
+// redeclaration, since it becomes the most recent one and
+// getMSInheritanceModel() reads the attribute off of that.
+TEST_P(ImportMSInheritanceAttr, PropagatedOntoImportedRedecl) {
+ Decl *ToTU = getToTuDecl(
+ R"(
+ namespace NS { class Inner {}; }
+ struct HasPM { void (NS::Inner::*PM)(); };
+ )",
+ Lang_CXX17);
+ auto *ToInner = FirstDeclMatcher<CXXRecordDecl>().match(
+ ToTU, cxxRecordDecl(hasName("Inner"), isDefinition()));
+ ASSERT_TRUE(ToInner->hasAttr<MSInheritanceAttr>());
+
+ Decl *FromTU =
+ getTuDecl("namespace NS { class Inner; }", Lang_CXX17, "from.cc");
+ auto *FromInner =
+ FirstDeclMatcher<CXXRecordDecl>().match(FromTU, cxxRecordDecl(hasName("Inner")));
+
+ auto *ToImportedInner = cast<CXXRecordDecl>(Import(FromInner, Lang_CXX17));
+ ASSERT_TRUE(ToImportedInner);
+
+ // The newly imported redeclaration must have become the most recent one.
+ ASSERT_EQ(ToImportedInner, ToInner->getMostRecentDecl());
+
+ EXPECT_TRUE(ToImportedInner->hasAttr<MSInheritanceAttr>());
+ EXPECT_EQ(ToImportedInner->getMSInheritanceModel(),
+ MSInheritanceModel::Single);
+}
+
template <typename T>
auto ExtendWithOptions(const T &Values, const std::vector<std::string> &Args) {
auto Copy = Values;
@@ -10867,6 +10903,9 @@ INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ImportWithExternalSource,
INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ImportAttributes,
DefaultTestValuesForRunOptions);
+INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ImportMSInheritanceAttr,
+ ::testing::Values(std::vector<std::string>()));
+
INSTANTIATE_TEST_SUITE_P(ParameterizedTests, ImportInjectedClassNameType,
DefaultTestValuesForRunOptions);
>From 07b68fb27409eec36e33ea8fe86298d323cc6737 Mon Sep 17 00:00:00 2001
From: guillem-bartrina-sonarsource <guillem.bartrina at sonarsource.com>
Date: Wed, 19 Aug 2026 10:48:38 +0200
Subject: [PATCH 3/3] Remove lit test
---
.../msinheritance-member-pointer.cpp | 115 ------------------
1 file changed, 115 deletions(-)
delete mode 100644 clang/test/Analysis/ctu/regression/msinheritance-member-pointer.cpp
diff --git a/clang/test/Analysis/ctu/regression/msinheritance-member-pointer.cpp b/clang/test/Analysis/ctu/regression/msinheritance-member-pointer.cpp
deleted file mode 100644
index d639f69e9b552..0000000000000
--- a/clang/test/Analysis/ctu/regression/msinheritance-member-pointer.cpp
+++ /dev/null
@@ -1,115 +0,0 @@
-// 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