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

Tom de Vries
2020-05-28T13:05:09Z

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);
> }
>