From 36ce2ece09f74cc8d71cc1d097b62be6264ddb86 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Fri, 4 Nov 2022 14:49:37 -0400 Subject: [PATCH] cmd/dist: restructure cgo_test Currently, dist test has a single test called "cgo_test" that runs a large number of different "go test"s. This commit restructures cgo_test into several individual tests, each of which runs a single "go test" that can be described by a goTest object and registered with registerTest. Since this lets us raise the abstraction level of constructing these tests and these tests are mostly covering the Cartesian product of a small number of orthogonal dimensions, we pull the common logic for constructing these tests into a helper function. For consistency, we now pass -tags=static to the static testtls and nocgo tests, but this tag doesn't affect the build of these tests at all. I traced all exec calls from cmd/dist on linux/amd64 and this is the only non-trivial change. For #37486. Change-Id: I53c1efa1c38d785dc71968f05e8d7d636b553e96 Reviewed-on: https://go-review.googlesource.com/c/go/+/450017 Run-TryBot: Austin Clements Reviewed-by: Dmitri Shuralyov Reviewed-by: Dmitri Shuralyov TryBot-Result: Gopher Robot --- src/cmd/dist/test.go | 143 +++++++++++++++++++++++++------------------ 1 file changed, 83 insertions(+), 60 deletions(-) diff --git a/src/cmd/dist/test.go b/src/cmd/dist/test.go index be1b2e8b34..e096c43806 100644 --- a/src/cmd/dist/test.go +++ b/src/cmd/dist/test.go @@ -895,11 +895,7 @@ func (t *tester) registerTests() { } } if t.cgoEnabled { - t.tests = append(t.tests, distTest{ - name: "cgo_test", - heading: "../misc/cgo/test", - fn: t.cgoTest, - }) + t.registerCgoTests() } // Don't run these tests with $GO_GCFLAGS because most of them @@ -1312,101 +1308,128 @@ func (t *tester) runHostTest(dir, pkg string) error { return t.dirCmd(dir, f.Name(), "-test.short="+short(), "-test.timeout="+t.timeoutDuration(300).String()).Run() } -func (t *tester) cgoTest(dt *distTest) error { - t.addCmd(dt, "misc/cgo/test", t.goTest(), "-ldflags=-linkmode=auto", ".") +func (t *tester) registerCgoTests() { + cgoTest := func(name string, subdir, linkmode, buildmode string, opts ...registerTestOpt) *goTest { + gt := &goTest{ + dir: "../misc/cgo/" + subdir, + buildmode: buildmode, + ldflags: "-linkmode=" + linkmode, + } + + if linkmode == "internal" { + gt.tags = append(gt.tags, "internal") + if buildmode == "pie" { + gt.tags = append(gt.tags, "internal_pie") + } + } + if buildmode == "static" { + // This isn't actually a Go buildmode, just a convenient way to tell + // cgoTest we want static linking. + gt.buildmode = "" + if linkmode == "external" { + gt.ldflags += ` -extldflags "-static -pthread"` + } else if linkmode == "auto" { + gt.env = append(gt.env, "CGO_LDFLAGS=-static -pthread") + } else { + panic("unknown linkmode with static build: " + linkmode) + } + gt.tags = append(gt.tags, "static") + } + + t.registerTest("cgo:"+name, "../misc/cgo/test", gt, opts...) + return gt + } + + cgoTest("test-auto", "test", "auto", "") // Stub out various buildmode=pie tests on alpine until 54354 resolved. builderName := os.Getenv("GO_BUILDER_NAME") disablePIE := strings.HasSuffix(builderName, "-alpine") if t.internalLink() { - t.addCmd(dt, "misc/cgo/test", t.goTest(), "-ldflags=-linkmode=internal", "-tags=internal", ".") + cgoTest("test-internal", "test", "internal", "") } - pair := gohostos + "-" + goarch - switch pair { - case "darwin-amd64", "darwin-arm64", - "windows-386", "windows-amd64", "windows-arm", "windows-arm64": - // test linkmode=external, but __thread not supported, so skip testtls. + os := gohostos + p := gohostos + "/" + goarch + switch { + case os == "darwin", os == "windows": if !t.extLink() { break } - t.addCmd(dt, "misc/cgo/test", t.goTest(), "-ldflags=-linkmode=external", ".") + // test linkmode=external, but __thread not supported, so skip testtls. + cgoTest("test-external", "test", "external", "") - t.addCmd(dt, "misc/cgo/test", t.goTest(), "-ldflags", "-linkmode=external -s", ".") + gt := cgoTest("test-external-s", "test", "external", "") + gt.ldflags += " -s" if t.supportedBuildmode("pie") && !disablePIE { - - t.addCmd(dt, "misc/cgo/test", t.goTest(), "-buildmode=pie", ".") + cgoTest("test-auto-pie", "test", "auto", "pie") if t.internalLink() && t.internalLinkPIE() { - t.addCmd(dt, "misc/cgo/test", t.goTest(), "-buildmode=pie", "-ldflags=-linkmode=internal", "-tags=internal,internal_pie", ".") + cgoTest("test-internal-pie", "test", "internal", "pie") } } - case "aix-ppc64", - "android-386", "android-amd64", "android-arm", "android-arm64", - "dragonfly-amd64", - "freebsd-386", "freebsd-amd64", "freebsd-arm", "freebsd-riscv64", - "linux-386", "linux-amd64", "linux-arm", "linux-arm64", "linux-loong64", "linux-mips", "linux-mipsle", "linux-mips64", "linux-mips64le", "linux-ppc64", "linux-ppc64le", "linux-riscv64", "linux-s390x", - "netbsd-386", "netbsd-amd64", "netbsd-arm", "netbsd-arm64", - "openbsd-386", "openbsd-amd64", "openbsd-arm", "openbsd-arm64", "openbsd-mips64": - - cmd := t.addCmd(dt, "misc/cgo/test", t.goTest(), "-ldflags=-linkmode=external", ".") - // cgo should be able to cope with both -g arguments and colored - // diagnostics. - setEnv(cmd, "CGO_CFLAGS", "-g0 -fdiagnostics-color") - - t.addCmd(dt, "misc/cgo/testtls", t.goTest(), "-ldflags", "-linkmode=auto", ".") - t.addCmd(dt, "misc/cgo/testtls", t.goTest(), "-ldflags", "-linkmode=external", ".") + case os == "aix", os == "android", os == "dragonfly", os == "freebsd", os == "linux", os == "netbsd", os == "openbsd": + gt := cgoTest("test-external-g0", "test", "external", "") + gt.env = append(gt.env, "CGO_CFLAGS=-g0 -fdiagnostics-color") - switch pair { - case "aix-ppc64": + cgoTest("testtls-auto", "testtls", "auto", "") + cgoTest("testtls-external", "testtls", "external", "") + switch { + case os == "aix": // no static linking - case "freebsd-arm": + case p == "freebsd/arm": // -fPIC compiled tls code will use __tls_get_addr instead // of __aeabi_read_tp, however, on FreeBSD/ARM, __tls_get_addr // is implemented in rtld-elf, so -fPIC isn't compatible with // static linking on FreeBSD/ARM with clang. (cgo depends on // -fPIC fundamentally.) default: + // Check for static linking support + var staticCheck rtPreFunc cmd := t.dirCmd("misc/cgo/test", compilerEnvLookup(defaultcc, goos, goarch), "-xc", "-o", "/dev/null", "-static", "-") cmd.Stdin = strings.NewReader("int main() {}") + cmd.Stdout, cmd.Stderr = nil, nil // Discard output if err := cmd.Run(); err != nil { - fmt.Println("No support for static linking found (lacks libc.a?), skip cgo static linking test.") - } else { - if goos != "android" && pair != "netbsd-arm" { - // TODO(#56629): Why does this fail on netbsd-arm? - t.addCmd(dt, "misc/cgo/testtls", t.goTest(), "-ldflags", `-linkmode=external -extldflags "-static -pthread"`, ".") + // Skip these tests + staticCheck.pre = func(*distTest) bool { + fmt.Println("No support for static linking found (lacks libc.a?), skip cgo static linking test.") + return false } - t.addCmd(dt, "misc/cgo/nocgo", t.goTest(), ".") - t.addCmd(dt, "misc/cgo/nocgo", t.goTest(), "-ldflags", `-linkmode=external`, ".") - if goos != "android" { - t.addCmd(dt, "misc/cgo/nocgo", t.goTest(), "-ldflags", `-linkmode=external -extldflags "-static -pthread"`, ".") - t.addCmd(dt, "misc/cgo/test", t.goTest(), "-tags=static", "-ldflags", `-linkmode=external -extldflags "-static -pthread"`, ".") - // -static in CGO_LDFLAGS triggers a different code path - // than -static in -extldflags, so test both. - // See issue #16651. - if goarch != "loong64" { - // TODO(#56623): Why does this fail on loong64? - cmd := t.addCmd(dt, "misc/cgo/test", t.goTest(), "-tags=static", ".") - setEnv(cmd, "CGO_LDFLAGS", "-static -pthread") - } + } + + // Static linking tests + if goos != "android" && p != "netbsd/arm" { + // TODO(#56629): Why does this fail on netbsd-arm? + cgoTest("testtls-static", "testtls", "external", "static", staticCheck) + } + cgoTest("nocgo-auto", "nocgo", "auto", "", staticCheck) + cgoTest("nocgo-external", "nocgo", "external", "", staticCheck) + if goos != "android" { + cgoTest("nocgo-static", "nocgo", "external", "static", staticCheck) + cgoTest("test-static", "test", "external", "static", staticCheck) + // -static in CGO_LDFLAGS triggers a different code path + // than -static in -extldflags, so test both. + // See issue #16651. + if goarch != "loong64" { + // TODO(#56623): Why does this fail on loong64? + cgoTest("test-static-env", "test", "auto", "static", staticCheck) } } + // PIE linking tests if t.supportedBuildmode("pie") && !disablePIE { - t.addCmd(dt, "misc/cgo/test", t.goTest(), "-buildmode=pie", ".") + cgoTest("test-pie", "test", "auto", "pie") if t.internalLink() && t.internalLinkPIE() { - t.addCmd(dt, "misc/cgo/test", t.goTest(), "-buildmode=pie", "-ldflags=-linkmode=internal", "-tags=internal,internal_pie", ".") + cgoTest("test-pie-internal", "test", "internal", "pie") } - t.addCmd(dt, "misc/cgo/testtls", t.goTest(), "-buildmode=pie", ".") - t.addCmd(dt, "misc/cgo/nocgo", t.goTest(), "-buildmode=pie", ".") + cgoTest("testtls-pie", "testtls", "auto", "pie") + cgoTest("nocgo-pie", "nocgo", "auto", "pie") } } } - - return nil } // run pending test commands, in parallel, emitting headers as appropriate. -- 2.48.1