From 7de02518a2868c91eeb288614bea7e0c3026cc9f Mon Sep 17 00:00:00 2001
From: Michael McLoughlin <mmcloughlin@gmail.com>
Date: Sat, 30 Oct 2021 13:32:25 -0700
Subject: [PATCH] gotypes: fix argument size for signatures without return
 types (#212)

This fixes a bug in argument size calculation in the case where the function
has no return values. Previously it was padding the argument struct to max
alignment, but this only happens if there are return values following.

Updates #191
---
 gotypes/signature.go                      | 26 ++++++++++++++++++-----
 gotypes/signature_test.go                 |  1 +
 tests/fixedbugs/issue191/asm.go           | 15 +++++++++++++
 tests/fixedbugs/issue191/doc.go           |  4 ++++
 tests/fixedbugs/issue191/issue191.s       |  5 +++++
 tests/fixedbugs/issue191/issue191_test.go |  9 ++++++++
 tests/fixedbugs/issue191/stub.go          |  6 ++++++
 7 files changed, 61 insertions(+), 5 deletions(-)
 create mode 100644 tests/fixedbugs/issue191/asm.go
 create mode 100644 tests/fixedbugs/issue191/doc.go
 create mode 100644 tests/fixedbugs/issue191/issue191.s
 create mode 100644 tests/fixedbugs/issue191/issue191_test.go
 create mode 100644 tests/fixedbugs/issue191/stub.go

diff --git a/gotypes/signature.go b/gotypes/signature.go
index e000020..ee514e3 100644
--- a/gotypes/signature.go
+++ b/gotypes/signature.go
@@ -97,20 +97,22 @@ func (s *Signature) init() {
 	p := s.sig.Params()
 	r := s.sig.Results()
 
-	// Compute parameter offsets.
+	// Compute parameter offsets. Note that if the function has results,
+	// additional padding up to max align is inserted between parameters and
+	// results.
 	vs := tuplevars(p)
 	vs = append(vs, types.NewParam(token.NoPos, nil, "sentinel", types.Typ[types.Uint64]))
 	paramsoffsets := Sizes.Offsetsof(vs)
 	paramssize := paramsoffsets[p.Len()]
+	if r.Len() == 0 {
+		paramssize = structsize(vs[:p.Len()])
+	}
 	s.params = newTuple(p, paramsoffsets, paramssize, "arg")
 
 	// Result offsets.
 	vs = tuplevars(r)
 	resultsoffsets := Sizes.Offsetsof(vs)
-	var resultssize int64
-	if n := len(vs); n > 0 {
-		resultssize = resultsoffsets[n-1] + Sizes.Sizeof(vs[n-1].Type())
-	}
+	resultssize := structsize(vs)
 	for i := range resultsoffsets {
 		resultsoffsets[i] += paramssize
 	}
@@ -175,3 +177,17 @@ func tuplevars(t *types.Tuple) []*types.Var {
 	}
 	return vs
 }
+
+// structsize computes the size of a struct containing the given variables as
+// fields. It would be equivalent to calculating the size of types.NewStruct(vs,
+// nil), apart from the fact that NewStruct panics if multiple fields have the
+// same name, and this happens for example if the variables represent return
+// types from a function.
+func structsize(vs []*types.Var) int64 {
+	n := len(vs)
+	if n == 0 {
+		return 0
+	}
+	offsets := Sizes.Offsetsof(vs)
+	return offsets[n-1] + Sizes.Sizeof(vs[n-1].Type())
+}
diff --git a/gotypes/signature_test.go b/gotypes/signature_test.go
index 91e688a..03997f1 100644
--- a/gotypes/signature_test.go
+++ b/gotypes/signature_test.go
@@ -159,6 +159,7 @@ func TestSignatureSizes(t *testing.T) {
 		{"func(uint64) uint64", 16},
 		{"func([7]byte) byte", 9},
 		{"func(uint64, uint64) (uint64, uint64)", 32},
+		{"func(uint16)", 2},
 	}
 	for _, c := range cases {
 		s, err := ParseSignature(c.Expr)
diff --git a/tests/fixedbugs/issue191/asm.go b/tests/fixedbugs/issue191/asm.go
new file mode 100644
index 0000000..f6373d5
--- /dev/null
+++ b/tests/fixedbugs/issue191/asm.go
@@ -0,0 +1,15 @@
+//go:build ignore
+// +build ignore
+
+package main
+
+import (
+	. "github.com/mmcloughlin/avo/build"
+)
+
+func main() {
+	TEXT("Uint16", 0, "func(n uint16)")
+	Doc("Uint16 tests argument size without return types.")
+	RET()
+	Generate()
+}
diff --git a/tests/fixedbugs/issue191/doc.go b/tests/fixedbugs/issue191/doc.go
new file mode 100644
index 0000000..f353ba0
--- /dev/null
+++ b/tests/fixedbugs/issue191/doc.go
@@ -0,0 +1,4 @@
+// Package issue191 tests for correct argument size for a function taking a
+// single uint16 argument. Prior to the fix, this test case triggered an asmdecl
+// error.
+package issue191
diff --git a/tests/fixedbugs/issue191/issue191.s b/tests/fixedbugs/issue191/issue191.s
new file mode 100644
index 0000000..8a2f927
--- /dev/null
+++ b/tests/fixedbugs/issue191/issue191.s
@@ -0,0 +1,5 @@
+// Code generated by command: go run asm.go -out issue191.s -stubs stub.go. DO NOT EDIT.
+
+// func Uint16(n uint16)
+TEXT ·Uint16(SB), $0-2
+	RET
diff --git a/tests/fixedbugs/issue191/issue191_test.go b/tests/fixedbugs/issue191/issue191_test.go
new file mode 100644
index 0000000..9fc1015
--- /dev/null
+++ b/tests/fixedbugs/issue191/issue191_test.go
@@ -0,0 +1,9 @@
+package issue191
+
+import "testing"
+
+//go:generate go run asm.go -out issue191.s -stubs stub.go
+
+func TestUint16(t *testing.T) {
+	Uint16(42)
+}
diff --git a/tests/fixedbugs/issue191/stub.go b/tests/fixedbugs/issue191/stub.go
new file mode 100644
index 0000000..5131b31
--- /dev/null
+++ b/tests/fixedbugs/issue191/stub.go
@@ -0,0 +1,6 @@
+// Code generated by command: go run asm.go -out issue191.s -stubs stub.go. DO NOT EDIT.
+
+package issue191
+
+// Uint16 tests argument size without return types.
+func Uint16(n uint16)