pkg/build: incorrect use of t.Fatal()/t.Fatalf() in tests

[ajdlinux]

Describe the bug

At least one test in pkg/build (but possibly in other parts) incorrectly calls t.Fatal()/t.Fatalf() from within a goroutine.

Per the documentation (https://pkg.go.dev/testing#T.FailNow), t.FailNow() (which is wrapped by t.Fatal() and t.Fatalf()) shouldn't be called from within a goroutine, other than the original test goroutine.

pkg/build/linux_test.go:43:

func enumerateFlags(t *testing.T, flags, allFlags []string) {
	if len(allFlags) != 0 {
		enumerateFlags(t, flags, allFlags[1:])
		enumerateFlags(t, append(flags, allFlags[0]), allFlags[1:])
		return
	}
	t.Run(strings.Join(flags, "-"), func(t *testing.T) {
		t.Parallel()
		sign1, sign2 := "", ""
		var wg sync.WaitGroup
		wg.Add(2)
		go func() {
			sign1 = sign(t, flags, false, false)
			wg.Done()
		}()
		go func() {
			sign2 = sign(t, flags, false, true)
			wg.Done()
		}()
		sign3 := sign(t, flags, true, false)
		wg.Wait()
		if sign1 != sign2 {
			t.Errorf("signature has changed after a comment-only change")
		}
		if sign1 == sign3 {
			t.Errorf("signature has not changed after a change")
		}
	})
}

func sign(t *testing.T, flags []string, changed, comment bool) string {
	buf := new(bytes.Buffer)
	if err := srcTemplate.Execute(buf, SrcParams{Changed: changed, Comment: comment}); err != nil {
		t.Fatal(err)
	}
	src := buf.Bytes()
	bin, err := osutil.TempFile("syz-build-test")
	if err != nil {
		t.Fatal(err) // <--- here
	}
	defer os.Remove(bin)
	cmd := osutil.Command("gcc", append(flags, "-pthread", "-o", bin, "-x", "c", "-")...)
	cmd.Stdin = buf
	out, err := cmd.CombinedOutput()
	if err != nil {
		t.Fatalf("compiler failed: %v\n%s\n\n%s", err, src, out) // <--- here
	}
	sign, err := elfBinarySignature(bin, &debugtracer.TestTracer{T: t})
	if err != nil {
		t.Fatal(err) // <--- here
	}
	return sign
}

I'm fairly sure this is why, when the test fails on line 88, you only see:

panic: Fail in goroutine after TestElfBinarySignature/-g--static has completed

followed by a stack trace, rather than the format string that is supposed to be printed.

To Reproduce n/a

Expected behavior t.Fatal()/t.Fatalf() should only be called from the top-level goroutine

Additional context n/a

[a-nogikh]

Thanks for reporting the problem! Indeed, the docs say that Fatal/Fatalf should not be used from other goroutines.

I'm curious why go vet tool did not detect this problem (it's reportedly able to do this since Go 1.16).

[a-nogikh]

Looked a bit closer: the vet tool does indeed check for Fatal calls, but does not yet detect the cases when Fatal() is called indirectly. So far it was deemed low-priority.