cmd/dist,cmd/go: set GOTRACEBACK to "system" for testing

If we're testing through dist, we're testing the implementation of Go,
so we're interested in any package failing with potential runtime
issues. In these cases, we'd like to have as much relevant detail as
possible, but currently runtime stack frames and goroutines are
suppressed due to the default GOTRACEBACK setting.

So, try to set GOTRACEBACK to system if it's unset. Check if it's unset
first so we don't override the user asking for a lower or higher level.

This change was brought up in the context of #45916, since there's an
apparent deadlock (or something!) in the runtime that appears when
running other code, but it's difficult to see exactly where it's
blocked. However, this change is very generally useful.

This change also runs scripted tests with GOTRACEBACK=system, upgrading
from GOTRACEBACK=all. Often, script tests can trigger failures deep in
the runtime in interesting ways because they start many individual Go
processes, so being able to identify points of interest in the runtime
is quite useful.

For #45916.

Change-Id: I3d50658d0d0090fb4c9182b87200d266c7f8f915
Reviewed-on: https://go-review.googlesource.com/c/go/+/316469
Trust: Michael Knyszek <mknyszek@google.com>
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>

diff --git a/src/cmd/dist/test.go b/src/cmd/dist/test.go
index 33fabd35541cb1e3a4500e7e34d508790632bf41..50bf80ba5960bd084f3f9d4453ff99c1283db57f 100644 (file)
--- a/src/cmd/dist/test.go
+++ b/src/cmd/dist/test.go
@@ -42,6 +42,7 @@ func cmdtest() {
        if noRebuild {
                t.rebuild = false
        }
+
        t.run()
 }
 
@@ -117,6 +118,21 @@ func (t *tester) run() {
                }
        }
 
+       // Set GOTRACEBACK to system if the user didn't set a level explicitly.
+       // Since we're running tests for Go, we want as much detail as possible
+       // if something goes wrong.
+       //
+       // Set it before running any commands just in case something goes wrong.
+       if ok := isEnvSet("GOTRACEBACK"); !ok {
+               if err := os.Setenv("GOTRACEBACK", "system"); err != nil {
+                       if t.keepGoing {
+                               log.Printf("Failed to set GOTRACEBACK: %v", err)
+                       } else {
+                               fatalf("Failed to set GOTRACEBACK: %v", err)
+                       }
+               }
+       }
+
        if t.rebuild {
                t.out("Building packages and commands.")
                // Force rebuild the whole toolchain.
@@ -1664,3 +1680,15 @@ func isUnsupportedVMASize(w *work) bool {
        unsupportedVMA := []byte("unsupported VMA range")
        return w.dt.name == "race" && bytes.Contains(w.out, unsupportedVMA)
 }
+
+// isEnvSet reports whether the environment variable evar is
+// set in the environment.
+func isEnvSet(evar string) bool {
+       evarEq := evar + "="
+       for _, e := range os.Environ() {
+               if strings.HasPrefix(e, evarEq) {
+                       return true
+               }
+       }
+       return false
+}

diff --git a/src/cmd/go/script_test.go b/src/cmd/go/script_test.go
index 2274335a75a2de6565993da2a84a47cbaf084a9d..327eaff44501b0451ff76567ddac712ba766e7e8 100644 (file)
--- a/src/cmd/go/script_test.go
+++ b/src/cmd/go/script_test.go
@@ -170,7 +170,7 @@ func (ts *testScript) setup() {
                "GOPRIVATE=",
                "GOROOT=" + testGOROOT,
                "GOROOT_FINAL=" + os.Getenv("GOROOT_FINAL"), // causes spurious rebuilds and breaks the "stale" built-in if not propagated
-               "GOTRACEBACK=all",
+               "GOTRACEBACK=system",
                "TESTGO_GOROOT=" + testGOROOT,
                "GOSUMDB=" + testSumDBVerifierKey,
                "GONOPROXY=",