runtime: fix generation skew with trace reentrancy

Currently when performing multiple nested traceAcquires, we re-read
trace.gen on subsequent reads. But this is invalid, since a generation
transition may happen in between a traceAcquire and a nested
traceAcquire. The first one will produce a traceLocker with a gen from
the previous generation, and the second will produce a traceLocker from
the next generation. (Note: generations cannot _complete_ advancement
under traceAcquire, but trace.gen can move forward.) The end result is
earlier events, from the nested traceAcquire, will write to a future
generation, and then previous events will write to a past generation.
This can break the trace.

(There are also a lot of comments left over talking about the
non-reentrancy of the tracer; we should look at those again.)

Change-Id: I08ac8cc86d41ab3e6061c5de58d657b6ad0d19d8
Reviewed-on: https://go-review.googlesource.com/c/go/+/708397
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>

diff --git a/src/runtime/traceruntime.go b/src/runtime/traceruntime.go
index 06e36fd8026d392a608bfd544761614ba75886b5..7df6f68b7061648b6130fdab9224c5cf5cc44cb5 100644 (file)
--- a/src/runtime/traceruntime.go
+++ b/src/runtime/traceruntime.go
@@ -29,6 +29,7 @@ type mTraceState struct {
        buf           [2][tracev2.NumExperiments]*traceBuf // Per-M traceBuf for writing. Indexed by trace.gen%2.
        link          *m                                   // Snapshot of alllink or freelink.
        reentered     uint32                               // Whether we've reentered tracing from within tracing.
+       entryGen      uintptr                              // The generation value on first entry.
        oldthrowsplit bool                                 // gp.throwsplit upon calling traceLocker.writer. For debugging.
 }
 
@@ -212,7 +213,7 @@ func traceAcquireEnabled() traceLocker {
        // that it is.
        if mp.trace.seqlock.Load()%2 == 1 {
                mp.trace.reentered++
-               return traceLocker{mp, trace.gen.Load()}
+               return traceLocker{mp, mp.trace.entryGen}
        }
 
        // Acquire the trace seqlock. This prevents traceAdvance from moving forward
@@ -240,6 +241,7 @@ func traceAcquireEnabled() traceLocker {
                releasem(mp)
                return traceLocker{}
        }
+       mp.trace.entryGen = gen
        return traceLocker{mp, gen}
 }