Make globals with mutable members non-constant, even in custom sections

[dblaikie]

Summary

Turned out we were making overly simple assumptions about which sections (& section flags) would be used when emitting a global into a custom section. This lead to sections with read-only flags being used for globals of struct types with mutable members.

Fixed by porting the codegen function with the more nuanced handling/checking for mutable members out of codegen for use in the sema code that does this initial checking/mapping to section flags.

[rnk]

clang/lib/Sema/SemaDecl.cpp

14363

I think this is not compatible with MSVC. MSVC uses simple logic, it doesn't look for mutable: https://gcc.godbolt.org/z/sj6d4saxx

The const mutable struct appears in the myrdata section in that example.

I think the solution is to separate the flag logic from the pragma stack selection logic, which has to remain MSVC-compatible.

[efriedma]

clang/lib/Sema/SemaDecl.cpp

14363

MSVC apparently looks at whether the variable is marked "const", and nothing else; it doesn't look at mutable, it doesn't look at whether the variable has a constant initializer. So the current code isn't right either; if we're trying to implement MSVC-compatible logic, we shouldn't check HasConstInit.

That said, I'm not sure how precisely/in what modes we want to precisely emulate MSVC. Probably anything we do here will be confusing.

[rsmith]

clang/lib/Sema/SemaDecl.cpp

14363

We should at least issue a warning if the sensible logic and the MSVC-compatible calculation differ. @rnk, do you know how important it is to follow the MSVC semantics in this regard?

[rnk]

clang/lib/Sema/SemaDecl.cpp

14363

I think it depends on whether you think that users are primarily using these pragmas to override the default rdata/bss/data sections without any care about precisely what goes where, or if they are using them to do something finer grained.

If I had to guess, I'd say it's more likely the former, given that __declspec(allocate) and #pragma(section) exist to handle cases where users are putting specific globals into specific sections.

Which, if we follow Richard's suggestion, would mean warning when we put a global marked const into a writable section when ConstSegStack is non-empty. That seems reasonable. -Wmicrosoft-const-seg for the new warning group?

[dblaikie]

clang/lib/Sema/SemaDecl.cpp

14363

Does the MSVC situation only apply to custom sections? (presumably when not customizing the section, MSVC gets it right and can support a const global with a runtime initializer, mutable member, or mutating dtor?)

I think this code still needs to be modified, since this is the code that drives the error about incompatible sections. So it'll need to behave differently depending on the target platform?

[efriedma]

clang/lib/Sema/SemaDecl.cpp

14363

Yes, the MSVC situation is specifically if you specify #pragma const_seg; without the pragma, it does what you'd expect.

[dblaikie]

Add a warning for MSVC pragma inconsistency

[dblaikie]

clang/lib/Sema/SemaDecl.cpp

14363

Went with the "let's do the thing that the user probably wants, but isn't what MSVC does, and warn when that difference comes up" - if that's OK with everyone.

(always open to wordsmithing the warning - and if we want to, can go to the extra layer and specifically diagnose which reason (mutable members, non-const init) - and I can't quite figure out the best phrasing to say "we're putting it in section X insetad of section Y, because Z, but Microsoft would use X because A" or something... it's all a bit of a mouthful)

[dblaikie]

@rnk & I figured that the user probably isn't relying on this much - if they cared about a particular variable, they could use an attribute instead of a pragma. The pragma just being about "put all my const things over here and all my non-const things over there" - bug-for-bug compatibility doesn't seem /as/ needed here.

Not sure how you feel about it, @efriedma ?

[efriedma]

Sure, diverging from MSVC here seems fine. I agree it's unlikely anyone would actually want to put a variable that will be modified in a "const" section.

clang/lib/Sema/SemaDecl.cpp

14363

Describing which reason actually applies would make the warning a lot easier to read.

[rnk]

Thanks! My concerns are addressed, but please confirm that Eli's are too.

clang/lib/Sema/SemaDecl.cpp

14363

That is true, but I think very few people will see this diagnostic. I'm not sure it's worth the added code complexity to implement that improvement.

[dblaikie]

Diagnose specific reasons

clang/lib/Sema/SemaDecl.cpp

14363

Updated with a specific diagnosis. Phrasing still feels a bit awkward/I'm open to wordsmithing suggestions.

[efriedma]

LGTM

[eddyz87]

Hi @dblaikie,

After this revision landed yesterday one BPF kernel selftest (written in C) stopped building. I narrowed the issue to the following example:

#define SEC(n) __attribute__((section(n)))

const int with_init SEC("foo") = 1;
const int no_init SEC("foo");

It fails with the following error:

$ clang -c t.c -o -
t.c:4:11: error: 'no_init' causes a section type conflict with 'with_init'
    4 | const int no_init SEC("foo");
      |           ^
t.c:3:11: note: declared here
    3 | const int with_init SEC("foo") = 1;
      |           ^
1 error generated.

The error occurs because clang infers PSF_Read attribute for section "foo" when with_init is processed and PSF_Read | PSF_Write attributes for section "foo" when no_init is processed, thus reporting diag::err_section_conflict. The behavior changed because of the following lines introduced in this revision:

void Sema::CheckCompleteVariableDeclaration(VarDecl *var) {
  ...
  if (GlobalStorage && var->isThisDeclarationADefinition() &&
      !inTemplateInstantiation()) {
    ...
    if (var->hasInit() && HasConstInit && !(Reason =
        var->getType().isNonConstantStorage(Context, true, false))) {
      Stack = &ConstSegStack;
    } else {
      SectionFlags |= ASTContext::PSF_Write;
      Stack = var->hasInit() && HasConstInit ? &DataSegStack : &BSSSegStack;
    }
    ...
  }
  ...
}

Because of the var->hasInit() check any global w/o initializer gets a PSF_Write flag, which was not the case before this revision. So, now if one wants to have some const globals in the same section, these globals need to have an initializer.

I checked C language standard (N3088, section "6.7.3 Type qualifiers") and it does not offer much to tell if my example is correct or not, so I guess it is implementation dependent. GCC accepts my example w/o errors or warnings.

So, a question: might it be the case that var->hasInit() check is too restrictive?
(I do understand that w/o some custom linker behavior const globals w/o initializer don't make much sense).

[eddyz87]

As an additional data point, the same example but w/o section specification compiles fine:

const int with_init = 1;
const int no_init;

And puts both globals to the same section:

$ clang -c t.c -o - | llvm-readelf --section-headers -s -
Section Headers:
  [Nr] Name              Type            Address          Off    Size   ES Flg Lk Inf Al
  ...
  [ 3] .rodata           PROGBITS        0000000000000000 000040 000008 00   A  0   0  4
  ...

Symbol table '.symtab' contains 4 entries:
   Num:    Value          Size Type    Bind   Vis       Ndx Name
     ...
     2: 0000000000000000     4 OBJECT  GLOBAL DEFAULT     3 with_init
     3: 0000000000000004     4 OBJECT  GLOBAL DEFAULT     3 no_init

(Ndx stands for section Nr).

[dblaikie]

In D156726#4591543, @eddyz87 wrote:

As an additional data point, the same example but w/o section specification compiles fine:

const int with_init = 1;
const int no_init;

And puts both globals to the same section:

$ clang -c t.c -o - | llvm-readelf --section-headers -s -
Section Headers:
  [Nr] Name              Type            Address          Off    Size   ES Flg Lk Inf Al
  ...
  [ 3] .rodata           PROGBITS        0000000000000000 000040 000008 00   A  0   0  4
  ...

Symbol table '.symtab' contains 4 entries:
   Num:    Value          Size Type    Bind   Vis       Ndx Name
     ...
     2: 0000000000000000     4 OBJECT  GLOBAL DEFAULT     3 with_init
     3: 0000000000000004     4 OBJECT  GLOBAL DEFAULT     3 no_init

(Ndx stands for section Nr).

Thanks for the details - looking into it!

[dblaikie]

Fixed in 19c216b5ab7c34c297b4c3c4da64b49b3c830a3a

[yonghong-song]

In D156726#4593671, @dblaikie wrote:

Fixed in 19c216b5ab7c34c297b4c3c4da64b49b3c830a3a

Hi, @dblaikie I cannot find the above commit in llvm-project and with latest llvm-project 'main' branch, the problem seems still exist.

[dblaikie]

In D156726#4594005, @yonghong-song wrote:

In D156726#4593671, @dblaikie wrote:

Fixed in 19c216b5ab7c34c297b4c3c4da64b49b3c830a3a

Hi, @dblaikie I cannot find the above commit in llvm-project and with latest llvm-project 'main' branch, the problem seems still exist.

Ah, right right - sorry, failed to push. Here it is: 2993243c45abdb4f2bc3979336d054be165b1134

[yonghong-song]

Ah, right right - sorry, failed to push. Here it is: 2993243c45abdb4f2bc3979336d054be165b1134

Thanks! I tested and the problem with bpf backend is resolved!

clang/include/clang/AST/Type.h

public:
   /// added "const" at a different level.
   bool isLocalConstQualified() const {
     return (getLocalFastQualifiers() & Qualifiers::Const);
   }
 
   /// Determine whether this type is const-qualified.
   bool isConstQualified() const;
 
+  enum class NonConstantStorageReason {
+    MutableField,
+    NonConstNonReferenceType,
+    NonTrivialCtor,
+    NonTrivialDtor,
+  };
+  /// Determine whether instances of this type can be placed in immutable
+  /// storage.
+  /// If ExcludeCtor is true, the duration when the object's constructor runs
+  /// will not be considered. The caller will need to verify that the object is
+  /// not written to during its construction. ExcludeDtor works similarly.
+  std::optional<NonConstantStorageReason>
+  isNonConstantStorage(const ASTContext &Ctx, bool ExcludeCtor,
+                       bool ExcludeDtor);
+
+  bool isConstantStorage(const ASTContext &Ctx, bool ExcludeCtor,
+                         bool ExcludeDtor) {
+    return !isNonConstantStorage(Ctx, ExcludeCtor, ExcludeDtor);
+  }
+
   /// Determine whether this particular QualType instance has the
   /// "restrict" qualifier set, without looking through typedefs that may have
   /// added "restrict" at a different level.
   bool isLocalRestrictQualified() const {
     return (getLocalFastQualifiers() & Qualifiers::Restrict);
   }
 
   /// Determine whether this type is restrict-qualified.

clang/include/clang/Basic/DiagnosticGroups.td

 def InfiniteRecursion : DiagGroup<"infinite-recursion">;
 def PureVirtualCallFromCtorDtor: DiagGroup<"call-to-pure-virtual-from-ctor-dtor">;
 def GNUImaginaryConstant : DiagGroup<"gnu-imaginary-constant">;
 def IgnoredReferenceQualifiers : DiagGroup<"ignored-reference-qualifiers">;
 def IgnoredQualifiers : DiagGroup<"ignored-qualifiers", [IgnoredReferenceQualifiers]>;
 def : DiagGroup<"import">;
 def GNUIncludeNext : DiagGroup<"gnu-include-next">;
 def IncompatibleMSStruct : DiagGroup<"incompatible-ms-struct">;
+def IncompatibleMSPragmaSection : DiagGroup<"incompatible-ms-pragma-section">;
 def IncompatiblePointerTypesDiscardsQualifiers
   : DiagGroup<"incompatible-pointer-types-discards-qualifiers">;
 def IncompatibleFunctionPointerTypes
   : DiagGroup<"incompatible-function-pointer-types">;
 def IncompatiblePointerTypes
   : DiagGroup<"incompatible-pointer-types",
     [IncompatiblePointerTypesDiscardsQualifiers,
      IncompatibleFunctionPointerTypes]>;

clang/include/clang/Basic/DiagnosticSemaKinds.td

def err_pragma_fenv_requires_precise : Error<
   "'#pragma STDC FENV_ACCESS ON' is illegal when precise is disabled">;
 def warn_cxx_ms_struct :
   Warning<"ms_struct may not produce Microsoft-compatible layouts for classes "
           "with base classes or virtual functions">,
   DefaultError, InGroup<IncompatibleMSStruct>;
 def err_pragma_pack_identifer_not_supported : Error<
   "specifying an identifier within `#pragma pack` is not supported on this target">;
 def err_section_conflict : Error<"%0 causes a section type conflict with %1">;
+def warn_section_msvc_compat : Warning<"`#pragma const_seg` for section %1 will"
+  " not apply to %0 due to the presence of a %select{mutable field||non-trivial constructor|non-trivial destructor}2">,
+  InGroup<IncompatibleMSPragmaSection>;
 def err_no_base_classes : Error<"invalid use of '__super', %0 has no base classes">;
 def err_invalid_super_scope : Error<"invalid use of '__super', "
   "this keyword can only be used inside class or member function scope">;
 def err_super_in_lambda_unsupported : Error<
   "use of '__super' inside a lambda is unsupported">;
 
 def err_pragma_expected_file_scope : Error<
   "'#pragma %0' can only appear at file scope">;

clang/lib/AST/Type.cpp

if (T.isConstQualified())
     return true;
 
   if (const ArrayType *AT = Ctx.getAsArrayType(T))
     return AT->getElementType().isConstant(Ctx);
 
   return T.getAddressSpace() == LangAS::opencl_constant;
 }
 
+std::optional<QualType::NonConstantStorageReason>
+QualType::isNonConstantStorage(const ASTContext &Ctx, bool ExcludeCtor,
+                            bool ExcludeDtor) {
+  if (!isConstant(Ctx) && !(*this)->isReferenceType())
+    return NonConstantStorageReason::NonConstNonReferenceType;
+  if (!Ctx.getLangOpts().CPlusPlus)
+    return std::nullopt;
+  if (const CXXRecordDecl *Record =
+          Ctx.getBaseElementType(*this)->getAsCXXRecordDecl()) {
+    if (!ExcludeCtor)
+      return NonConstantStorageReason::NonTrivialCtor;
+    if (Record->hasMutableFields())
+      return NonConstantStorageReason::MutableField;
+    if (!Record->hasTrivialDestructor() && !ExcludeDtor)
+      return NonConstantStorageReason::NonTrivialDtor;
+  }
+  return std::nullopt;
+}
+
 // C++ [temp.dep.type]p1:
 //   A type is dependent if it is...
 //     - an array type constructed from any dependent type or whose
 //       size is specified by a constant expression that is
 //       value-dependent,
 ArrayType::ArrayType(TypeClass tc, QualType et, QualType can,
                      ArraySizeModifier sm, unsigned tq, const Expr *sz)
     // Note, we need to check for DependentSizedArrayType explicitly here

clang/lib/CodeGen/CGDecl.cpp

if (GV->getValueType() != Init->getType()) {
 
     // Erase the old global, since it is no longer used.
     OldGV->eraseFromParent();
   }
 
   bool NeedsDtor =
       D.needsDestruction(getContext()) == QualType::DK_cxx_destructor;
 
-  GV->setConstant(CGM.isTypeConstant(D.getType(), true, !NeedsDtor));
+  GV->setConstant(
+      D.getType().isConstantStorage(getContext(), true, !NeedsDtor));
   GV->setInitializer(Init);
 
   emitter.finalize(GV);
 
   if (NeedsDtor && HaveInsertPoint()) {
     // We have a constant initializer, but a nontrivial destructor. We still
     // need to perform a guarded "initialization" in order to register the
     // destructor.
if (D.getInit() && (Ty->isArrayType() || Ty->isRecordType()) &&
       // emit it as a global instead.
       // Exception is if a variable is located in non-constant address space
       // in OpenCL.
       bool NeedsDtor =
           D.needsDestruction(getContext()) == QualType::DK_cxx_destructor;
       if ((!getLangOpts().OpenCL ||
            Ty.getAddressSpace() == LangAS::opencl_constant) &&
           (CGM.getCodeGenOpts().MergeAllConstants && !NRVO &&
-           !isEscapingByRef && CGM.isTypeConstant(Ty, true, !NeedsDtor))) {
+           !isEscapingByRef &&
+           Ty.isConstantStorage(getContext(), true, !NeedsDtor))) {
         EmitStaticVarDecl(D, llvm::GlobalValue::InternalLinkage);
 
         // Signal this condition to later callbacks.
         emission.Addr = Address::invalid();
         assert(emission.wasEmittedAsGlobal());
         return emission;
       }
 

clang/lib/CodeGen/CGDeclCXX.cpp

if (getLangOpts().OpenMP && !getLangOpts().OpenMPSimd &&
       (void)CGM.getOpenMPRuntime().emitThreadPrivateVarDefinition(
           &D, DeclAddr, D.getAttr<OMPThreadPrivateDeclAttr>()->getLocation(),
           PerformInit, this);
     }
     bool NeedsDtor =
         D.needsDestruction(getContext()) == QualType::DK_cxx_destructor;
     if (PerformInit)
       EmitDeclInit(*this, D, DeclAddr);
-    if (CGM.isTypeConstant(D.getType(), true, !NeedsDtor))
+    if (D.getType().isConstantStorage(getContext(), true, !NeedsDtor))
       EmitDeclInvariant(*this, D, DeclPtr);
     else
       EmitDeclDestroy(*this, D, DeclAddr);
     return;
   }
 
   assert(PerformInit && "cannot have constant initializer which needs "
          "destruction for reference");

clang/lib/CodeGen/CGExpr.cpp

static Address createReferenceTemporary(CodeGenFunction &CGF,
   case SD_Automatic: {
     // If we have a constant temporary array or record try to promote it into a
     // constant global under the same rules a normal constant would've been
     // promoted. This is easier on the optimizer and generally emits fewer
     // instructions.
     QualType Ty = Inner->getType();
     if (CGF.CGM.getCodeGenOpts().MergeAllConstants &&
         (Ty->isArrayType() || Ty->isRecordType()) &&
-        CGF.CGM.isTypeConstant(Ty, true, false))
+        Ty.isConstantStorage(CGF.getContext(), true, false))
       if (auto Init = ConstantEmitter(CGF).tryEmitAbstract(Inner, Ty)) {
         auto AS = CGF.CGM.GetGlobalConstantAddressSpace();
         auto *GV = new llvm::GlobalVariable(
             CGF.CGM.getModule(), Init->getType(), /*isConstant=*/true,
             llvm::GlobalValue::PrivateLinkage, Init, ".ref.tmp", nullptr,
             llvm::GlobalValue::NotThreadLocal,
             CGF.getContext().getTargetAddressSpace(AS));
         CharUnits alignment = CGF.getContext().getTypeAlignInChars(Ty);

clang/lib/CodeGen/CGExprConstant.cpp

tryEmitGlobalCompoundLiteral(ConstantEmitter &emitter,
   if (!C) {
     assert(!E->isFileScope() &&
            "file-scope compound literal did not have constant initializer!");
     return ConstantAddress::invalid();
   }
 
   auto GV = new llvm::GlobalVariable(
       CGM.getModule(), C->getType(),
-      CGM.isTypeConstant(E->getType(), true, false),
+      E->getType().isConstantStorage(CGM.getContext(), true, false),
       llvm::GlobalValue::InternalLinkage, C, ".compoundliteral", nullptr,
       llvm::GlobalVariable::NotThreadLocal,
       CGM.getContext().getTargetAddressSpace(addressSpace));
   emitter.finalize(GV);
   GV->setAlignment(Align.getAsAlign());
   CGM.setAddrOfConstantCompoundLiteral(E, GV);
   return ConstantAddress(GV, GV->getValueType(), Align);
 }

clang/lib/CodeGen/CodeGenModule.h

public:
   /// getTBAAInfoForSubobject - Get TBAA information for an access with a given
   /// base lvalue.
   TBAAAccessInfo getTBAAInfoForSubobject(LValue Base, QualType AccessType) {
     if (Base.getTBAAInfo().isMayAlias())
       return TBAAAccessInfo::getMayAliasInfo();
     return getTBAAAccessInfo(AccessType);
   }
 
-  bool isTypeConstant(QualType QTy, bool ExcludeCtor, bool ExcludeDtor);
-
   bool isPaddedAtomicType(QualType type);
   bool isPaddedAtomicType(const AtomicType *type);
 
   /// DecorateInstructionWithTBAA - Decorate the instruction with a TBAA tag.
   void DecorateInstructionWithTBAA(llvm::Instruction *Inst,
                                    TBAAAccessInfo TBAAInfo);
 
   /// Adds !invariant.barrier !tag to instruction

clang/lib/CodeGen/CodeGenModule.cpp

if (CXX20ModuleInits && VD->getOwningModule() &&
       // as part of the initializer for an imported one.
       return false;
     }
   }
   // If OpenMP is enabled and threadprivates must be generated like TLS, delay
   // codegen for global variables, because they may be marked as threadprivate.
   if (LangOpts.OpenMP && LangOpts.OpenMPUseTLS &&
       getContext().getTargetInfo().isTLSSupported() && isa<VarDecl>(Global) &&
-      !isTypeConstant(Global->getType(), false, false) &&
+      !Global->getType().isConstantStorage(getContext(), false, false) &&
       !OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(Global))
     return false;
 
   return true;
 }
 
 ConstantAddress CodeGenModule::GetAddrOfMSGuidDecl(const MSGuidDecl *GD) {
   StringRef Name = getMangledName(GD);
if (F->empty()) {
       }
       setDSOLocal(F);
     }
   }
 
   return {FTy, C};
 }
 
-/// isTypeConstant - Determine whether an object of this type can be emitted
-/// as a constant.
-///
-/// If ExcludeCtor is true, the duration when the object's constructor runs
-/// will not be considered. The caller will need to verify that the object is
-/// not written to during its construction. ExcludeDtor works similarly.
-bool CodeGenModule::isTypeConstant(QualType Ty, bool ExcludeCtor,
-                                   bool ExcludeDtor) {
-  if (!Ty.isConstant(Context) && !Ty->isReferenceType())
-    return false;
-
-  if (Context.getLangOpts().CPlusPlus) {
-    if (const CXXRecordDecl *Record
-          = Context.getBaseElementType(Ty)->getAsCXXRecordDecl())
-      return ExcludeCtor && !Record->hasMutableFields() &&
-             (Record->hasTrivialDestructor() || ExcludeDtor);
-  }
-
-  return true;
-}
-
 /// GetOrCreateLLVMGlobal - If the specified mangled name is not in the module,
 /// create and return an llvm GlobalVariable with the specified type and address
 /// space. If there is something in the module with the specified name, return
 /// it potentially bitcasted to the right type.
 ///
 /// If D is non-null, it specifies a decl that correspond to this.  This is used
 /// to set the attributes on the global when it is first created.
 ///
CodeGenModule::GetOrCreateLLVMGlobal(StringRef MangledName, llvm::Type *Ty,
 
   // Handle things which are present even on external declarations.
   if (D) {
     if (LangOpts.OpenMP && !LangOpts.OpenMPSimd)
       getOpenMPRuntime().registerTargetGlobalVariable(D, GV);
 
     // FIXME: This code is overly simple and should be merged with other global
     // handling.
-    GV->setConstant(isTypeConstant(D->getType(), false, false));
+    GV->setConstant(D->getType().isConstantStorage(getContext(), false, false));
 
     GV->setAlignment(getContext().getDeclAlign(D).getAsAlign());
 
     setLinkageForGV(GV, D);
 
     if (D->getTLSKind()) {
       if (D->getTLSKind() == VarDecl::TLS_Dynamic)
         CXXThreadLocals.push_back(D);
#endif
   }
 
   GV->setInitializer(Init);
   if (emitter)
     emitter->finalize(GV);
 
   // If it is safe to mark the global 'constant', do so now.
   GV->setConstant(!NeedsGlobalCtor && !NeedsGlobalDtor &&
-                  isTypeConstant(D->getType(), true, true));
+                  D->getType().isConstantStorage(getContext(), true, true));
 
   // If it is in a read-only section, mark it 'constant'.
   if (const SectionAttr *SA = D->getAttr<SectionAttr>()) {
     const ASTContext::SectionInfo &SI = Context.SectionInfos[SA->getName()];
     if ((SI.SectionFlags & ASTContext::PSF_Write) == 0)
       GV->setConstant(true);
   }
 
ConstantAddress CodeGenModule::GetAddrOfGlobalTemporary(
   llvm::Constant *InitialValue = nullptr;
   bool Constant = false;
   llvm::Type *Type;
   if (Value) {
     // The temporary has a constant initializer, use it.
     emitter.emplace(*this);
     InitialValue = emitter->emitForInitializer(*Value, AddrSpace,
                                                MaterializedType);
-    Constant = isTypeConstant(MaterializedType, /*ExcludeCtor*/ Value,
+    Constant =
+        MaterializedType.isConstantStorage(getContext(), /*ExcludeCtor*/ Value,
                                            /*ExcludeDtor*/ false);
     Type = InitialValue->getType();
   } else {
     // No initializer, the initialization will be provided when we
     // initialize the declaration which performed lifetime extension.
     Type = getTypes().ConvertTypeForMem(MaterializedType);
   }
 
   // Create a global variable for this lifetime-extended temporary.

clang/lib/CodeGen/Targets/AMDGPU.cpp

if (!D)
     return DefaultGlobalAS;
 
   LangAS AddrSpace = D->getType().getAddressSpace();
   assert(AddrSpace == LangAS::Default || isTargetAddressSpace(AddrSpace));
   if (AddrSpace != LangAS::Default)
     return AddrSpace;
 
   // Only promote to address space 4 if VarDecl has constant initialization.
-  if (CGM.isTypeConstant(D->getType(), false, false) &&
+  if (D->getType().isConstantStorage(CGM.getContext(), false, false) &&
       D->hasConstantInitialization()) {
     if (auto ConstAS = CGM.getTarget().getConstantAddressSpace())
       return *ConstAS;
   }
   return DefaultGlobalAS;
 }
 
 llvm::SyncScope::ID

clang/lib/Sema/SemaDecl.cpp

if (getLangOpts().CPlusPlus && !type->isDependentType() && Init &&
     }
   }
 
   // Apply section attributes and pragmas to global variables.
   if (GlobalStorage && var->isThisDeclarationADefinition() &&
       !inTemplateInstantiation()) {
     PragmaStack<StringLiteral *> *Stack = nullptr;
     int SectionFlags = ASTContext::PSF_Read;
-    if (var->getType().isConstQualified()) {
-      if (HasConstInit)
-        Stack = &ConstSegStack;
-      else {
-        Stack = &BSSSegStack;
-        SectionFlags |= ASTContext::PSF_Write;
-      }
-    } else if (var->hasInit() && HasConstInit) {
-      Stack = &DataSegStack;
-      SectionFlags |= ASTContext::PSF_Write;
+    bool MSVCEnv =
+        Context.getTargetInfo().getTriple().isWindowsMSVCEnvironment();
+    std::optional<QualType::NonConstantStorageReason> Reason;
+    if (var->hasInit() && HasConstInit && !(Reason =
+        var->getType().isNonConstantStorage(Context, true, false))) {
     } else {
-      Stack = &BSSSegStack;
       SectionFlags |= ASTContext::PSF_Write;
+      Stack = var->hasInit() && HasConstInit ? &DataSegStack : &BSSSegStack;
     }
     if (const SectionAttr *SA = var->getAttr<SectionAttr>()) {
       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;
       auto SectionName = Stack->CurrentValue->getString();
       var->addAttr(SectionAttr::CreateImplicit(Context, SectionName,
                                                Stack->CurrentPragmaLocation,
                                                SectionAttr::Declspec_allocate));
       if (UnifySection(SectionName, SectionFlags, var))
         var->dropAttr<SectionAttr>();
     }

clang/test/CodeGenCXX/sections.cpp

-// RUN: %clang_cc1 -emit-llvm -triple i686-pc-win32 -fms-extensions -o - %s | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -triple i686-pc-win32 -fms-extensions -verify -o - %s | FileCheck %s
 
 extern "C" {
 
+struct Mutable {
+mutable int i = 3;
+};
+extern const Mutable mutable_default_section;
+const Mutable mutable_default_section;
+struct Normal {
+  int i = 2;
+};
+extern const Normal normal_default_section;
+const Normal normal_default_section;
 #pragma const_seg(".my_const")
 #pragma bss_seg(".my_bss")
 int D = 1;
 #pragma data_seg(".data")
 int a = 1;
+extern const Mutable mutable_custom_section;
+const Mutable mutable_custom_section; // expected-warning {{`#pragma const_seg` for section ".my_const" will not apply to 'mutable_custom_section' due to the presence of a mutable field}}
+extern const Normal normal_custom_section;
+const Normal normal_custom_section;
+struct NonTrivialDtor {
+  ~NonTrivialDtor();
+};
+extern const NonTrivialDtor non_trivial_dtor_custom_section;
+const NonTrivialDtor non_trivial_dtor_custom_section; // expected-warning {{`#pragma const_seg` for section ".my_const" will not apply to 'non_trivial_dtor_custom_section' due to the presence of a non-trivial destructor}}
+struct NonTrivialCtor {
+  NonTrivialCtor();
+};
+extern const NonTrivialCtor non_trivial_ctor_custom_section;
+const NonTrivialCtor non_trivial_ctor_custom_section; // expected-warning {{`#pragma const_seg` for section ".my_const" will not apply to 'non_trivial_ctor_custom_section' due to the presence of a non-trivial constructor}}
 #pragma data_seg(push, label, ".data2")
 extern const int b;
 const int b = 1;
 const char* s = "my string!";
 #pragma data_seg(push, ".my_seg")
 int c = 1;
 #pragma data_seg(pop, label)
 int d = 1;

 
 #pragma section("long_section", long)
 // Pragma section ignores "long".
 __declspec(allocate("long_section")) long long_var = 42;
 
 #pragma section("short_section", short)
 // Pragma section ignores "short".
 __declspec(allocate("short_section")) short short_var = 42;
+
+struct t2 { t2(); };
+extern const t2 non_trivial_ctor;
+__declspec(allocate("non_trivial_ctor_section")) const t2 non_trivial_ctor_var;
 }
 
+
+//CHECK: @mutable_default_section = dso_local global %struct.Mutable { i32 3 }, align 4{{$}}
+//CHECK: @normal_default_section = dso_local constant %struct.Normal { i32 2 }, align 4{{$}}
 //CHECK: @D = dso_local global i32 1
 //CHECK: @a = dso_local global i32 1, section ".data"
+//CHECK: @mutable_custom_section = dso_local global %struct.Mutable { i32 3 }, section ".data", align 4
+//CHECK: @normal_custom_section = dso_local constant %struct.Normal { i32 2 }, section ".my_const", align 4
 //CHECK: @b = dso_local constant i32 1, section ".my_const"
 //CHECK: @[[MYSTR:.*]] = {{.*}} unnamed_addr constant [11 x i8] c"my string!\00"
 //CHECK: @s = dso_local global ptr @[[MYSTR]], section ".data2"
 //CHECK: @c = dso_local global i32 1, section ".my_seg"
 //CHECK: @d = dso_local global i32 1, section ".data"
 //CHECK: @e = dso_local global i32 0, section ".my_bss"
 //CHECK: @f = dso_local global i32 0, section ".c"
 //CHECK: @i = dso_local global i32 0
 //CHECK: @TEST1 = dso_local global i32 0
 //CHECK: @TEST2 = dso_local global i32 0, section ".bss1"
 //CHECK: @TEST3 = dso_local global i32 0, section ".bss1"
 //CHECK: @d2 = dso_local global i32 1, section ".data"
 //CHECK: @b2 = dso_local constant i32 1, section ".my_const"
 //CHECK: @unreferenced = dso_local constant i32 0, section "read_flag_section"
 //CHECK: @referenced = dso_local constant i32 42, section "read_flag_section"
 //CHECK: @implicitly_read_write = dso_local global i32 42, section "no_section_attributes"
 //CHECK: @long_var = dso_local global i32 42, section "long_section"
 //CHECK: @short_var = dso_local global i16 42, section "short_section"
+//CHECK: @non_trivial_ctor_var = internal global %struct.t2 zeroinitializer, section "non_trivial_ctor_section"
 //CHECK: define dso_local void @g()
 //CHECK: define dso_local void @h() {{.*}} section ".my_code"
 //CHECK: define dso_local void @h2() {{.*}} section ".my_code"

clang/test/SemaCXX/attr-section.cpp

namespace override {
   void C::f() {}
 }
 
 // Check for section type conflicts between global variables and function templates
 template <typename> __attribute__((section("template_fn1"))) void template_fn1() {} // expected-note {{declared here}}
 const int const_global_var __attribute__((section("template_fn1"))) = 42;           // expected-error {{'const_global_var' causes a section type conflict with 'template_fn1'}}
 int mut_global_var __attribute__((section("template_fn2"))) = 42;                   // expected-note {{declared here}}
 template <typename> __attribute__((section("template_fn2"))) void template_fn2() {} // expected-error {{'template_fn2' causes a section type conflict with 'mut_global_var'}}
+
+namespace mutable_member {
+struct t1 {
+  mutable int i;
+};
+extern const t1 v1;
+__attribute__((section("mutable_member"))) const t1 v1{};
+extern int i;
+__attribute__((section("mutable_member"))) int i{};
+} // namespace mutable_member
+
+namespace non_trivial_ctor {
+struct t1 {
+  t1();
+  constexpr t1(int) { }
+};
+extern const t1 v1;
+__attribute__((section("non_trivial_ctor"))) const t1 v1; // expected-note {{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