[WebAssembly] Correctly handle va_arg of zero-sized structures

[quantum]

Summary

D66168 passes size 0 structs indirectly, while the wasm backend expects it to
be passed directly. This causes subsequent variadic arguments to be read
incorrectly.

This diff changes it so that size 0 structs are passed directly.

[dschuff]

clang/test/CodeGen/wasm-varargs.c

104 ↗

(On Diff #215242)

This should maybe be called "test_empty_struct" since its size is actually 1 and not zero.

115 ↗

(On Diff #215242)

could use CHECK_LABEL here and above.

125 ↗

(On Diff #215242)

It looks like most of the output (up to here) is boilerplate for va_arg setup and duplicates the CHECKs above. It might be more clear if we just omitted the duplicated stuff and only CHECK for the things that are different from the above test.

[quantum]

Address review feedback.

clang/test/CodeGen/wasm-varargs.c

104 ↗

(On Diff #215242)

Done.

115 ↗

(On Diff #215242)

Can't because the line has match patterns.

125 ↗

(On Diff #215242)

We can't remove the duplicate checks for the second argument because the broken code resulted from the second argument being incorrectly read when the first argument is a blank structure.

Instead, I am changing the whole test to use CHECK-NEXT so we can be sure the exact generated code is correct.

cfe/trunk/lib/CodeGen/TargetInfo.cpp

ABIArgInfo WebAssemblyABIInfo::classifyReturnType(QualType RetTy) const {
   }
 
   // Otherwise just do the default thing.
   return defaultInfo.classifyReturnType(RetTy);
 }
 
 Address WebAssemblyABIInfo::EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
                                       QualType Ty) const {
-  bool IsIndirect =
-      isAggregateTypeForABI(Ty) && !isSingleElementStruct(Ty, getContext());
+  bool IsIndirect = isAggregateTypeForABI(Ty) &&
+                    !isEmptyRecord(getContext(), Ty, true) &&
+                    !isSingleElementStruct(Ty, getContext());
   return emitVoidPtrVAArg(CGF, VAListAddr, Ty, IsIndirect,
                           getContext().getTypeInfoInChars(Ty),
                           CharUnits::fromQuantity(4),
                           /*AllowHigherAlign=*/true);
 }
 
 //===----------------------------------------------------------------------===//
 // le32/PNaCl bitcode ABI Implementation

cfe/trunk/test/CodeGen/wasm-varargs.c

struct S test_struct(char *fmt, ...) {
 
   va_start(va, fmt);
   struct S v = va_arg(va, struct S);
   va_end(va);
 
   return v;
 }
 
-// CHECK: define void @test_struct([[STRUCT_S:%[^,=]+]]*{{.*}} noalias sret [[AGG_RESULT:%.*]], i8*{{.*}} %fmt, ...) {{.*}} {
-// CHECK:   [[FMT_ADDR:%[^,=]+]] = alloca i8*, align 4
-// CHECK:   [[VA:%[^,=]+]] = alloca i8*, align 4
-// CHECK:   store i8* %fmt, i8** [[FMT_ADDR]], align 4
-// CHECK:   [[VA1:%[^,=]+]] = bitcast i8** [[VA]] to i8*
-// CHECK:   call void @llvm.va_start(i8* [[VA1]])
-// CHECK:   [[ARGP_CUR:%[^,=]+]] = load i8*, i8** [[VA]], align 4
-// CHECK:   [[ARGP_NEXT:%[^,=]+]] = getelementptr inbounds i8, i8* [[ARGP_CUR]], i32 4
-// CHECK:   store i8* [[ARGP_NEXT]], i8** [[VA]], align 4
-// CHECK:   [[R3:%[^,=]+]] = bitcast i8* [[ARGP_CUR]] to [[STRUCT_S]]**
-// CHECK:   [[R4:%[^,=]+]] = load [[STRUCT_S]]*, [[STRUCT_S]]** %0, align 4
-// CHECK:   [[R5:%[^,=]+]] = bitcast [[STRUCT_S]]* [[AGG_RESULT]] to i8*
-// CHECK:   [[R6:%[^,=]+]] = bitcast [[STRUCT_S]]* [[R4]] to i8*
-// CHECK:   call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 [[R5]], i8* align 4 [[R6]], i32 12, i1 false)
-// CHECK:   [[VA2:%[^,=]+]] = bitcast i8** [[VA]] to i8*
-// CHECK:   call void @llvm.va_end(i8* [[VA2]])
-// CHECK:   ret void
-// CHECK: }
+// CHECK:      define void @test_struct([[STRUCT_S:%[^,=]+]]*{{.*}} noalias sret [[AGG_RESULT:%.*]], i8*{{.*}} %fmt, ...) {{.*}} {
+// CHECK:        [[FMT_ADDR:%[^,=]+]] = alloca i8*, align 4
+// CHECK-NEXT:   [[VA:%[^,=]+]] = alloca i8*, align 4
+// CHECK-NEXT:   store i8* %fmt, i8** [[FMT_ADDR]], align 4
+// CHECK-NEXT:   [[VA1:%[^,=]+]] = bitcast i8** [[VA]] to i8*
+// CHECK-NEXT:   call void @llvm.va_start(i8* [[VA1]])
+// CHECK-NEXT:   [[ARGP_CUR:%[^,=]+]] = load i8*, i8** [[VA]], align 4
+// CHECK-NEXT:   [[ARGP_NEXT:%[^,=]+]] = getelementptr inbounds i8, i8* [[ARGP_CUR]], i32 4
+// CHECK-NEXT:   store i8* [[ARGP_NEXT]], i8** [[VA]], align 4
+// CHECK-NEXT:   [[R3:%[^,=]+]] = bitcast i8* [[ARGP_CUR]] to [[STRUCT_S]]**
+// CHECK-NEXT:   [[R4:%[^,=]+]] = load [[STRUCT_S]]*, [[STRUCT_S]]** [[R3]], align 4
+// CHECK-NEXT:   [[R5:%[^,=]+]] = bitcast [[STRUCT_S]]* [[AGG_RESULT]] to i8*
+// CHECK-NEXT:   [[R6:%[^,=]+]] = bitcast [[STRUCT_S]]* [[R4]] to i8*
+// CHECK-NEXT:   call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 [[R5]], i8* align 4 [[R6]], i32 12, i1 false)
+// CHECK-NEXT:   [[VA2:%[^,=]+]] = bitcast i8** [[VA]] to i8*
+// CHECK-NEXT:   call void @llvm.va_end(i8* [[VA2]])
+// CHECK-NEXT:   ret void
+// CHECK-NEXT: }
+
+struct Z {};
+
+struct S test_empty_struct(char *fmt, ...) {
+  va_list va;
+
+  va_start(va, fmt);
+  struct Z u = va_arg(va, struct Z);
+  struct S v = va_arg(va, struct S);
+  va_end(va);
+
+  return v;
+}
+
+// CHECK:      define void @test_empty_struct([[STRUCT_S:%[^,=]+]]*{{.*}} noalias sret [[AGG_RESULT:%.*]], i8*{{.*}} %fmt, ...) {{.*}} {
+// CHECK:        [[FMT_ADDR:%[^,=]+]] = alloca i8*, align 4
+// CHECK-NEXT:   [[VA:%[^,=]+]] = alloca i8*, align 4
+// CHECK-NEXT:   [[U:%[^,=]+]] = alloca [[STRUCT_Z:%[^,=]+]], align 1
+// CHECK-NEXT:   store i8* %fmt, i8** [[FMT_ADDR]], align 4
+// CHECK-NEXT:   [[VA1:%[^,=]+]] = bitcast i8** [[VA]] to i8*
+// CHECK-NEXT:   call void @llvm.va_start(i8* [[VA1]])
+// CHECK-NEXT:   [[ARGP_CUR:%[^,=]+]] = load i8*, i8** [[VA]], align 4
+// CHECK-NEXT:   [[ARGP_NEXT:%[^,=]+]] = getelementptr inbounds i8, i8* [[ARGP_CUR]], i32 0
+// CHECK-NEXT:   store i8* [[ARGP_NEXT]], i8** [[VA]], align 4
+// CHECK-NEXT:   [[R0:%[^,=]+]] = bitcast i8* [[ARGP_CUR]] to [[STRUCT_Z]]*
+// CHECK-NEXT:   [[R1:%[^,=]+]] = bitcast [[STRUCT_Z]]* [[U]] to i8*
+// CHECK-NEXT:   [[R2:%[^,=]+]] = bitcast [[STRUCT_Z]]* [[R0]] to i8*
+// CHECK-NEXT:   call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 1 [[R1]], i8* align 4 [[R2]], i32 0, i1 false)
+// CHECK-NEXT:   [[ARGP_CUR2:%[^,=]+]] = load i8*, i8** [[VA]], align 4
+// CHECK-NEXT:   [[ARGP_NEXT2:%[^,=]+]] = getelementptr inbounds i8, i8* [[ARGP_CUR2]], i32 4
+// CHECK-NEXT:   store i8* [[ARGP_NEXT2]], i8** [[VA]], align 4
+// CHECK-NEXT:   [[R3:%[^,=]+]] = bitcast i8* [[ARGP_CUR2]] to [[STRUCT_S]]**
+// CHECK-NEXT:   [[R4:%[^,=]+]] = load [[STRUCT_S]]*, [[STRUCT_S]]** [[R3]], align 4
+// CHECK-NEXT:   [[R5:%[^,=]+]] = bitcast [[STRUCT_S]]* [[AGG_RESULT]] to i8*
+// CHECK-NEXT:   [[R6:%[^,=]+]] = bitcast [[STRUCT_S]]* [[R4]] to i8*
+// CHECK-NEXT:   call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 [[R5]], i8* align 4 [[R6]], i32 12, i1 false)
+// CHECK-NEXT:   [[VA2:%[^,=]+]] = bitcast i8** [[VA]] to i8*
+// CHECK-NEXT:   call void @llvm.va_end(i8* [[VA2]])
+// CHECK-NEXT:   ret void
+// CHECK-NEXT: }