[clang] 19f2b68 - Make globals with mutable members non-constant, even in custom sections
David Blaikie via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 14 15:26:08 PDT 2023
Author: David Blaikie
Date: 2023-08-14T22:25:42Z
New Revision: 19f2b68095fe727e40079b7c6380b36b6462e691
URL: https://github.com/llvm/llvm-project/commit/19f2b68095fe727e40079b7c6380b36b6462e691
DIFF: https://github.com/llvm/llvm-project/commit/19f2b68095fe727e40079b7c6380b36b6462e691.diff
LOG: Make globals with mutable members non-constant, even in custom sections
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.
Differential Revision: https://reviews.llvm.org/D156726
Added:
Modified:
clang/include/clang/AST/Type.h
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/AST/Type.cpp
clang/lib/CodeGen/CGDecl.cpp
clang/lib/CodeGen/CGDeclCXX.cpp
clang/lib/CodeGen/CGExpr.cpp
clang/lib/CodeGen/CGExprConstant.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/CodeGen/CodeGenModule.h
clang/lib/CodeGen/Targets/AMDGPU.cpp
clang/lib/Sema/SemaDecl.cpp
clang/test/CodeGenCXX/sections.cpp
clang/test/SemaCXX/attr-section.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 82a68e332765e8..ed424ffb748014 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -817,6 +817,26 @@ class QualType {
/// 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
diff erent level.
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 990f223f755490..6f8386cd10922f 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -446,6 +446,7 @@ def IgnoredQualifiers : DiagGroup<"ignored-qualifiers", [IgnoredReferenceQualifi
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
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 95f8f0247d4b78..935fad819b82a4 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -996,6 +996,9 @@ def warn_cxx_ms_struct :
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">;
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index b3fd6fc3d823e2..e023cb46b9bb85 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -112,6 +112,25 @@ bool QualType::isConstant(QualType T, const ASTContext &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
diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index 155157776315b8..b68fe3c3c15f81 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -398,7 +398,8 @@ CodeGenFunction::AddInitializerToStaticVarDecl(const VarDecl &D,
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);
@@ -1498,7 +1499,8 @@ CodeGenFunction::EmitAutoVarAlloca(const VarDecl &D) {
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.
diff --git a/clang/lib/CodeGen/CGDeclCXX.cpp b/clang/lib/CodeGen/CGDeclCXX.cpp
index a9c88110d6f0b2..80543ba6311de4 100644
--- a/clang/lib/CodeGen/CGDeclCXX.cpp
+++ b/clang/lib/CodeGen/CGDeclCXX.cpp
@@ -217,7 +217,7 @@ void CodeGenFunction::EmitCXXGlobalVarDeclInit(const VarDecl &D,
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);
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 0aadaeaba69f3d..31c04255ddcbc0 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -391,7 +391,7 @@ static Address createReferenceTemporary(CodeGenFunction &CGF,
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(
diff --git a/clang/lib/CodeGen/CGExprConstant.cpp b/clang/lib/CodeGen/CGExprConstant.cpp
index b0f52066daeeca..622d586254f336 100644
--- a/clang/lib/CodeGen/CGExprConstant.cpp
+++ b/clang/lib/CodeGen/CGExprConstant.cpp
@@ -934,7 +934,7 @@ tryEmitGlobalCompoundLiteral(ConstantEmitter &emitter,
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));
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 3a79dec5359260..0d22713dbb310c 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -3377,7 +3377,7 @@ bool CodeGenModule::MayBeEmittedEagerly(const ValueDecl *Global) {
// 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;
@@ -4581,27 +4581,6 @@ CodeGenModule::CreateRuntimeFunction(llvm::FunctionType *FTy, StringRef Name,
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
@@ -4709,7 +4688,7 @@ CodeGenModule::GetOrCreateLLVMGlobal(StringRef MangledName, llvm::Type *Ty,
// 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());
@@ -5270,7 +5249,7 @@ void CodeGenModule::EmitGlobalVarDefinition(const VarDecl *D,
// 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>()) {
@@ -6344,8 +6323,9 @@ ConstantAddress CodeGenModule::GetAddrOfGlobalTemporary(
emitter.emplace(*this);
InitialValue = emitter->emitForInitializer(*Value, AddrSpace,
MaterializedType);
- Constant = isTypeConstant(MaterializedType, /*ExcludeCtor*/ Value,
- /*ExcludeDtor*/ false);
+ Constant =
+ MaterializedType.isConstantStorage(getContext(), /*ExcludeCtor*/ Value,
+ /*ExcludeDtor*/ false);
Type = InitialValue->getType();
} else {
// No initializer, the initialization will be provided when we
diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h
index 8517b1c93b40f1..601530a9dd8514 100644
--- a/clang/lib/CodeGen/CodeGenModule.h
+++ b/clang/lib/CodeGen/CodeGenModule.h
@@ -814,8 +814,6 @@ class CodeGenModule : public CodeGenTypeCache {
return getTBAAAccessInfo(AccessType);
}
- bool isTypeConstant(QualType QTy, bool ExcludeCtor, bool ExcludeDtor);
-
bool isPaddedAtomicType(QualType type);
bool isPaddedAtomicType(const AtomicType *type);
diff --git a/clang/lib/CodeGen/Targets/AMDGPU.cpp b/clang/lib/CodeGen/Targets/AMDGPU.cpp
index 1e7b036de82efd..b7d3978ecc22f2 100644
--- a/clang/lib/CodeGen/Targets/AMDGPU.cpp
+++ b/clang/lib/CodeGen/Targets/AMDGPU.cpp
@@ -418,7 +418,7 @@ AMDGPUTargetCodeGenInfo::getGlobalVarAddressSpace(CodeGenModule &CGM,
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;
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index c4bb9b15269f76..0ad807cca36c7b 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -14360,25 +14360,33 @@ void Sema::CheckCompleteVariableDeclaration(VarDecl *var) {
!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))) {
+ Stack = &ConstSegStack;
} 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,
diff --git a/clang/test/CodeGenCXX/sections.cpp b/clang/test/CodeGenCXX/sections.cpp
index 32e7d9402e3167..c7fe4e45ea05a6 100644
--- a/clang/test/CodeGenCXX/sections.cpp
+++ b/clang/test/CodeGenCXX/sections.cpp
@@ -1,12 +1,36 @@
-// 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;
@@ -74,10 +98,19 @@ __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"
@@ -96,6 +129,7 @@ __declspec(allocate("short_section")) short short_var = 42;
//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"
diff --git a/clang/test/SemaCXX/attr-section.cpp b/clang/test/SemaCXX/attr-section.cpp
index 12c0da283997df..e2418e0abcc5ff 100644
--- a/clang/test/SemaCXX/attr-section.cpp
+++ b/clang/test/SemaCXX/attr-section.cpp
@@ -48,3 +48,24 @@ template <typename> __attribute__((section("template_fn1"))) void template_fn1()
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
More information about the cfe-commits
mailing list