From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id GBW3B6kvW2HmdgAAWB0awg (envelope-from ) for ; Mon, 04 Oct 2021 12:45:29 -0400 Received: by simark.ca (Postfix, from userid 112) id 1D1CA1EE1A; Mon, 4 Oct 2021 12:45:29 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.0 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,NICE_REPLY_A,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 103761E813 for ; Mon, 4 Oct 2021 12:45:28 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 8A8FF3857011 for ; Mon, 4 Oct 2021 16:45:27 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8A8FF3857011 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1633365927; bh=QbZ5xL1P6k/9ZiOoruhOikQ6+cp/YoWDHMTbu1nhIhE=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=XrXHDzpR0ge7tP/8KPYeRoWgBQYeG/XIoNW2o/5EGVdH29nMBhpHj9Y/8p1gF+B72 UBMKXYOjXhjmUaQlBAbnbMZ5qPWUidCIqrZGEu52JaCOgdwM0ohDvUgliD8vFOgqQ7 lcAfDJmSe1cbpdqV+81mL0qI7diLy0HYer4Ki7GE= Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id DD8AB3858D28 for ; Mon, 4 Oct 2021 16:45:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org DD8AB3858D28 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 226211FDFC; Mon, 4 Oct 2021 16:45:07 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 0376813A83; Mon, 4 Oct 2021 16:45:06 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id WdBYOZIvW2ElQAAAMHmgww (envelope-from ); Mon, 04 Oct 2021 16:45:06 +0000 Subject: Re: [PATCH 3/4] [gdb/symtab] Add call_site::pc () To: Simon Marchi , gdb-patches@sourceware.org References: <20211001123328.22314-1-tdevries@suse.de> <20211001123328.22314-3-tdevries@suse.de> <60e7ef77-9dac-b9b2-ff90-d15e1ec9b6ab@simark.ca> Message-ID: <008441f8-3964-db22-56ad-39b6c75cbe53@suse.de> Date: Mon, 4 Oct 2021 18:45:06 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: <60e7ef77-9dac-b9b2-ff90-d15e1ec9b6ab@simark.ca> Content-Type: multipart/mixed; boundary="------------F7F2A4FB249D6312A6BCC9E0" Content-Language: en-US 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: , From: Tom de Vries via Gdb-patches Reply-To: Tom de Vries Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" This is a multi-part message in MIME format. --------------F7F2A4FB249D6312A6BCC9E0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 10/1/21 8:10 PM, Simon Marchi wrote: > On 2021-10-01 8:33 a.m., Tom de Vries via Gdb-patches wrote: >> From: Simon Marchi >> >> Add member function call_site::pc () and update all uses. >> >> Tested on x86_64-linux. >> >> Co-Authored-By: Tom de Vries >> --- >> gdb/dwarf2/frame-tailcall.c | 4 ++-- >> gdb/dwarf2/loc.c | 18 +++++++++--------- >> gdb/dwarf2/read.c | 2 +- >> gdb/gdbtypes.c | 9 +++++++++ >> gdb/gdbtypes.h | 6 +++++- >> 5 files changed, 26 insertions(+), 13 deletions(-) >> >> diff --git a/gdb/dwarf2/frame-tailcall.c b/gdb/dwarf2/frame-tailcall.c >> index f112b4ecca4..9fe498b0924 100644 >> --- a/gdb/dwarf2/frame-tailcall.c >> +++ b/gdb/dwarf2/frame-tailcall.c >> @@ -240,14 +240,14 @@ pretend_pc (struct frame_info *this_frame, struct tailcall_cache *cache) >> gdb_assert (next_levels >= 0); >> >> if (next_levels < chain->callees) >> - return chain->call_site[chain->length - next_levels - 1]->pc; >> + return chain->call_site[chain->length - next_levels - 1]->pc (); >> next_levels -= chain->callees; >> >> /* Otherwise CHAIN->CALLEES are already covered by CHAIN->CALLERS. */ >> if (chain->callees != chain->length) >> { >> if (next_levels < chain->callers) >> - return chain->call_site[chain->callers - next_levels - 1]->pc; >> + return chain->call_site[chain->callers - next_levels - 1]->pc (); >> next_levels -= chain->callers; >> } >> >> diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c >> index 2322a01f396..27a1c97682a 100644 >> --- a/gdb/dwarf2/loc.c >> +++ b/gdb/dwarf2/loc.c >> @@ -654,10 +654,10 @@ call_site_to_target_addr (struct gdbarch *call_site_gdbarch, >> { >> struct bound_minimal_symbol msym; >> >> - msym = lookup_minimal_symbol_by_pc (call_site->pc - 1); >> + msym = lookup_minimal_symbol_by_pc (call_site->pc () - 1); >> throw_error (NO_ENTRY_VALUE_ERROR, >> _("DW_AT_call_target is not specified at %s in %s"), >> - paddress (call_site_gdbarch, call_site->pc), >> + paddress (call_site_gdbarch, call_site->pc ()), >> (msym.minsym == NULL ? "???" >> : msym.minsym->print_name ())); >> >> @@ -666,12 +666,12 @@ call_site_to_target_addr (struct gdbarch *call_site_gdbarch, >> { >> struct bound_minimal_symbol msym; >> >> - msym = lookup_minimal_symbol_by_pc (call_site->pc - 1); >> + msym = lookup_minimal_symbol_by_pc (call_site->pc () - 1); >> throw_error (NO_ENTRY_VALUE_ERROR, >> _("DW_AT_call_target DWARF block resolving " >> "requires known frame which is currently not " >> "available at %s in %s"), >> - paddress (call_site_gdbarch, call_site->pc), >> + paddress (call_site_gdbarch, call_site->pc ()), >> (msym.minsym == NULL ? "???" >> : msym.minsym->print_name ())); >> >> @@ -700,11 +700,11 @@ call_site_to_target_addr (struct gdbarch *call_site_gdbarch, >> msym = lookup_minimal_symbol (physname, NULL, NULL); >> if (msym.minsym == NULL) >> { >> - msym = lookup_minimal_symbol_by_pc (call_site->pc - 1); >> + msym = lookup_minimal_symbol_by_pc (call_site->pc () - 1); >> throw_error (NO_ENTRY_VALUE_ERROR, >> _("Cannot find function \"%s\" for a call site target " >> "at %s in %s"), >> - physname, paddress (call_site_gdbarch, call_site->pc), >> + physname, paddress (call_site_gdbarch, call_site->pc ()), >> (msym.minsym == NULL ? "???" >> : msym.minsym->print_name ())); >> >> @@ -810,7 +810,7 @@ func_verify_no_selftailcall (struct gdbarch *gdbarch, CORE_ADDR verify_addr) >> static void >> tailcall_dump (struct gdbarch *gdbarch, const struct call_site *call_site) >> { >> - CORE_ADDR addr = call_site->pc; >> + CORE_ADDR addr = call_site->pc (); >> struct bound_minimal_symbol msym = lookup_minimal_symbol_by_pc (addr - 1); >> >> fprintf_unfiltered (gdb_stdlog, " %s(%s)", paddress (gdbarch, addr), >> @@ -986,7 +986,7 @@ call_site_find_chain_1 (struct gdbarch *gdbarch, CORE_ADDR caller_pc, >> >> if (target_call_site) >> { >> - if (addr_hash.insert (target_call_site->pc).second) >> + if (addr_hash.insert (target_call_site->pc ()).second) >> { >> /* Successfully entered TARGET_CALL_SITE. */ >> >> @@ -1005,7 +1005,7 @@ call_site_find_chain_1 (struct gdbarch *gdbarch, CORE_ADDR caller_pc, >> call_site = chain.back (); >> chain.pop_back (); >> >> - size_t removed = addr_hash.erase (call_site->pc); >> + size_t removed = addr_hash.erase (call_site->pc ()); >> gdb_assert (removed == 1); >> >> target_call_site = call_site->tail_call_next; >> diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c >> index ac8460df9a4..d0460674d10 100644 >> --- a/gdb/dwarf2/read.c >> +++ b/gdb/dwarf2/read.c >> @@ -13405,7 +13405,7 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu) >> + (sizeof (*call_site->parameter) * (nparams - 1)))); >> *slot = call_site; >> memset (call_site, 0, sizeof (*call_site) - sizeof (*call_site->parameter)); >> - call_site->pc = pc; >> + call_site->m_pc = pc; > > I did this as a quick hack in my proof of concept patch, but I think we > should make this a bit nicer than setting the (conceptually) private > field here. We can have add a constructor to call_site and use > placement new, see patch below (only built-tested). > > Ack, integrated in patch, and renamed $subject to indicate larger scope of patch. Also some trivial updates due to commit "[gdb/symtab] Add call_site_eq and call_site_hash". Committed as attached. Thanks, - Tom --------------F7F2A4FB249D6312A6BCC9E0 Content-Type: text/x-patch; charset=UTF-8; name="0002-gdb-symtab-C-ify-call_site.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="0002-gdb-symtab-C-ify-call_site.patch" [gdb/symtab] C++-ify call_site - add constructor - add member function call_site::pc () Tested on x86_64-linux. Co-Authored-By: Tom de Vries --- gdb/dwarf2/frame-tailcall.c | 4 +-- gdb/dwarf2/loc.c | 18 ++++++------ gdb/dwarf2/read.c | 27 +++++++++--------- gdb/gdbtypes.c | 9 ++++++ gdb/gdbtypes.h | 67 +++++++++++++++++++++++++++++---------------- gdb/symtab.c | 3 +- 6 files changed, 77 insertions(+), 51 deletions(-) diff --git a/gdb/dwarf2/frame-tailcall.c b/gdb/dwarf2/frame-tailcall.c index f112b4ecca4..9fe498b0924 100644 --- a/gdb/dwarf2/frame-tailcall.c +++ b/gdb/dwarf2/frame-tailcall.c @@ -240,14 +240,14 @@ pretend_pc (struct frame_info *this_frame, struct tailcall_cache *cache) gdb_assert (next_levels >= 0); if (next_levels < chain->callees) - return chain->call_site[chain->length - next_levels - 1]->pc; + return chain->call_site[chain->length - next_levels - 1]->pc (); next_levels -= chain->callees; /* Otherwise CHAIN->CALLEES are already covered by CHAIN->CALLERS. */ if (chain->callees != chain->length) { if (next_levels < chain->callers) - return chain->call_site[chain->callers - next_levels - 1]->pc; + return chain->call_site[chain->callers - next_levels - 1]->pc (); next_levels -= chain->callers; } diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c index 2322a01f396..27a1c97682a 100644 --- a/gdb/dwarf2/loc.c +++ b/gdb/dwarf2/loc.c @@ -654,10 +654,10 @@ call_site_to_target_addr (struct gdbarch *call_site_gdbarch, { struct bound_minimal_symbol msym; - msym = lookup_minimal_symbol_by_pc (call_site->pc - 1); + msym = lookup_minimal_symbol_by_pc (call_site->pc () - 1); throw_error (NO_ENTRY_VALUE_ERROR, _("DW_AT_call_target is not specified at %s in %s"), - paddress (call_site_gdbarch, call_site->pc), + paddress (call_site_gdbarch, call_site->pc ()), (msym.minsym == NULL ? "???" : msym.minsym->print_name ())); @@ -666,12 +666,12 @@ call_site_to_target_addr (struct gdbarch *call_site_gdbarch, { struct bound_minimal_symbol msym; - msym = lookup_minimal_symbol_by_pc (call_site->pc - 1); + msym = lookup_minimal_symbol_by_pc (call_site->pc () - 1); throw_error (NO_ENTRY_VALUE_ERROR, _("DW_AT_call_target DWARF block resolving " "requires known frame which is currently not " "available at %s in %s"), - paddress (call_site_gdbarch, call_site->pc), + paddress (call_site_gdbarch, call_site->pc ()), (msym.minsym == NULL ? "???" : msym.minsym->print_name ())); @@ -700,11 +700,11 @@ call_site_to_target_addr (struct gdbarch *call_site_gdbarch, msym = lookup_minimal_symbol (physname, NULL, NULL); if (msym.minsym == NULL) { - msym = lookup_minimal_symbol_by_pc (call_site->pc - 1); + msym = lookup_minimal_symbol_by_pc (call_site->pc () - 1); throw_error (NO_ENTRY_VALUE_ERROR, _("Cannot find function \"%s\" for a call site target " "at %s in %s"), - physname, paddress (call_site_gdbarch, call_site->pc), + physname, paddress (call_site_gdbarch, call_site->pc ()), (msym.minsym == NULL ? "???" : msym.minsym->print_name ())); @@ -810,7 +810,7 @@ func_verify_no_selftailcall (struct gdbarch *gdbarch, CORE_ADDR verify_addr) static void tailcall_dump (struct gdbarch *gdbarch, const struct call_site *call_site) { - CORE_ADDR addr = call_site->pc; + CORE_ADDR addr = call_site->pc (); struct bound_minimal_symbol msym = lookup_minimal_symbol_by_pc (addr - 1); fprintf_unfiltered (gdb_stdlog, " %s(%s)", paddress (gdbarch, addr), @@ -986,7 +986,7 @@ call_site_find_chain_1 (struct gdbarch *gdbarch, CORE_ADDR caller_pc, if (target_call_site) { - if (addr_hash.insert (target_call_site->pc).second) + if (addr_hash.insert (target_call_site->pc ()).second) { /* Successfully entered TARGET_CALL_SITE. */ @@ -1005,7 +1005,7 @@ call_site_find_chain_1 (struct gdbarch *gdbarch, CORE_ADDR caller_pc, call_site = chain.back (); chain.pop_back (); - size_t removed = addr_hash.erase (call_site->pc); + size_t removed = addr_hash.erase (call_site->pc ()); gdb_assert (removed == 1); target_call_site = call_site->tail_call_next; diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index f8421aeb344..230eb6a04b1 100644 --- a/gdb/dwarf2/read.c +++ b/gdb/dwarf2/read.c @@ -13341,7 +13341,6 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu) struct gdbarch *gdbarch = objfile->arch (); CORE_ADDR pc, baseaddr; struct attribute *attr; - struct call_site *call_site, call_site_local; void **slot; int nparams; struct die_info *child_die; @@ -13366,10 +13365,11 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu) pc = gdbarch_adjust_dwarf2_addr (gdbarch, pc); if (cu->call_site_htab == NULL) - cu->call_site_htab = htab_create_alloc_ex (16, call_site_hash, call_site_eq, - NULL, &objfile->objfile_obstack, + cu->call_site_htab = htab_create_alloc_ex (16, call_site::hash, + call_site::eq, NULL, + &objfile->objfile_obstack, hashtab_obstack_allocate, NULL); - call_site_local.pc = pc; + struct call_site call_site_local (pc, nullptr, nullptr); slot = htab_find_slot (cu->call_site_htab, &call_site_local, INSERT); if (*slot != NULL) { @@ -13399,14 +13399,16 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu) nparams++; } - call_site - = ((struct call_site *) - obstack_alloc (&objfile->objfile_obstack, - sizeof (*call_site) - + (sizeof (*call_site->parameter) * (nparams - 1)))); + struct call_site *call_site + = new (XOBNEWVAR (&objfile->objfile_obstack, + struct call_site, + sizeof (*call_site) + sizeof (call_site->parameter[0]) * nparams)) + struct call_site (pc, cu->per_cu, per_objfile); *slot = call_site; - memset (call_site, 0, sizeof (*call_site) - sizeof (*call_site->parameter)); - call_site->pc = pc; + + /* We never call the destructor of call_site, so we must ensure it is + trivially destructible. */ + gdb_static_assert(std::is_trivially_destructible::value); if (dwarf2_flag_true_p (die, DW_AT_call_tail_call, cu) || dwarf2_flag_true_p (die, DW_AT_GNU_tail_call, cu)) @@ -13527,9 +13529,6 @@ read_call_site_scope (struct die_info *die, struct dwarf2_cu *cu) "block nor reference, for DIE %s [in module %s]"), sect_offset_str (die->sect_off), objfile_name (objfile)); - call_site->per_cu = cu->per_cu; - call_site->per_objfile = per_objfile; - for (child_die = die->child; child_die && child_die->tag; child_die = child_die->sibling) diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c index 97f016fccd1..8954c370b7b 100644 --- a/gdb/gdbtypes.c +++ b/gdb/gdbtypes.c @@ -37,6 +37,7 @@ #include "cp-support.h" #include "bcache.h" #include "dwarf2/loc.h" +#include "dwarf2/read.h" #include "gdbcore.h" #include "floatformat.h" #include "f-lang.h" @@ -6308,6 +6309,14 @@ objfile_type (struct objfile *objfile) return objfile_type; } +/* See gdbtypes.h. */ + +CORE_ADDR +call_site::pc () const +{ + return m_pc; +} + void _initialize_gdbtypes (); void _initialize_gdbtypes () diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h index 8021cb21ecc..5fe10cb7f1e 100644 --- a/gdb/gdbtypes.h +++ b/gdb/gdbtypes.h @@ -1793,52 +1793,71 @@ struct call_site_parameter struct call_site { - /* Address of the first instruction after this call. */ - - CORE_ADDR pc; + call_site (CORE_ADDR pc, dwarf2_per_cu_data *per_cu, + dwarf2_per_objfile *per_objfile) + : per_cu (per_cu), per_objfile (per_objfile), m_pc (pc) + {} + + static int + eq (const call_site *a, const call_site *b) + { + return core_addr_eq (&a->m_pc, &b->m_pc); + } + + static hashval_t + hash (const call_site *a) + { + return core_addr_hash (&a->m_pc); + } + + static int + eq (const void *a, const void *b) + { + return eq ((const call_site *)a, (const call_site *)b); + } + + static hashval_t + hash (const void *a) + { + return hash ((const call_site *)a); + } + + /* Return the address of the first instruction after this call. */ + + CORE_ADDR pc () const; /* * List successor with head in FUNC_TYPE.TAIL_CALL_LIST. */ - struct call_site *tail_call_next; + struct call_site *tail_call_next = nullptr; /* * Describe DW_AT_call_target. Missing attribute uses FIELD_LOC_KIND_DWARF_BLOCK with FIELD_DWARF_BLOCK == NULL. */ - struct call_site_target target; + struct call_site_target target {}; /* * Size of the PARAMETER array. */ - unsigned parameter_count; + unsigned parameter_count = 0; /* * CU of the function where the call is located. It gets used for DWARF blocks execution in the parameter array below. */ - dwarf2_per_cu_data *per_cu; + dwarf2_per_cu_data *const per_cu = nullptr; /* objfile of the function where the call is located. */ - dwarf2_per_objfile *per_objfile; + dwarf2_per_objfile *const per_objfile = nullptr; + private: + /* Address of the first instruction after this call. */ + const CORE_ADDR m_pc; + + public: /* * Describe DW_TAG_call_site's DW_TAG_formal_parameter. */ - struct call_site_parameter parameter[1]; + struct call_site_parameter parameter[]; }; -static inline int -call_site_eq (const void *a_, const void *b_) -{ - const struct call_site *a = (const call_site *)a_; - const struct call_site *b = (const call_site *)b_; - return core_addr_eq (&a->pc, &b->pc); -} - -static inline hashval_t -call_site_hash (const void *a_) -{ - const struct call_site *a = (const call_site *)a_; - return core_addr_hash (&a->pc); -} - /* The type-specific info for TYPE_CODE_FIXED_POINT types. */ struct fixed_point_type_info diff --git a/gdb/symtab.c b/gdb/symtab.c index bd1eb429b94..cb583846efa 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -334,11 +334,10 @@ search_domain_name (enum search_domain e) call_site * compunit_symtab::find_call_site (CORE_ADDR pc) const { - struct call_site call_site_local; if (m_call_site_htab == nullptr) return nullptr; - call_site_local.pc = pc; + struct call_site call_site_local (pc, nullptr, nullptr); void **slot = htab_find_slot (m_call_site_htab, &call_site_local, NO_INSERT); if (slot == nullptr) --------------F7F2A4FB249D6312A6BCC9E0--