From 29ea99fc5f3373620c1e431c07cf431d5eb644db Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=AD?= <mvdan@mvdan.cc>
Date: Sun, 2 Jan 2022 23:34:33 +0000
Subject: [PATCH] CI: test on GOARCH=386

Note that this cross-compilation disables cgo by default,
and so the cgo.txt test script isn't run on GOARCH=386.
That seems fine for now, as the test isn't arch-specific.

This testing uncovered one build failure in internal/literals;
the comparison between int and math.MaxUint32 is invalid on 32-bit.
To fix that build failure, use int64 consistently.

One test also incorrectly assumed amd64; it now supports 386 too.
For any other architecture, it's being skipped for now.

I also had to increase the -race test timeout,
as it usually takes 8-9m on GitHub Actions,
and the timeout would sometimes trigger.

Finally, use "go env" rather than "go version" on CI,
which gives us much more useful information,
and also includes Go's own version now via GOVERSION.

Fixes #426.
---
 .github/workflows/test.yml | 27 ++++++++++++++++++++++++---
 internal/literals/swap.go  |  4 ++--
 main_test.go               | 18 ++++++++++++++++++
 testdata/scripts/asm.txt   | 35 +++++++++++++++++++++--------------
 testdata/scripts/cgo.txt   |  2 ++
 5 files changed, 67 insertions(+), 19 deletions(-)

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 138b829..6bc5577 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -22,14 +22,35 @@ jobs:
       uses: actions/checkout@v2
     - name: Test
       run: |
-        go version
+        go env
         go test ./...
     - name: Test with -race
       # macos and windows tend to be a bit slower,
       # and it's rare that a race in garble would be OS-specific.
+      # also note that this can take 5-10m, so use a larger timeout.
       if: matrix.os == 'ubuntu-latest'
       run: |
-        go test -race ./...
+        go test -race -timeout=20m ./...
+
+  # We don't care about GOARCH=386 particularly,
+  # but it helps ensure we support 32-bit hosts and targets well.
+  # TODO: use CGO_ENABLED=1 once we figure out how to install gcc-multilib,
+  # and once gotooltest forwards the value of CGO_ENABLED to the scripts.
+  test-386:
+    runs-on: ubuntu-latest
+    env:
+      GOARCH: 386
+    steps:
+    - name: Install Go
+      uses: actions/setup-go@v2
+      with:
+        go-version: 1.17.x
+    - name: Checkout code
+      uses: actions/checkout@v2
+    - name: Test
+      run: |
+        go env
+        go test ./...
 
   test-gotip:
     runs-on: ubuntu-latest
@@ -55,7 +76,7 @@ jobs:
       uses: actions/checkout@v2
     - name: Test
       run: |
-        go version
+        go env
         go test ./...
 
   code-checks:
diff --git a/internal/literals/swap.go b/internal/literals/swap.go
index d18e448..5fd88c1 100644
--- a/internal/literals/swap.go
+++ b/internal/literals/swap.go
@@ -17,7 +17,7 @@ type swap struct{}
 // check that the obfuscator interface is implemented
 var _ obfuscator = swap{}
 
-func getIndexType(dataLen int) string {
+func getIndexType(dataLen int64) string {
 	switch {
 	case dataLen <= math.MaxUint8:
 		return "byte"
@@ -34,7 +34,7 @@ func positionsToSlice(data []int) *ast.CompositeLit {
 	arr := &ast.CompositeLit{
 		Type: &ast.ArrayType{
 			Len: &ast.Ellipsis{}, // Performance optimization
-			Elt: ast.NewIdent(getIndexType(len(data))),
+			Elt: ast.NewIdent(getIndexType(int64(len(data)))),
 		},
 		Elts: []ast.Expr{},
 	}
diff --git a/main_test.go b/main_test.go
index 237d295..7f60fd8 100644
--- a/main_test.go
+++ b/main_test.go
@@ -87,6 +87,24 @@ func TestScripts(t *testing.T) {
 			}
 			return nil
 		},
+		// TODO: this condition should probably be supported by gotooltest
+		Condition: func(cond string) (bool, error) {
+			switch cond {
+			case "cgo":
+				out, err := exec.Command("go", "env", "CGO_ENABLED").CombinedOutput()
+				if err != nil {
+					return false, err
+				}
+				result := strings.TrimSpace(string(out))
+				switch result {
+				case "0", "1":
+					return result == "1", nil
+				default:
+					return false, fmt.Errorf("unknown CGO_ENABLED: %q", result)
+				}
+			}
+			return false, fmt.Errorf("unknown condition")
+		},
 		Cmds: map[string]func(ts *testscript.TestScript, neg bool, args []string){
 			"binsubstr":         binsubstr,
 			"bincmp":            bincmp,
diff --git a/testdata/scripts/asm.txt b/testdata/scripts/asm.txt
index 33efd5a..2e45dff 100644
--- a/testdata/scripts/asm.txt
+++ b/testdata/scripts/asm.txt
@@ -1,3 +1,6 @@
+# TODO: support arm64, at least
+[!386] [!amd64] skip 'the assembly is only written for 386 and amd64'
+
 env GOGARBLE=test/main
 
 garble build
@@ -28,29 +31,33 @@ import (
 	"test/main/imported"
 )
 
-func privateAdd(x, y int64) int64
+func privateAdd(x, y int32) int32
 
 func main() {
 	println(privateAdd(1, 2))
 	println(imported.PublicAdd(3, 4))
 }
--- main.s --
-TEXT ·privateAdd(SB),$0-24
-	MOVQ x+0(FP), BX
-	MOVQ y+8(FP), BP
-	ADDQ BP, BX
-	MOVQ BX, ret+16(FP)
+-- main_x86.s --
+//go:build 386 || amd64
+
+TEXT ·privateAdd(SB),$0-16
+	MOVL x+0(FP), BX
+	MOVL y+4(FP), BP
+	ADDL BP, BX
+	MOVL BX, ret+8(FP)
 	RET
 -- imported/imported.go --
 package imported
 
-func PublicAdd(x, y int64) int64
--- imported/imported.s --
-TEXT ·PublicAdd(SB),$0-24
-	MOVQ x+0(FP), BX
-	MOVQ y+8(FP), BP
-	ADDQ BP, BX
-	MOVQ BX, ret+16(FP)
+func PublicAdd(x, y int32) int32
+-- imported/imported_x86.s --
+//go:build 386 || amd64
+
+TEXT ·PublicAdd(SB),$0-16
+	MOVL x+0(FP), BX
+	MOVL y+4(FP), BP
+	ADDL BP, BX
+	MOVL BX, ret+8(FP)
 	RET
 -- main.stderr --
 3
diff --git a/testdata/scripts/cgo.txt b/testdata/scripts/cgo.txt
index e14c969..9144e91 100644
--- a/testdata/scripts/cgo.txt
+++ b/testdata/scripts/cgo.txt
@@ -1,3 +1,5 @@
+[!cgo] skip 'this test requires cgo to be enabled'
+
 env GOGARBLE=test/main
 
 garble build