From patchwork Mon Jun 3 23:08:01 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Lance Taylor X-Patchwork-Id: 1109554 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-502261-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="VLn0aK4c"; 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="q5Me5nmm"; 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 45HrMG137vz9s3Z for ; Tue, 4 Jun 2019 09:08:24 +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=CaXyeVYQrt7c3Gb7Qo/DIgWpZzDNPeAQiM7hNrxKkNBdlq b8Cuwnc9a6V7pnaSyZdGSaSvuvypDw0uAM9UkWJG+HsCPMlGR4OqakbSiOkiz2LL EvuptnNdKISdixdEx+9lbCtRYJgLycaJt+W5cBfH8vAlMCtHF5tbNTPNKa/qc= 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=icjePofzn7YAiDiZngCIhCLIThs=; b=VLn0aK4cnIFYWwg2KVMJ Y/HBUPvz/4vHNwc5OIZURQ5dMRnlTz9rMZnzeJpvwz7Wfq87TGsdOERGleG9JMcj bDbgUGs+bGpiHTctOrWccL7+ossXW40eD7AQToO2g2ef5b9TpevTIv3nq+ff/SkT icZWdAvCsCBE2VU0eETZIUw= Received: (qmail 100356 invoked by alias); 3 Jun 2019 23:08:17 -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 100348 invoked by uid 89); 3 Jun 2019 23:08:17 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-12.6 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=associate, artifacts, Delay X-HELO: mail-lf1-f45.google.com Received: from mail-lf1-f45.google.com (HELO mail-lf1-f45.google.com) (209.85.167.45) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 03 Jun 2019 23:08:15 +0000 Received: by mail-lf1-f45.google.com with SMTP id y198so4413062lfa.1 for ; Mon, 03 Jun 2019 16:08:14 -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=zgdmwqkfMrI7Wygt5Iy5bzUQljd8Tsjo3zBCIIdreuI=; b=q5Me5nmmViNy6Hl/4eHhoiF83WC8h3+bJHS/KktRcdtstGV7aQWi+cgqS/ZiDpeoc9 wCVAewDwv4VQub9UHQPCrGpAg4Xsy538dxwpbJchLtGdPNTgub832NEB+C3vaE4EJpLW EuoKOwGCDWqV7+ArlovGhW5bCAA/t/A3cXVEKL3TFBuq2VANY1kZ+t4mlGo10UI+s4io bc4pzw++C/iPC01JxblY3UUpl6T372CYC7JvyjWXR4BT1RBsjfIhjsr63ptHIQMDe0LR nOBfq1nIGZ5/GkQBbTpl5MTnAL5e5LBsGnRv3Q6/RW3f9KKB/hWxCzR4SQJZ0BrwZDeb OQzg== MIME-Version: 1.0 From: Ian Lance Taylor Date: Mon, 3 Jun 2019 16:08:01 -0700 Message-ID: Subject: libgo patch committed: Delay applying profiling stack skips To: gcc-patches , gofrontend-dev This libgo patch by Than McIntosh delays applying the requested skip count to a profiling stack trace until it is symbolized. When the runtime collects a stack trace to associate it with some profiling event (mem alloc, mutex, etc) there is a skip count passed to runtime.Callers (or equivalent) to skip some known count of frames in order to get to the "interesting" frame corresponding to the profile event. Now that the profiling mechanism uses lazy fixup (when removing compiler artifacts like thunks, morestack calls etc), we also need to move the frame skipping logic after the fixup, so as to insure that the skip count isn't thrown off by these artifacts. This fixes https://golang.org/issue/32290. 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 271891) +++ gcc/go/gofrontend/MERGE (working copy) @@ -1,4 +1,4 @@ -37a47e4691b4602dd167f82c64a6569019584a80 +951c83af46375019b2fe262635746368a6b9c4ba The first line of this file holds the git revision number of the last merge done from the gofrontend repository. Index: libgo/go/runtime/mprof.go =================================================================== --- libgo/go/runtime/mprof.go (revision 271669) +++ libgo/go/runtime/mprof.go (working copy) @@ -56,6 +56,7 @@ type bucket struct { hash uintptr size uintptr nstk uintptr + skip int } // A memRecord is the bucket data for a bucket of type memProfile, @@ -185,7 +186,7 @@ func max(x, y uintptr) uintptr { } // newBucket allocates a bucket with the given type and number of stack entries. -func newBucket(typ bucketType, nstk int) *bucket { +func newBucket(typ bucketType, nstk int, skipCount int) *bucket { size := payloadOffset(typ, uintptr(nstk)) switch typ { default: @@ -203,6 +204,7 @@ func newBucket(typ bucketType, nstk int) bucketmem += size b.typ = typ b.nstk = uintptr(nstk) + b.skip = skipCount return b } @@ -229,7 +231,7 @@ func (b *bucket) bp() *blockRecord { } // Return the bucket for stk[0:nstk], allocating new bucket if needed. -func stkbucket(typ bucketType, size uintptr, stk []uintptr, alloc bool) *bucket { +func stkbucket(typ bucketType, size uintptr, skip int, stk []uintptr, alloc bool) *bucket { if buckhash == nil { buckhash = (*[buckHashSize]*bucket)(sysAlloc(unsafe.Sizeof(*buckhash), &memstats.buckhash_sys)) if buckhash == nil { @@ -264,7 +266,7 @@ func stkbucket(typ bucketType, size uint } // Create new bucket. - b := newBucket(typ, len(stk)) + b := newBucket(typ, len(stk), skip) copy(b.stk(), stk) b.hash = h b.size = size @@ -369,9 +371,10 @@ func mProf_PostSweep() { // Called by malloc to record a profiled block. func mProf_Malloc(p unsafe.Pointer, size uintptr) { var stk [maxStack]uintptr - nstk := callersRaw(1, stk[:]) + nstk := callersRaw(stk[:]) lock(&proflock) - b := stkbucket(memProfile, size, stk[:nstk], true) + skip := 1 + b := stkbucket(memProfile, size, skip, stk[:nstk], true) c := mProf.cycle mp := b.mp() mpc := &mp.future[(c+2)%uint32(len(mp.future))] @@ -446,14 +449,14 @@ func saveblockevent(cycles int64, skip i var nstk int var stk [maxStack]uintptr if gp.m.curg == nil || gp.m.curg == gp { - nstk = callersRaw(skip, stk[:]) + nstk = callersRaw(stk[:]) } else { // FIXME: This should get a traceback of gp.m.curg. // nstk = gcallers(gp.m.curg, skip, stk[:]) - nstk = callersRaw(skip, stk[:]) + nstk = callersRaw(stk[:]) } lock(&proflock) - b := stkbucket(which, 0, stk[:nstk], true) + b := stkbucket(which, 0, skip, stk[:nstk], true) b.bp().count++ b.bp().cycles += cycles unlock(&proflock) @@ -605,9 +608,12 @@ func freebucket(tofree *bucket) *bucket // later. Note: there is code in go-callers.c's backtrace_full callback() // function that performs very similar fixups; these two code paths // should be kept in sync. -func fixupStack(stk []uintptr, canonStack *[maxStack]uintptr, size uintptr) int { +func fixupStack(stk []uintptr, skip int, canonStack *[maxStack]uintptr, size uintptr) int { var cidx int var termTrace bool + // Increase the skip count to take into account the frames corresponding + // to runtime.callersRaw and to the C routine that it invokes. + skip += 2 for _, pc := range stk { // Subtract 1 from PC to undo the 1 we added in callback in // go-callers.c. @@ -669,6 +675,16 @@ func fixupStack(stk []uintptr, canonStac break } } + + // Apply skip count. Needs to be done after expanding inline frames. + if skip != 0 { + if skip >= cidx { + return 0 + } + copy(canonStack[:cidx-skip], canonStack[skip:]) + return cidx - skip + } + return cidx } @@ -680,8 +696,8 @@ func fixupStack(stk []uintptr, canonStac // the new bucket. func fixupBucket(b *bucket) { var canonStack [maxStack]uintptr - frames := fixupStack(b.stk(), &canonStack, b.size) - cb := stkbucket(prunedProfile, b.size, canonStack[:frames], true) + frames := fixupStack(b.stk(), b.skip, &canonStack, b.size) + cb := stkbucket(prunedProfile, b.size, 0, canonStack[:frames], true) switch b.typ { default: throw("invalid profile bucket type") Index: libgo/go/runtime/traceback_gccgo.go =================================================================== --- libgo/go/runtime/traceback_gccgo.go (revision 271669) +++ libgo/go/runtime/traceback_gccgo.go (working copy) @@ -63,11 +63,11 @@ func callers(skip int, locbuf []location //go:noescape //extern runtime_callersRaw -func c_callersRaw(skip int32, pcs *uintptr, max int32) int32 +func c_callersRaw(pcs *uintptr, max int32) int32 // callersRaw returns a raw (PCs only) stack trace of the current goroutine. -func callersRaw(skip int, pcbuf []uintptr) int { - n := c_callersRaw(int32(skip)+1, &pcbuf[0], int32(len(pcbuf))) +func callersRaw(pcbuf []uintptr) int { + n := c_callersRaw(&pcbuf[0], int32(len(pcbuf))) return int(n) } Index: libgo/runtime/go-callers.c =================================================================== --- libgo/runtime/go-callers.c (revision 271669) +++ libgo/runtime/go-callers.c (working copy) @@ -268,7 +268,6 @@ Callers (intgo skip, struct __go_open_ar struct callersRaw_data { uintptr* pcbuf; - int skip; int index; int max; }; @@ -280,12 +279,6 @@ static int callback_raw (void *data, uin { struct callersRaw_data *arg = (struct callersRaw_data *) data; - if (arg->skip > 0) - { - --arg->skip; - return 0; - } - /* On the call to backtrace_simple the pc value was most likely decremented if there was a normal call, since the pc referred to the instruction where the call returned and not the call itself. @@ -306,13 +299,12 @@ static int callback_raw (void *data, uin /* runtime_callersRaw is similar to runtime_callers() above, but it returns raw PC values as opposed to file/func/line locations. */ int32 -runtime_callersRaw (int32 skip, uintptr *pcbuf, int32 m) +runtime_callersRaw (uintptr *pcbuf, int32 m) { struct callersRaw_data data; struct backtrace_state* state; data.pcbuf = pcbuf; - data.skip = skip + 1; data.index = 0; data.max = m; runtime_xadd (&__go_runtime_in_callers, 1);