This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC] [PATCH] Provide the ability to write the frame unwinder in Python


>> So here's the new proposal for the Python API, hopefully in
>> line with what you have in mind for Guile:
>>
>> If a sniffer is able to unwind a frame, it should return an instance of
>> gdb.sniffer.UnwindInfo class, which has the following methods:
>> * UnwindInfo(registers)
>>   Constructor. `registers' is a tuple of (register_number, register_value)
>>   2-tuples for the registers that can be unwound.
>> * frame_id_build_wild(SP)
>>   frame_id_build(SP, PC)
>>   frame_id_build_special(SP, PC, SPECIAL)
>>   Sets frame ID by calling the corresponding GDB function. It is an error
>>   to return UnwindInfo object before one of these methods is called (a
>>   sniffer should return None if it cannot unwind a frame)
>> * set_register(register_number, register_value)
>>   Adds a 2-tuple to the list of unwound registers. Not sure this is needed.
>
> You'll need a link to the sniffer_info in order to be able to give good
> errors for set_register, to check that the register exists and that the
> value is of the correct type and size.  For that reason, in my first
> draft of a Guile interface, the "ephemeral frame" is like your
> sniffer_info and unwind_info together.  Perhaps this is a bad idea
> though.
I am somewhat confused, so maybe my proposal wasn't clear, let
me try again.
In the current implementation for Python a sniffer return something
similar to this (x86_64 architecture):
    return (((AMD64_RBP_NUM, fp), (AMD64_RIP_NUM, pc), (AMD64_RSP_NUM, sp)),
                (AMD64_RSP_NUM, AMD64_RIP_NUM))
(the variables `fp', 'pc', 'sp' contain the values of the respective
variables in the returned frame).
I am proposing to return an object as follows:
    previous_frame = UnwindInfo(((AMD64_RBP_NUM, fp), (AMD64_RIP_NUM,
pc), (AMD64_RSP_NUM, sp)))
    previous_frame.frame_id_build(sp, pc)
    return previous_frame
or
    previous_frame = UnwindInfo(((AMD64_RBP_NUM, fp), (AMD64_RIP_NUM, pc)))
    previous_frame.set_register(AMD64_RSP_NUM, sp)
    previous_frame.frame_id_build(sp, pc)
    return previous_frame
Checking the type and size of the a register in the implementation of
the `set_register' method depends only on the current architecture,
not on what's available in the sniffer_info.

>>> [W]hy not specify registers as strings, as elsewhere
>>> (e.g. gdb.Frame.read_register)?
>> My concern is that name lookups are expensive
>
> Are they?  I wouldn't think so, no more than anything that happens in
> Python.
The function mapping a register name to the number,
`user_reg_map_name_to_regnum', compares given register name
with all known register names, one at a time. I thought Python interpreter
is somewhat smarter than that and use dictionaries for lookups. Even if
it is not, there is no need to pile up inefficiency.
Actually, it's not that difficult to be able to pass either a register number,
or register name.

>>> The sniffer_info object is unfortunate -- it's a frame, but without
>>> frame methods.  You can't get its architecture from python, for
>>> example, or get the next frame.  More about that later.
>> I guess you know by now that it is not a frame. The interface
>> reflects that.
>
> Well.  I mean, it's not a frame to Python, but its only state is a
> "struct frame_info" pointer, and its only method is also present on
> gdb.Frame, so it looks a lot like a frame to me :)
>
>>> In the read_register() function, I believe you can use
>>> get_frame_register_value instead of deprecated_frame_register_read.
>> You can't, get frame_register_value wiil assert because the frame
>> has no frame ID yet.
>
> The comment in the source says:
>
>           /* Call `deprecated_frame_register_read' -- calling
>              `value_of_register' would an assert in `get_frame_id'
>              because our frame is incomplete.  */
>
> Whereas get_frame_register_value looks something like this:
>
>   struct value *
>   frame_unwind_register_value (struct frame_info *frame, int regnum)
>   {
>     /* Find the unwinder.  */
>     if (frame->unwind == NULL)
>       frame_unwind_find_by_frame (frame, &frame->prologue_cache);
>
>     /* Ask this frame to unwind its register.  */
>     return frame->unwind->prev_register (frame, &frame->prologue_cache, regnum);
>   }
>
>   struct value *
>   get_frame_register_value (struct frame_info *frame, int regnum)
>   {
>     return frame_unwind_register_value (frame->next, regnum);
>   }
>
> So it doesn't touch THIS_FRAME.

Here's traceback I get when I try to call value_of_register:
#5  0x00000000006b5212 in internal_error (file=file@entry=0x838760
"<...>/gdb/frame.c", line=line@entry=478, fmt=<optimized out>)
    at gdb/common/errors.c:55
#6  0x0000000000688796 in get_frame_id (fi=fi@entry=0x24b63f0) at
gdb/frame.c:478
#7  0x0000000000551ea3 in value_of_register_lazy
(frame=frame@entry=0x24b63f0, regnum=regnum@entry=6) at
gdb/findvar.c:290
#8  0x0000000000551fef in value_of_register (regnum=6,
frame=0x24b63f0) at gdb/findvar.c:271

I am not sure why this particular comment I've added attracts so much
attention :-)

> Alexander, did you not run into nasty crashes while doing random Python
> things inside your unwind handler?
I used to have random crashes before I learned to call `ensure_python_env'
to establish Python runtime environment before suing Python API, but after
that, no. Do you have an example that you can share?


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]