From: Tom Tromey <tromey@adacore.com>
To: gdb-patches@sourceware.org
Cc: Tom Tromey <tromey@adacore.com>
Subject: [PATCH 5/6] Change call_site_target to iterate over addresses
Date: Wed, 1 Dec 2021 15:04:31 -0700 [thread overview]
Message-ID: <20211201220432.4105152-6-tromey@adacore.com> (raw)
In-Reply-To: <20211201220432.4105152-1-tromey@adacore.com>
In order to handle the case where a call site target might refer to
multiple addresses, we change the code to use a callback style. Any
spot using call_site_target::address now passes in a callback function
that may be called multiple times.
---
gdb/dwarf2/loc.c | 134 +++++++++++++++++++++++++++++------------------
gdb/gdbtypes.h | 34 +++++++-----
2 files changed, 105 insertions(+), 63 deletions(-)
diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c
index 398538b54f8..e1001eb7adb 100644
--- a/gdb/dwarf2/loc.c
+++ b/gdb/dwarf2/loc.c
@@ -634,10 +634,12 @@ show_entry_values_debug (struct ui_file *file, int from_tty,
/* See gdbtypes.h. */
-CORE_ADDR
-call_site_target::address (struct gdbarch *call_site_gdbarch,
- const struct call_site *call_site,
- struct frame_info *caller_frame) const
+void
+call_site_target::iterate_over_addresses
+ (struct gdbarch *call_site_gdbarch,
+ const struct call_site *call_site,
+ struct frame_info *caller_frame,
+ iterate_ftype callback) const
{
switch (m_loc_kind)
{
@@ -683,10 +685,11 @@ call_site_target::address (struct gdbarch *call_site_gdbarch,
dwarf_block->per_objfile);
/* DW_AT_call_target is a DWARF expression, not a DWARF location. */
if (VALUE_LVAL (val) == lval_memory)
- return value_address (val);
+ callback (value_address (val));
else
- return value_as_address (val);
+ callback (value_as_address (val));
}
+ break;
case call_site_target::PHYSNAME:
{
@@ -708,8 +711,9 @@ call_site_target::address (struct gdbarch *call_site_gdbarch,
: msym.minsym->print_name ()));
}
- return BMSYMBOL_VALUE_ADDRESS (msym);
+ callback (BMSYMBOL_VALUE_ADDRESS (msym));
}
+ break;
case call_site_target::PHYSADDR:
{
@@ -718,8 +722,9 @@ call_site_target::address (struct gdbarch *call_site_gdbarch,
int sect_idx = COMPUNIT_BLOCK_LINE_SECTION (cust);
CORE_ADDR delta = per_objfile->objfile->section_offsets[sect_idx];
- return m_loc.physaddr + delta;
+ callback (m_loc.physaddr + delta);
}
+ break;
default:
internal_error (__FILE__, __LINE__, _("invalid call site target kind"));
@@ -784,28 +789,28 @@ func_verify_no_selftailcall (struct gdbarch *gdbarch, CORE_ADDR verify_addr)
for (call_site = TYPE_TAIL_CALL_LIST (SYMBOL_TYPE (func_sym));
call_site; call_site = call_site->tail_call_next)
{
- CORE_ADDR target_addr;
-
/* CALLER_FRAME with registers is not available for tail-call jumped
frames. */
- target_addr = call_site->address (gdbarch, nullptr);
-
- if (target_addr == verify_addr)
+ call_site->iterate_over_addresses (gdbarch, nullptr,
+ [&] (CORE_ADDR target_addr)
{
- struct bound_minimal_symbol msym;
-
- msym = lookup_minimal_symbol_by_pc (verify_addr);
- throw_error (NO_ENTRY_VALUE_ERROR,
- _("DW_OP_entry_value resolving has found "
- "function \"%s\" at %s can call itself via tail "
- "calls"),
- (msym.minsym == NULL ? "???"
- : msym.minsym->print_name ()),
- paddress (gdbarch, verify_addr));
- }
-
- if (addr_hash.insert (target_addr).second)
- todo.push_back (target_addr);
+ if (target_addr == verify_addr)
+ {
+ struct bound_minimal_symbol msym;
+
+ msym = lookup_minimal_symbol_by_pc (verify_addr);
+ throw_error (NO_ENTRY_VALUE_ERROR,
+ _("DW_OP_entry_value resolving has found "
+ "function \"%s\" at %s can call itself via tail "
+ "calls"),
+ (msym.minsym == NULL ? "???"
+ : msym.minsym->print_name ()),
+ paddress (gdbarch, verify_addr));
+ }
+
+ if (addr_hash.insert (target_addr).second)
+ todo.push_back (target_addr);
+ });
}
}
}
@@ -942,11 +947,18 @@ call_site_find_chain_2
struct call_site *call_site,
CORE_ADDR callee_pc)
{
- /* CALLER_FRAME with registers is not available for tail-call jumped
- frames. */
- CORE_ADDR target_func_addr = call_site->address (gdbarch, nullptr);
+ std::vector<CORE_ADDR> addresses;
+ bool found_exact = false;
+ call_site->iterate_over_addresses (gdbarch, nullptr,
+ [&] (CORE_ADDR addr)
+ {
+ if (addr == callee_pc)
+ found_exact = true;
+ else
+ addresses.push_back (addr);
+ });
- if (target_func_addr == callee_pc)
+ if (found_exact)
{
chain_candidate (gdbarch, resultp, chain);
/* If RESULTP was reset, then chain_candidate failed, and so we
@@ -954,26 +966,29 @@ call_site_find_chain_2
return *resultp != nullptr;
}
- struct symbol *target_func
- = func_addr_to_tail_call_list (gdbarch, target_func_addr);
- for (struct call_site *target_call_site
- = TYPE_TAIL_CALL_LIST (SYMBOL_TYPE (target_func));
- target_call_site != nullptr;
- target_call_site = target_call_site->tail_call_next)
+ for (CORE_ADDR target_func_addr : addresses)
{
- if (addr_hash.insert (target_call_site->pc ()).second)
+ struct symbol *target_func
+ = func_addr_to_tail_call_list (gdbarch, target_func_addr);
+ for (struct call_site *target_call_site
+ = TYPE_TAIL_CALL_LIST (SYMBOL_TYPE (target_func));
+ target_call_site != nullptr;
+ target_call_site = target_call_site->tail_call_next)
{
- /* Successfully entered TARGET_CALL_SITE. */
- chain.push_back (target_call_site);
+ if (addr_hash.insert (target_call_site->pc ()).second)
+ {
+ /* Successfully entered TARGET_CALL_SITE. */
+ chain.push_back (target_call_site);
- if (!call_site_find_chain_2 (gdbarch, resultp, chain,
- addr_hash, target_call_site,
- callee_pc))
- return false;
+ if (!call_site_find_chain_2 (gdbarch, resultp, chain,
+ addr_hash, target_call_site,
+ callee_pc))
+ return false;
- size_t removed = addr_hash.erase (target_call_site->pc ());
- gdb_assert (removed == 1);
- chain.pop_back ();
+ size_t removed = addr_hash.erase (target_call_site->pc ());
+ gdb_assert (removed == 1);
+ chain.pop_back ();
+ }
}
}
@@ -998,6 +1013,12 @@ call_site_find_chain_1 (struct gdbarch *gdbarch, CORE_ADDR caller_pc,
TAIL_CALL_NEXT. This is inappropriate for CALLER_PC's call_site. */
std::vector<struct call_site *> chain;
+ /* A given call site may have multiple associated addresses. This
+ can happen if, e.g., the caller is split by hot/cold
+ partitioning. This vector tracks the ones we haven't visited
+ yet. */
+ std::vector<std::vector<CORE_ADDR>> unvisited_addresses;
+
/* We are not interested in the specific PC inside the callee function. */
callee_pc = get_pc_function_start (callee_pc);
if (callee_pc == 0)
@@ -1147,19 +1168,32 @@ dwarf_expr_reg_to_entry_parameter (struct frame_info *frame,
caller_pc = get_frame_pc (caller_frame);
call_site = call_site_for_pc (gdbarch, caller_pc);
- target_addr = call_site->address (gdbarch, caller_frame);
- if (target_addr != func_addr)
+ bool found = false;
+ unsigned count = 0;
+ call_site->iterate_over_addresses (gdbarch, caller_frame,
+ [&] (CORE_ADDR addr)
+ {
+ /* Preserve any address. */
+ target_addr = addr;
+ ++count;
+ if (addr == func_addr)
+ found = true;
+ });
+ if (!found)
{
struct minimal_symbol *target_msym, *func_msym;
target_msym = lookup_minimal_symbol_by_pc (target_addr).minsym;
func_msym = lookup_minimal_symbol_by_pc (func_addr).minsym;
throw_error (NO_ENTRY_VALUE_ERROR,
- _("DW_OP_entry_value resolving expects callee %s at %s "
+ _("DW_OP_entry_value resolving expects callee %s at %s %s"
"but the called frame is for %s at %s"),
(target_msym == NULL ? "???"
: target_msym->print_name ()),
paddress (gdbarch, target_addr),
+ (count > 0
+ ? _("(but note there are multiple addresses not listed)")
+ : ""),
func_msym == NULL ? "???" : func_msym->print_name (),
paddress (gdbarch, func_addr));
}
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index d2b5483c893..eaa3cd0dcb7 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -51,6 +51,7 @@
#include "gdbsupport/enum-flags.h"
#include "gdbsupport/underlying.h"
#include "gdbsupport/print-utils.h"
+#include "gdbsupport/function-view.h"
#include "dwarf2.h"
#include "gdb_obstack.h"
#include "gmp-utils.h"
@@ -1839,14 +1840,18 @@ struct call_site_target
m_loc.dwarf_block = dwarf_block;
}
- /* Find DW_TAG_call_site's DW_AT_call_target address. CALLER_FRAME
- (for registers) can be NULL if it is not known. This function
- always returns valid address or it throws
- NO_ENTRY_VALUE_ERROR. */
+ /* Callback type for iterate_over_addresses. */
- CORE_ADDR address (struct gdbarch *call_site_gdbarch,
- const struct call_site *call_site,
- struct frame_info *caller_frame) const;
+ using iterate_ftype = gdb::function_view<void (CORE_ADDR)>;
+
+ /* Call CALLBACK for each DW_TAG_call_site's DW_AT_call_target
+ address. CALLER_FRAME (for registers) can be NULL if it is not
+ known. This function always may throw NO_ENTRY_VALUE_ERROR. */
+
+ void iterate_over_addresses (struct gdbarch *call_site_gdbarch,
+ const struct call_site *call_site,
+ struct frame_info *caller_frame,
+ iterate_ftype callback) const;
private:
@@ -1941,14 +1946,17 @@ struct call_site
CORE_ADDR pc () const;
- /* Find the target address. CALLER_FRAME (for registers) can be
- NULL if it is not known. This function always returns valid
- address or it throws NO_ENTRY_VALUE_ERROR. */
+ /* Call CALLBACK for each target address. CALLER_FRAME (for
+ registers) can be NULL if it is not known. This function may
+ throw NO_ENTRY_VALUE_ERROR. */
- CORE_ADDR address (struct gdbarch *call_site_gdbarch,
- struct frame_info *caller_frame) const
+ void iterate_over_addresses (struct gdbarch *call_site_gdbarch,
+ struct frame_info *caller_frame,
+ call_site_target::iterate_ftype callback)
+ const
{
- return target.address (call_site_gdbarch, this, caller_frame);
+ return target.iterate_over_addresses (call_site_gdbarch, this,
+ caller_frame, callback);
}
/* * List successor with head in FUNC_TYPE.TAIL_CALL_LIST. */
--
2.31.1
next prev parent reply other threads:[~2021-12-01 22:04 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-01 22:04 [PATCH 0/6] Handle split functions in call site chains Tom Tromey
2021-12-01 22:04 ` [PATCH 1/6] Change call_site_target to use custom type and enum Tom Tromey
2021-12-01 22:04 ` [PATCH 2/6] Make call_site_target members private Tom Tromey
2021-12-01 22:04 ` [PATCH 3/6] Constify chain_candidate Tom Tromey
2021-12-01 22:04 ` [PATCH 4/6] Change call_site_find_chain_1 to work recursively Tom Tromey
2021-12-01 22:04 ` Tom Tromey [this message]
2021-12-01 22:04 ` [PATCH 6/6] Handle multiple addresses in call_site_target Tom Tromey
2022-02-28 18:34 ` [PATCH 0/6] Handle split functions in call site chains Tom Tromey
2022-03-28 19:54 ` Tom Tromey
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=20211201220432.4105152-6-tromey@adacore.com \
--to=tromey@adacore.com \
--cc=gdb-patches@sourceware.org \
/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).