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 75876385842B for ; Fri, 1 Oct 2021 18:10:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 75876385842B Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca Received: from [172.16.0.95] (192-222-180-24.qc.cable.ebox.net [192.222.180.24]) (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 087871E813; Fri, 1 Oct 2021 14:10:22 -0400 (EDT) Subject: Re: [PATCH 3/4] [gdb/symtab] Add call_site::pc () To: Tom de Vries , gdb-patches@sourceware.org References: <20211001123328.22314-1-tdevries@suse.de> <20211001123328.22314-3-tdevries@suse.de> From: Simon Marchi Message-ID: <60e7ef77-9dac-b9b2-ff90-d15e1ec9b6ab@simark.ca> Date: Fri, 1 Oct 2021 14:10:21 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: <20211001123328.22314-3-tdevries@suse.de> Content-Type: text/plain; charset=utf-8 Content-Language: tl Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.8 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, NICE_REPLY_A, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) 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: Fri, 01 Oct 2021 18:10:24 -0000 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). >From b0515b5273d725d32bfdd0ec4eb9c346217d57a8 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Fri, 1 Oct 2021 13:50:52 -0400 Subject: [PATCH] fixup for patch 3 Change-Id: I291008c07c17cca1f8c8cd08954c308eff4611d1 --- gdb/dwarf2/read.c | 20 +++++++++----------- gdb/gdbtypes.h | 28 +++++++++++++++++----------- 2 files changed, 26 insertions(+), 22 deletions(-) diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c index 2648faf6289..ec2f64c4014 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; void **slot; int nparams; struct die_info *child_die; @@ -13398,14 +13397,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->m_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)) @@ -13526,9 +13527,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.h b/gdb/gdbtypes.h index ed4bc25afa1..e181a899981 100644 --- a/gdb/gdbtypes.h +++ b/gdb/gdbtypes.h @@ -1793,39 +1793,45 @@ struct call_site_parameter struct call_site { - /* As m_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) + {} - CORE_ADDR pc () const; - - /* Address of the first instruction after this call. */ + /* Return the address of the first instruction after this call. */ - CORE_ADDR m_pc; + 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[]; }; /* The type-specific info for TYPE_CODE_FIXED_POINT types. */ -- 2.33.0