Re: [PATCH 12/43] Merge evaluate_for_locexpr_baton evaluator

Simon Marchi via Gdb-patches
2021-04-28T01:34:03Z

On 2021-03-01 9:45 a.m., Zoran Zaric via Gdb-patches wrote:
> From: Zoran Zaric <zoran.zaric@amd.com>
>
> The evaluate_for_locexpr_baton is the last derived class from the
> dwarf_expr_context class. It's purpose is to support the passed in
> buffer functionality.
>
> Although, it is not really necessary to merge this class with it's
> base class, doing that simplifies new expression evaluator design.
>
> Considering that this functionality is going around the DWARF standard,
> it is also reasonable to expect that with a new evaluator design and
> extending the push object address functionality to accept any location
> description, there will be no need to support passed in buffers.
>
> Alternatively, it would also makes sense to abstract the interaction
> between the evaluator and a given resource in the near future. The
> passed in buffer would then be a specialization of that abstraction.
>
> gdb/ChangeLog:
>
> * dwarf2/expr.c (dwarf_expr_context::read_mem): Merge with
> evaluate_for_locexpr_baton implementation.
> * dwarf2/loc.c (class evaluate_for_locexpr_baton): Remove
> class.
> (evaluate_for_locexpr_baton::read_mem): Move to
> dwarf_expr_context.
> (dwarf2_locexpr_baton_eval): Instantiate dwarf_expr_context
> instead of evaluate_for_locexpr_baton class.
> ---
> gdb/dwarf2/expr.c | 19 +++++++++++++++++--
> gdb/dwarf2/expr.h | 13 +++----------
> gdb/dwarf2/loc.c | 45 ++-------------------------------------------
> 3 files changed, 22 insertions(+), 55 deletions(-)
>
> diff --git a/gdb/dwarf2/expr.c b/gdb/dwarf2/expr.c
> index 1212be9c984..fe6d072ac8f 100644
> --- a/gdb/dwarf2/expr.c
> +++ b/gdb/dwarf2/expr.c
> @@ -252,6 +252,18 @@ void
> dwarf_expr_context::read_mem (gdb_byte *buf, CORE_ADDR addr,
> size_t length)
> {
> + if (length == 0)
> + return;

I'm really curious about when `length == 0` happens, but I see it was
there previously, so this ok. But if you know the answer, I want to
know.

> @@ -239,6 +231,7 @@ struct dwarf_expr_context
> void push_dwarf_reg_entry_value (enum call_site_parameter_kind kind,
> union call_site_parameter_u kind_u,
> int deref_size);
> + void read_mem (gdb_byte *buf, CORE_ADDR addr, size_t length);

You could keep the comment that was there previously over the read_mem
method signature.

Simon