From e07ea69d611991a24d67076df906866c198252c0 Mon Sep 17 00:00:00 2001
From: Michael Forney <mforney@mforney.org>
Date: Sat, 4 Sep 2021 13:53:09 -0700
Subject: Use architecture-specific va_list type

Previously, cproc effectively used used

	typedef struct { /* 32 bytes, 8-byte aligned */ } __builtin_va_list[1];

However, this is not quite correct for x86_64 nor aarch64, though
it was close enough for both to work in most cases.

In actuality, for x86_64 we want

	typedef struct { /* 24 bytes, 8-byte aligned */ } __builtin_va_list[1];

and for aarch64 we want

	typedef struct { /* 32 bytes, 8-byte aligned */ } __builtin_va_list;

The difference only appears when the size of va_list matters, or
when va_list is passed as a parameter. However, the former is not
often the case, and the aarch64 ABI replaces aggregate arguments
with pointers to caller-allocated memory, which is quite similar
to arrays decaying to pointers in C except that the struct is not
copied.

Additionally, riscv64 simply uses

	typedef void *__builtin_va_list;

which again has a different size and calling convention.

To fix this, make the __builtin_va_list type architecture-specific
and use architecture-specific tests for varargs-related functionality.
---
 cc.h                                 |  2 +-
 expr.c                               | 10 +++++-----
 qbe.c                                |  4 ++++
 scope.c                              |  5 ++++-
 targ.c                               | 18 ++++++++++++++++++
 test/builtin-va-copy+aarch64.c       |  4 ++++
 test/builtin-va-copy+aarch64.qbe     | 22 ++++++++++++++++++++++
 test/builtin-va-copy+x86_64.c        |  4 ++++
 test/builtin-va-copy+x86_64.qbe      | 18 ++++++++++++++++++
 test/builtin-va-copy.c               |  4 ----
 test/builtin-va-copy.qbe             | 22 ----------------------
 test/varargs+aarch64.c               | 12 ++++++++++++
 test/varargs+aarch64.qbe             | 22 ++++++++++++++++++++++
 test/varargs+riscv64.c               | 12 ++++++++++++
 test/varargs+riscv64.qbe             | 22 ++++++++++++++++++++++
 test/varargs+x86_64.c                | 12 ++++++++++++
 test/varargs+x86_64.qbe              | 22 ++++++++++++++++++++++
 test/varargs-pass-valist+aarch64.c   |  5 +++++
 test/varargs-pass-valist+aarch64.qbe |  9 +++++++++
 test/varargs-pass-valist+riscv64.c   |  5 +++++
 test/varargs-pass-valist+riscv64.qbe |  9 +++++++++
 test/varargs-pass-valist+x86_64.c    |  5 +++++
 test/varargs-pass-valist+x86_64.qbe  |  8 ++++++++
 test/varargs.c                       | 12 ------------
 test/varargs.qbe                     | 22 ----------------------
 type.c                               |  9 ---------
 26 files changed, 223 insertions(+), 76 deletions(-)
 create mode 100644 test/builtin-va-copy+aarch64.c
 create mode 100644 test/builtin-va-copy+aarch64.qbe
 create mode 100644 test/builtin-va-copy+x86_64.c
 create mode 100644 test/builtin-va-copy+x86_64.qbe
 delete mode 100644 test/builtin-va-copy.c
 delete mode 100644 test/builtin-va-copy.qbe
 create mode 100644 test/varargs+aarch64.c
 create mode 100644 test/varargs+aarch64.qbe
 create mode 100644 test/varargs+riscv64.c
 create mode 100644 test/varargs+riscv64.qbe
 create mode 100644 test/varargs+x86_64.c
 create mode 100644 test/varargs+x86_64.qbe
 create mode 100644 test/varargs-pass-valist+aarch64.c
 create mode 100644 test/varargs-pass-valist+aarch64.qbe
 create mode 100644 test/varargs-pass-valist+riscv64.c
 create mode 100644 test/varargs-pass-valist+riscv64.qbe
 create mode 100644 test/varargs-pass-valist+x86_64.c
 create mode 100644 test/varargs-pass-valist+x86_64.qbe
 delete mode 100644 test/varargs.c
 delete mode 100644 test/varargs.qbe

diff --git a/cc.h b/cc.h
index 9c4484b..c4dbee9 100644
--- a/cc.h
+++ b/cc.h
@@ -428,12 +428,12 @@ extern struct type typeint, typeuint;
 extern struct type typelong, typeulong;
 extern struct type typellong, typeullong;
 extern struct type typefloat, typedouble, typeldouble;
-extern struct type typevalist;
 
 /* targ */
 
 struct target {
 	const char *name;
+	struct type *typevalist;
 	struct type *typewchar;
 	int signedchar;
 };
diff --git a/expr.c b/expr.c
index 6837adb..1c91556 100644
--- a/expr.c
+++ b/expr.c
@@ -643,7 +643,7 @@ builtinfunc(struct scope *s, enum builtinkind kind)
 		e = mkexpr(EXPRBUILTIN, NULL);
 		e->builtin.kind = BUILTINVAARG;
 		e->base = mkunaryexpr(TBAND, assignexpr(s));
-		if (e->base->base->type != &typevalist)
+		if (e->base->base->type != targ->typevalist)
 			error(&tok.loc, "va_arg argument must have type va_list");
 		expect(TCOMMA, "after va_list");
 		e->type = typename(s, &e->qual);
@@ -653,20 +653,20 @@ builtinfunc(struct scope *s, enum builtinkind kind)
 		e->assign.l = assignexpr(s);
 		if (e->assign.l->decayed)
 			e->assign.l = e->assign.l->base;
-		if (e->assign.l->type != &typevalist)
+		if (e->assign.l->type != targ->typevalist)
 			error(&tok.loc, "va_copy destination must have type va_list");
 		expect(TCOMMA, "after target va_list");
 		e->assign.r = assignexpr(s);
 		if (e->assign.r->decayed)
 			e->assign.r = e->assign.r->base;
-		if (e->assign.r->type != &typevalist)
+		if (e->assign.r->type != targ->typevalist)
 			error(&tok.loc, "va_copy source must have type va_list");
 		break;
 	case BUILTINVAEND:
 		e = assignexpr(s);
 		if (e->decayed)
 			e = e->base;
-		if (e->type != &typevalist)
+		if (e->type != targ->typevalist)
 			error(&tok.loc, "va_end argument must have type va_list");
 		e = mkexpr(EXPRBUILTIN, &typevoid);
 		e->builtin.kind = BUILTINVAEND;
@@ -675,7 +675,7 @@ builtinfunc(struct scope *s, enum builtinkind kind)
 		e = mkexpr(EXPRBUILTIN, &typevoid);
 		e->builtin.kind = BUILTINVASTART;
 		e->base = mkunaryexpr(TBAND, assignexpr(s));
-		if (e->base->base->type != &typevalist)
+		if (e->base->base->type != targ->typevalist)
 			error(&tok.loc, "va_start argument must have type va_list");
 		expect(TCOMMA, "after va_list");
 		param = assignexpr(s);
diff --git a/qbe.c b/qbe.c
index df363c2..be7dce7 100644
--- a/qbe.c
+++ b/qbe.c
@@ -1104,6 +1104,10 @@ emittype(struct type *t)
 	}
 	fputs("type ", stdout);
 	emitvalue(t->value);
+	if (t == targ->typevalist) {
+		printf(" = align %d { %" PRIu64 " }\n", t->align, t->size);
+		return;
+	}
 	fputs(" = { ", stdout);
 	for (m = t->structunion.members, off = 0; m;) {
 		if (t->kind == TYPESTRUCT) {
diff --git a/scope.c b/scope.c
index 72025f5..c116670 100644
--- a/scope.c
+++ b/scope.c
@@ -25,13 +25,16 @@ scopeinit(void)
 		{"__builtin_va_arg",     {.kind = DECLBUILTIN, .builtin = BUILTINVAARG}},
 		{"__builtin_va_copy",    {.kind = DECLBUILTIN, .builtin = BUILTINVACOPY}},
 		{"__builtin_va_end",     {.kind = DECLBUILTIN, .builtin = BUILTINVAEND}},
-		{"__builtin_va_list",    {.kind = DECLTYPE, .type = &typevalist}},
 		{"__builtin_va_start",   {.kind = DECLBUILTIN, .builtin = BUILTINVASTART}},
 	};
+	static struct decl valist;
 	struct builtin *b;
 
 	for (b = builtins; b < builtins + LEN(builtins); ++b)
 		scopeputdecl(&filescope, b->name, &b->decl);
+	valist.kind = DECLTYPE;
+	valist.type = targ->typevalist;
+	scopeputdecl(&filescope, "__builtin_va_list", &valist);
 }
 
 struct scope *
diff --git a/targ.c b/targ.c
index 2859975..f2be127 100644
--- a/targ.c
+++ b/targ.c
@@ -9,14 +9,32 @@ static const struct target alltargs[] = {
 	{
 		.name = "x86_64",
 		.typewchar = &typeint,
+		.typevalist = &(struct type){
+			.kind = TYPEARRAY, .prop = PROPOBJECT|PROPDERIVED|PROPAGGR,
+			.align = 8, .size = 24,
+			.array = {1}, .base = &(struct type){
+				.kind = TYPESTRUCT, .prop = PROPOBJECT|PROPAGGR,
+				.align = 8, .size = 24,
+			},
+		},
 		.signedchar = 1,
 	},
 	{
 		.name = "aarch64",
+		.typevalist = &(struct type){
+			.kind = TYPESTRUCT, .prop = PROPOBJECT|PROPAGGR,
+			.align = 8, .size = 32,
+			.structunion.tag = "va_list",
+		},
 		.typewchar = &typeuint,
 	},
 	{
 		.name = "riscv64",
+		.typevalist = &(struct type){
+			.kind = TYPEPOINTER, .prop = PROPOBJECT|PROPDERIVED|PROPSCALAR,
+			.align = 8, .size = 8,
+			.base = &typevoid,
+		},
 		.typewchar = &typeint,
 	},
 };
diff --git a/test/builtin-va-copy+aarch64.c b/test/builtin-va-copy+aarch64.c
new file mode 100644
index 0000000..7a69596
--- /dev/null
+++ b/test/builtin-va-copy+aarch64.c
@@ -0,0 +1,4 @@
+void f(void) {
+	static __builtin_va_list a, b;
+	__builtin_va_copy(a, b);
+}
diff --git a/test/builtin-va-copy+aarch64.qbe b/test/builtin-va-copy+aarch64.qbe
new file mode 100644
index 0000000..6c8d288
--- /dev/null
+++ b/test/builtin-va-copy+aarch64.qbe
@@ -0,0 +1,22 @@
+data $.La.2 = align 8 { z 32 }
+data $.Lb.3 = align 8 { z 32 }
+export
+function $f() {
+@start.1
+@body.2
+	%.1 =l loadl $.Lb.3
+	storel %.1, $.La.2
+	%.2 =l add $.Lb.3, 8
+	%.3 =l add $.La.2, 8
+	%.4 =l loadl %.2
+	storel %.4, %.3
+	%.5 =l add %.2, 8
+	%.6 =l add %.3, 8
+	%.7 =l loadl %.5
+	storel %.7, %.6
+	%.8 =l add %.5, 8
+	%.9 =l add %.6, 8
+	%.10 =l loadl %.8
+	storel %.10, %.9
+	ret
+}
diff --git a/test/builtin-va-copy+x86_64.c b/test/builtin-va-copy+x86_64.c
new file mode 100644
index 0000000..7a69596
--- /dev/null
+++ b/test/builtin-va-copy+x86_64.c
@@ -0,0 +1,4 @@
+void f(void) {
+	static __builtin_va_list a, b;
+	__builtin_va_copy(a, b);
+}
diff --git a/test/builtin-va-copy+x86_64.qbe b/test/builtin-va-copy+x86_64.qbe
new file mode 100644
index 0000000..47e1d9e
--- /dev/null
+++ b/test/builtin-va-copy+x86_64.qbe
@@ -0,0 +1,18 @@
+data $.La.2 = align 8 { z 24 }
+data $.Lb.3 = align 8 { z 24 }
+export
+function $f() {
+@start.1
+@body.2
+	%.1 =l loadl $.Lb.3
+	storel %.1, $.La.2
+	%.2 =l add $.Lb.3, 8
+	%.3 =l add $.La.2, 8
+	%.4 =l loadl %.2
+	storel %.4, %.3
+	%.5 =l add %.2, 8
+	%.6 =l add %.3, 8
+	%.7 =l loadl %.5
+	storel %.7, %.6
+	ret
+}
diff --git a/test/builtin-va-copy.c b/test/builtin-va-copy.c
deleted file mode 100644
index 7a69596..0000000
--- a/test/builtin-va-copy.c
+++ /dev/null
@@ -1,4 +0,0 @@
-void f(void) {
-	static __builtin_va_list a, b;
-	__builtin_va_copy(a, b);
-}
diff --git a/test/builtin-va-copy.qbe b/test/builtin-va-copy.qbe
deleted file mode 100644
index 6c8d288..0000000
--- a/test/builtin-va-copy.qbe
+++ /dev/null
@@ -1,22 +0,0 @@
-data $.La.2 = align 8 { z 32 }
-data $.Lb.3 = align 8 { z 32 }
-export
-function $f() {
-@start.1
-@body.2
-	%.1 =l loadl $.Lb.3
-	storel %.1, $.La.2
-	%.2 =l add $.Lb.3, 8
-	%.3 =l add $.La.2, 8
-	%.4 =l loadl %.2
-	storel %.4, %.3
-	%.5 =l add %.2, 8
-	%.6 =l add %.3, 8
-	%.7 =l loadl %.5
-	storel %.7, %.6
-	%.8 =l add %.5, 8
-	%.9 =l add %.6, 8
-	%.10 =l loadl %.8
-	storel %.10, %.9
-	ret
-}
diff --git a/test/varargs+aarch64.c b/test/varargs+aarch64.c
new file mode 100644
index 0000000..0b5a73d
--- /dev/null
+++ b/test/varargs+aarch64.c
@@ -0,0 +1,12 @@
+void f(int n, ...) {
+	__builtin_va_list ap;
+
+	__builtin_va_start(ap, n);
+	while (n) {
+		__builtin_va_arg(ap, int);
+		__builtin_va_arg(ap, float);
+		__builtin_va_arg(ap, char *);
+		--n;
+	}
+	__builtin_va_end(ap);
+}
diff --git a/test/varargs+aarch64.qbe b/test/varargs+aarch64.qbe
new file mode 100644
index 0000000..512a8c7
--- /dev/null
+++ b/test/varargs+aarch64.qbe
@@ -0,0 +1,22 @@
+export
+function $f(w %.1, ...) {
+@start.1
+	%.2 =l alloc4 4
+	storew %.1, %.2
+	%.3 =l alloc8 32
+@body.2
+	vastart %.3
+@while_cond.3
+	%.4 =w loadw %.2
+	jnz %.4, @while_body.4, @while_join.5
+@while_body.4
+	%.5 =w vaarg %.3
+	%.6 =s vaarg %.3
+	%.7 =l vaarg %.3
+	%.8 =w loadw %.2
+	%.9 =w sub %.8, 1
+	storew %.9, %.2
+	jmp @while_cond.3
+@while_join.5
+	ret
+}
diff --git a/test/varargs+riscv64.c b/test/varargs+riscv64.c
new file mode 100644
index 0000000..0b5a73d
--- /dev/null
+++ b/test/varargs+riscv64.c
@@ -0,0 +1,12 @@
+void f(int n, ...) {
+	__builtin_va_list ap;
+
+	__builtin_va_start(ap, n);
+	while (n) {
+		__builtin_va_arg(ap, int);
+		__builtin_va_arg(ap, float);
+		__builtin_va_arg(ap, char *);
+		--n;
+	}
+	__builtin_va_end(ap);
+}
diff --git a/test/varargs+riscv64.qbe b/test/varargs+riscv64.qbe
new file mode 100644
index 0000000..7fcda8d
--- /dev/null
+++ b/test/varargs+riscv64.qbe
@@ -0,0 +1,22 @@
+export
+function $f(w %.1, ...) {
+@start.1
+	%.2 =l alloc4 4
+	storew %.1, %.2
+	%.3 =l alloc8 8
+@body.2
+	vastart %.3
+@while_cond.3
+	%.4 =w loadw %.2
+	jnz %.4, @while_body.4, @while_join.5
+@while_body.4
+	%.5 =w vaarg %.3
+	%.6 =s vaarg %.3
+	%.7 =l vaarg %.3
+	%.8 =w loadw %.2
+	%.9 =w sub %.8, 1
+	storew %.9, %.2
+	jmp @while_cond.3
+@while_join.5
+	ret
+}
diff --git a/test/varargs+x86_64.c b/test/varargs+x86_64.c
new file mode 100644
index 0000000..0b5a73d
--- /dev/null
+++ b/test/varargs+x86_64.c
@@ -0,0 +1,12 @@
+void f(int n, ...) {
+	__builtin_va_list ap;
+
+	__builtin_va_start(ap, n);
+	while (n) {
+		__builtin_va_arg(ap, int);
+		__builtin_va_arg(ap, float);
+		__builtin_va_arg(ap, char *);
+		--n;
+	}
+	__builtin_va_end(ap);
+}
diff --git a/test/varargs+x86_64.qbe b/test/varargs+x86_64.qbe
new file mode 100644
index 0000000..641afa1
--- /dev/null
+++ b/test/varargs+x86_64.qbe
@@ -0,0 +1,22 @@
+export
+function $f(w %.1, ...) {
+@start.1
+	%.2 =l alloc4 4
+	storew %.1, %.2
+	%.3 =l alloc8 24
+@body.2
+	vastart %.3
+@while_cond.3
+	%.4 =w loadw %.2
+	jnz %.4, @while_body.4, @while_join.5
+@while_body.4
+	%.5 =w vaarg %.3
+	%.6 =s vaarg %.3
+	%.7 =l vaarg %.3
+	%.8 =w loadw %.2
+	%.9 =w sub %.8, 1
+	storew %.9, %.2
+	jmp @while_cond.3
+@while_join.5
+	ret
+}
diff --git a/test/varargs-pass-valist+aarch64.c b/test/varargs-pass-valist+aarch64.c
new file mode 100644
index 0000000..c1ede84
--- /dev/null
+++ b/test/varargs-pass-valist+aarch64.c
@@ -0,0 +1,5 @@
+void f(__builtin_va_list ap);
+void g(void) {
+	static __builtin_va_list ap;
+	f(ap);
+}
diff --git a/test/varargs-pass-valist+aarch64.qbe b/test/varargs-pass-valist+aarch64.qbe
new file mode 100644
index 0000000..67cef8a
--- /dev/null
+++ b/test/varargs-pass-valist+aarch64.qbe
@@ -0,0 +1,9 @@
+data $.Lap.2 = align 8 { z 32 }
+type :va_list.1 = align 8 { 32 }
+export
+function $g() {
+@start.1
+@body.2
+	call $f(:va_list.1 $.Lap.2)
+	ret
+}
diff --git a/test/varargs-pass-valist+riscv64.c b/test/varargs-pass-valist+riscv64.c
new file mode 100644
index 0000000..c1ede84
--- /dev/null
+++ b/test/varargs-pass-valist+riscv64.c
@@ -0,0 +1,5 @@
+void f(__builtin_va_list ap);
+void g(void) {
+	static __builtin_va_list ap;
+	f(ap);
+}
diff --git a/test/varargs-pass-valist+riscv64.qbe b/test/varargs-pass-valist+riscv64.qbe
new file mode 100644
index 0000000..937f50a
--- /dev/null
+++ b/test/varargs-pass-valist+riscv64.qbe
@@ -0,0 +1,9 @@
+data $.Lap.2 = align 8 { z 8 }
+export
+function $g() {
+@start.1
+@body.2
+	%.1 =l loadl $.Lap.2
+	call $f(l %.1)
+	ret
+}
diff --git a/test/varargs-pass-valist+x86_64.c b/test/varargs-pass-valist+x86_64.c
new file mode 100644
index 0000000..c1ede84
--- /dev/null
+++ b/test/varargs-pass-valist+x86_64.c
@@ -0,0 +1,5 @@
+void f(__builtin_va_list ap);
+void g(void) {
+	static __builtin_va_list ap;
+	f(ap);
+}
diff --git a/test/varargs-pass-valist+x86_64.qbe b/test/varargs-pass-valist+x86_64.qbe
new file mode 100644
index 0000000..50d9283
--- /dev/null
+++ b/test/varargs-pass-valist+x86_64.qbe
@@ -0,0 +1,8 @@
+data $.Lap.2 = align 8 { z 24 }
+export
+function $g() {
+@start.1
+@body.2
+	call $f(l $.Lap.2)
+	ret
+}
diff --git a/test/varargs.c b/test/varargs.c
deleted file mode 100644
index 0b5a73d..0000000
--- a/test/varargs.c
+++ /dev/null
@@ -1,12 +0,0 @@
-void f(int n, ...) {
-	__builtin_va_list ap;
-
-	__builtin_va_start(ap, n);
-	while (n) {
-		__builtin_va_arg(ap, int);
-		__builtin_va_arg(ap, float);
-		__builtin_va_arg(ap, char *);
-		--n;
-	}
-	__builtin_va_end(ap);
-}
diff --git a/test/varargs.qbe b/test/varargs.qbe
deleted file mode 100644
index 512a8c7..0000000
--- a/test/varargs.qbe
+++ /dev/null
@@ -1,22 +0,0 @@
-export
-function $f(w %.1, ...) {
-@start.1
-	%.2 =l alloc4 4
-	storew %.1, %.2
-	%.3 =l alloc8 32
-@body.2
-	vastart %.3
-@while_cond.3
-	%.4 =w loadw %.2
-	jnz %.4, @while_body.4, @while_join.5
-@while_body.4
-	%.5 =w vaarg %.3
-	%.6 =s vaarg %.3
-	%.7 =l vaarg %.3
-	%.8 =w loadw %.2
-	%.9 =w sub %.8, 1
-	storew %.9, %.2
-	jmp @while_cond.3
-@while_join.5
-	ret
-}
diff --git a/type.c b/type.c
index a97175a..1d28213 100644
--- a/type.c
+++ b/type.c
@@ -39,15 +39,6 @@ struct type typefloat   = FLTTYPE(TYPEFLOAT, 4);
 struct type typedouble  = FLTTYPE(TYPEDOUBLE, 8);
 struct type typeldouble = FLTTYPE(TYPELDOUBLE, 16);
 
-struct type typevalist = {
-	.kind = TYPEARRAY, .size = 32, .align = 8, .array = {1},
-	.prop = PROPOBJECT|PROPDERIVED|PROPAGGR,
-	.base = &(struct type){
-		.kind = TYPESTRUCT, .size = 32, .align = 8,
-		.prop = PROPOBJECT|PROPAGGR,
-	},
-};
-
 struct type *
 mktype(enum typekind kind, enum typeprop prop)
 {
-- 
cgit v1.2.3