cmd/trace: don't filter events for profile by whether they have stack

Right now the profile-from-trace code blindly discards events that don't
have a stack, but this means it can discard 'end' events for goroutine
time ranges that don't have stacks, like when a goroutine exits a
syscall. This means we drop stack samples we *do* have, because we
correctly already only use the stack trace of the corresponding 'start'
event for a time-range-of-interest anyway.

This change means that some events will be tracked that have no stack in
their start event, but that's fine. It won't end up in the profile
anyway because the stack is empty! And the rest of the code appears to
be robust to an empty stack already.

Thank you to Rhys Hiltner for reporting this issue and for the
reproducer, which I have worked into a test for this change.

Fixes #74850.

Change-Id: I943b97ecf6b82803e4a778a0f83a14473d32254e
Reviewed-on: https://go-review.googlesource.com/c/go/+/694156
Reviewed-by: Rhys Hiltner <rhys.hiltner@gmail.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Carlos Amedee <carlos@golang.org>

2 files changed, 103 insertions, 4 deletions

diff --git a/src/cmd/trace/pprof.go b/src/cmd/trace/pprof.go
index a66419aedf..b472ffa759 100644
--- a/src/cmd/trace/pprof.go
+++ b/src/cmd/trace/pprof.go
@@ -153,10 +153,6 @@ func makeComputePprofFunc(state trace.GoState, trackReason func(string) bool) co
 			if ev.Kind() != trace.EventStateTransition {
 				continue
 			}
-			stack := ev.Stack()
-			if stack == trace.NoStack {
-				continue
-			}
 
 			// The state transition has to apply to a goroutine.
 			st := ev.StateTransition()
diff --git a/src/cmd/trace/pprof_test.go b/src/cmd/trace/pprof_test.go
new file mode 100644
index 0000000000..6d18e7d5d1
--- /dev/null
+++ b/src/cmd/trace/pprof_test.go
@@ -0,0 +1,103 @@
+// Copyright 2025 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import (
+	"bytes"
+	"net/http"
+	"os"
+	"runtime/trace"
+	"strings"
+	"testing"
+	"testing/synctest"
+	"time"
+
+	"internal/trace/testtrace"
+)
+
+// Regression test for go.dev/issue/74850.
+func TestSyscallProfile74850(t *testing.T) {
+	testtrace.MustHaveSyscallEvents(t)
+
+	var buf bytes.Buffer
+	err := trace.Start(&buf)
+	if err != nil {
+		t.Fatalf("start tracing: %v", err)
+	}
+
+	synctest.Test(t, func(t *testing.T) {
+		go hidden1(t)
+		go hidden2(t)
+		go visible(t)
+		synctest.Wait()
+		time.Sleep(1 * time.Millisecond)
+		synctest.Wait()
+	})
+	trace.Stop()
+
+	if t.Failed() {
+		return
+	}
+
+	parsed, err := parseTrace(&buf, int64(buf.Len()))
+	if err != nil {
+		t.Fatalf("parsing trace: %v", err)
+	}
+
+	records, err := pprofByGoroutine(computePprofSyscall(), parsed)(&http.Request{})
+	if err != nil {
+		t.Fatalf("failed to generate pprof: %v\n", err)
+	}
+
+	for _, r := range records {
+		t.Logf("Record: n=%d, total=%v", r.Count, r.Time)
+		for _, f := range r.Stack {
+			t.Logf("\t%s", f.Func)
+			t.Logf("\t\t%s:%d @ 0x%x", f.File, f.Line, f.PC)
+		}
+	}
+	if len(records) == 0 {
+		t.Error("empty profile")
+	}
+
+	// Make sure we see the right frames.
+	wantSymbols := []string{"cmd/trace.visible", "cmd/trace.hidden1", "cmd/trace.hidden2"}
+	haveSymbols := make([]bool, len(wantSymbols))
+	for _, r := range records {
+		for _, f := range r.Stack {
+			for i, s := range wantSymbols {
+				if strings.Contains(f.Func, s) {
+					haveSymbols[i] = true
+				}
+			}
+		}
+	}
+	for i, have := range haveSymbols {
+		if !have {
+			t.Errorf("expected %s in syscall profile", wantSymbols[i])
+		}
+	}
+}
+
+func stat(t *testing.T) {
+	_, err := os.Stat(".")
+	if err != nil {
+		t.Errorf("os.Stat: %v", err)
+	}
+}
+
+func hidden1(t *testing.T) {
+	stat(t)
+}
+
+func hidden2(t *testing.T) {
+	stat(t)
+	stat(t)
+}
+
+func visible(t *testing.T) {
+	stat(t)
+	time.Sleep(1 * time.Millisecond)
+}