[clang] [MS] Follow up fix to pass aligned args to variadic x86_32 functions (PR #65692)

Reid Kleckner via cfe-commits cfe-commits at lists.llvm.org
Thu Sep 7 16:04:58 PDT 2023


https://github.com/rnk created https://github.com/llvm/llvm-project/pull/65692:

MSVC allows users to pass structures with required alignments greater than 4 to variadic functions. It does not pass them indirectly to correctly align them. Instead, it passes them directly with the usual 4 byte stack alignment.

This change implements the same logic in clang on the passing side.  The receiving side (va_arg) never implemented any of this indirect logic, so it doesn't need to be updated.

This issue pre-existed, but @aaron.ballman noticed it when we started passing structs containing aligned fields indirectly in D152752.

>From 98d560c8057b171c81b43d93c1a0c26f1d27cf5b Mon Sep 17 00:00:00 2001
From: Reid Kleckner <rnk at ad.corp.google.com>
Date: Tue, 29 Aug 2023 14:26:10 -0700
Subject: [PATCH] [MS] Follow up fix to pass aligned args to variadic x86_32
 functions

MSVC allows users to pass structures with required alignments greater
than 4 to variadic functions. It does not pass them indirectly to
correctly align them. Instead, it passes them directly with the usual 4
byte stack alignment.

This change implements the same logic in clang on the passing side.  The
receiving side (va_arg) never implemented any of this indirect logic, so
it doesn't need to be updated.

This issue pre-existed, but @aaron.ballman noticed it when we started
passing structs containing aligned fields indirectly in D152752.
---
 clang/lib/CodeGen/Targets/X86.cpp             | 27 ++++++++++-------
 .../test/CodeGen/X86/x86_32-arguments-win32.c | 29 +++++++++++++++++++
 2 files changed, 45 insertions(+), 11 deletions(-)

diff --git a/clang/lib/CodeGen/Targets/X86.cpp b/clang/lib/CodeGen/Targets/X86.cpp
index 9f5c3258d65cb47..f04e0c67a807e1a 100644
--- a/clang/lib/CodeGen/Targets/X86.cpp
+++ b/clang/lib/CodeGen/Targets/X86.cpp
@@ -87,12 +87,15 @@ static ABIArgInfo getDirectX86Hva(llvm::Type* T = nullptr) {
 /// Similar to llvm::CCState, but for Clang.
 struct CCState {
   CCState(CGFunctionInfo &FI)
-      : IsPreassigned(FI.arg_size()), CC(FI.getCallingConvention()) {}
+      : IsPreassigned(FI.arg_size()), CC(FI.getCallingConvention()),
+	Required(FI.getRequiredArgs()), IsDelegateCall(FI.isDelegateCall()) {}
 
   llvm::SmallBitVector IsPreassigned;
   unsigned CC = CallingConv::CC_C;
   unsigned FreeRegs = 0;
   unsigned FreeSSERegs = 0;
+  RequiredArgs Required;
+  bool IsDelegateCall = false;
 };
 
 /// X86_32ABIInfo - The X86-32 ABI information.
@@ -141,7 +144,7 @@ class X86_32ABIInfo : public ABIInfo {
   Class classify(QualType Ty) const;
   ABIArgInfo classifyReturnType(QualType RetTy, CCState &State) const;
   ABIArgInfo classifyArgumentType(QualType RetTy, CCState &State,
-                                  bool isDelegateCall) const;
+                                  unsigned ArgIndex) const;
 
   /// Updates the number of available free registers, returns
   /// true if any registers were allocated.
@@ -739,7 +742,7 @@ void X86_32ABIInfo::runVectorCallFirstPass(CGFunctionInfo &FI, CCState &State) c
 }
 
 ABIArgInfo X86_32ABIInfo::classifyArgumentType(QualType Ty, CCState &State,
-                                               bool isDelegateCall) const {
+                                               unsigned ArgIndex) const {
   // FIXME: Set alignment on indirect arguments.
   bool IsFastCall = State.CC == llvm::CallingConv::X86_FastCall;
   bool IsRegCall = State.CC == llvm::CallingConv::X86_RegCall;
@@ -754,7 +757,7 @@ ABIArgInfo X86_32ABIInfo::classifyArgumentType(QualType Ty, CCState &State,
     CGCXXABI::RecordArgABI RAA = getRecordArgABI(RT, getCXXABI());
     if (RAA == CGCXXABI::RAA_Indirect) {
       return getIndirectResult(Ty, false, State);
-    } else if (isDelegateCall) {
+    } else if (State.IsDelegateCall) {
       // Avoid having different alignments on delegate call args by always
       // setting the alignment to 4, which is what we do for inallocas.
       ABIArgInfo Res = getIndirectResult(Ty, false, State);
@@ -812,11 +815,13 @@ ABIArgInfo X86_32ABIInfo::classifyArgumentType(QualType Ty, CCState &State,
     }
     llvm::IntegerType *PaddingType = NeedsPadding ? Int32 : nullptr;
 
-    // Pass over-aligned aggregates on Windows indirectly. This behavior was
-    // added in MSVC 2015. Use the required alignment from the record layout,
-    // since that may be less than the regular type alignment, and types with
-    // required alignment of less than 4 bytes are not passed indirectly.
-    if (IsWin32StructABI) {
+    // Pass over-aligned aggregates to non-variadic functions on Windows
+    // indirectly. This behavior was added in MSVC 2015. Use the required
+    // alignment from the record layout, since that may be less than the
+    // regular type alignment, and types with required alignment of less than 4
+    // bytes are not passed indirectly.
+    if (IsWin32StructABI && (!State.Required.allowsOptionalArgs() ||
+                             ArgIndex < State.Required.getNumRequiredArgs())) {
       unsigned AlignInBits = 0;
       if (RT) {
         const ASTRecordLayout &Layout =
@@ -942,13 +947,13 @@ void X86_32ABIInfo::computeInfo(CGFunctionInfo &FI) const {
 
   bool UsedInAlloca = false;
   MutableArrayRef<CGFunctionInfoArgInfo> Args = FI.arguments();
-  for (int I = 0, E = Args.size(); I < E; ++I) {
+  for (unsigned I = 0, E = Args.size(); I < E; ++I) {
     // Skip arguments that have already been assigned.
     if (State.IsPreassigned.test(I))
       continue;
 
     Args[I].info =
-        classifyArgumentType(Args[I].type, State, FI.isDelegateCall());
+        classifyArgumentType(Args[I].type, State, I);
     UsedInAlloca |= (Args[I].info.getKind() == ABIArgInfo::InAlloca);
   }
 
diff --git a/clang/test/CodeGen/X86/x86_32-arguments-win32.c b/clang/test/CodeGen/X86/x86_32-arguments-win32.c
index 53f6b5b79642c37..5b81c43f4bbb878 100644
--- a/clang/test/CodeGen/X86/x86_32-arguments-win32.c
+++ b/clang/test/CodeGen/X86/x86_32-arguments-win32.c
@@ -128,3 +128,32 @@ void pass_underaligned_record_field() {
 // CHECK: call void @receive_falign1(i64 {{[^,)]*}})
 // CHECK: call void @receive_falign4(i64 {{[^,)]*}})
 // CHECK: call void @receive_falign8(ptr {{[^,)]*}})
+
+struct __declspec(align(8)) BigAligned {
+  int big[5];
+};
+
+void receive_aligned_variadic(int f, ...);
+void pass_aligned_variadic() {
+  struct Align8 a8 = {42};
+  struct FieldAlign8 f8 = {42};
+  struct BigAligned big;
+  receive_aligned_variadic(1, a8, f8, big);
+}
+// MSVC doesn't pass aligned objects to variadic functions indirectly.
+// CHECK-LABEL: define dso_local void @pass_aligned_variadic()
+// CHECK: call void (i32, ...) @receive_aligned_variadic(i32 noundef 1, i64 %{{[^,]*}}, i64 %{{[^,]*}}, ptr noundef byval(%struct.BigAligned) align 4 %{{[^)]*}})
+
+
+void receive_fixed_align_variadic(struct BigAligned big, ...);
+void pass_fixed_align_variadic() {
+  struct BigAligned big;
+  receive_fixed_align_variadic(big, 42);
+}
+// MSVC emits error C2719 and C3916 when receiving and passing arguments with
+// required alignment greater than 4 to the fixed part of a variadic function
+// prototype, but it's actually easier to just implement this functionality
+// correctly in Clang than it is to be bug for bug compatible, so we pass such
+// arguments indirectly.
+// CHECK-LABEL: define dso_local void @pass_fixed_align_variadic()
+// CHECK: call void (ptr, ...) @receive_fixed_align_variadic(ptr noundef %{{[^)]*}}, i32 noundef 42)



More information about the cfe-commits mailing list