From: Guinevere Larsen <blarsen@redhat.com>
To: "Rémi Bernon" <rbernon@codeweavers.com>, gdb-patches@sourceware.org
Subject: Re: [PATCH] gdb/python: Allow SIGTRAMP_FRAME python unwinders to be created.
Date: Thu, 14 Mar 2024 14:00:37 +0100 [thread overview]
Message-ID: <3d80a22b-3ccf-4da7-9c40-59a3c74ccaa8@redhat.com> (raw)
In-Reply-To: <20240214215916.2655301-1-rbernon@codeweavers.com>
On 14/02/2024 22:58, Rémi Bernon wrote:
> Wine now executes Win32 code on a separate stack for its Unix code. It
> switches from one stack to another on through specific functions, and
> without any custom unwinders, debugging Wine in Gdb will only let you
> see the frames of either the Win32 side, or the Unix side.
>
> The Win32 and Unix call stacks are actually interleaved, with Unix code
> sometimes calling back into Win32. Using a custom Python frame unwinder
> we can provide Gdb with the information it needs to join both toghether
> and show a complete interleaved call stack. However, Gdb will often stop
> unwinding as it will see the frames from one stack as inner the frames
> from the other stack.
>
> This allows to write custom unwinders to produce SIGTRAMP_FRAME typed
> frames, which bypasses this restriction and will show the Win32 / Unix
> gate as a signal frame.
Hi!
Sorry for taking a long time for to review.
This looks like a good improvement. It would be nice if you could
provide a test, but I'm not sure if you are able to manufacture a
trampoline frame for a test. If it isn't possible to make a test patch
LGTM: Reviewed-By: Guinevere Larsen <blarsen@redhat.com>
I hope a responsible maintainer is able to approve this soon (or explain
how to make a faux trampoline frame to test).
--
Cheers,
Guinevere Larsen
She/Her/Hers
> ---
> gdb/python/lib/gdb/__init__.py | 8 ++++----
> gdb/python/lib/gdb/unwinder.py | 7 ++++++-
> gdb/python/py-unwind.c | 16 +++++++++++++++-
> 3 files changed, 25 insertions(+), 6 deletions(-)
>
> diff --git a/gdb/python/lib/gdb/__init__.py b/gdb/python/lib/gdb/__init__.py
> index b3124369fe8..d9226440cbc 100644
> --- a/gdb/python/lib/gdb/__init__.py
> +++ b/gdb/python/lib/gdb/__init__.py
> @@ -86,7 +86,7 @@ frame_filters = {}
> frame_unwinders = []
>
>
> -def _execute_unwinders(pending_frame):
> +def _execute_unwinders(pending_frame, frame_type):
> """Internal function called from GDB to execute all unwinders.
>
> Runs each currently enabled unwinder until it finds the one that
> @@ -105,19 +105,19 @@ def _execute_unwinders(pending_frame):
> """
> for objfile in objfiles():
> for unwinder in objfile.frame_unwinders:
> - if unwinder.enabled:
> + if unwinder.enabled and unwinder.frame_type == frame_type:
> unwind_info = unwinder(pending_frame)
> if unwind_info is not None:
> return (unwind_info, unwinder.name)
>
> for unwinder in current_progspace().frame_unwinders:
> - if unwinder.enabled:
> + if unwinder.enabled and unwinder.frame_type == frame_type:
> unwind_info = unwinder(pending_frame)
> if unwind_info is not None:
> return (unwind_info, unwinder.name)
>
> for unwinder in frame_unwinders:
> - if unwinder.enabled:
> + if unwinder.enabled and unwinder.frame_type == frame_type:
> unwind_info = unwinder(pending_frame)
> if unwind_info is not None:
> return (unwind_info, unwinder.name)
> diff --git a/gdb/python/lib/gdb/unwinder.py b/gdb/python/lib/gdb/unwinder.py
> index 140b84d3374..7e23a662a32 100644
> --- a/gdb/python/lib/gdb/unwinder.py
> +++ b/gdb/python/lib/gdb/unwinder.py
> @@ -29,7 +29,7 @@ class Unwinder(object):
> enabled: A boolean indicating whether the unwinder is enabled.
> """
>
> - def __init__(self, name):
> + def __init__(self, name, frame_type=gdb.NORMAL_FRAME):
> """Constructor.
>
> Args:
> @@ -39,9 +39,14 @@ class Unwinder(object):
> if not isinstance(name, str):
> raise TypeError("incorrect type for name: %s" % type(name))
>
> + self._frame_type = frame_type
> self._name = name
> self._enabled = True
>
> + @property
> + def frame_type(self):
> + return self._frame_type
> +
> @property
> def name(self):
> return self._name
> diff --git a/gdb/python/py-unwind.c b/gdb/python/py-unwind.c
> index 1856e41e2a1..2d36c43d342 100644
> --- a/gdb/python/py-unwind.c
> +++ b/gdb/python/py-unwind.c
> @@ -844,7 +844,8 @@ pyuw_sniffer (const struct frame_unwind *self, frame_info_ptr this_frame,
> /* A (gdb.UnwindInfo, str) tuple, or None. */
> gdbpy_ref<> pyo_execute_ret
> (PyObject_CallFunctionObjArgs (pyo_execute.get (),
> - pyo_pending_frame.get (), NULL));
> + pyo_pending_frame.get (),
> + PyLong_FromLong(self->type), NULL));
> if (pyo_execute_ret == nullptr)
> {
> /* If the unwinder is cancelled due to a Ctrl-C, then propagate
> @@ -965,6 +966,19 @@ pyuw_on_new_gdbarch (struct gdbarch *newarch)
> unwinder->sniffer = pyuw_sniffer;
> unwinder->dealloc_cache = pyuw_dealloc_cache;
> frame_unwind_prepend_unwinder (newarch, unwinder);
> +
> + struct frame_unwind *unwinder_signals
> + = GDBARCH_OBSTACK_ZALLOC (newarch, struct frame_unwind);
> +
> + unwinder_signals->name = "python-sigtramp";
> + unwinder_signals->type = SIGTRAMP_FRAME;
> + unwinder_signals->stop_reason = default_frame_unwind_stop_reason;
> + unwinder_signals->this_id = pyuw_this_id;
> + unwinder_signals->prev_register = pyuw_prev_register;
> + unwinder_signals->unwind_data = (const struct frame_data *) newarch;
> + unwinder_signals->sniffer = pyuw_sniffer;
> + unwinder_signals->dealloc_cache = pyuw_dealloc_cache;
> + frame_unwind_prepend_unwinder (newarch, unwinder_signals);
> data->unwinder_registered = 1;
> }
> }
next prev parent reply other threads:[~2024-03-14 13:00 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-14 21:58 Rémi Bernon
2024-02-21 16:41 ` [PING][PATCH] " Rémi Bernon
2024-02-29 8:17 ` Rémi Bernon
2024-03-14 13:00 ` Guinevere Larsen [this message]
2024-03-19 15:58 ` [PATCH] " Rémi Bernon
2024-03-20 14:13 ` Andrew Burgess
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=3d80a22b-3ccf-4da7-9c40-59a3c74ccaa8@redhat.com \
--to=blarsen@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=rbernon@codeweavers.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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).