From: "Metzger, Markus T" <markus.t.metzger@intel.com>
To: Jan Kratochvil <jan.kratochvil@redhat.com>
Cc: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: RE: [+rfc] Re: [patch v6 00/21] record-btrace: reverse
Date: Thu, 28 Nov 2013 10:54:00 -0000 [thread overview]
Message-ID: <A78C989F6D9628469189715575E55B230AA2DBEE@IRSMSX104.ger.corp.intel.com> (raw)
In-Reply-To: <20131127185727.GA18038@host2.jankratochvil.net>
> -----Original Message-----
> From: Jan Kratochvil [mailto:jan.kratochvil@redhat.com]
> Sent: Wednesday, November 27, 2013 7:57 PM
> To: Metzger, Markus T
> Cc: gdb-patches@sourceware.org
> Subject: Re: [+rfc] Re: [patch v6 00/21] record-btrace: reverse
>
> On Thu, 07 Nov 2013 16:41:40 +0100, Metzger, Markus T wrote:
> > I hacked a first prototype of this (see below). It passes most tests but
> > results in three fails in the record_goto suite.
> >
> > One thing that it shows, though, is that it only removes the 'mostly
> harmless'
> > hack in the various goto functions shown above.
> >
> > The more serious hacks in record_btrace_start_replaying
> >
> > /* Make sure we're not using any stale registers. */
> > registers_changed_ptid (tp->ptid);
> >
> > /* We just started replaying. The frame id cached for stepping is
> based
> > on unwinding, not on branch tracing. Recompute it. */
> > frame = get_current_frame_nocheck ();
> > insn = btrace_insn_get (replay);
> > sal = find_pc_line (insn->pc, 0);
> > set_step_info (frame, sal);
> >
> > and record_btrace_stop_replaying
> >
> > /* Make sure we're not leaving any stale registers. */
> > registers_changed_ptid (tp->ptid);
> >
> > however, are not removed by this.
>
> In such case it is not finished. These hacks should not be needed.
See below.
> > They are required when reverse-stepping the first time or when
> > stepping past the end of the execution trace.
>
> I have patched what you describe as the problem. But as I do not have a box
> with reliably working BTS so it is difficult for me to say whether it works or
> not. I can look at other problems if you describe them from a reliable box.
Those hacks are not related to "record goto" and are thus also not affected
by the patch to implement "record goto" via wait/resume.
Let me try to describe the problem. It is also exposed by the next.exp test.
Assume we enable btrace and next over a function call. We will end up
right after the call instruction.
(gdb) record btrace
(gdb) n
50 return 0; /* main.3 */
(gdb) record instruction-history -
31 0x0000000000400590 <fun1+0>: push %rbp
32 0x0000000000400591 <fun1+1>: mov %rsp,%rbp
33 0x0000000000400594 <fun1+4>: leaveq
34 0x0000000000400595 <fun1+5>: retq
35 0x000000000040059f <fun2+9>: leaveq
36 0x00000000004005a0 <fun2+10>: retq
37 0x00000000004005af <fun3+14>: leaveq
38 0x00000000004005b0 <fun3+15>: retq
39 0x00000000004005c4 <fun4+19>: leaveq
40 0x00000000004005c5 <fun4+20>: retq
(gdb) disas
Dump of assembler code for function main:
0x00000000004005c6 <+0>: push %rbp
0x00000000004005c7 <+1>: mov %rsp,%rbp
0x00000000004005ca <+4>: callq 0x4005b1 <fun4>
=> 0x00000000004005cf <+9>: mov $0x0,%eax
0x00000000004005d4 <+14>: leaveq
0x00000000004005d5 <+15>: retq
End of assembler dump.
(gdb)
If we now do a reverse-next, we end up inside the function
we were supposed to step over.
(gdb) reverse-next
fun4 () at record_goto.c:44
44 } /* fun4.5 */
(gdb) record instruction-history -
30 0x000000000040059a <fun2+4>: callq 0x400590 <fun1>
31 0x0000000000400590 <fun1+0>: push %rbp
32 0x0000000000400591 <fun1+1>: mov %rsp,%rbp
33 0x0000000000400594 <fun1+4>: leaveq
34 0x0000000000400595 <fun1+5>: retq
35 0x000000000040059f <fun2+9>: leaveq
36 0x00000000004005a0 <fun2+10>: retq
37 0x00000000004005af <fun3+14>: leaveq
38 0x00000000004005b0 <fun3+15>: retq
39 => 0x00000000004005c4 <fun4+19>: leaveq
(gdb) disas
Dump of assembler code for function fun4:
0x00000000004005b1 <+0>: push %rbp
0x00000000004005b2 <+1>: mov %rsp,%rbp
0x00000000004005b5 <+4>: callq 0x400590 <fun1>
0x00000000004005ba <+9>: callq 0x400596 <fun2>
0x00000000004005bf <+14>: callq 0x4005a1 <fun3>
=> 0x00000000004005c4 <+19>: leaveq
0x00000000004005c5 <+20>: retq
End of assembler dump.
(gdb)
The reason is the way how GDB implements next/reverse-next.
We store the frame_id of the current frame and do a single-step.
Then we try to detect stepping into a subroutine by unwinding
the stack frames and comparing the frame_id's with our stored
frame_id.
The stored frame_id has been computed using dwarf2 frame
unwind.
After single-stepping, we're replaying the recorded execution.
The frame_id's are now computed using btrace frame unwind.
Our parent's frame_id does not compare equal to the stored
frame_id. We fail to detect that we just reverse-stepped into
a subroutine.
The s/w record implementation does not suffer from this problem
because it traces data and is hence able to use the dwarf2 frame
unwinder also when replacing.
The way I tried to overcome this is to recompute all frame_id's
when we start replaying. This will cause us to store a btrace
frame_id in the stepping algorithm. Now we are able to detect
that we reverse-stepped into a subroutine.
Do you have a better idea?
Regards,
Markus.
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052
next prev parent reply other threads:[~2013-11-28 8:42 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-20 11:30 Markus Metzger
2013-09-20 11:30 ` [patch v6 06/21] btrace: increase buffer size Markus Metzger
2013-09-20 11:30 ` [patch v6 12/21] frame, backtrace: allow targets to supply a frame unwinder Markus Metzger
2013-09-20 11:30 ` [patch v6 10/21] target: add ops parameter to to_prepare_to_store method Markus Metzger
2013-09-20 11:30 ` [patch v6 16/21] record-btrace: provide target_find_new_threads method Markus Metzger
2013-09-20 11:30 ` [patch v6 15/21] record-btrace: add to_wait and to_resume target methods Markus Metzger
2013-09-20 11:30 ` [patch v6 14/21] record-btrace: provide xfer_partial target method Markus Metzger
2013-09-20 11:30 ` [patch v6 02/21] gdbarch: add instruction predicate methods Markus Metzger
2013-09-20 11:31 ` [patch v6 07/21] record-btrace: optionally indent function call history Markus Metzger
2013-10-06 19:47 ` Jan Kratochvil
2013-09-20 11:31 ` [patch v6 01/21] btrace, linux: fix memory leak when reading branch trace Markus Metzger
2013-09-20 11:31 ` [patch v6 18/21] record-btrace: extend unwinder Markus Metzger
2013-10-06 19:49 ` Jan Kratochvil
2013-11-06 13:45 ` Metzger, Markus T
2013-11-25 21:11 ` Jan Kratochvil
2013-09-20 11:31 ` [patch v6 03/21] btrace: change branch trace data structure Markus Metzger
2013-10-06 19:46 ` Jan Kratochvil
2013-09-20 11:31 ` [patch v6 13/21] record-btrace, frame: supply target-specific unwinder Markus Metzger
2013-09-20 11:31 ` [patch v6 05/21] record-btrace: start counting at one Markus Metzger
2013-09-20 11:31 ` [patch v6 19/21] btrace, gdbserver: read branch trace incrementally Markus Metzger
2013-10-06 19:51 ` Jan Kratochvil
2013-09-20 11:31 ` [patch v6 08/21] record-btrace: make ranges include begin and end Markus Metzger
2013-09-20 11:31 ` [patch v6 04/21] record-btrace: fix insn range in function call history Markus Metzger
2013-09-20 11:31 ` [patch v6 20/21] record-btrace: show trace from enable location Markus Metzger
2013-09-20 11:31 ` [patch v6 21/21] record-btrace: add (reverse-)stepping support Markus Metzger
2013-10-06 19:52 ` Jan Kratochvil
2013-11-06 15:06 ` Metzger, Markus T
2013-11-26 13:48 ` Jan Kratochvil
2013-09-20 11:31 ` [patch v6 11/21] record-btrace: supply register target methods Markus Metzger
2013-09-20 11:31 ` [patch v6 09/21] btrace: add replay position to btrace thread info Markus Metzger
2013-09-20 11:31 ` [patch v6 17/21] record-btrace: add record goto target methods Markus Metzger
2013-10-06 19:48 ` Jan Kratochvil
2013-09-26 19:16 ` v6 crash bugreport [Re: [patch v6 00/21] record-btrace: reverse] Jan Kratochvil
2013-09-27 6:37 ` Metzger, Markus T
2013-10-06 19:59 ` [+rfc] Re: [patch v6 00/21] record-btrace: reverse Jan Kratochvil
2013-11-07 15:44 ` Metzger, Markus T
2013-11-27 20:35 ` Jan Kratochvil
2013-11-28 10:54 ` Metzger, Markus T [this message]
2013-11-28 22:35 ` Jan Kratochvil
2013-11-29 14:27 ` Metzger, Markus T
2013-12-11 19:57 ` Jan Kratochvil
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=A78C989F6D9628469189715575E55B230AA2DBEE@IRSMSX104.ger.corp.intel.com \
--to=markus.t.metzger@intel.com \
--cc=gdb-patches@sourceware.org \
--cc=jan.kratochvil@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox