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

[Tom de Vries <tdevries@suse.de>]

On 28-05-2020 04:00, Simon Marchi wrote:
> On 2020-05-27 6:16 p.m., Simon Marchi via Gdb-patches wrote:
>> Ok, I reproduced it with opensuse/tumbleweed, after installing glibc's debug info.  I'll
>> look into it.
> 
> Quick update to say that I understand a bit more what happens.  It turns out that call sites
> can refer indirectly to other objfiles.  Here, we deal with an indirect call (through a function
> pointer probably).  In that case, the call site doesn't know at compile time what it calls.  It
> provides a DWARF expression to compute the called address. The DIE corresponding to the call site
> is:
> 
> 0x0091aa10:     DW_TAG_GNU_call_site
>                   DW_AT_low_pc [DW_FORM_addr]   (0x00000000001398e0)
>                   DW_AT_GNU_call_site_target [DW_FORM_exprloc]  (DW_OP_fbreg -272, DW_OP_deref)
>                   DW_AT_sibling [DW_FORM_ref4]  (0x0091aa2b)
> 
> This call site is in the _dl_catch_exception function of libc (which indeed takes a function pointer
> as argument).  In our case, the function pointer is a function in ld-linux.so.  So this is how the
> two objfiles can be different.
> 
> For comparison, when the function being called is known at compile time, the call site DIE will refer
> directly to the function being called, like in this case:
> 
> 0x0091a9f2:     DW_TAG_GNU_call_site
>                   DW_AT_low_pc [DW_FORM_addr]   (0x00000000001398cd)
>                   DW_AT_abstract_origin [DW_FORM_ref4]  (0x0091ad45 "__GI___sigsetjmp")
>                   DW_AT_sibling [DW_FORM_ref4]  (0x0091aa10)
> 
> In that case, I presume that it's not really possible for the caller and calle to be in different
> objfiles (this is the scenarion I had in my mind, I didn't know that indirect call sites could be
> described as well).
> 
> See the patch below (still missing the commit message). 

Hi,

thanks for looking into this.

I tried out the patch, and it fixes all regressions for me.

Thanks,
- Tom

> I think we only need to put and restore the
> caller's per_objfile in this->per_objfile.  Alternatively, we could maybe just create a new
> dwarf_evaluate_loc_desc local object, instead of setting and restoring all the fields.  I'll try that
> tomorrow.
> 
> Simon
> 
> 
> From 2381d562b3130b94ad8e266dd034431f80275c8b Mon Sep 17 00:00:00 2001
> From: Simon Marchi <simon.marchi@efficios.com>
> Date: Wed, 27 May 2020 21:54:45 -0400
> Subject: [PATCH] gdb: use caller objfile in
>  dwarf_evaluate_loc_desc::push_dwarf_reg_entry_value
> 
> Change-Id: Ib227d767ce525c10607ab6621a373aaae982c67a
> ---
>  gdb/dwarf2/loc.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
> index 7953361adeed..1aab1a4f51bc 100644
> --- a/gdb/dwarf2/loc.c
> +++ b/gdb/dwarf2/loc.c
> @@ -726,8 +726,6 @@ class dwarf_evaluate_loc_desc : public dwarf_expr_context
>      data_src = deref_size == -1 ? parameter->value : parameter->data_value;
>      size = deref_size == -1 ? parameter->value_size : parameter->data_value_size;
> 
> -    gdb_assert (this->per_objfile == caller_per_objfile);
> -
>      /* DEREF_SIZE size is not verified here.  */
>      if (data_src == NULL)
>        throw_error (NO_ENTRY_VALUE_ERROR,
> @@ -739,11 +737,13 @@ class dwarf_evaluate_loc_desc : public dwarf_expr_context
>  						      caller_per_cu);
>      scoped_restore save_obj_addr = make_scoped_restore (&this->obj_address,
>  							(CORE_ADDR) 0);
> +    scoped_restore save_per_objfile = make_scoped_restore (&this->per_objfile,
> +							   caller_per_objfile);
> 
>      scoped_restore save_arch = make_scoped_restore (&this->gdbarch);
>      this->gdbarch = this->per_objfile->objfile->arch ();
>      scoped_restore save_addr_size = make_scoped_restore (&this->addr_size);
> -    this->addr_size = per_cu->addr_size ();
> +    this->addr_size = this->per_cu->addr_size ();
> 
>      this->eval (data_src, size);
>    }
>