From: Simon Marchi <simark@simark.ca>
To: Tom de Vries <tdevries@suse.de>, gdb-patches@sourceware.org
Subject: Re: [PATCH 3/4] [gdb/symtab] Add call_site::pc ()
Date: Fri, 1 Oct 2021 14:10:21 -0400 [thread overview]
Message-ID: <60e7ef77-9dac-b9b2-ff90-d15e1ec9b6ab@simark.ca> (raw)
In-Reply-To: <20211001123328.22314-3-tdevries@suse.de>
On 2021-10-01 8:33 a.m., Tom de Vries via Gdb-patches wrote:
> From: Simon Marchi <simon.marchi@polymtl.ca>
>
> Add member function call_site::pc () and update all uses.
>
> Tested on x86_64-linux.
>
> Co-Authored-By: Tom de Vries <tdevries@suse.de>
> ---
> 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 <simon.marchi@efficios.com>
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<struct call_site>::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
next prev parent reply other threads:[~2021-10-01 18:10 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-01 12:33 [PATCH 1/4] [gdb/symtab] Fix htab_find_slot call in read_call_site_scope Tom de Vries
2021-10-01 12:33 ` [PATCH 2/4] [gdb/symtab] Remove COMPUNIT_CALL_SITE_HTAB Tom de Vries
2021-10-01 13:13 ` Simon Marchi
2021-10-01 12:33 ` [PATCH 3/4] [gdb/symtab] Add call_site::pc () Tom de Vries
2021-10-01 18:10 ` Simon Marchi [this message]
2021-10-04 16:45 ` Tom de Vries
2021-10-01 12:33 ` [PATCH 4/4] [gdb/symtab] Use unrelocated addresses in call_site Tom de Vries
2021-10-01 20:56 ` Simon Marchi
2021-10-04 16:47 ` Tom de Vries
2021-10-01 13:09 ` [PATCH 1/4] [gdb/symtab] Fix htab_find_slot call in read_call_site_scope Simon Marchi
2021-10-03 19:34 ` Tom de Vries
2021-10-04 12:05 ` Simon Marchi
2021-10-04 12:46 ` Tom de Vries
2021-10-04 15:41 ` Simon Marchi
2021-10-04 16:14 ` Tom de Vries
2021-10-04 16:34 ` Simon Marchi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=60e7ef77-9dac-b9b2-ff90-d15e1ec9b6ab@simark.ca \
--to=simark@simark.ca \
--cc=gdb-patches@sourceware.org \
--cc=tdevries@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).