[PATCH v3 4/5] Continue making GDB use frame_info_ptr
Andrew Burgess
aburgess@redhat.com
Thu Jul 28 16:44:39 GMT 2022
Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
<snip>
> diff --git a/gdb/dwarf2/frame-tailcall.c b/gdb/dwarf2/frame-tailcall.c
> index 7f30a0d3126..e4c931ad3b9 100644
> --- a/gdb/dwarf2/frame-tailcall.c
> +++ b/gdb/dwarf2/frame-tailcall.c
> @@ -39,7 +39,7 @@ static htab_t cache_htab;
> struct tailcall_cache
> {
> /* It must be the first one of this struct. It is the furthest callee. */
> - struct frame_info *next_bottom_frame;
> + frame_info *next_bottom_frame;
>
> /* Reference count. The whole chain of virtual tail call frames shares one
> tailcall_cache. */
> @@ -90,12 +90,12 @@ cache_eq (const void *arg1, const void *arg2)
> tailcall_cache. */
>
> static struct tailcall_cache *
> -cache_new_ref1 (struct frame_info *next_bottom_frame)
> +cache_new_ref1 (frame_info_ptr next_bottom_frame)
> {
> struct tailcall_cache *cache = XCNEW (struct tailcall_cache);
> void **slot;
>
> - cache->next_bottom_frame = next_bottom_frame;
> + cache->next_bottom_frame = next_bottom_frame.get();
Ooops! Missing space before ().
<snip>
> diff --git a/gdb/guile/scm-frame.c b/gdb/guile/scm-frame.c
> index d49233661e6..6152659c940 100644
> --- a/gdb/guile/scm-frame.c
> +++ b/gdb/guile/scm-frame.c
> @@ -204,7 +204,7 @@ gdbscm_frame_p (SCM scm)
> Returns a <gdb:exception> object if there is an error. */
>
> static SCM
> -frscm_scm_from_frame (struct frame_info *frame, struct inferior *inferior)
> +frscm_scm_from_frame (frame_info_ptr frame, struct inferior *inferior)
> {
> frame_smob *f_smob, f_smob_for_lookup;
> SCM f_scm;
> @@ -263,7 +263,7 @@ frscm_scm_from_frame (struct frame_info *frame, struct inferior *inferior)
> A Scheme exception is thrown if there is an error. */
>
> static SCM
> -frscm_scm_from_frame_unsafe (struct frame_info *frame,
> +frscm_scm_from_frame_unsafe (frame_info_ptr frame,
> struct inferior *inferior)
> {
> SCM f_scm = frscm_scm_from_frame (frame, inferior);
> @@ -287,7 +287,7 @@ frscm_get_frame_arg_unsafe (SCM self, int arg_pos, const char *func_name)
> }
>
> /* There is no gdbscm_scm_to_frame function because translating
> - a frame SCM object to a struct frame_info * can throw a GDB error.
> + a frame SCM object to a frame_info_ptr can throw a GDB error.
> Thus code working with frames has to handle both Scheme errors (e.g., the
> object is not a frame) and GDB errors (e.g., the frame lookup failed).
>
> @@ -331,10 +331,10 @@ frscm_get_frame_smob_arg_unsafe (SCM self, int arg_pos, const char *func_name)
> This function calls GDB routines, so don't assume a GDB error will
> not be thrown. */
>
> -struct frame_info *
> +frame_info_ptr
> frscm_frame_smob_to_frame (frame_smob *f_smob)
> {
> - struct frame_info *frame;
> + frame_info_ptr frame;
>
> frame = frame_find_by_id (f_smob->frame_id);
> if (frame == NULL)
> @@ -386,7 +386,7 @@ static SCM
> gdbscm_frame_valid_p (SCM self)
> {
> frame_smob *f_smob;
> - struct frame_info *frame = NULL;
> + frame_info_ptr frame = NULL;
>
> f_smob = frscm_get_frame_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
>
> @@ -414,7 +414,7 @@ gdbscm_frame_name (SCM self)
> frame_smob *f_smob;
> gdb::unique_xmalloc_ptr<char> name;
> enum language lang = language_minimal;
> - struct frame_info *frame = NULL;
> + frame_info_ptr frame = NULL;
> SCM result;
>
> f_smob = frscm_get_frame_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
> @@ -454,7 +454,7 @@ gdbscm_frame_type (SCM self)
> {
> frame_smob *f_smob;
> enum frame_type type = NORMAL_FRAME;
> - struct frame_info *frame = NULL;
> + frame_info_ptr frame = NULL;
>
> f_smob = frscm_get_frame_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
>
> @@ -487,7 +487,7 @@ static SCM
> gdbscm_frame_arch (SCM self)
> {
> frame_smob *f_smob;
> - struct frame_info *frame = NULL;
> + frame_info_ptr frame = NULL;
>
> f_smob = frscm_get_frame_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
>
> @@ -518,7 +518,7 @@ static SCM
> gdbscm_frame_unwind_stop_reason (SCM self)
> {
> frame_smob *f_smob;
> - struct frame_info *frame = NULL;
> + frame_info_ptr frame = NULL;
> enum unwind_stop_reason stop_reason;
>
> f_smob = frscm_get_frame_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
> @@ -553,7 +553,7 @@ gdbscm_frame_pc (SCM self)
> {
> frame_smob *f_smob;
> CORE_ADDR pc = 0;
> - struct frame_info *frame = NULL;
> + frame_info_ptr frame = NULL;
>
> f_smob = frscm_get_frame_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
>
> @@ -587,7 +587,7 @@ gdbscm_frame_block (SCM self)
> {
> frame_smob *f_smob;
> const struct block *block = NULL, *fn_block;
> - struct frame_info *frame = NULL;
> + frame_info_ptr frame = NULL;
>
> f_smob = frscm_get_frame_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
>
> @@ -639,7 +639,7 @@ gdbscm_frame_function (SCM self)
> {
> frame_smob *f_smob;
> struct symbol *sym = NULL;
> - struct frame_info *frame = NULL;
> + frame_info_ptr frame = NULL;
>
> f_smob = frscm_get_frame_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
>
> @@ -676,8 +676,8 @@ static SCM
> gdbscm_frame_older (SCM self)
> {
> frame_smob *f_smob;
> - struct frame_info *prev = NULL;
> - struct frame_info *frame = NULL;
> + frame_info_ptr prev = NULL;
> + frame_info_ptr frame = NULL;
>
> f_smob = frscm_get_frame_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
>
> @@ -714,8 +714,8 @@ static SCM
> gdbscm_frame_newer (SCM self)
> {
> frame_smob *f_smob;
> - struct frame_info *next = NULL;
> - struct frame_info *frame = NULL;
> + frame_info_ptr next = NULL;
> + frame_info_ptr frame = NULL;
>
> f_smob = frscm_get_frame_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
>
> @@ -752,7 +752,7 @@ gdbscm_frame_sal (SCM self)
> {
> frame_smob *f_smob;
> struct symtab_and_line sal;
> - struct frame_info *frame = NULL;
> + frame_info_ptr frame = NULL;
>
> f_smob = frscm_get_frame_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
>
> @@ -786,7 +786,7 @@ gdbscm_frame_read_register (SCM self, SCM register_scm)
> {
> char *register_str;
> struct value *value = NULL;
> - struct frame_info *frame = NULL;
> + frame_info_ptr frame = NULL;
> frame_smob *f_smob;
>
> f_smob = frscm_get_frame_smob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
> @@ -847,7 +847,7 @@ gdbscm_frame_read_var (SCM self, SCM symbol_scm, SCM rest)
> frame_smob *f_smob;
> int block_arg_pos = -1;
> SCM block_scm = SCM_UNDEFINED;
> - struct frame_info *frame = NULL;
> + frame_info_ptr frame = NULL;
Unfortunately, at least this change, and possibly other changes in the
guile code isn't safe, this causes a test failure in gdb.guile/scm-frame.exp.
The guile API is not great for embedding into C++ as it makes use of
longjmp for exception handling. Any call that ends up at scm_throw will
perform a longjmp, this includes things like GDBSCM_HANDLE_GDB_EXCEPTION
and gdbscm_invalid_object_error.
We either need to be really careful about the scope of any
frame_info_ptr objects in guile code, or, avoid frame_info_ptr in guile
code and just fall back to raw frame_info*.
<snip>
> diff --git a/gdb/ia64-tdep.c b/gdb/ia64-tdep.c
> index ab4e52d16d7..7eea8e72616 100644
> --- a/gdb/ia64-tdep.c
> +++ b/gdb/ia64-tdep.c
> @@ -1217,7 +1217,7 @@ ia64_convert_register_p (struct gdbarch *gdbarch, int regno, struct type *type)
> }
>
> static int
> -ia64_register_to_value (struct frame_info *frame, int regnum,
> +ia64_register_to_value (frame_info_ptr frame, int regnum,
> struct type *valtype, gdb_byte *out,
> int *optimizedp, int *unavailablep)
> {
> @@ -1238,7 +1238,7 @@ ia64_register_to_value (struct frame_info *frame, int regnum,
> }
>
> static void
> -ia64_value_to_register (struct frame_info *frame, int regnum,
> +ia64_value_to_register (frame_info_ptr frame, int regnum,
> struct type *valtype, const gdb_byte *in)
> {
> struct gdbarch *gdbarch = get_frame_arch (frame);
> @@ -1358,7 +1358,7 @@ ia64_alloc_frame_cache (void)
>
> static CORE_ADDR
> examine_prologue (CORE_ADDR pc, CORE_ADDR lim_pc,
> - struct frame_info *this_frame,
> + frame_info_ptr this_frame,
> struct ia64_frame_cache *cache)
> {
> CORE_ADDR next_pc;
> @@ -1839,7 +1839,7 @@ ia64_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
> /* Normal frames. */
>
> static struct ia64_frame_cache *
> -ia64_frame_cache (struct frame_info *this_frame, void **this_cache)
> +ia64_frame_cache (frame_info_ptr this_frame, void **this_cache)
> {
> struct gdbarch *gdbarch = get_frame_arch (this_frame);
> enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> @@ -1884,7 +1884,7 @@ ia64_frame_cache (struct frame_info *this_frame, void **this_cache)
> }
>
> static void
> -ia64_frame_this_id (struct frame_info *this_frame, void **this_cache,
> +ia64_frame_this_id (frame_info_ptr this_frame, void **this_cache,
> struct frame_id *this_id)
> {
> struct gdbarch *gdbarch = get_frame_arch (this_frame);
> @@ -1901,11 +1901,11 @@ ia64_frame_this_id (struct frame_info *this_frame, void **this_cache,
> paddress (gdbarch, this_id->code_addr),
> paddress (gdbarch, this_id->stack_addr),
> paddress (gdbarch, cache->bsp),
> - host_address_to_string (this_frame));
> + host_address_to_string (this_frame.get ()));
> }
>
> static struct value *
> -ia64_frame_prev_register (struct frame_info *this_frame, void **this_cache,
> +ia64_frame_prev_register (frame_info_ptr this_frame, void **this_cache,
> int regnum)
> {
> struct gdbarch *gdbarch = get_frame_arch (this_frame);
> @@ -2173,7 +2173,7 @@ static const struct frame_unwind ia64_frame_unwind =
> /* Signal trampolines. */
>
> static void
> -ia64_sigtramp_frame_init_saved_regs (struct frame_info *this_frame,
> +ia64_sigtramp_frame_init_saved_regs (frame_info_ptr this_frame,
> struct ia64_frame_cache *cache)
> {
> struct gdbarch *gdbarch = get_frame_arch (this_frame);
> @@ -2227,7 +2227,7 @@ ia64_sigtramp_frame_init_saved_regs (struct frame_info *this_frame,
> }
>
> static struct ia64_frame_cache *
> -ia64_sigtramp_frame_cache (struct frame_info *this_frame, void **this_cache)
> +ia64_sigtramp_frame_cache (frame_info_ptr this_frame, void **this_cache)
> {
> struct gdbarch *gdbarch = get_frame_arch (this_frame);
> enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> @@ -2258,7 +2258,7 @@ ia64_sigtramp_frame_cache (struct frame_info *this_frame, void **this_cache)
> }
>
> static void
> -ia64_sigtramp_frame_this_id (struct frame_info *this_frame,
> +ia64_sigtramp_frame_this_id (frame_info_ptr this_frame,
> void **this_cache, struct frame_id *this_id)
> {
> struct gdbarch *gdbarch = get_frame_arch (this_frame);
> @@ -2275,11 +2275,11 @@ ia64_sigtramp_frame_this_id (struct frame_info *this_frame,
> paddress (gdbarch, this_id->code_addr),
> paddress (gdbarch, this_id->stack_addr),
> paddress (gdbarch, cache->bsp),
> - host_address_to_string (this_frame));
> + host_address_to_string (this_frame.get ()));
> }
>
> static struct value *
> -ia64_sigtramp_frame_prev_register (struct frame_info *this_frame,
> +ia64_sigtramp_frame_prev_register (frame_info_ptr this_frame,
> void **this_cache, int regnum)
> {
> struct ia64_frame_cache *cache =
> @@ -2332,7 +2332,7 @@ ia64_sigtramp_frame_prev_register (struct frame_info *this_frame,
>
> static int
> ia64_sigtramp_frame_sniffer (const struct frame_unwind *self,
> - struct frame_info *this_frame,
> + frame_info_ptr this_frame,
> void **this_cache)
> {
> gdbarch *arch = get_frame_arch (this_frame);
> @@ -2362,7 +2362,7 @@ static const struct frame_unwind ia64_sigtramp_frame_unwind =
>
>
> static CORE_ADDR
> -ia64_frame_base_address (struct frame_info *this_frame, void **this_cache)
> +ia64_frame_base_address (frame_info_ptr this_frame, void **this_cache)
> {
> struct ia64_frame_cache *cache = ia64_frame_cache (this_frame, this_cache);
>
> @@ -2483,7 +2483,7 @@ ia64_access_reg (unw_addr_space_t as, unw_regnum_t uw_regnum, unw_word_t *val,
> {
> int regnum = ia64_uw2gdb_regnum (uw_regnum);
> unw_word_t bsp, sof, cfm, psr, ip;
> - struct frame_info *this_frame = (struct frame_info *) arg;
> + frame_info_ptr this_frame = (frame_info_ptr ) arg;
I'm a little suspicious about this change. ARG arrives as a void*,
while a frame_info_ptr is larger than a single void*. I suspect there's
probably a place in ia64-libunwind-tdep.c, maybe in
libunwind_frame_cache where we are trying to pass in a frame_info_ptr
when we probably need to pass a raw frame_info*.
I suspect (but haven't checked) that this code is only compiled on a
native ia64 target.
<snip>
> diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
> index 366f3745b9d..b5a8280b350 100644
> --- a/gdb/python/py-framefilter.c
> +++ b/gdb/python/py-framefilter.c
> @@ -419,7 +419,7 @@ enumerate_args (PyObject *iter,
> struct ui_out *out,
> enum ext_lang_frame_args args_type,
> int print_args_field,
> - struct frame_info *frame)
> + frame_info_ptr frame)
> {
> struct value_print_options opts;
>
> @@ -550,7 +550,7 @@ enumerate_locals (PyObject *iter,
> int indent,
> enum ext_lang_frame_args args_type,
> int print_args_field,
> - struct frame_info *frame)
> + frame_info_ptr frame)
> {
> struct value_print_options opts;
>
> @@ -638,7 +638,7 @@ static enum ext_lang_bt_status
> py_mi_print_variables (PyObject *filter, struct ui_out *out,
> struct value_print_options *opts,
> enum ext_lang_frame_args args_type,
> - struct frame_info *frame)
> + frame_info_ptr frame)
> {
> gdbpy_ref<> args_iter (get_py_iter_from_func (filter, "frame_args"));
> if (args_iter == NULL)
> @@ -672,7 +672,7 @@ py_print_locals (PyObject *filter,
> struct ui_out *out,
> enum ext_lang_frame_args args_type,
> int indent,
> - struct frame_info *frame)
> + frame_info_ptr frame)
> {
> gdbpy_ref<> locals_iter (get_py_iter_from_func (filter, "frame_locals"));
> if (locals_iter == NULL)
> @@ -697,7 +697,7 @@ static enum ext_lang_bt_status
> py_print_args (PyObject *filter,
> struct ui_out *out,
> enum ext_lang_frame_args args_type,
> - struct frame_info *frame)
> + frame_info_ptr frame)
> {
> gdbpy_ref<> args_iter (get_py_iter_from_func (filter, "frame_args"));
> if (args_iter == NULL)
> @@ -754,7 +754,7 @@ py_print_frame (PyObject *filter, frame_filter_flags flags,
> int has_addr = 0;
> CORE_ADDR address = 0;
> struct gdbarch *gdbarch = NULL;
> - struct frame_info *frame = NULL;
> + frame_info_ptr frame = NULL;
> struct value_print_options opts;
>
> int print_level, print_frame_info, print_args, print_locals;
> @@ -859,11 +859,11 @@ py_print_frame (PyObject *filter, frame_filter_flags flags,
> && (location_print
> || (out->is_mi_like_p () && (print_frame_info || print_args))))
> {
> - struct frame_info **slot;
> + frame_info_ptr *slot;
> int level;
>
> - slot = (struct frame_info **) htab_find_slot (levels_printed,
> - frame, INSERT);
> + slot = (frame_info_ptr *) htab_find_slot (levels_printed,
> + frame.get(), INSERT);
This doesn't look right, and is causing test failures for
gdb.python/py-framefilter-mi.exp.
As you use 'frame.get ()' in the updated htab_find_slot call, which is
passing the raw frame_info*, effectively, the htab_find_slot call hasn't
changed with this patch. That you are now casting the return type to a
different type indicates the bug.
You might be tempted to pass '&frame' to htab_find_slot, but don't do
that, the frame is a local variable, so hashing based on its address is
not going to work.
I think that levels_printed really does need to hold the raw
frame_info*.
Thanks,
Andrew
More information about the Gdb-patches
mailing list