]> Cypherpunks repositories - gostls13.git/commitdiff
runtime/pprof: assert that labels never appear on unexpected samples
authorMichael Pratt <mpratt@google.com>
Tue, 7 Dec 2021 17:01:04 +0000 (12:01 -0500)
committerMichael Pratt <mpratt@google.com>
Wed, 19 Jan 2022 16:33:11 +0000 (16:33 +0000)
This makes TestLabelSystemstack much more strict, enabling it to detect
any misplacement of labels.

Unfortunately, there are several edge cases where we may not have an
obviously correct stack trace, so we generally except the runtime
package, with the exception of background goroutines that we know should
not be labeled.

For #50007
For #50032

Change-Id: I8dce7e7da04f278ce297422227901efe52782ca0
Reviewed-on: https://go-review.googlesource.com/c/go/+/369984
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
src/runtime/pprof/pprof_test.go

index b8b1382ad1560987b35a1c350ef24ce1dc577756..44d27d2b318b6f7d613e8eadd710620f0bbada4d 100644 (file)
@@ -1435,47 +1435,89 @@ func TestLabelSystemstack(t *testing.T) {
 
        matches := matchAndAvoidStacks(stackContainsLabeled, []string{"runtime.systemstack;key=value"}, avoidFunctions())
        p := testCPUProfile(t, matches, func(dur time.Duration) {
-               Do(context.Background(), Labels("key", "value"), func(context.Context) {
-                       var wg sync.WaitGroup
-                       stop := make(chan struct{})
-                       for i := 0; i < runtime.GOMAXPROCS(0); i++ {
-                               wg.Add(1)
-                               go func() {
-                                       defer wg.Done()
-                                       labelHog(stop, gogc)
-                               }()
-                       }
-
-                       time.Sleep(dur)
-                       close(stop)
-                       wg.Wait()
+               Do(context.Background(), Labels("key", "value"), func(ctx context.Context) {
+                       parallelLabelHog(ctx, dur, gogc)
                })
        })
 
-       // labelHog should always be labeled.
+       // Two conditions to check:
+       // * labelHog should always be labeled.
+       // * The label should _only_ appear on labelHog and the Do call above.
        for _, s := range p.Sample {
+               isLabeled := s.Label != nil && contains(s.Label["key"], "value")
+               var (
+                       mayBeLabeled     bool
+                       mustBeLabeled    bool
+                       mustNotBeLabeled bool
+               )
                for _, loc := range s.Location {
                        for _, l := range loc.Line {
-                               if l.Function.Name != "runtime/pprof.labelHog" {
-                                       continue
+                               switch l.Function.Name {
+                               case "runtime/pprof.labelHog", "runtime/pprof.parallelLabelHog":
+                                       mustBeLabeled = true
+                               case "runtime/pprof.Do":
+                                       // Do sets the labels, so samples may
+                                       // or may not be labeled depending on
+                                       // which part of the function they are
+                                       // at.
+                                       mayBeLabeled = true
+                               case "runtime.bgsweep", "runtime.bgscavenge", "runtime.forcegchelper", "runtime.gcBgMarkWorker", "runtime.runfinq", "runtime.sysmon":
+                                       // Runtime system goroutines or threads
+                                       // (such as those identified by
+                                       // runtime.isSystemGoroutine). These
+                                       // should never be labeled.
+                                       mustNotBeLabeled = true
+                               case "gogo", "gosave_systemstack_switch":
+                                       // These are context switch critical
+                                       // that we can't do a full traceback
+                                       // from. Typically this would be
+                                       // covered by the runtime check below,
+                                       // but these symbols don't have the
+                                       // package name.
+                                       mayBeLabeled = true
                                }
 
-                               if s.Label == nil {
-                                       t.Errorf("labelHog sample labels got nil want key=value")
-                                       continue
-                               }
-                               if !contains(s.Label["key"], "value") {
-                                       t.Errorf("labelHog sample labels got %+v want contains key=value", s.Label)
-                                       continue
+                               if strings.HasPrefix(l.Function.Name, "runtime.") {
+                                       // There are many places in the runtime
+                                       // where we can't do a full traceback.
+                                       // Ideally we'd list them all, but
+                                       // barring that allow anything in the
+                                       // runtime, unless explicitly excluded
+                                       // above.
+                                       mayBeLabeled = true
                                }
                        }
                }
+               if mustNotBeLabeled {
+                       // If this must not be labeled, then mayBeLabeled hints
+                       // are not relevant.
+                       mayBeLabeled = false
+               }
+               if mustBeLabeled && !isLabeled {
+                       var buf bytes.Buffer
+                       fprintStack(&buf, s.Location)
+                       t.Errorf("Sample labeled got false want true: %s", buf.String())
+               }
+               if mustNotBeLabeled && isLabeled {
+                       var buf bytes.Buffer
+                       fprintStack(&buf, s.Location)
+                       t.Errorf("Sample labeled got true want false: %s", buf.String())
+               }
+               if isLabeled && !(mayBeLabeled || mustBeLabeled) {
+                       var buf bytes.Buffer
+                       fprintStack(&buf, s.Location)
+                       t.Errorf("Sample labeled got true want false: %s", buf.String())
+               }
        }
 }
 
 // labelHog is designed to burn CPU time in a way that a high number of CPU
 // samples end up running on systemstack.
 func labelHog(stop chan struct{}, gogc int) {
+       // Regression test for issue 50032. We must give GC an opportunity to
+       // be initially triggered by a labelled goroutine.
+       runtime.GC()
+
        for i := 0; ; i++ {
                select {
                case <-stop:
@@ -1486,6 +1528,23 @@ func labelHog(stop chan struct{}, gogc int) {
        }
 }
 
+// parallelLabelHog runs GOMAXPROCS goroutines running labelHog.
+func parallelLabelHog(ctx context.Context, dur time.Duration, gogc int) {
+       var wg sync.WaitGroup
+       stop := make(chan struct{})
+       for i := 0; i < runtime.GOMAXPROCS(0); i++ {
+               wg.Add(1)
+               go func() {
+                       defer wg.Done()
+                       labelHog(stop, gogc)
+               }()
+       }
+
+       time.Sleep(dur)
+       close(stop)
+       wg.Wait()
+}
+
 // Check that there is no deadlock when the program receives SIGPROF while in
 // 64bit atomics' critical section. Used to happen on mips{,le}. See #20146.
 func TestAtomicLoadStore64(t *testing.T) {