Author: Oliver Stannard
Date: 2025-01-29T15:02:37Z
New Revision: e9c2e0acd747b7b5260cf022233fa9b5336d2d30

URL: 
https://github.com/llvm/llvm-project/commit/e9c2e0acd747b7b5260cf022233fa9b5336d2d30
DIFF: 
https://github.com/llvm/llvm-project/commit/e9c2e0acd747b7b5260cf022233fa9b5336d2d30.diff

LOG: [AArch64] Match GCC behaviour for zero-size structs (#124760)

We had a test claiming that this empty struct type consumes a register
slot when passing it to a function with GCC, but that does not appear to
be the case, at least with GCC versions going back to 4.8.

This also caused a miscompilation when passing one of these structs to a
variadic function, but it turned out that our implementation of `va_arg`
matches GCC's ABI, so the one change fixes both bugs.

Added: 
    

Modified: 
    clang/lib/CodeGen/Targets/AArch64.cpp
    clang/test/CodeGen/AArch64/args.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/Targets/AArch64.cpp 
b/clang/lib/CodeGen/Targets/AArch64.cpp
index 057199c66f5a10..e2e434815d43af 100644
--- a/clang/lib/CodeGen/Targets/AArch64.cpp
+++ b/clang/lib/CodeGen/Targets/AArch64.cpp
@@ -415,18 +415,21 @@ ABIArgInfo AArch64ABIInfo::classifyArgumentType(QualType 
Ty, bool IsVariadicFn,
                                      CGCXXABI::RAA_DirectInMemory);
   }
 
-  // Empty records are always ignored on Darwin, but actually passed in C++ 
mode
-  // elsewhere for GNU compatibility.
+  // Empty records:
   uint64_t Size = getContext().getTypeSize(Ty);
   bool IsEmpty = isEmptyRecord(getContext(), Ty, true);
   if (!Ty->isSVESizelessBuiltinType() && (IsEmpty || Size == 0)) {
+    // Empty records are ignored in C mode, and in C++ on Darwin.
     if (!getContext().getLangOpts().CPlusPlus || isDarwinPCS())
       return ABIArgInfo::getIgnore();
 
-    // GNU C mode. The only argument that gets ignored is an empty one with 
size
-    // 0.
-    if (IsEmpty && Size == 0)
+    // In C++ mode, arguments which have sizeof() == 0 (which are non-standard
+    // C++) are ignored. This isn't defined by any standard, so we copy GCC's
+    // behaviour here.
+    if (Size == 0)
       return ABIArgInfo::getIgnore();
+
+    // Otherwise, they are passed as if they have a size of 1 byte.
     return ABIArgInfo::getDirect(llvm::Type::getInt8Ty(getVMContext()));
   }
 

diff  --git a/clang/test/CodeGen/AArch64/args.cpp 
b/clang/test/CodeGen/AArch64/args.cpp
index fe1298cc683a40..6f1f3d5e2a062b 100644
--- a/clang/test/CodeGen/AArch64/args.cpp
+++ b/clang/test/CodeGen/AArch64/args.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -triple arm64-apple-ios7.0 -target-abi darwinpcs -emit-llvm 
-o - %s | FileCheck %s
-// RUN: %clang_cc1 -triple aarch64-linux-gnu -emit-llvm -o - -x c %s | 
FileCheck %s --check-prefix=CHECK-GNU-C
-// RUN: %clang_cc1 -triple aarch64-linux-gnu -emit-llvm -o - %s | FileCheck %s 
--check-prefix=CHECK-GNU-CXX
+// RUN: %clang_cc1 -triple arm64-apple-ios7.0 -target-abi darwinpcs -emit-llvm 
-o - %s | FileCheck %s --check-prefixes=CHECK,DARWIN
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -emit-llvm -o - -x c %s | 
FileCheck %s --check-prefixes=CHECK,C
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -emit-llvm -o - %s | FileCheck %s 
--check-prefixes=CHECK,CXX
 
 // Empty structs are ignored for PCS purposes on Darwin and in C mode 
elsewhere.
 // In C++ mode on ELF they consume a register slot though. Functions are
@@ -15,16 +15,16 @@
 
 struct Empty {};
 
-// CHECK: define{{.*}} i32 @empty_arg(i32 noundef %a)
-// CHECK-GNU-C: define{{.*}} i32 @empty_arg(i32 noundef %a)
-// CHECK-GNU-CXX: define{{.*}} i32 @empty_arg(i8 %e.coerce, i32 noundef %a)
+// DARWIN: define{{.*}} i32 @empty_arg(i32 noundef %a)
+// C: define{{.*}} i32 @empty_arg(i32 noundef %a)
+// CXX: define{{.*}} i32 @empty_arg(i8 %e.coerce, i32 noundef %a)
 EXTERNC int empty_arg(struct Empty e, int a) {
   return a;
 }
 
-// CHECK: define{{.*}} void @empty_ret()
-// CHECK-GNU-C: define{{.*}} void @empty_ret()
-// CHECK-GNU-CXX: define{{.*}} void @empty_ret()
+// DARWIN: define{{.*}} void @empty_ret()
+// C: define{{.*}} void @empty_ret()
+// CXX: define{{.*}} void @empty_ret()
 EXTERNC struct Empty empty_ret(void) {
   struct Empty e;
   return e;
@@ -38,30 +38,75 @@ struct SuperEmpty {
   int arr[0];
 };
 
-// CHECK: define{{.*}} i32 @super_empty_arg(i32 noundef %a)
-// CHECK-GNU-C: define{{.*}} i32 @super_empty_arg(i32 noundef %a)
-// CHECK-GNU-CXX: define{{.*}} i32 @super_empty_arg(i32 noundef %a)
+// DARWIN: define{{.*}} i32 @super_empty_arg(i32 noundef %a)
+// C: define{{.*}} i32 @super_empty_arg(i32 noundef %a)
+// CXX: define{{.*}} i32 @super_empty_arg(i32 noundef %a)
 EXTERNC int super_empty_arg(struct SuperEmpty e, int a) {
   return a;
 }
 
-// This is not empty. It has 0 size but consumes a register slot for GCC.
+// This is also not empty, and non-standard. We previously considered it to
+// consume a register slot, but GCC does not, so we match that.
 
 struct SortOfEmpty {
   struct SuperEmpty e;
 };
 
-// CHECK: define{{.*}} i32 @sort_of_empty_arg(i32 noundef %a)
-// CHECK-GNU-C: define{{.*}} i32 @sort_of_empty_arg(i32 noundef %a)
-// CHECK-GNU-CXX: define{{.*}} i32 @sort_of_empty_arg(i8 %e.coerce, i32 
noundef %a)
-EXTERNC int sort_of_empty_arg(struct Empty e, int a) {
+// DARWIN: define{{.*}} i32 @sort_of_empty_arg(i32 noundef %a)
+// C: define{{.*}} i32 @sort_of_empty_arg(i32 noundef %a)
+// CXX: define{{.*}} i32 @sort_of_empty_arg(i32 noundef %a)
+EXTERNC int sort_of_empty_arg(struct SortOfEmpty e, int a) {
   return a;
 }
 
-// CHECK: define{{.*}} void @sort_of_empty_ret()
-// CHECK-GNU-C: define{{.*}} void @sort_of_empty_ret()
-// CHECK-GNU-CXX: define{{.*}} void @sort_of_empty_ret()
+// DARWIN: define{{.*}} void @sort_of_empty_ret()
+// C: define{{.*}} void @sort_of_empty_ret()
+// CXX: define{{.*}} void @sort_of_empty_ret()
 EXTERNC struct SortOfEmpty sort_of_empty_ret(void) {
   struct SortOfEmpty e;
   return e;
 }
+
+#include <stdarg.h>
+
+// va_arg matches the above rules, consuming an incoming argument in cases
+// where one would be passed, and not doing so when the argument should be
+// ignored.
+
+EXTERNC struct Empty empty_arg_variadic(int a, ...) {
+// CHECK-LABEL: @empty_arg_variadic(
+// DARWIN-NOT: {{ getelementptr }}
+// C-NOT: {{ getelementptr }}
+// CXX: %new_reg_offs = add i32 %gr_offs, 8
+// CXX: %new_stack = getelementptr inbounds i8, ptr %stack, i64 8
+  va_list vl;
+  va_start(vl, a);
+  struct Empty b = va_arg(vl, struct Empty);
+  va_end(vl);
+  return b;
+}
+
+EXTERNC struct SuperEmpty super_empty_arg_variadic(int a, ...) {
+// CHECK-LABEL: @super_empty_arg_variadic(
+// DARWIN-NOT: {{ getelementptr }}
+// C-NOT: {{ getelementptr }}
+// CXX-NOT: {{ getelementptr }}
+  va_list vl;
+  va_start(vl, a);
+  struct SuperEmpty b = va_arg(vl, struct SuperEmpty);
+  va_end(vl);
+  return b;
+}
+
+EXTERNC struct SortOfEmpty sort_of_empty_arg_variadic(int a, ...) {
+// CHECK-LABEL: @sort_of_empty_arg_variadic(
+// DARWIN: %argp.next = getelementptr inbounds i8, ptr %argp.cur, i64 0
+// C-NOT: {{ getelementptr }}
+// CXX-NOT: {{ getelementptr }}
+  va_list vl;
+  va_start(vl, a);
+  struct SortOfEmpty b = va_arg(vl, struct SortOfEmpty);
+  va_end(vl);
+  return b;
+}
+


        
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to