[binutils-gdb] gdb: use caller objfile in dwarf_evaluate_loc_desc::push_dwarf_reg_entry_value

[binutils-gdb] gdb: use caller objfile in dwarf_evaluate_loc_desc::push_dwarf_reg_entry_value

[Simon Marchi]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=44486dcf19b62708ad49bbb6094e065a223dea99

commit 44486dcf19b62708ad49bbb6094e065a223dea99
Author: Simon Marchi <simon.marchi@efficios.com>
Date:   Thu May 28 11:30:11 2020 -0400

    gdb: use caller objfile in dwarf_evaluate_loc_desc::push_dwarf_reg_entry_value
    
    In commit
    
        89b07335fe ("Add dwarf2_per_objfile to dwarf_expr_context and dwarf2_frame_cache")
    
    I replaced the offset property of dwarf_expr_context by a per_objfile
    property (since we can get the text offset from the objfile).  The
    previous code in dwarf_evaluate_loc_desc::push_dwarf_reg_entry_value
    (dwarf_evaluate_loc_desc derives from dwarf_expr_context) did
    temporarily override the offset property while evaluating a DWARF
    sub-expression.  I speculated that this sub-expression always came from
    the same objfile as the outer expression, so I didn't see the need to
    temporarily override the per_objfile property in the new code.  A later
    commit:
    
        9f47c70716 ("Remove dwarf2_per_cu_data::objfile ()")
    
    added the following assertion to verify this:
    
        gdb_assert (this->per_objfile == caller_per_objfile);
    
    It turns out that this is not true.  Call sites can refer to function in
    another objfile, and therefore the caller's objfile can be different
    from the callee's objfile.  This can happen when the call site DIE in the
    DWARF represents a function call done through a function pointer.  The
    DIE can't describe statically which function is being called, since it's
    variable and not known at compile time.  Instead, it provides an
    expression that evaluates to the address of the function being called.
    In this case, the called function can very well be in a separate
    objfile.
    
    Fix this by overriding the per_objfile property while evaluating the
    sub-expression.
    
    This was exposed by the gdb.base/catch-load.exp test failing on openSUSE
    Tumbleweed with the glibc debug info installed.  It was also reported to
    fail on Fedora.
    
    When I investigated the problem, the particular call site on which we
    did hit the assert was coming from this DIE, in
    /usr/lib/debug/lib64/libc-2.31.so-2.31-5.1.x86_64.debug on openSUSE
    Tumbleweed:
    
        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)
    
    And for you curious out there, this call site is found in this function:
    
        0x0091a91d:   DW_TAG_subprogram
                        DW_AT_external [DW_FORM_flag_present]   (true)
                        DW_AT_name [DW_FORM_strp]       ("_dl_catch_exception")
                        DW_AT_decl_file [DW_FORM_data1] ("/usr/src/debug/glibc-2.31-5.1.x86_64/elf/dl-error-skeleton.c")
                        ...
    
    Which is a function that indeed uses a function pointer.
    
    gdb/ChangeLog:
    
            * dwarf2/loc.c (class dwarf_evaluate_loc_desc)
            <push_dwarf_reg_entry_value>: Remove assert.  Override
            per_objfile with caller_per_objfile.
    
    Change-Id: Ib227d767ce525c10607ab6621a373aaae982c67a

Diff:
---
 gdb/ChangeLog    | 6 ++++++
 gdb/dwarf2/loc.c | 6 +++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 593ff01cc9d..e5b4019dd64 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2020-05-28  Simon Marchi  <simon.marchi@efficios.com>
+
+	* dwarf2/loc.c (class dwarf_evaluate_loc_desc)
+	<push_dwarf_reg_entry_value>: Remove assert.  Override
+	per_objfile with caller_per_objfile.
+
 2020-05-28  Tom de Vries  <tdevries@suse.de>
 
 	* dwarf2/read.c	(dw2_symtab_iter_next, dw2_expand_marked_cus): Limit
diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index 7953361adee..1aab1a4f51b 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -726,8 +726,6 @@ public:
     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 @@ public:
 						      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);
   }