mstorsjo created this revision.
mstorsjo added reviewers: aaron.ballman, efriedma.
Herald added subscribers: steven.zhang, pengfei.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a project: clang.
An empty struct is handled as a struct with a dummy i8, on all targets.
Most targets treat an empty struct return value as essentially
void - but some don't. (Currently, at least x86_64-windows-* and
powerpc64le-* don't treat it as void.)
When intializing a struct with such a no_unique_address member,
make sure we don't write the dummy i8 into the struct where there's
no space allocated for it.
Previously it would clobber the actual valid data of the struct.
The existing clang tests in CodeGenCXX/tail-padding.cpp contain
cases of fields that have no_unique_address on non-empty struct
members; the check for isEmptyRecord() is needed to retain the
previous behaviour in that test.
Fixes https://github.com/llvm/llvm-project/issues/64253, and
possibly https://github.com/llvm/llvm-project/issues/64077
and https://github.com/llvm/llvm-project/issues/64427 as well.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D157332
Files:
clang/lib/CodeGen/CGClass.cpp
clang/test/CodeGenCXX/ctor-empty-nounique.cpp
Index: clang/test/CodeGenCXX/ctor-empty-nounique.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/ctor-empty-nounique.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -triple x86_64-windows-gnu -emit-llvm -o - %s | FileCheck %s
+
+// An empty struct is handled as a struct with a dummy i8, on all targets.
+// Most targets treat an empty struct return value as essentially void - but
+// some don't. (Currently, at least x86_64-windows-* and powerpc64le-* don't
+// treat it as void.)
+//
+// When intializing a struct with such a no_unique_address member, make sure we
+// don't write the dummy i8 into the struct where there's no space allocated
for
+// it.
+//
+// This can only be tested with targets that don't treat empty struct returns
as
+// void.
+
+struct S {};
+S f();
+struct Z {
+ int x;
+ [[no_unique_address]] S y;
+ Z();
+};
+Z::Z() : x(111), y(f()) {}
+
+// CHECK: define {{.*}} @_ZN1ZC2Ev
+
+// CHECK: %coerce = alloca %struct.S, align 1
+
+// CHECK: %call = call i8 @_Z1fv()
+// CHECK-NEXT: %coerce.dive = getelementptr inbounds %struct.S, ptr %coerce,
i32 0, i32 0
+// CHECK-NEXT: store i8 %call, ptr %coerce.dive, align 1
+// CHECK-NEXT: ret void
Index: clang/lib/CodeGen/CGClass.cpp
===================================================================
--- clang/lib/CodeGen/CGClass.cpp
+++ clang/lib/CodeGen/CGClass.cpp
@@ -10,6 +10,7 @@
//
//===----------------------------------------------------------------------===//
+#include "ABIInfoImpl.h"
#include "CGBlocks.h"
#include "CGCXXABI.h"
#include "CGDebugInfo.h"
@@ -701,6 +702,9 @@
getOverlapForFieldInit(Field), AggValueSlot::IsNotZeroed,
// Checks are made by the code that calls constructor.
AggValueSlot::IsSanitizerChecked);
+ if (Field->hasAttr<NoUniqueAddressAttr>() &&
+ isEmptyRecord(getContext(), FieldType, true))
+ Slot = AggValueSlot::ignored();
EmitAggExpr(Init, Slot);
break;
}
Index: clang/test/CodeGenCXX/ctor-empty-nounique.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/ctor-empty-nounique.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -triple x86_64-windows-gnu -emit-llvm -o - %s | FileCheck %s
+
+// An empty struct is handled as a struct with a dummy i8, on all targets.
+// Most targets treat an empty struct return value as essentially void - but
+// some don't. (Currently, at least x86_64-windows-* and powerpc64le-* don't
+// treat it as void.)
+//
+// When intializing a struct with such a no_unique_address member, make sure we
+// don't write the dummy i8 into the struct where there's no space allocated for
+// it.
+//
+// This can only be tested with targets that don't treat empty struct returns as
+// void.
+
+struct S {};
+S f();
+struct Z {
+ int x;
+ [[no_unique_address]] S y;
+ Z();
+};
+Z::Z() : x(111), y(f()) {}
+
+// CHECK: define {{.*}} @_ZN1ZC2Ev
+
+// CHECK: %coerce = alloca %struct.S, align 1
+
+// CHECK: %call = call i8 @_Z1fv()
+// CHECK-NEXT: %coerce.dive = getelementptr inbounds %struct.S, ptr %coerce, i32 0, i32 0
+// CHECK-NEXT: store i8 %call, ptr %coerce.dive, align 1
+// CHECK-NEXT: ret void
Index: clang/lib/CodeGen/CGClass.cpp
===================================================================
--- clang/lib/CodeGen/CGClass.cpp
+++ clang/lib/CodeGen/CGClass.cpp
@@ -10,6 +10,7 @@
//
//===----------------------------------------------------------------------===//
+#include "ABIInfoImpl.h"
#include "CGBlocks.h"
#include "CGCXXABI.h"
#include "CGDebugInfo.h"
@@ -701,6 +702,9 @@
getOverlapForFieldInit(Field), AggValueSlot::IsNotZeroed,
// Checks are made by the code that calls constructor.
AggValueSlot::IsSanitizerChecked);
+ if (Field->hasAttr<NoUniqueAddressAttr>() &&
+ isEmptyRecord(getContext(), FieldType, true))
+ Slot = AggValueSlot::ignored();
EmitAggExpr(Init, Slot);
break;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
