From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 3A246396E415 for ; Thu, 28 May 2020 15:33:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 3A246396E415 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark@simark.ca Received: from [10.0.0.11] (173-246-6-90.qc.cable.ebox.net [173.246.6.90]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 3AF6E1E072; Thu, 28 May 2020 11:33:25 -0400 (EDT) Subject: Re: [PATCH v2 28/42] Remove dwarf2_per_cu_data::objfile () To: Tom de Vries , Simon Marchi , gdb-patches@sourceware.org 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> From: Simon Marchi Message-ID: Date: Thu, 28 May 2020 11:33:23 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.8.0 MIME-Version: 1.0 In-Reply-To: <2cdb90c2-ad74-310d-75db-65a3d694049f@suse.de> Content-Type: text/plain; charset=utf-8 Content-Language: fr Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-9.1 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, SPF_HELO_PASS, SPF_PASS, TXREP 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 15:33:28 -0000 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. Simon >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