From 87312bc3edd028a31d662e145d02b38790f4c716 Mon Sep 17 00:00:00 2001
From: Alan Donovan <adonovan@google.com>
Date: Thu, 11 Oct 2018 23:10:21 -0400
Subject: [PATCH] go/packages: use effective GOARCH to determine type size
 function

The typechecker's Sizes function is not currently set correctly.
The correct answer requires information known only to the build
system's query tool (go list, blaze query, etc), and we need to
add a mechanism for it to return this information.

In the meantime, we use this simple workaround: if the GOARCH
environment variable is set, we use that to determine sizes according
to the conventions of gc. Otherwise, we use the architecture for which
the application was compiled. Both could easily be incorrect, but this
is nonetheless progress.

This change should fix the tests of go/analysis/passes/shift,
which are currently broken for GOARCH=386 because the analysistest
driver uses go/packages, which ignores GOARCH.

Change-Id: Iabe3211ad513a9a94eadd6d8f4b2068f7abdd053
Reviewed-on: https://go-review.googlesource.com/c/141757
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
---
 go/packages/packages.go      | 17 ++++++++++++-----
 go/packages/packages_test.go | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/go/packages/packages.go b/go/packages/packages.go
index 92fce7585ee..f40720d0630 100644
--- a/go/packages/packages.go
+++ b/go/packages/packages.go
@@ -15,13 +15,14 @@ import (
 	"go/scanner"
 	"go/token"
 	"go/types"
+	"io/ioutil"
 	"log"
 	"os"
+	"path/filepath"
+	"runtime"
 	"sync"
 
 	"golang.org/x/tools/go/gcexportdata"
-	"io/ioutil"
-	"path/filepath"
 )
 
 // A LoadMode specifies the amount of detail to return when loading.
@@ -681,6 +682,14 @@ func (ld *loader) loadPackage(lpkg *loaderPackage) {
 		panic("unreachable")
 	})
 
+	// This is only an approximation.
+	// TODO(adonovan): derive Sizes from the underlying build system.
+	goarch := runtime.GOARCH
+	if x, ok := os.LookupEnv("GOARCH"); ok {
+		goarch = x
+	}
+	sizes := types.SizesFor("gc", goarch)
+
 	// type-check
 	tc := &types.Config{
 		Importer: importer,
@@ -691,9 +700,7 @@ func (ld *loader) loadPackage(lpkg *loaderPackage) {
 		IgnoreFuncBodies: ld.Mode < LoadAllSyntax && !lpkg.initial,
 
 		Error: appendError,
-
-		// TODO(adonovan): derive Sizes from the underlying
-		// build system.
+		Sizes: sizes,
 	}
 	types.NewChecker(tc, ld.Fset, lpkg.Types, lpkg.TypesInfo).Files(lpkg.Syntax)
 
diff --git a/go/packages/packages_test.go b/go/packages/packages_test.go
index 555a4908732..f5e99ba8ec6 100644
--- a/go/packages/packages_test.go
+++ b/go/packages/packages_test.go
@@ -9,6 +9,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"go/ast"
+	constantpkg "go/constant"
 	"go/parser"
 	"go/token"
 	"go/types"
@@ -1041,6 +1042,41 @@ func TestContains(t *testing.T) {
 	}
 }
 
+// This test ensures that the effective GOARCH variable in the
+// application determines the Sizes function used by the type checker.
+// This behavior is a stop-gap until we make the build system's query
+// too report the correct sizes function for the actual configuration.
+func TestSizes(t *testing.T) {
+	tmp, cleanup := makeTree(t, map[string]string{
+		"src/a/a.go": `package a; import "unsafe"; const WordSize = 8*unsafe.Sizeof(int(0))`,
+	})
+	defer cleanup()
+
+	savedGOARCH := os.Getenv("GOARCH")
+	defer os.Setenv("GOARCH", savedGOARCH)
+
+	for arch, wantWordSize := range map[string]int64{"386": 32, "amd64": 64} {
+		os.Setenv("GOARCH", arch)
+		cfg := &packages.Config{
+			Mode: packages.LoadSyntax,
+			Dir:  tmp,
+			Env:  append(os.Environ(), "GOPATH="+tmp, "GO111MODULE=off"),
+		}
+		initial, err := packages.Load(cfg, "a")
+		if err != nil {
+			t.Fatal(err)
+		}
+		if packages.PrintErrors(initial) > 0 {
+			t.Fatal("there were errors")
+		}
+		gotWordSize, _ := constantpkg.Int64Val(constant(initial[0], "WordSize").Val())
+		if gotWordSize != wantWordSize {
+			t.Errorf("for GOARCH=%s, got word size %d, want %d", arch, gotWordSize, wantWordSize)
+		}
+	}
+
+}
+
 // TestContains_FallbackSticks ensures that when there are both contains and non-contains queries
 // the decision whether to fallback to the pre-1.11 go list sticks across both sets of calls to
 // go list.