From patchwork Fri Jul 27 18:43:55 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Lance Taylor X-Patchwork-Id: 950300 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-482554-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=golang.org Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="G5ADTmg8"; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=golang-org.20150623.gappssmtp.com header.i=@golang-org.20150623.gappssmtp.com header.b="C1Owsk0t"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 41cdCx6TgFz9ryt for ; Sat, 28 Jul 2018 04:44:11 +1000 (AEST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:from:date:message-id:subject:to:content-type; q= dns; s=default; b=ALpL90mCXIKY2SdLC0IORDQu1fdfG7AgAF3b9uPRoBYom7 OTaL7+BatFytgBPb1N3szhjLuswOgwR+aGWVqzRhK/sbeQeHIarTwpQh6z28nixI ltpLEvLzhxgiMGExYSsYe7OknjULwzAD4kAS572mt+6T71hxYbFM0aZUmBrwg= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:from:date:message-id:subject:to:content-type; s= default; bh=FVU0dlv7KHMOLhw1HN/EjSEzvOQ=; b=G5ADTmg8fuk282Sgf3Vb Ew40LuiZoaQv8Rq/5iECq09zvNg8Eq6oa8NfLzG8kyVtkceUbmE/hph8F+GDjDPE xX1OoKcriIC1hy+SyYQWp3oFnx5w5+1oSKG1puGMwxalASPpiuaWrcRTow42xvhI o9mN+SRwLmQJP8dO9wOIb8k= Received: (qmail 96812 invoked by alias); 27 Jul 2018 18:44:01 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 96791 invoked by uid 89); 27 Jul 2018 18:44:01 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-11.1 required=5.0 tests=BAYES_00, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=HTo:D*googlegroups.com, H*Ad:D*googlegroups.com, macbook, profiles X-HELO: mail-lf1-f43.google.com Received: from mail-lf1-f43.google.com (HELO mail-lf1-f43.google.com) (209.85.167.43) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 27 Jul 2018 18:43:58 +0000 Received: by mail-lf1-f43.google.com with SMTP id u14-v6so4170340lfu.0 for ; Fri, 27 Jul 2018 11:43:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=golang-org.20150623.gappssmtp.com; s=20150623; h=mime-version:from:date:message-id:subject:to; bh=tFwGdbZiU2cilLGPKJQ5y7jDfWiQfbu2wDkiw5BKvgk=; b=C1Owsk0tn7KdQP5bstSqznNMAegR0cJK+ZC4jzJCzMf8+IXAbcMpzwEtZnwsCIaYKd bo2Cl4/QjyrBZ/QIa6P0vu8TPqp4nEt/mHsyhSjj5TsROQcYY3SjbKoej5jrcKS7Ge1p a0itML12VCB50z5NnGGaF/xe+Ic82pRq0o8iAxbFD6M/nItnrqt/UcuJo7VPPtdt6McA ncDsRW8qzMNf0T423tJB1UjYTuMsEgIEa8tgGtfg2Nw1t9eKhJbqM0EsXLYcD1dSfjGS f8B1FdiepyV2ZthGJeAmME2vEJsoflizM9DQrc+1ZdQVEeF4fEG8cdCzElRRMqqDZhik mt7Q== MIME-Version: 1.0 Received: by 2002:ab3:66d2:0:0:0:0:0 with HTTP; Fri, 27 Jul 2018 11:43:55 -0700 (PDT) From: Ian Lance Taylor Date: Fri, 27 Jul 2018 11:43:55 -0700 Message-ID: Subject: libgo patch committed: Prune sighandler frames in runtime.sigprof To: gcc-patches , gofrontend-dev@googlegroups.com This libgo patch by Than McIntosh prunes sighandler frames in runtime.sigprof. When writing stack frames to the pprof CPU profile machinery, it is very important to insure that the frames emitted do not contain any frames corresponding to artifacts of the profiling process itself (signal handlers, sigprof, etc). This patch changes runtime.sigprof to strip out those frames from the raw stack generated by "runtime.callers". This fixes https://golang.org/issue/26595. Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu. Committed to mainline. Ian Index: gcc/go/gofrontend/MERGE =================================================================== --- gcc/go/gofrontend/MERGE (revision 262908) +++ gcc/go/gofrontend/MERGE (working copy) @@ -1,4 +1,4 @@ -39d4d755db7d71b5e770ca435a8b1d1f08f53185 +a2e0ad16555b2698df8e71f4c0fe02e185715bc1 The first line of this file holds the git revision number of the last merge done from the gofrontend repository. Index: libgo/go/runtime/pprof/pprof_test.go =================================================================== --- libgo/go/runtime/pprof/pprof_test.go (revision 262658) +++ libgo/go/runtime/pprof/pprof_test.go (working copy) @@ -72,15 +72,24 @@ func cpuHog2(x int) int { return foo } +// Return a list of functions that we don't want to ever appear in CPU +// profiles. For gccgo, that list includes the sigprof handler itself. +func avoidFunctions() []string { + if runtime.Compiler == "gccgo" { + return []string{"runtime.sigprof"} + } + return nil +} + func TestCPUProfile(t *testing.T) { - testCPUProfile(t, []string{"pprof.cpuHog1"}, func(dur time.Duration) { + testCPUProfile(t, []string{"pprof.cpuHog1"}, avoidFunctions(), func(dur time.Duration) { cpuHogger(cpuHog1, &salt1, dur) }) } func TestCPUProfileMultithreaded(t *testing.T) { defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(2)) - testCPUProfile(t, []string{"pprof.cpuHog1", "pprof.cpuHog2"}, func(dur time.Duration) { + testCPUProfile(t, []string{"pprof.cpuHog1", "pprof.cpuHog2"}, avoidFunctions(), func(dur time.Duration) { c := make(chan int) go func() { cpuHogger(cpuHog1, &salt1, dur) @@ -92,7 +101,7 @@ func TestCPUProfileMultithreaded(t *test } func TestCPUProfileInlining(t *testing.T) { - testCPUProfile(t, []string{"pprof.inlinedCallee", "pprof.inlinedCaller"}, func(dur time.Duration) { + testCPUProfile(t, []string{"pprof.inlinedCallee", "pprof.inlinedCaller"}, avoidFunctions(), func(dur time.Duration) { cpuHogger(inlinedCaller, &salt1, dur) }) } @@ -130,7 +139,7 @@ func parseProfile(t *testing.T, valBytes } } -func testCPUProfile(t *testing.T, need []string, f func(dur time.Duration)) { +func testCPUProfile(t *testing.T, need []string, avoid []string, f func(dur time.Duration)) { switch runtime.GOOS { case "darwin": switch runtime.GOARCH { @@ -169,7 +178,7 @@ func testCPUProfile(t *testing.T, need [ f(duration) StopCPUProfile() - if profileOk(t, need, prof, duration) { + if profileOk(t, need, avoid, prof, duration) { return } @@ -202,11 +211,13 @@ func contains(slice []string, s string) return false } -func profileOk(t *testing.T, need []string, prof bytes.Buffer, duration time.Duration) (ok bool) { +func profileOk(t *testing.T, need []string, avoid []string, prof bytes.Buffer, duration time.Duration) (ok bool) { ok = true - // Check that profile is well formed and contains need. + // Check that profile is well formed, contains 'need', and does not contain + // anything from 'avoid'. have := make([]uintptr, len(need)) + avoidSamples := make([]uintptr, len(avoid)) var samples uintptr var buf bytes.Buffer parseProfile(t, prof.Bytes(), func(count uintptr, stk []*profile.Location, labels map[string][]string) { @@ -229,6 +240,15 @@ func profileOk(t *testing.T, need []stri } } } + for i, name := range avoid { + for _, loc := range stk { + for _, line := range loc.Line { + if strings.Contains(line.Function.Name, name) { + avoidSamples[i] += count + } + } + } + } fmt.Fprintf(&buf, "\n") }) t.Logf("total %d CPU profile samples collected:\n%s", samples, buf.String()) @@ -251,6 +271,14 @@ func profileOk(t *testing.T, need []stri ok = false } + for i, name := range avoid { + bad := avoidSamples[i] + if bad != 0 { + t.Logf("found %d samples in avoid-function %s\n", bad, name) + ok = false + } + } + if len(need) == 0 { return ok } @@ -318,6 +346,9 @@ func TestCPUProfileWithFork(t *testing.T // If it did, it would see inconsistent state and would either record an incorrect stack // or crash because the stack was malformed. func TestGoroutineSwitch(t *testing.T) { + if runtime.Compiler == "gccgo" { + t.Skip("not applicable for gccgo") + } // How much to try. These defaults take about 1 seconds // on a 2012 MacBook Pro. The ones in short mode take // about 0.1 seconds. @@ -377,7 +408,7 @@ func fprintStack(w io.Writer, stk []*pro // Test that profiling of division operations is okay, especially on ARM. See issue 6681. func TestMathBigDivide(t *testing.T) { - testCPUProfile(t, nil, func(duration time.Duration) { + testCPUProfile(t, nil, nil, func(duration time.Duration) { t := time.After(duration) pi := new(big.Int) for { @@ -851,7 +882,7 @@ func TestEmptyCallStack(t *testing.T) { } func TestCPUProfileLabel(t *testing.T) { - testCPUProfile(t, []string{"pprof.cpuHogger;key=value"}, func(dur time.Duration) { + testCPUProfile(t, []string{"pprof.cpuHogger;key=value"}, avoidFunctions(), func(dur time.Duration) { Do(context.Background(), Labels("key", "value"), func(context.Context) { cpuHogger(cpuHog1, &salt1, dur) }) @@ -862,7 +893,7 @@ func TestLabelRace(t *testing.T) { // Test the race detector annotations for synchronization // between settings labels and consuming them from the // profile. - testCPUProfile(t, []string{"pprof.cpuHogger;key=value"}, func(dur time.Duration) { + testCPUProfile(t, []string{"pprof.cpuHogger;key=value"}, avoidFunctions(), func(dur time.Duration) { start := time.Now() var wg sync.WaitGroup for time.Since(start) < dur { Index: libgo/go/runtime/proc.go =================================================================== --- libgo/go/runtime/proc.go (revision 262658) +++ libgo/go/runtime/proc.go (working copy) @@ -3418,8 +3418,36 @@ func sigprof(pc uintptr, gp *g, mp *m) { var stklocs [maxCPUProfStack]location n = callers(0, stklocs[:]) + // Issue 26595: the stack trace we've just collected is going + // to include frames that we don't want to report in the CPU + // profile, including signal handler frames. Here is what we + // might typically see at the point of "callers" above for a + // signal delivered to the application routine "interesting" + // called by "main". + // + // 0: runtime.sigprof + // 1: runtime.sighandler + // 2: runtime.sigtrampgo + // 3: runtime.sigtramp + // 4: + // 5: main.interesting_routine + // 6: main.main + // + // To ensure a sane profile, walk through the frames in + // "stklocs" until we find the "runtime.sigtramp" frame, then + // report only those frames below the frame one down from + // that. If for some reason "runtime.sigtramp" is not present, + // don't make any changes. + framesToDiscard := 0 for i := 0; i < n; i++ { - stk[i] = stklocs[i].pc + if stklocs[i].function == "runtime.sigtramp" && i+2 < n { + framesToDiscard = i + 2 + n -= framesToDiscard + break + } + } + for i := 0; i < n; i++ { + stk[i] = stklocs[i+framesToDiscard].pc } }