This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v4] Add Guile frame unwinder interface
- From: Andy Wingo <wingo at igalia dot com>
- To: Doug Evans <dje at google dot com>
- Cc: gdb-patches at sourceware dot org, Pedro Alves <palves at redhat dot com>
- Date: Thu, 09 Apr 2015 22:02:38 +0200
- Subject: Re: [PATCH v4] Add Guile frame unwinder interface
- Authentication-results: sourceware.org; auth=none
- References: <87bnjtc4o4 dot fsf at igalia dot com> <21777 dot 57403 dot 725615 dot 308934 at ruffy2 dot mtv dot corp dot google dot com>
Hi!
I have a new version of this patch that I'll post as a separate thread,
addressing all the issues except a couple that I comment below.
On Tue 24 Mar 2015 23:07, Doug Evans <dje@google.com> writes:
> > +@deffn {Scheme Procedure} unwind-info-add-saved-register! unwind-info register value
> > +Set the saved value of a register in a ephemeral frame.
> > +
> > +@var{register} names a register, and should be a string, for example
> > +@samp{rip}. @var{value} is the register value, as a @value{GDBN}
>
> To be more universal let's replace "rip" here with "pc".
I kept in "rip" because user regs like "pc" don't work on pending
frames, and I didn't want to mislead the reader that user regs would
work. (OK, "pc" is probably a user reg only on some architectures.)
WDYT? Still change to "pc"?
> > +Any register whose value is not recorded as saved via
> > +@code{unwind-info-add-saved-register!} will be marked as ``not saved''
> > +in the outer frame.
>
> "outer frame" here is confusing.
> I'd have thought this would read "``not saved'' in the frame".
> [with the frame?]
Let's say you are unwinding pending frame 1, which will be frame-1. You
add some saved registers to it. Those registers will become the values
of (frame-read-register (frame-older frame-1) "foo"). If a register
isn't added to the set, then the frame-read-register on the older frame
will return "not saved". Is that clear? It's a bit confusing but I
think it reflects the problem space...
> > +(define* (make-frame-unwinder name procedure #:key (priority 20) (enabled? #t))
>
> In addition to removing priority (for now), let's remove enabled? as
> a parameter here. The user can disable the unwinder before registering it.
I kept it in, as noted in the patch set header of the next mail, because
that's more like frame filters which indeed have a priority and also
have an enabled flag. Please let me know again if this bugs you and
I'll take them both out.
> > +(define* (register-frame-unwinder! unwinder #:key scope)
>
> Guile question: Does scope default to #f?
Yep.
> > +gdb_test_no_output "guile (enable-frame-unwinder! \"Synthetic\")" \
> > + "enable synthetic unwinder"
> > +
> > +gdb_breakpoint "f2"
> > +gdb_continue_to_breakpoint "breakpoint at f2"
> > +
> > +gdb_test "bt 10" " f2 .*cabba9e5 .*cabba9e5 .*"
>
> IWBN to test that unwinding handles unwinder enabling.
> E.g., try a backtrace with and without an unwinder enabled.
>
> This brings up an issue I haven't seen discussed before.
> Apologies if I missed it.
> Should enabling/disabling/registering/removing an unwinder
> invalidate the frame cache? Seems like it.
I can't do it here for the reason you mention. I have added something
that does a bt from before the enable!, and also doesn't have a trailing
.*. A followup perhaps?
> > +(define (synthetic-unwinder frame)
> > + ;; Increment the stack pointer, set IP to 0xdeadbeef
> > + (let* ((this-pc (ephemeral-frame-read-register frame "rip"))
> > + (this-sp (ephemeral-frame-read-register frame "rsp"))
>
> rip,rsp are architecture-specific names but the .exp file
> doesn't check for amd64. IWBN if the test wasn't architecture-specific.
> Will "pc" and "sp" work here?
They won't currently. Should I look into making user regs work or
somehow mark this test as architecture-specific?
Thanks for your time, Doug!
Andy