From 539b4d8d5bb91f74f580a1bc6e8e28b9de38aedd Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Tue, 21 Nov 2023 17:23:43 +0000 Subject: [PATCH] internal/trace/v2: forward Event.Stack to StateTransition.Stack Currently StateTransition.Stack is only set for the GoCreate case, because there are two stacks and we need to distinguish them. But the docs for StateTransition.Stack say that that stack always references the resource that is transitioning. There are quite a few cases where Event.Stack is actually the appropriate stack to for StateTransition.Stack, but in these cases it's left empty, and the caller just needs to understand which one to look at. This isn't great. Forward Event.Stack to StateTransition.Stack whenever Event.Stack also refers to the resource experiencing the state transition. Change-Id: Ie43fc6036f2712c7982174d5739d95765312dfcc Reviewed-on: https://go-review.googlesource.com/c/go/+/544316 Auto-Submit: Michael Knyszek Reviewed-by: Michael Pratt LUCI-TryBot-Result: Go LUCI --- src/internal/trace/v2/event.go | 6 ++++++ src/internal/trace/v2/testtrace/validation.go | 5 +++++ 2 files changed, 11 insertions(+) diff --git a/src/internal/trace/v2/event.go b/src/internal/trace/v2/event.go index 7ec4698d88..3700cbcc2f 100644 --- a/src/internal/trace/v2/event.go +++ b/src/internal/trace/v2/event.go @@ -568,22 +568,28 @@ func (e Event) StateTransition() StateTransition { s = goStateTransition(GoID(e.base.args[0]), GoRunnable, GoRunning) case go122.EvGoDestroy: s = goStateTransition(e.ctx.G, GoRunning, GoNotExist) + s.Stack = e.Stack() // This event references the resource the event happened on. case go122.EvGoDestroySyscall: s = goStateTransition(e.ctx.G, GoSyscall, GoNotExist) case go122.EvGoStop: s = goStateTransition(e.ctx.G, GoRunning, GoRunnable) s.Reason = e.table.strings.mustGet(stringID(e.base.args[0])) + s.Stack = e.Stack() // This event references the resource the event happened on. case go122.EvGoBlock: s = goStateTransition(e.ctx.G, GoRunning, GoWaiting) s.Reason = e.table.strings.mustGet(stringID(e.base.args[0])) + s.Stack = e.Stack() // This event references the resource the event happened on. case go122.EvGoUnblock: s = goStateTransition(GoID(e.base.args[0]), GoWaiting, GoRunnable) case go122.EvGoSyscallBegin: s = goStateTransition(e.ctx.G, GoRunning, GoSyscall) + s.Stack = e.Stack() // This event references the resource the event happened on. case go122.EvGoSyscallEnd: s = goStateTransition(e.ctx.G, GoSyscall, GoRunning) + s.Stack = e.Stack() // This event references the resource the event happened on. case go122.EvGoSyscallEndBlocked: s = goStateTransition(e.ctx.G, GoSyscall, GoRunnable) + s.Stack = e.Stack() // This event references the resource the event happened on. case go122.EvGoStatus: // N.B. ordering.advance populates e.base.extra. s = goStateTransition(GoID(e.base.args[0]), GoState(e.base.extra(version.Go122)[0]), go122GoStatus2GoState[e.base.args[2]]) diff --git a/src/internal/trace/v2/testtrace/validation.go b/src/internal/trace/v2/testtrace/validation.go index fcbc10801b..a2654a10e4 100644 --- a/src/internal/trace/v2/testtrace/validation.go +++ b/src/internal/trace/v2/testtrace/validation.go @@ -169,6 +169,11 @@ func (v *Validator) Event(ev trace.Event) error { state.binding = ctx } } else if old.Executing() && !new.Executing() { + if tr.Stack != ev.Stack() { + // This is a case where the transition is happening to a goroutine that is also executing, so + // these two stacks should always match. + e.Errorf("StateTransition.Stack doesn't match Event.Stack") + } ctx := state.binding if ctx != nil { if ctx.G != id { -- 2.48.1