From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qv1-xf42.google.com (mail-qv1-xf42.google.com [IPv6:2607:f8b0:4864:20::f42]) by sourceware.org (Postfix) with ESMTPS id B88FC386F430 for ; Thu, 28 May 2020 16:24:50 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org B88FC386F430 Received: by mail-qv1-xf42.google.com with SMTP id y9so3520000qvs.4 for ; Thu, 28 May 2020 09:24:50 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=egJWCho0KChgarahAvGtmlKhug32AQzlv2kOdehGVAM=; b=NLckrtrmgFU8oM+3v8pyryQn/bbFtCJW6PanJSxOlvuCiri7PR2KJZ/xhkbjB3AbQP 6LNlvmP3Ff6/a5yY1ly9LZMQcyRgkVRiP8iNPoMsl4QBpioi4a/kVkBM6+gjVkOOZEkE cqmNpPDve+Mo667suX37kqK3wRkQpb8fYamN/2VXInC97joeoYi/qB2deldi/tEARUe8 ixws7kd9njVAoF1Mi0QFT7AlS+01SLicV6KJ7HzPIRBeUt90pE83vqwJur6Sfu4A0LAr SsnPIhXtV9E9vHsT1nAXL5yE0E5S0UJmAsrkMn48qq5lOJKmzinRvJ0Cw6rlOhqiJ2JF sW4Q== X-Gm-Message-State: AOAM530PgP+0IHWS5TaWrnJfkhUDqb8RLBdugzMmIJGuBydFrxokrXO2 weBrg33B1akR/SwdnbpYE7nM/LVFMyypEd9fhUHBEQ== X-Google-Smtp-Source: ABdhPJzIg7uOw7aiZZlAJNtvSTwqLqLQC1xX1BO/d33Vd3z1SWVB4oP5jvABWvoYL9cCmQ03jRYmo2tMHrtfZe+vKOU= X-Received: by 2002:ad4:4572:: with SMTP id o18mr3875955qvu.204.1590683089803; Thu, 28 May 2020 09:24:49 -0700 (PDT) MIME-Version: 1.0 References: <20200512210913.5593-1-simon.marchi@efficios.com> <20200512211250.6230-29-simon.marchi@efficios.com> <04473d43-9719-436b-648a-bdc8e5f55751@suse.de> <354bbf9d-6bca-deaa-c64b-714c8d6b477d@suse.de> <2cdb90c2-ad74-310d-75db-65a3d694049f@suse.de> In-Reply-To: From: Christian Biesinger Date: Thu, 28 May 2020 11:24:13 -0500 Message-ID: Subject: Re: [PATCH v2 28/42] Remove dwarf2_per_cu_data::objfile () To: Simon Marchi Cc: Tom de Vries , Simon Marchi , gdb-patches Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-24.2 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 May 2020 16:24:52 -0000 On Thu, May 28, 2020 at 10:33 AM Simon Marchi 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? > From 44486dcf19b62708ad49bbb6094e065a223dea99 Mon Sep 17 00:00:00 2001 > From: Simon Marchi > Date: Thu, 28 May 2020 11:30:11 -0400 > Subject: [PATCH] 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) > : Remove assert. Override > per_objfile with caller_per_objfile. > > Change-Id: Ib227d767ce525c10607ab6621a373aaae982c67a > --- > gdb/ChangeLog | 6 ++++++ > gdb/dwarf2/loc.c | 6 +++--- > 2 files changed, 9 insertions(+), 3 deletions(-) > > diff --git a/gdb/ChangeLog b/gdb/ChangeLog > index 593ff01cc9d0..e5b4019dd646 100644 > --- a/gdb/ChangeLog > +++ b/gdb/ChangeLog > @@ -1,3 +1,9 @@ > +2020-05-28 Simon Marchi > + > + * dwarf2/loc.c (class dwarf_evaluate_loc_desc) > + : Remove assert. Override > + per_objfile with caller_per_objfile. > + > 2020-05-28 Tom de Vries > > * 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 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); > } > -- > 2.26.2 >