https://github.com/erichkeane updated https://github.com/llvm/llvm-project/pull/228506
>From 791edcc985a374d4e0e72138fb3f77c55d22a884 Mon Sep 17 00:00:00 2001 From: erichkeane <[email protected]> Date: Fri, 2 Oct 2026 09:23:16 -0700 Subject: [PATCH 1/2] [CIR] Correct const lowering with potentially-overlapping-types. The below bug report found a case where our base lowering and our const-record lowering got out of sync with no unique address, so this patch makes sure we get it right. Fixes: #228300 --- clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp | 28 ++++++++++++++++++-- clang/test/CIR/CodeGen/no-unique-address.cpp | 23 ++++++++++++++-- 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp b/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp index 46aabba5b567a..db7a070adcf10 100644 --- a/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp @@ -286,6 +286,24 @@ setBitfieldInit(CIRGenModule &cgm, const CIRGenRecordLayout &cirLayout, field->getType()->isSignedIntegerOrEnumerationType(), info); } +// Handle potentially-overlapping field rewrite for the const subobject records. +mlir::Attribute asBaseSubobject(CIRGenBuilderTy &builder, mlir::Attribute attr, + cir::RecordType baseSubobjTy) { + auto typedAttr = mlir::cast<mlir::TypedAttr>(attr); + if (typedAttr.getType() == baseSubobjTy) + return attr; + + if (mlir::isa<cir::ZeroAttr>(attr)) + return builder.getZeroInitAttr(baseSubobjTy); + + auto recordAttr = mlir::cast<cir::ConstRecordAttr>(attr); + mlir::ArrayAttr members = recordAttr.getMembers(); + llvm::SmallVector<mlir::Attribute> baseMembers( + members.begin(), members.begin() + baseSubobjTy.getNumElements()); + return cir::ConstRecordAttr::get(baseSubobjTy, + builder.getArrayAttr(baseMembers)); +} + mlir::Attribute buildRecordHelper(ConstantEmitter &emitter, const RecordDecl *rd, const RecordDecl *vtableBaseTy, @@ -406,11 +424,17 @@ mlir::Attribute buildRecordHelper(ConstantEmitter &emitter, if (!eltAttr) return {}; - if (field->isBitField()) + if (field->isBitField()) { elements[fieldIdx] = setBitfieldInit(cgm, cirLayout, builder, field, elements[fieldIdx], eltAttr); - else + } else { + if (field->isPotentiallyOverlapping()) { + if (auto expectedTy = mlir::dyn_cast<cir::RecordType>( + recordTy.getMembers()[fieldIdx])) + eltAttr = asBaseSubobject(builder, eltAttr, expectedTy); + } elements[fieldIdx] = eltAttr; + } } // Anything we haven't initialized, we try to zero init. We could/should diff --git a/clang/test/CIR/CodeGen/no-unique-address.cpp b/clang/test/CIR/CodeGen/no-unique-address.cpp index bad94dae9c417..87640bd98d97f 100644 --- a/clang/test/CIR/CodeGen/no-unique-address.cpp +++ b/clang/test/CIR/CodeGen/no-unique-address.cpp @@ -68,6 +68,7 @@ struct Outer { // LLVM-DAG: %struct.UnionAllEmptyBits.base = type { [3 x i8] } // LLVM-DAG: @oubp = {{(dso_local )?}}global %struct.OuterUnionBitPad zeroinitializer, align 8 // LLVM-DAG: @oaeb = {{(dso_local )?}}global %struct.OuterAllEmptyBits zeroinitializer, align 8 +// LLVM-DAG: @ndo = {{(dso_local )?}}global %struct.NUADtorOuter { %struct.NUADtor.base <{ i32 42, [3 x i8] zeroinitializer }>, i8 0 } // OGCG-DAG: %struct.OuterUnion = type { %union.UnionForNUA, i32 } // OGCG-DAG: %union.UnionForNUA = type { i64 } // OGCG-DAG: %struct.OuterFinal = type { %struct.FinalForNUA, i8 } @@ -94,6 +95,7 @@ struct Outer { // OGCG-DAG: %union.UnionAllEmptyBits.base = type { [3 x i8] } // OGCG-DAG: @oubp = {{(dso_local )?}}global %struct.OuterUnionBitPad zeroinitializer, align 8 // OGCG-DAG: @oaeb = {{(dso_local )?}}global %struct.OuterAllEmptyBits zeroinitializer, align 8 +// OGCG-DAG: @ndo = {{(dso_local )?}}global { i32, [3 x i8], i8 } { i32 42, [3 x i8] zeroinitializer, i8 0 }, align 4 // LLVM-LABEL: define {{.*}} void @_ZN5OuterC2ERK6Middlec( // LLVM: %[[GEP:.*]] = getelementptr inbounds nuw %struct.Outer, ptr %{{.+}}, i32 0, i32 0 @@ -127,8 +129,7 @@ struct OuterUnion { OuterUnion ou; struct FinalForNUA final { - int a; - char b; + int a; char b; }; struct OuterFinal { @@ -322,3 +323,21 @@ OuterAllEmpty oae; // CIR-NUA-DAG: !rec_UnionZeroDataSize = !cir.union<"UnionZeroDataSize" {empty !rec_EmptyForNUA, data !s32i}> // CIR-NUA-DAG: !rec_OuterZeroData = !cir.struct<"OuterZeroData" {data !rec_UnionZeroDataSize, data !cir.bool}> // CIR-NUA-DAG: cir.global external @ozd = #cir.zero : !rec_OuterZeroData + +struct NUADtor { + ~NUADtor(); + int n; + char c[3]; +}; + +struct NUADtorOuter { + [[no_unique_address]] NUADtor a; + char k; +}; + +NUADtorOuter ndo = {42}; + +// CIR-NUA-DAG: !rec_NUADtor2Ebase = !cir.struct<"NUADtor.base" packed {data !s32i, data !cir.array<!s8i x 3>}> +// CIR-NUA-DAG: !rec_NUADtorOuter = !cir.struct<"NUADtorOuter" {data !rec_NUADtor2Ebase, data !s8i}> +// CIR-NUA-DAG: cir.global external {{.*}}@ndo = #cir.const_record<{#cir.const_record<{#cir.int<42> : !s32i, #cir.zero : !cir.array<!s8i x 3>}> : !rec_NUADtor2Ebase, #cir.int<0> : !s8i}> : !rec_NUADtorOuter + >From 7478b9314f978da4849c931db7ddb643d0e6fc3b Mon Sep 17 00:00:00 2001 From: erichkeane <[email protected]> Date: Fri, 2 Oct 2026 12:55:10 -0700 Subject: [PATCH 2/2] Add assert bruno requested --- clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp | 23 ++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp b/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp index db7a070adcf10..1d7206f430c8c 100644 --- a/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp @@ -300,6 +300,29 @@ mlir::Attribute asBaseSubobject(CIRGenBuilderTy &builder, mlir::Attribute attr, mlir::ArrayAttr members = recordAttr.getMembers(); llvm::SmallVector<mlir::Attribute> baseMembers( members.begin(), members.begin() + baseSubobjTy.getNumElements()); + + // The base-subobject type is assumed to be a prefix of the complete-object + // type (i.e. the complete type minus tail padding). Verify that assumption + // holds so a layout change doesn't silently produce a wrong initializer. + // + // ConstRecordAttr's members skip zero-width bit-fields and store a + // bit-field's storage type rather than the bit-field type itself (see + // ConstRecordAttr::verify), so baseSubobjTy's raw member list has to be + // filtered the same way before comparing against baseMembers index-for- + // index. + assert(llvm::all_of( + llvm::zip_equal(baseMembers, + llvm::map_range(llvm::make_filter_range( + baseSubobjTy.getMembers(), + cir::memberOwnsBytes), + cir::memberStorageType)), + [](const auto &pair) { + auto &[member, baseTy] = pair; + return mlir::cast<mlir::TypedAttr>(member).getType() == baseTy; + }) && + "base-subobject member type does not match complete-object member " + "type at the same index"); + return cir::ConstRecordAttr::get(baseSubobjTy, builder.getArrayAttr(baseMembers)); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
