Re: [PATCH 12/43] Merge evaluate_for_locexpr_baton evaluator

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

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