Re: [PATCH v2 3/4] gdb: dwarf2 generic implementation for caching function data

Torbjorn SVENSSON via Gdb-patches
2022-11-18T16:02:24Z

Hi,

On 2022-11-18 16:52, Torbjörn SVENSSON wrote:
> When there is no dwarf2 data for a register, a function can be called
> to provide the value of this register. In some situations, it might
> not be trivial to determine the value to return and it would cause a
> performance bottleneck to do the computation each time.
>
> This patch allows the called function to have a "cache" object that it
> can use to store some metadata between calls to reduce the performance
> impact of the complex logic.
>
> The cache object is unique for each function and frame, so if there are
> more than one function pointer stored in the dwarf2_frame_cache->reg
> array, then the appropriate pointer will be supplied (the type is not
> known by the dwarf2 implementation).
>
> dwarf2_frame_get_fn_data can be used to retrieve the function unique
> cache object.
> dwarf2_frame_allocate_fn_data can be used to allocate and retrieve the
> function unqiue cache object.
>
> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
> Signed-off-by: Yvan Roux <yvan.roux@foss.st.com>
> ---
> gdb/dwarf2/frame.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++
> gdb/dwarf2/frame.h | 20 +++++++++++++++++--
> 2 files changed, 66 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/dwarf2/frame.c b/gdb/dwarf2/frame.c
> index 3f884abe1d5..bff3b706e7e 100644
> --- a/gdb/dwarf2/frame.c
> +++ b/gdb/dwarf2/frame.c
> @@ -831,6 +831,14 @@ dwarf2_fetch_cfa_info (struct gdbarch *gdbarch, CORE_ADDR pc,
> }
>
> \f
> +struct dwarf2_frame_fn_data
> +{
> + struct value *(*fn) (frame_info_ptr this_frame, void **this_cache,
> + int regnum);
> + void *data;
> + struct dwarf2_frame_fn_data* next;
> +};
> +
> struct dwarf2_frame_cache
> {
> /* DWARF Call Frame Address. */
> @@ -862,6 +870,8 @@ struct dwarf2_frame_cache
> dwarf2_tailcall_frame_unwind unwinder so this field does not apply for
> them. */
> void *tailcall_cache;
> +
> + struct dwarf2_frame_fn_data *fn_data;
> };
>
> static struct dwarf2_frame_cache *
> @@ -1221,6 +1231,44 @@ dwarf2_frame_prev_register (frame_info_ptr this_frame, void **this_cache,
> }
> }
>
> +void *dwarf2_frame_get_fn_data (frame_info_ptr this_frame, void **this_cache,
> + fn_prev_register fn)
> +{
> + struct dwarf2_frame_fn_data *fn_data = nullptr;
> + struct dwarf2_frame_cache *cache
> + = dwarf2_frame_cache (this_frame, this_cache);
> +
> + /* Find the object for the function. */
> + for (fn_data = cache->fn_data; fn_data; fn_data = fn_data->next)
> + if (fn_data->fn == fn)
> + return fn_data->data;
> +
> + return nullptr;
> +}
> +
> +void *dwarf2_frame_allocate_fn_data (frame_info_ptr this_frame,
> + void **this_cache,
> + fn_prev_register fn, unsigned long size)
> +{
> + struct dwarf2_frame_fn_data *fn_data = nullptr;
> + struct dwarf2_frame_cache *cache
> + = dwarf2_frame_cache (this_frame, this_cache);
> +
> + /* First try to find an existing object. */
> + void *data = dwarf2_frame_get_fn_data (this_frame, this_cache, fn);
> + if (data)
> + return data;
> +
> + /* No object found, lets create a new instance. */
> + fn_data = FRAME_OBSTACK_ZALLOC (struct dwarf2_frame_fn_data);
> + fn_data->fn = fn;
> + fn_data->data = frame_obstack_zalloc (size);

Since these 2 blocks (fn_data and fn_data->fn) are allocated on the
obstack, do we need to release them in dwarf2_frame_dealloc_cache() or
just leave them to the generic garbage collection?

Kind regards,
Torbjörn

> + fn_data->next = cache->fn_data;
> + cache->fn_data = fn_data;
> +
> + return fn_data->data;
> +}
> +
> /* Proxy for tailcall_frame_dealloc_cache for bottom frame of a virtual tail
> call frames chain. */
>
> diff --git a/gdb/dwarf2/frame.h b/gdb/dwarf2/frame.h
> index 06c8a10c178..444afd9f8eb 100644
> --- a/gdb/dwarf2/frame.h
> +++ b/gdb/dwarf2/frame.h
> @@ -66,6 +66,9 @@ enum dwarf2_frame_reg_rule
>
> /* Register state. */
>
> +typedef struct value *(*fn_prev_register) (frame_info_ptr this_frame,
> + void **this_cache, int regnum);
> +
> struct dwarf2_frame_state_reg
> {
> /* Each register save state can be described in terms of a CFA slot,
> @@ -78,8 +81,7 @@ struct dwarf2_frame_state_reg
> const gdb_byte *start;
> ULONGEST len;
> } exp;
> - struct value *(*fn) (frame_info_ptr this_frame, void **this_cache,
> - int regnum);
> + fn_prev_register fn;
> } loc;
> enum dwarf2_frame_reg_rule how;
> };
> @@ -262,4 +264,18 @@ extern int dwarf2_fetch_cfa_info (struct gdbarch *gdbarch, CORE_ADDR pc,
> const gdb_byte **cfa_start_out,
> const gdb_byte **cfa_end_out);
>
> +
> +/* Allocate a new instance of the function unique data. */
> +
> +extern void *dwarf2_frame_allocate_fn_data (frame_info_ptr this_frame,
> + void **this_cache,
> + fn_prev_register fn,
> + unsigned long size);
> +
> +/* Retrieve the function unique data for this frame. */
> +
> +extern void *dwarf2_frame_get_fn_data (frame_info_ptr this_frame,
> + void **this_cache,
> + fn_prev_register fn);
> +
> #endif /* dwarf2-frame.h */