From a46d41190f491e7f6f3cf7bcdbbbdf1d4ab7b030 Mon Sep 17 00:00:00 2001 From: Akira Hatanaka Date: Wed, 1 Oct 2025 11:58:31 -0700 Subject: [PATCH 1/3] [Sema] Don't call isNonConstantStorage on incomplete variable types The code that applies section attributes to global variables was crashing when it encountered a variable whose type was incomplete. This change adds a check to ensure the variable's type is complete before calling the function. Fixes #120371 rdar://155577913 --- clang/lib/Sema/SemaDecl.cpp | 95 +++++++++++++++-------------- clang/test/SemaCXX/attr-section.cpp | 8 +++ 2 files changed, 58 insertions(+), 45 deletions(-) diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 0069b08f1991a..e6271745e7ac1 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -14918,52 +14918,57 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl *var) { } // Apply section attributes and pragmas to global variables. - if (GlobalStorage && var->isThisDeclarationADefinition() && - !inTemplateInstantiation()) { - PragmaStack *Stack = nullptr; - int SectionFlags = ASTContext::PSF_Read; - bool MSVCEnv = - Context.getTargetInfo().getTriple().isWindowsMSVCEnvironment(); - std::optional Reason; - if (HasConstInit && - !(Reason = var->getType().isNonConstantStorage(Context, true, false))) { - Stack = &ConstSegStack; - } else { - SectionFlags |= ASTContext::PSF_Write; - Stack = var->hasInit() && HasConstInit ? &DataSegStack : &BSSSegStack; - } - if (const SectionAttr *SA = var->getAttr()) { - if (SA->getSyntax() == AttributeCommonInfo::AS_Declspec) + [&]() { + if (GlobalStorage && var->isThisDeclarationADefinition() && + !inTemplateInstantiation()) { + PragmaStack *Stack = nullptr; + int SectionFlags = ASTContext::PSF_Read; + bool MSVCEnv = + Context.getTargetInfo().getTriple().isWindowsMSVCEnvironment(); + std::optional Reason; + if (HasConstInit && var->getType()->isIncompleteType()) + return; + if (HasConstInit && !(Reason = var->getType().isNonConstantStorage( + Context, true, false))) { + Stack = &ConstSegStack; + } else { + SectionFlags |= ASTContext::PSF_Write; + Stack = var->hasInit() && HasConstInit ? &DataSegStack : &BSSSegStack; + } + if (const SectionAttr *SA = var->getAttr()) { + if (SA->getSyntax() == AttributeCommonInfo::AS_Declspec) + SectionFlags |= ASTContext::PSF_Implicit; + UnifySection(SA->getName(), SectionFlags, var); + } else if (Stack->CurrentValue) { + if (Stack != &ConstSegStack && MSVCEnv && + ConstSegStack.CurrentValue != ConstSegStack.DefaultValue && + var->getType().isConstQualified()) { + assert((!Reason || Reason != QualType::NonConstantStorageReason:: + NonConstNonReferenceType) && + "This case should've already been handled elsewhere"); + Diag(var->getLocation(), diag::warn_section_msvc_compat) + << var << ConstSegStack.CurrentValue + << (int)(!HasConstInit + ? QualType::NonConstantStorageReason::NonTrivialCtor + : *Reason); + } SectionFlags |= ASTContext::PSF_Implicit; - UnifySection(SA->getName(), SectionFlags, var); - } else if (Stack->CurrentValue) { - if (Stack != &ConstSegStack && MSVCEnv && - ConstSegStack.CurrentValue != ConstSegStack.DefaultValue && - var->getType().isConstQualified()) { - assert((!Reason || Reason != QualType::NonConstantStorageReason:: - NonConstNonReferenceType) && - "This case should've already been handled elsewhere"); - Diag(var->getLocation(), diag::warn_section_msvc_compat) - << var << ConstSegStack.CurrentValue << (int)(!HasConstInit - ? QualType::NonConstantStorageReason::NonTrivialCtor - : *Reason); - } - SectionFlags |= ASTContext::PSF_Implicit; - auto SectionName = Stack->CurrentValue->getString(); - var->addAttr(SectionAttr::CreateImplicit(Context, SectionName, - Stack->CurrentPragmaLocation, - SectionAttr::Declspec_allocate)); - if (UnifySection(SectionName, SectionFlags, var)) - var->dropAttr(); - } - - // Apply the init_seg attribute if this has an initializer. If the - // initializer turns out to not be dynamic, we'll end up ignoring this - // attribute. - if (CurInitSeg && var->getInit()) - var->addAttr(InitSegAttr::CreateImplicit(Context, CurInitSeg->getString(), - CurInitSegLoc)); - } + auto SectionName = Stack->CurrentValue->getString(); + var->addAttr(SectionAttr::CreateImplicit( + Context, SectionName, Stack->CurrentPragmaLocation, + SectionAttr::Declspec_allocate)); + if (UnifySection(SectionName, SectionFlags, var)) + var->dropAttr(); + } + + // Apply the init_seg attribute if this has an initializer. If the + // initializer turns out to not be dynamic, we'll end up ignoring this + // attribute. + if (CurInitSeg && var->getInit()) + var->addAttr(InitSegAttr::CreateImplicit( + Context, CurInitSeg->getString(), CurInitSegLoc)); + } + }(); // All the following checks are C++ only. if (!getLangOpts().CPlusPlus) { diff --git a/clang/test/SemaCXX/attr-section.cpp b/clang/test/SemaCXX/attr-section.cpp index 1c07e3dd8bba2..3ec455d860ff0 100644 --- a/clang/test/SemaCXX/attr-section.cpp +++ b/clang/test/SemaCXX/attr-section.cpp @@ -69,3 +69,11 @@ __attribute__((section("non_trivial_ctor"))) const t1 v1; // expected-note {{dec extern const t1 v2; __attribute__((section("non_trivial_ctor"))) const t1 v2{3}; // expected-error {{'v2' causes a section type conflict with 'v1'}} } // namespace non_trivial_ctor + +namespace incomplete_type { +template +struct A { + struct B; + static constexpr B b{nullptr}; +}; +} From 1ce03356207e1ea745ffa0b8a2d04de1d857e81a Mon Sep 17 00:00:00 2001 From: Akira Hatanaka Date: Thu, 2 Oct 2025 12:53:40 -0700 Subject: [PATCH 2/3] Don't emit the diagnostic unless the variable is instantiated --- clang/lib/Sema/SemaDecl.cpp | 95 ++++++++++++++--------------- clang/test/SemaCXX/attr-section.cpp | 14 ++++- 2 files changed, 56 insertions(+), 53 deletions(-) diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index e6271745e7ac1..95aa7fbc0a321 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -14918,57 +14918,52 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl *var) { } // Apply section attributes and pragmas to global variables. - [&]() { - if (GlobalStorage && var->isThisDeclarationADefinition() && - !inTemplateInstantiation()) { - PragmaStack *Stack = nullptr; - int SectionFlags = ASTContext::PSF_Read; - bool MSVCEnv = - Context.getTargetInfo().getTriple().isWindowsMSVCEnvironment(); - std::optional Reason; - if (HasConstInit && var->getType()->isIncompleteType()) - return; - if (HasConstInit && !(Reason = var->getType().isNonConstantStorage( - Context, true, false))) { - Stack = &ConstSegStack; - } else { - SectionFlags |= ASTContext::PSF_Write; - Stack = var->hasInit() && HasConstInit ? &DataSegStack : &BSSSegStack; - } - if (const SectionAttr *SA = var->getAttr()) { - if (SA->getSyntax() == AttributeCommonInfo::AS_Declspec) - SectionFlags |= ASTContext::PSF_Implicit; - UnifySection(SA->getName(), SectionFlags, var); - } else if (Stack->CurrentValue) { - if (Stack != &ConstSegStack && MSVCEnv && - ConstSegStack.CurrentValue != ConstSegStack.DefaultValue && - var->getType().isConstQualified()) { - assert((!Reason || Reason != QualType::NonConstantStorageReason:: - NonConstNonReferenceType) && - "This case should've already been handled elsewhere"); - Diag(var->getLocation(), diag::warn_section_msvc_compat) - << var << ConstSegStack.CurrentValue - << (int)(!HasConstInit - ? QualType::NonConstantStorageReason::NonTrivialCtor - : *Reason); - } + if (GlobalStorage && var->isThisDeclarationADefinition() && + !var->getDeclContext()->isDependentContext()) { + PragmaStack *Stack = nullptr; + int SectionFlags = ASTContext::PSF_Read; + bool MSVCEnv = + Context.getTargetInfo().getTriple().isWindowsMSVCEnvironment(); + std::optional Reason; + if (HasConstInit && + !(Reason = var->getType().isNonConstantStorage(Context, true, false))) { + Stack = &ConstSegStack; + } else { + SectionFlags |= ASTContext::PSF_Write; + Stack = var->hasInit() && HasConstInit ? &DataSegStack : &BSSSegStack; + } + if (const SectionAttr *SA = var->getAttr()) { + if (SA->getSyntax() == AttributeCommonInfo::AS_Declspec) SectionFlags |= ASTContext::PSF_Implicit; - auto SectionName = Stack->CurrentValue->getString(); - var->addAttr(SectionAttr::CreateImplicit( - Context, SectionName, Stack->CurrentPragmaLocation, - SectionAttr::Declspec_allocate)); - if (UnifySection(SectionName, SectionFlags, var)) - var->dropAttr(); - } - - // Apply the init_seg attribute if this has an initializer. If the - // initializer turns out to not be dynamic, we'll end up ignoring this - // attribute. - if (CurInitSeg && var->getInit()) - var->addAttr(InitSegAttr::CreateImplicit( - Context, CurInitSeg->getString(), CurInitSegLoc)); - } - }(); + UnifySection(SA->getName(), SectionFlags, var); + } else if (Stack->CurrentValue) { + if (Stack != &ConstSegStack && MSVCEnv && + ConstSegStack.CurrentValue != ConstSegStack.DefaultValue && + var->getType().isConstQualified()) { + assert((!Reason || Reason != QualType::NonConstantStorageReason:: + NonConstNonReferenceType) && + "This case should've already been handled elsewhere"); + Diag(var->getLocation(), diag::warn_section_msvc_compat) + << var << ConstSegStack.CurrentValue << (int)(!HasConstInit + ? QualType::NonConstantStorageReason::NonTrivialCtor + : *Reason); + } + SectionFlags |= ASTContext::PSF_Implicit; + auto SectionName = Stack->CurrentValue->getString(); + var->addAttr(SectionAttr::CreateImplicit(Context, SectionName, + Stack->CurrentPragmaLocation, + SectionAttr::Declspec_allocate)); + if (UnifySection(SectionName, SectionFlags, var)) + var->dropAttr(); + } + + // Apply the init_seg attribute if this has an initializer. If the + // initializer turns out to not be dynamic, we'll end up ignoring this + // attribute. + if (CurInitSeg && var->getInit()) + var->addAttr(InitSegAttr::CreateImplicit(Context, CurInitSeg->getString(), + CurInitSegLoc)); + } // All the following checks are C++ only. if (!getLangOpts().CPlusPlus) { diff --git a/clang/test/SemaCXX/attr-section.cpp b/clang/test/SemaCXX/attr-section.cpp index 3ec455d860ff0..76fd97e34ad87 100644 --- a/clang/test/SemaCXX/attr-section.cpp +++ b/clang/test/SemaCXX/attr-section.cpp @@ -65,15 +65,23 @@ struct t1 { constexpr t1(int) { } }; extern const t1 v1; -__attribute__((section("non_trivial_ctor"))) const t1 v1; // expected-note {{declared here}} +__attribute__((section("non_trivial_ctor"))) const t1 v1; // expected-note 2 {{declared here}} extern const t1 v2; __attribute__((section("non_trivial_ctor"))) const t1 v2{3}; // expected-error {{'v2' causes a section type conflict with 'v1'}} } // namespace non_trivial_ctor -namespace incomplete_type { +namespace dependent_context { template struct A { struct B; - static constexpr B b{nullptr}; + static constexpr B b{nullptr}; // This used to crash. }; + +template +struct C { + __attribute__((section("non_trivial_ctor"))) + static constexpr int m{123}; // expected-error {{'m' causes a section type conflict with 'v1'}} +}; + +auto *p = &C::m; // expected-note {{in instantiation of static data member 'dependent_context::C::m' requested here}} } From f74f86cc547e704e23472fb3d6da2fec26307183 Mon Sep 17 00:00:00 2001 From: Akira Hatanaka Date: Thu, 9 Oct 2025 11:04:03 -0700 Subject: [PATCH 3/3] Add test case --- clang/test/SemaCXX/attr-section.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/clang/test/SemaCXX/attr-section.cpp b/clang/test/SemaCXX/attr-section.cpp index 76fd97e34ad87..6ed432f1cf7e0 100644 --- a/clang/test/SemaCXX/attr-section.cpp +++ b/clang/test/SemaCXX/attr-section.cpp @@ -77,6 +77,13 @@ struct A { static constexpr B b{nullptr}; // This used to crash. }; +struct B1 { void *p; }; +template +struct A1 { + __attribute__((section("non_trivial_ctor"))) + static constexpr B1 b{nullptr}; // no diagnostic expected +}; + template struct C { __attribute__((section("non_trivial_ctor")))