https://github.com/erichkeane updated https://github.com/llvm/llvm-project/pull/229478
>From 4685a9ce490f5e538a6e74c1fed77b35a6d1d818 Mon Sep 17 00:00:00 2001 From: erichkeane <[email protected]> Date: Tue, 6 Oct 2026 08:47:35 -0700 Subject: [PATCH 1/2] [CIR] Fix consteval store of no_unqiue_address_members We were unconditionally overwriting the members of a consteval type, even if there was padding / no_unique_address in the way. This patch corrects that behavior by picking up the relevant parts of #93115 which ensures we only write the constexpr initialized fields. --- clang/lib/CIR/CodeGen/CIRGenExprAggregate.cpp | 41 ++++- clang/test/CIR/CodeGen/agg-init-constexpr.cpp | 14 +- .../no-unique-address-empty-consteval.cpp | 150 ++++++++++++++++++ 3 files changed, 197 insertions(+), 8 deletions(-) create mode 100644 clang/test/CIR/CodeGen/no-unique-address-empty-consteval.cpp diff --git a/clang/lib/CIR/CodeGen/CIRGenExprAggregate.cpp b/clang/lib/CIR/CodeGen/CIRGenExprAggregate.cpp index 81a6eae7effcf9..43f7842b03b3c9 100644 --- a/clang/lib/CIR/CodeGen/CIRGenExprAggregate.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenExprAggregate.cpp @@ -340,12 +340,39 @@ class AggExprEmitter : public StmtVisitor<AggExprEmitter> { Visit(e->getSelectedExpr()); } void VisitConstantExpr(ConstantExpr *e) { - ensureDest(cgf.getLoc(e->getSourceRange()), e->getType()); + mlir::Location loc = cgf.getLoc(e->getSourceRange()); + ensureDest(loc, e->getType()); if (mlir::Attribute result = ConstantEmitter(cgf).tryEmitConstantExpr(e)) { + QualType ty = e->getType(); + + // If the destination's tail padding may overlap another object's + // storage (e.g. a [[no_unique_address]] member or base), only store + // the type's data size rather than its full size, so we don't + // clobber bytes that belong to that other object. In particular, a + // genuinely empty class has a data size of zero, so nothing should + // be stored at all. + if (dest.mayOverlap()) { + CharUnits dataSize = + cgf.getContext().getTypeInfoDataSizeInChars(ty).Width; + if (dataSize.isZero()) + return; + + if (dataSize != cgf.getContext().getTypeSizeInChars(ty)) { + Address temp = cgf.createMemTemp(ty, loc); + mlir::Value resultVal = cgf.getBuilder().getConstant( + loc, mlir::cast<mlir::TypedAttr>(result)); + cgf.getBuilder().createStore(loc, resultVal, temp); + cgf.getBuilder().createCopy(dest.getAddress(), temp, + /*isVolatile=*/false, + /*skipTailPadding=*/true); + return; + } + } + mlir::Value resultVal = cgf.getBuilder().getConstant( - cgf.getLoc(e->getSourceRange()), mlir::cast<mlir::TypedAttr>(result)); - LValue destLVal = cgf.makeAddrLValue(dest.getAddress(), e->getType()); + loc, mlir::cast<mlir::TypedAttr>(result)); + LValue destLVal = cgf.makeAddrLValue(dest.getAddress(), ty); cgf.emitStoreThroughLValue(RValue::get(resultVal), destLVal); return; } @@ -1257,6 +1284,10 @@ AggValueSlot::Overlap_t CIRGenFunction::getOverlapForBaseInit( if (isVirtual) return AggValueSlot::MayOverlap; + // Empty bases can overlap earlier bases. + if (baseRD->isEmpty()) + return AggValueSlot::MayOverlap; + // If the base class is laid out entirely within the nvsize of the derived // class, its tail padding cannot yet be initialized, so we can issue // stores at the full width of the base class. @@ -1347,6 +1378,10 @@ CIRGenFunction::getOverlapForFieldInit(const FieldDecl *fd) { if (!fd->hasAttr<NoUniqueAddressAttr>() || !fd->getType()->isRecordType()) return AggValueSlot::DoesNotOverlap; + // Empty fields can overlap earlier fields. + if (fd->getType()->getAsCXXRecordDecl()->isEmpty()) + return AggValueSlot::MayOverlap; + // If the field lies entirely within the enclosing class's nvsize, its tail // padding cannot overlap any already-initialized object. (The only subobjects // with greater addresses that might already be initialized are vbases.) diff --git a/clang/test/CIR/CodeGen/agg-init-constexpr.cpp b/clang/test/CIR/CodeGen/agg-init-constexpr.cpp index 250d10faa6e1de..43809c723f9536 100644 --- a/clang/test/CIR/CodeGen/agg-init-constexpr.cpp +++ b/clang/test/CIR/CodeGen/agg-init-constexpr.cpp @@ -19,17 +19,21 @@ extern "C" void construct() { WithCtor c(2,5); } -// CIR: cir.global "private" constant cir_private @__const.construct.c = #cir.const_record<{#cir.int<4> : !s32i, #cir.zero : !cir.array<!u8i x 4>, #cir.int<10> : !s64i, #cir.const_record<{#cir.int<5> : !s32i}> : !rec_HasVal, #cir.zero : !cir.array<!u8i x 4>}> : !rec_WithCtor +// CIR: cir.global "private" constant cir_private @__const.construct.tmp = #cir.const_record<{#cir.int<4> : !s32i, #cir.zero : !cir.array<!u8i x 4>, #cir.int<10> : !s64i, #cir.const_record<{#cir.int<5> : !s32i}> : !rec_HasVal, #cir.zero : !cir.array<!u8i x 4>}> : !rec_WithCtor // CIR-LABEL: construct() // CIR-NEXT: %[[WC_ALLOCA:.*]] = cir.alloca "c" {{.*}} : !cir.ptr<!rec_WithCtor> -// CIR-NEXT: %[[GET_GLOBAL:.*]] = cir.get_global @__const.construct.c : !cir.ptr<!rec_WithCtor> -// CIR-NEXT: cir.copy %[[GET_GLOBAL]] align(8) to %[[WC_ALLOCA]] align(8) : !cir.ptr<!rec_WithCtor> +// CIR-NEXT: %[[TMP_ALLOCA:.*]] = cir.alloca "tmp" {{.*}} : !cir.ptr<!rec_WithCtor> +// CIR-NEXT: %[[GET_GLOBAL:.*]] = cir.get_global @__const.construct.tmp : !cir.ptr<!rec_WithCtor> +// CIR-NEXT: cir.copy %[[GET_GLOBAL]] align(8) to %[[TMP_ALLOCA]] align(8) : !cir.ptr<!rec_WithCtor> +// CIR-NEXT: cir.copy %[[TMP_ALLOCA]] align(8) to %[[WC_ALLOCA]] align(8) skip_tail_padding : !cir.ptr<!rec_WithCtor> // CIR-NEXT: cir.return -// LLVM: @__const.construct.c = private constant %struct.WithCtor <{ i32 4, [4 x i8] zeroinitializer, i64 10, %struct.HasVal { i32 5 }, [4 x i8] zeroinitializer }> +// LLVM: @__const.construct.tmp = private constant %struct.WithCtor <{ i32 4, [4 x i8] zeroinitializer, i64 10, %struct.HasVal { i32 5 }, [4 x i8] zeroinitializer }> // LLVM-LABEL: construct() // LLVM-NEXT: %[[WC_ALLOCA:.*]] = alloca %struct.WithCtor -// LLVM-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 8 %1, ptr align 8 @__const.construct.c, i64 24, i1 false) +// LLVM-NEXT: %[[TMP_ALLOCA:.*]] = alloca %struct.WithCtor +// LLVM-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 8 %[[TMP_ALLOCA]], ptr align 8 @__const.construct.tmp, i64 24, i1 false) +// LLVM-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr align 8 %[[WC_ALLOCA]], ptr align 8 %[[TMP_ALLOCA]], i64 20, i1 false) // LLVM-NEXT: ret void // OGCG-LABEL: construct() diff --git a/clang/test/CIR/CodeGen/no-unique-address-empty-consteval.cpp b/clang/test/CIR/CodeGen/no-unique-address-empty-consteval.cpp new file mode 100644 index 00000000000000..0691d808b55825 --- /dev/null +++ b/clang/test/CIR/CodeGen/no-unique-address-empty-consteval.cpp @@ -0,0 +1,150 @@ +// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu \ +// RUN: -fclangir -emit-cir %s -o %t.cir +// RUN: FileCheck --check-prefix=CIR --input-file=%t.cir %s +// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu \ +// RUN: -fclangir -emit-llvm %s -o %t.ll +// RUN: FileCheck --check-prefix=LLVM --input-file=%t.ll %s +// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu \ +// RUN: -emit-llvm %s -o %t.og.ll +// RUN: FileCheck --check-prefix=OGCG --input-file=%t.og.ll %s + +// Regression test: a [[no_unique_address]] empty member/base initialized by +// a consteval constructor is folded by Sema into a ConstantExpr. CIRGen's +// AggExprEmitter::VisitConstantExpr used to unconditionally store the +// folded (zero-size) constant at the member's address. Because a truly +// empty [[no_unique_address]] subobject overlaps the storage of an earlier +// field, that store clobbered the field's just-initialized value. +// VisitConstantExpr must honor the destination's overlap flag and skip the +// store entirely when the type's data size is zero. +// +// The CHECK blocks below are ordered to match the actual emission order +// (base-object '*C2' constructors are grouped together after all the +// '*C1' complete-object constructors and free functions), not the source +// order of the structs. + +struct Empty { + consteval Empty(int) {} +}; + +// Member case: 'e' is empty and [[no_unique_address]], so it is laid out at +// offset 0, overlapping 'v'. Initializing 'e' must not touch 'v's bytes. +struct S { + unsigned long v; + [[no_unique_address]] Empty e; + S(unsigned long x) : v(x), e(0) {} +}; + +unsigned long test_member() { + S s(0x1234); + return s.v; +} + +// Base case: EmptyBase is empty, so empty-base optimization places it at the +// same address as 'v2'. +struct EmptyBase { + consteval EmptyBase(int) {} +}; + +struct T : EmptyBase { + unsigned long v2; + T(unsigned long x) : EmptyBase(0), v2(x) {} +}; + +unsigned long test_base() { + T t(0x5678); + return t.v2; +} + +// Aggregate-initialization case: the init-list path already requests +// MayOverlap for TEK_Aggregate fields, so this exercises the same fix in +// VisitConstantExpr via a different caller. +struct U { + unsigned long v3; + [[no_unique_address]] Empty e; +}; + +// CIR-LABEL: cir.func {{.*}} @_Z19test_aggregate_initv( +// CIR: %[[V3:.*]] = cir.get_member %{{.+}}[0] {name = "v3"} +// CIR-NEXT: %[[C:.*]] = cir.const #cir.int<39612> +// CIR-NEXT: cir.store align(8) %[[C]], %[[V3]] : !u64i, !cir.ptr<!u64i> +// CIR-NOT: #cir.zero : !rec_Empty + +// LLVM-LABEL: define {{.*}} i64 @_Z19test_aggregate_initv( +// LLVM: %[[V3:.*]] = getelementptr inbounds nuw %struct.U, ptr %{{.+}}, i32 0, i32 0 +// LLVM-NEXT: store i64 39612, ptr %[[V3]], align 8 + +// OGCG-LABEL: define {{.*}} i64 @_Z19test_aggregate_initv( +// OGCG: %[[V3:.*]] = getelementptr inbounds nuw %struct.U, ptr %{{.+}}, i32 0, i32 0 +// OGCG-NEXT: store i64 39612, ptr %[[V3]], align 8 + +unsigned long test_aggregate_init() { + U u{0x9abc, 0}; + return u.v3; +} + +// Default-member-initializer case. +struct V { + unsigned long v4; + [[no_unique_address]] Empty e = Empty(0); + V(unsigned long x) : v4(x) {} +}; + +unsigned long test_default_member_init() { + V v(0xdef0); + return v.v4; +} + +// CIR-LABEL: cir.func {{.*}} @_ZN1SC2Em( +// CIR: %[[V:.*]] = cir.get_member %{{.+}}[0] {name = "v"} +// CIR: %[[X:.*]] = cir.load align(8) %{{.+}} : !cir.ptr<!u64i>, !u64i +// CIR-NEXT: cir.store align(8) %[[X]], %[[V]] : !u64i, !cir.ptr<!u64i> +// CIR-NEXT: cir.return + +// LLVM-LABEL: define {{.*}} void @_ZN1SC2Em( +// LLVM: %[[V:.*]] = getelementptr inbounds nuw %struct.S, ptr %{{.+}}, i32 0, i32 0 +// LLVM: %[[X:.*]] = load i64, ptr %{{.+}}, align 8 +// LLVM-NEXT: store i64 %[[X]], ptr %[[V]], align 8 +// LLVM-NEXT: ret void + +// OGCG-LABEL: define {{.*}} void @_ZN1SC2Em( +// OGCG: %[[V:.*]] = getelementptr inbounds nuw %struct.S, ptr %{{.+}}, i32 0, i32 0 +// OGCG: %[[X:.*]] = load i64, ptr %{{.+}}, align 8 +// OGCG-NEXT: store i64 %[[X]], ptr %[[V]], align 8 +// OGCG-NEXT: ret void + +// CIR-LABEL: cir.func {{.*}} @_ZN1TC2Em( +// CIR: cir.base_class_addr +// CIR: %[[V2:.*]] = cir.get_member %{{.+}}[0] {name = "v2"} +// CIR: %[[X2:.*]] = cir.load align(8) %{{.+}} : !cir.ptr<!u64i>, !u64i +// CIR-NEXT: cir.store align(8) %[[X2]], %[[V2]] : !u64i, !cir.ptr<!u64i> +// CIR-NEXT: cir.return + +// LLVM-LABEL: define {{.*}} void @_ZN1TC2Em( +// LLVM: %[[V2:.*]] = getelementptr inbounds nuw %struct.T, ptr %{{.+}}, i32 0, i32 0 +// LLVM: %[[X2:.*]] = load i64, ptr %{{.+}}, align 8 +// LLVM-NEXT: store i64 %[[X2]], ptr %[[V2]], align 8 +// LLVM-NEXT: ret void + +// OGCG-LABEL: define {{.*}} void @_ZN1TC2Em( +// OGCG: %[[V2:.*]] = getelementptr inbounds nuw %struct.T, ptr %{{.+}}, i32 0, i32 0 +// OGCG: %[[X2:.*]] = load i64, ptr %{{.+}}, align 8 +// OGCG-NEXT: store i64 %[[X2]], ptr %[[V2]], align 8 +// OGCG-NEXT: ret void + +// CIR-LABEL: cir.func {{.*}} @_ZN1VC2Em( +// CIR: %[[V4:.*]] = cir.get_member %{{.+}}[0] {name = "v4"} +// CIR: %[[X4:.*]] = cir.load align(8) %{{.+}} : !cir.ptr<!u64i>, !u64i +// CIR-NEXT: cir.store align(8) %[[X4]], %[[V4]] : !u64i, !cir.ptr<!u64i> +// CIR-NEXT: cir.return + +// LLVM-LABEL: define {{.*}} void @_ZN1VC2Em( +// LLVM: %[[V4:.*]] = getelementptr inbounds nuw %struct.V, ptr %{{.+}}, i32 0, i32 0 +// LLVM: %[[X4:.*]] = load i64, ptr %{{.+}}, align 8 +// LLVM-NEXT: store i64 %[[X4]], ptr %[[V4]], align 8 +// LLVM-NEXT: ret void + +// OGCG-LABEL: define {{.*}} void @_ZN1VC2Em( +// OGCG: %[[V4:.*]] = getelementptr inbounds nuw %struct.V, ptr %{{.+}}, i32 0, i32 0 +// OGCG: %[[X4:.*]] = load i64, ptr %{{.+}}, align 8 +// OGCG-NEXT: store i64 %[[X4]], ptr %[[V4]], align 8 +// OGCG-NEXT: ret void >From 7e6282edb35ee61bff1d4a4eb80b5e272d3ef76c Mon Sep 17 00:00:00 2001 From: erichkeane <[email protected]> Date: Tue, 6 Oct 2026 15:51:37 -0700 Subject: [PATCH 2/2] update test to remove OGCG --- .../no-unique-address-empty-consteval.cpp | 24 +------------------ 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/clang/test/CIR/CodeGen/no-unique-address-empty-consteval.cpp b/clang/test/CIR/CodeGen/no-unique-address-empty-consteval.cpp index 0691d808b55825..403a191a40f751 100644 --- a/clang/test/CIR/CodeGen/no-unique-address-empty-consteval.cpp +++ b/clang/test/CIR/CodeGen/no-unique-address-empty-consteval.cpp @@ -6,7 +6,7 @@ // RUN: FileCheck --check-prefix=LLVM --input-file=%t.ll %s // RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-linux-gnu \ // RUN: -emit-llvm %s -o %t.og.ll -// RUN: FileCheck --check-prefix=OGCG --input-file=%t.og.ll %s +// RUN: FileCheck --check-prefix=LLVM --input-file=%t.og.ll %s // Regression test: a [[no_unique_address]] empty member/base initialized by // a consteval constructor is folded by Sema into a ConstantExpr. CIRGen's @@ -73,10 +73,6 @@ struct U { // LLVM: %[[V3:.*]] = getelementptr inbounds nuw %struct.U, ptr %{{.+}}, i32 0, i32 0 // LLVM-NEXT: store i64 39612, ptr %[[V3]], align 8 -// OGCG-LABEL: define {{.*}} i64 @_Z19test_aggregate_initv( -// OGCG: %[[V3:.*]] = getelementptr inbounds nuw %struct.U, ptr %{{.+}}, i32 0, i32 0 -// OGCG-NEXT: store i64 39612, ptr %[[V3]], align 8 - unsigned long test_aggregate_init() { U u{0x9abc, 0}; return u.v3; @@ -106,12 +102,6 @@ unsigned long test_default_member_init() { // LLVM-NEXT: store i64 %[[X]], ptr %[[V]], align 8 // LLVM-NEXT: ret void -// OGCG-LABEL: define {{.*}} void @_ZN1SC2Em( -// OGCG: %[[V:.*]] = getelementptr inbounds nuw %struct.S, ptr %{{.+}}, i32 0, i32 0 -// OGCG: %[[X:.*]] = load i64, ptr %{{.+}}, align 8 -// OGCG-NEXT: store i64 %[[X]], ptr %[[V]], align 8 -// OGCG-NEXT: ret void - // CIR-LABEL: cir.func {{.*}} @_ZN1TC2Em( // CIR: cir.base_class_addr // CIR: %[[V2:.*]] = cir.get_member %{{.+}}[0] {name = "v2"} @@ -125,12 +115,6 @@ unsigned long test_default_member_init() { // LLVM-NEXT: store i64 %[[X2]], ptr %[[V2]], align 8 // LLVM-NEXT: ret void -// OGCG-LABEL: define {{.*}} void @_ZN1TC2Em( -// OGCG: %[[V2:.*]] = getelementptr inbounds nuw %struct.T, ptr %{{.+}}, i32 0, i32 0 -// OGCG: %[[X2:.*]] = load i64, ptr %{{.+}}, align 8 -// OGCG-NEXT: store i64 %[[X2]], ptr %[[V2]], align 8 -// OGCG-NEXT: ret void - // CIR-LABEL: cir.func {{.*}} @_ZN1VC2Em( // CIR: %[[V4:.*]] = cir.get_member %{{.+}}[0] {name = "v4"} // CIR: %[[X4:.*]] = cir.load align(8) %{{.+}} : !cir.ptr<!u64i>, !u64i @@ -142,9 +126,3 @@ unsigned long test_default_member_init() { // LLVM: %[[X4:.*]] = load i64, ptr %{{.+}}, align 8 // LLVM-NEXT: store i64 %[[X4]], ptr %[[V4]], align 8 // LLVM-NEXT: ret void - -// OGCG-LABEL: define {{.*}} void @_ZN1VC2Em( -// OGCG: %[[V4:.*]] = getelementptr inbounds nuw %struct.V, ptr %{{.+}}, i32 0, i32 0 -// OGCG: %[[X4:.*]] = load i64, ptr %{{.+}}, align 8 -// OGCG-NEXT: store i64 %[[X4]], ptr %[[V4]], align 8 -// OGCG-NEXT: ret void _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
