Re: [PATCH v2 28/42] Remove dwarf2_per_cu_data::objfile ()

Simon Marchi
2020-05-28T19:49:46Z

On 2020-05-28 12:52 p.m., Simon Marchi wrote:
> On 2020-05-28 12:24 p.m., Christian Biesinger wrote:
>> On Thu, May 28, 2020 at 10:33 AM Simon Marchi <simark@simark.ca> wrote:
>>>
>>> On 2020-05-28 9:05 a.m., Tom de Vries wrote:
>>>> Hi,
>>>>
>>>> thanks for looking into this.
>>>>
>>>> I tried out the patch, and it fixes all regressions for me.
>>>>
>>>> Thanks,
>>>> - Tom
>>>
>>> Thanks, I pushed it with the following commit message.
>>
>> Maybe add a comment describing this case, so that future developers
>> looking at the code know why the two objfiles can be different?
>
> I am the first one to ask others to comment their code and not assume that everybody
> is a domain expert, so yes :)
>
> I would add this:
>
> From 54c8c8f45b2196538a6c87c4cc7d498a8a5ade36 Mon Sep 17 00:00:00 2001
> From: Simon Marchi <simon.marchi@polymtl.ca>
> Date: Thu, 28 May 2020 12:38:03 -0400
> Subject: [PATCH] gdb: add comment in
> dwarf_evaluate_loc_desc::push_dwarf_reg_entry_value
>
> Add a comment to clarify why we temporarily override some of the
> context's fields, and especially the per_objfile field. A longer
> explanation can be found in this previous commit
>
> 44486dcf19b ("gdb: use caller objfile in dwarf_evaluate_loc_desc::push_dwarf_reg_entry_value")
>
> gdb/ChangeLog:
>
> * dwarf2/loc.c (class dwarf_evaluate_loc_desc)
> <push_dwarf_reg_entry_value>: Add comment.
>
> Change-Id: I60c6e1062799f729b30a9db78bcb6448783324b4
> ---
> gdb/dwarf2/loc.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
> index 1aab1a4f51bc..400bb4d16fc7 100644
> --- a/gdb/dwarf2/loc.c
> +++ b/gdb/dwarf2/loc.c
> @@ -731,6 +731,12 @@ class dwarf_evaluate_loc_desc : public dwarf_expr_context
> throw_error (NO_ENTRY_VALUE_ERROR,
> _("Cannot resolve DW_AT_call_data_value"));
>
> + /* We are about to evaluate an expression in the context of the caller
> + of the current frame. This evaluation context may be different from
> + the current (callee's) context), so temporarily set the caller's context.
> +
> + It is possible for the caller to be from a different objfile from the
> + callee if the call is made through a function pointer. */
> scoped_restore save_frame = make_scoped_restore (&this->frame,
> caller_frame);
> scoped_restore save_per_cu = make_scoped_restore (&this->per_cu,
> --
> 2.26.2
>

I pushed this.

Simon