[PATCH] D157332: [clang] Make init for empty no_unique_address fields a no-op write

[Martin Storsjö via Phabricator via cfe-commits]

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;
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D157332.547951.patch
Type: text/x-patch
Size: 1989 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230807/85c8bbe0/attachment.bin>