From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 768 invoked by alias); 6 Feb 2017 23:56:41 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 743 invoked by uid 89); 6 Feb 2017 23:56:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=unexplained, sk:_dl_run, 0x4, sk:markus X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 06 Feb 2017 23:56:29 +0000 Received: from svr-orw-mbx-03.mgc.mentorg.com ([147.34.90.203]) by relay1.mentorg.com with esmtp id 1cat91-0002n4-L4 from Luis_Gustavo@mentor.com ; Mon, 06 Feb 2017 15:56:27 -0800 Received: from [172.30.11.152] (147.34.91.1) by svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) with Microsoft SMTP Server (TLS) id 15.0.1210.3; Mon, 6 Feb 2017 15:56:24 -0800 Subject: Re: [PATCH] btrace: preserve call stack on function switch References: <1485940343-29405-1-git-send-email-markus.t.metzger@intel.com> To: Markus Metzger , Reply-To: Luis Machado From: Luis Machado Message-ID: <826ae22f-eec3-2f4b-756b-a8ecee8defbc@codesourcery.com> Date: Mon, 06 Feb 2017 23:56:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: <1485940343-29405-1-git-send-email-markus.t.metzger@intel.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) To svr-orw-mbx-03.mgc.mentorg.com (147.34.90.203) X-IsSubscribed: yes X-SW-Source: 2017-02/txt/msg00139.txt.bz2 On 02/01/2017 03:12 AM, Markus Metzger wrote: > On 64-bit FC25, the _dl_runtime_resolve function uses a conditional branch to > 'call' a particular variant optimized for that system: > > (gdb) disas _dl_runtime_resolve_avx_opt > Dump of assembler code for function _dl_runtime_resolve_avx_opt: > 0x00007ffff7deeb60 <+0>: push %rax > 0x00007ffff7deeb61 <+1>: push %rcx > 0x00007ffff7deeb62 <+2>: push %rdx > 0x00007ffff7deeb63 <+3>: mov $0x1,%ecx > 0x00007ffff7deeb68 <+8>: xgetbv > 0x00007ffff7deeb6b <+11>: mov %eax,%r11d > 0x00007ffff7deeb6e <+14>: pop %rdx > 0x00007ffff7deeb6f <+15>: pop %rcx > 0x00007ffff7deeb70 <+16>: pop %rax > 0x00007ffff7deeb71 <+17>: and $0x4,%r11d > 0x00007ffff7deeb75 <+21>: bnd je 0x7ffff7def4a0 <_dl_runtime_resolve_sse_vex> > End of assembler dump. > > When computing the function-level trace, btrace treats this as a switch from > _dl_runtime_resolve_avx_opt to _dl_runtime_resolve_sse_vex. We know that we > switched functions but we can't really say in which caller/callee relationship > those two functions are. > > In addition to preserving the indentaion level, also preserve the caller > information. This is a heuristic since we don't really know. But at least in > this case, this seems to be the right thing to do. > > This fixes a fail in gdb.btrace/rn-dl-bind.exp on 64-bit FC25. > > 2017-02-01 Markus Metzger > > * btrace.c (ftrace_new_switch): Preserve up link and flags. > --- > gdb/btrace.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/gdb/btrace.c b/gdb/btrace.c > index 6d621e4..ddf6692 100644 > --- a/gdb/btrace.c > +++ b/gdb/btrace.c > @@ -448,9 +448,11 @@ ftrace_new_switch (struct btrace_function *prev, > { > struct btrace_function *bfun; > > - /* This is an unexplained function switch. The call stack will likely > - be wrong at this point. */ > + /* This is an unexplained function switch. We can't really be sure about the > + call stack, yet the best I can think of right now is to preserve it. */ > bfun = ftrace_new_function (prev, mfun, fun); > + bfun->up = prev->up; > + bfun->flags = prev->flags; > > ftrace_debug (bfun, "new switch"); > > I don't know much about btrace, but the patch looks reasonable given the explanation. From what i understood, this adds an heuristic where previously there was none? We just declared defeat before the patch?