* [PATCH 0/4] Allow access to target_sections even without an executable
@ 2021-02-16 17:50 Andrew Burgess
2021-02-16 17:50 ` [PATCH 1/4] gdb: add a new 'maint info target-sections' command Andrew Burgess
` (5 more replies)
0 siblings, 6 replies; 19+ messages in thread
From: Andrew Burgess @ 2021-02-16 17:50 UTC (permalink / raw)
To: gdb-patches
The last patch in this series is the interesting one.
I ran into a situation where a user was using GDB against a remote
target without specifying an executable (either on the command line,
or using the file command). Instead, they loaded the executabe with
add-symbol-file and then continued to debug their remote target.
This works fine, except that 'set trust-readonly-sections' doesn't
work.
This series fixes this.
Thanks,
Andrew
---
Andrew Burgess (4):
gdb: add a new 'maint info target-sections' command
gdb: spread a little 'const' through the target_section_table code
gdb: make the target_sections table private within program_space
gdb: move get_section_table from exec_target to dummy_target
gdb/ChangeLog | 75 +++++++++++++
gdb/NEWS | 3 +
gdb/bfd-target.c | 4 +-
gdb/doc/ChangeLog | 5 +
gdb/doc/gdb.texinfo | 8 ++
gdb/exec.c | 102 ++++++++++--------
gdb/exec.h | 2 +-
gdb/maint.c | 59 ++++++++++
gdb/ppc64-tdep.c | 2 +-
gdb/progspace.h | 30 +++++-
gdb/record-btrace.c | 2 +-
gdb/remote.c | 6 +-
gdb/s390-tdep.c | 4 +-
gdb/solib-dsbt.c | 3 +-
gdb/solib-svr4.c | 3 +-
gdb/target-debug.h | 2 +-
gdb/target-delegates.c | 16 +--
gdb/target.c | 24 +++--
gdb/target.h | 14 ++-
gdb/testsuite/ChangeLog | 5 +
.../gdb.base/maint-info-sections.exp | 66 +++++++++++-
21 files changed, 349 insertions(+), 86 deletions(-)
--
2.25.4
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH 1/4] gdb: add a new 'maint info target-sections' command 2021-02-16 17:50 [PATCH 0/4] Allow access to target_sections even without an executable Andrew Burgess @ 2021-02-16 17:50 ` Andrew Burgess 2021-02-16 17:55 ` Eli Zaretskii 2021-02-17 16:19 ` Christian Biesinger 2021-02-16 17:50 ` [PATCH 2/4] gdb: spread a little 'const' through the target_section_table code Andrew Burgess ` (4 subsequent siblings) 5 siblings, 2 replies; 19+ messages in thread From: Andrew Burgess @ 2021-02-16 17:50 UTC (permalink / raw) To: gdb-patches We already have a command 'maint info sections', this command prints all sections from all known object files. However, GDB maintains a second section table internally. This section table is used when GDB wants to read directly from an object file rather than actually reading memory on the target. As such only some sections (the allocatable ones) are added to this secondary section table. I recently ran into a situation where some of GDB's optimisations for reading directly from the files were not working. In 'maint info sections' I could see that GDB knew about the object file, and did know about the sections that it _should_ have been reading from. But I couldn't ask GDB which sections it had copied into its secondary section table. This commit adds a new command 'maint info target-sections' that fills this gap. This command lists only those sections that GDB has copied into its secondary table. You'll notice that the testsuite includes a comment indicating that there's a bug in GDB. Normally this is not something I would add to the testsuite, instead we should raise an actual bugzilla bug and then mark an xfail, however, a later patch in this series will remove this comment once the actual bug in GDB is fixed. gdb/ChangeLog: * NEWS: Mention new 'maint info target-sections' command. * maint.c (maintenance_info_target_sections): New function. (_initialize_maint_cmds): Register new command. gdb/doc/ChangeLog: * gdb.texinfo (Files): Document new 'maint info target-sections' command. gdb/testsuite/ChangeLog: * gdb.base/maint-info-sections.exp: Add new tests. (check_maint_info_target_sections_output): New proc. --- gdb/ChangeLog | 6 ++ gdb/NEWS | 3 + gdb/doc/ChangeLog | 5 ++ gdb/doc/gdb.texinfo | 8 +++ gdb/maint.c | 59 ++++++++++++++++ gdb/testsuite/ChangeLog | 5 ++ .../gdb.base/maint-info-sections.exp | 68 ++++++++++++++++++- 7 files changed, 153 insertions(+), 1 deletion(-) diff --git a/gdb/NEWS b/gdb/NEWS index 1dfbbc648eb..7f5a745d0c0 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -47,6 +47,9 @@ maintenance flush register-cache maintenance flush dcache A new command to flush the dcache. +maintenance info target-sections + Print GDB's internal target sections table. + * Changed commands break [PROBE_MODIFIER] [LOCATION] [thread THREADNUM] diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 0b1deba9667..7189e698436 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -20668,6 +20668,14 @@ Section contains common symbols. @end table @end table + +@kindex maint info target-sections +@item maint info target-sections +This command prints @value{GDBN}'s internal section table. For each +target @value{GDBN} maintains a table containing the allocatable +sections from all currently mapped objects, along with information +about where the section is mapped. + @kindex set trust-readonly-sections @cindex read-only sections @item set trust-readonly-sections on diff --git a/gdb/maint.c b/gdb/maint.c index 707d156ec2a..bfdd1d5ca3e 100644 --- a/gdb/maint.c +++ b/gdb/maint.c @@ -464,6 +464,56 @@ maintenance_info_sections (const char *arg, int from_tty) maint_print_all_sections (_("Core file: "), core_bfd, nullptr, arg); } +/* Implement the "maintenance info target-sections" command. */ + +static void +maintenance_info_target_sections (const char *arg, int from_tty) +{ + bfd *abfd = nullptr; + int digits = 0; + const target_section_table *table + = target_get_section_table (current_top_target ()); + if (table == nullptr) + return; + + for (const target_section &sec : *table) + { + if (abfd == nullptr || sec.the_bfd_section->owner != abfd) + { + abfd = sec.the_bfd_section->owner; + digits = std::max (index_digits (gdb_bfd_count_sections (abfd)), + digits); + } + } + + struct gdbarch *gdbarch = nullptr; + int addr_size = 0; + abfd = nullptr; + for (const target_section &sec : *table) + { + if (sec.the_bfd_section->owner != abfd) + { + abfd = sec.the_bfd_section->owner; + gdbarch = gdbarch_from_bfd (abfd); + addr_size = gdbarch_addr_bit (gdbarch) / 8; + + printf_filtered (_("From '%s', file type %s:\n"), + bfd_get_filename (abfd), bfd_get_target (abfd)); + } + print_bfd_section_info (abfd, + sec.the_bfd_section, + nullptr, + digits); + /* The magic '8 + digits' here ensures that the 'Start' is aligned + with the output of print_bfd_section_info. */ + printf_filtered ("%*sStart: %s, End: %s, Owner token: %p\n", + (8 + digits), "", + hex_string_custom (sec.addr, addr_size), + hex_string_custom (sec.endaddr, addr_size), + sec.owner); + } +} + static void maintenance_print_statistics (const char *args, int from_tty) { @@ -1122,6 +1172,15 @@ Options:\n\ &maintenanceinfolist); set_cmd_completer_handle_brkchars (cmd, maint_info_sections_completer); + add_cmd ("target-sections", class_maintenance, + maintenance_info_target_sections, _("\ +List GDB's internal section table.\n\ +\n\ +Print the current targets section list. This is a sub-set of all\n\ +sections, from all objects currently loaded. Usually the ALLOC\n\ +sectoins."), + &maintenanceinfolist); + add_basic_prefix_cmd ("print", class_maintenance, _("Maintenance command for printing GDB internal state."), &maintenanceprintlist, "maintenance print ", 0, diff --git a/gdb/testsuite/gdb.base/maint-info-sections.exp b/gdb/testsuite/gdb.base/maint-info-sections.exp index 07c53b16177..b86812e4f12 100644 --- a/gdb/testsuite/gdb.base/maint-info-sections.exp +++ b/gdb/testsuite/gdb.base/maint-info-sections.exp @@ -13,7 +13,8 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see <http://www.gnu.org/licenses/>. -# Test just for the 'maintenance info sections' command. +# Test just for the 'maintenance info sections' command and the +# 'maintenance info target-sections' command. load_lib completion-support.exp @@ -29,6 +30,65 @@ if ![runto_main] then { return -1 } +# Check the output of 'maint info target-sections' command. +proc check_maint_info_target_sections_output {prefix} { + global hex gdb_prompt + + with_test_prefix $prefix { + # Check the output of the 'maint info target-sections' command. + # Ensures that all the lines have the expected format, and that we see + # an auxiliary information line after every section information line. + # + # We also check that every bfd section has the ALLOC flag. + set seen_header false + set seen_a_section false + set seen_aux_info false + set expecting_aux_info false + gdb_test_multiple "maint info target-sections" "" { + -re "^maint info target-sections\r\n" { + # Consume the command we sent to GDB that the terminal echo'd + # back. + exp_continue + } + -re "^From '\[^\r\n\]+', file type \[^\r\n\]+:\r\n" { + # There might be more than one header, but there should be at + # least one. + set seen_header true + if { $expecting_aux_info } { + fail $gdb_test_name + } else { + exp_continue + } + } + -re "^ \\\[\[0-9\]+\\\]\\s+$hex->$hex at $hex: \[^*\r\]+\\yALLOC\\y\[^*\r\]*\r\n" { + # A bfd section description line. + set seen_a_section true + if { $expecting_aux_info } { + fail $gdb_test_name + } else { + set expecting_aux_info true + exp_continue + } + } + -re "^\\s+Start: $hex, End: $hex, Owner token: $hex\r\n" { + # A target section auxiliary information line. + set seen_aux_info true + if { !$expecting_aux_info } { + fail $gdb_test_name + } else { + set expecting_aux_info false + exp_continue + } + } + -re "^$gdb_prompt $" { + gdb_assert { $seen_header && $seen_a_section && $seen_aux_info } + gdb_assert { !$expecting_aux_info } + pass $gdb_test_name + } + } + } +} + # Check that 'maint info sections' output looks correct. When # checking the lines for each section we reject section names starting # with a '*' character, the internal *COM*, *UND*, *ABS*, and *IND* @@ -128,6 +188,8 @@ gdb_test_multiple "maint info sections DATA" "" { } } +check_maint_info_target_sections_output "with executable" + # Restart GDB, but don't load the executable. clean_restart @@ -161,6 +223,10 @@ gdb_test_multiple "maint info sections -all-objects" "" { } } +# NOTE: We would like to check 'maint info target-sections' again +# here, but GDB currently doesn't display the target sections table in +# this case. This is a bug and fill be fixed shortly!! + # Test the command line completion on 'maint info sections'. First # the command line flag. test_gdb_complete_unique \ -- 2.25.4 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/4] gdb: add a new 'maint info target-sections' command 2021-02-16 17:50 ` [PATCH 1/4] gdb: add a new 'maint info target-sections' command Andrew Burgess @ 2021-02-16 17:55 ` Eli Zaretskii 2021-02-17 16:19 ` Christian Biesinger 1 sibling, 0 replies; 19+ messages in thread From: Eli Zaretskii @ 2021-02-16 17:55 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches > From: Andrew Burgess <andrew.burgess@embecosm.com> > Date: Tue, 16 Feb 2021 17:50:27 +0000 > > gdb/ChangeLog: > > * NEWS: Mention new 'maint info target-sections' command. > * maint.c (maintenance_info_target_sections): New function. > (_initialize_maint_cmds): Register new command. > > gdb/doc/ChangeLog: > > * gdb.texinfo (Files): Document new 'maint info target-sections' > command. > > gdb/testsuite/ChangeLog: > > * gdb.base/maint-info-sections.exp: Add new tests. > (check_maint_info_target_sections_output): New proc. Thanks, the documentation parts of this are okay. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/4] gdb: add a new 'maint info target-sections' command 2021-02-16 17:50 ` [PATCH 1/4] gdb: add a new 'maint info target-sections' command Andrew Burgess 2021-02-16 17:55 ` Eli Zaretskii @ 2021-02-17 16:19 ` Christian Biesinger 2021-02-18 16:25 ` Andrew Burgess 1 sibling, 1 reply; 19+ messages in thread From: Christian Biesinger @ 2021-02-17 16:19 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches On Tue, Feb 16, 2021 at 6:50 PM Andrew Burgess <andrew.burgess@embecosm.com> wrote: > diff --git a/gdb/testsuite/gdb.base/maint-info-sections.exp b/gdb/testsuite/gdb.base/maint-info-sections.exp > index 07c53b16177..b86812e4f12 100644 > --- a/gdb/testsuite/gdb.base/maint-info-sections.exp > +++ b/gdb/testsuite/gdb.base/maint-info-sections.exp ... > @@ -161,6 +223,10 @@ gdb_test_multiple "maint info sections -all-objects" "" { > } > } > > +# NOTE: We would like to check 'maint info target-sections' again > +# here, but GDB currently doesn't display the target sections table in > +# this case. This is a bug and fill be fixed shortly!! fill -> will? ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/4] gdb: add a new 'maint info target-sections' command 2021-02-17 16:19 ` Christian Biesinger @ 2021-02-18 16:25 ` Andrew Burgess 0 siblings, 0 replies; 19+ messages in thread From: Andrew Burgess @ 2021-02-18 16:25 UTC (permalink / raw) To: Christian Biesinger; +Cc: gdb-patches * Christian Biesinger <cbiesinger@google.com> [2021-02-17 17:19:48 +0100]: > On Tue, Feb 16, 2021 at 6:50 PM Andrew Burgess > <andrew.burgess@embecosm.com> wrote: > > diff --git a/gdb/testsuite/gdb.base/maint-info-sections.exp b/gdb/testsuite/gdb.base/maint-info-sections.exp > > index 07c53b16177..b86812e4f12 100644 > > --- a/gdb/testsuite/gdb.base/maint-info-sections.exp > > +++ b/gdb/testsuite/gdb.base/maint-info-sections.exp > ... > > @@ -161,6 +223,10 @@ gdb_test_multiple "maint info sections -all-objects" "" { > > } > > } > > > > +# NOTE: We would like to check 'maint info target-sections' again > > +# here, but GDB currently doesn't display the target sections table in > > +# this case. This is a bug and fill be fixed shortly!! > > fill -> will? Indeed. I'll fix this before I push. thanks, Andrew ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/4] gdb: spread a little 'const' through the target_section_table code 2021-02-16 17:50 [PATCH 0/4] Allow access to target_sections even without an executable Andrew Burgess 2021-02-16 17:50 ` [PATCH 1/4] gdb: add a new 'maint info target-sections' command Andrew Burgess @ 2021-02-16 17:50 ` Andrew Burgess 2021-02-16 17:50 ` [PATCH 3/4] gdb: make the target_sections table private within program_space Andrew Burgess ` (3 subsequent siblings) 5 siblings, 0 replies; 19+ messages in thread From: Andrew Burgess @ 2021-02-16 17:50 UTC (permalink / raw) To: gdb-patches The code to access the target section table can be made more const, so lets do that. There should be no user visible changes after this commit. gdb/ChangeLog: * gdb/bfd-target.c (class target_bfd) <get_section_table>: Make return type const. * gdb/exec.c (struct exec_target) <get_section_table>: Likewise. (section_table_read_available_memory): Make local const. (exec_target::xfer_partial): Make local const. (print_section_info): Make parameter const. * gdb/exec.h (print_section_info): Likewise. * gdb/ppc64-tdep.c (ppc64_convert_from_func_ptr_addr): Make local const. * gdb/record-btrace.c (record_btrace_target::xfer_partial): Likewise. * gdb/remote.c (remote_target::remote_xfer_live_readonly_partial): Likewise. * gdb/s390-tdep.c (s390_load): Likewise. * gdb/solib-dsbt.c (scan_dyntag): Likewise. * gdb/solib-svr4.c (scan_dyntag): Likewise. * gdb/target-debug.h (target_debug_print_target_section_table_p): Rename to... (target_debug_print_const_target_section_table_p): ...this. * gdb/target-delegates.c: Regenerate. * gdb/target.c (target_get_section_table): Make return type const. (target_section_by_addr): Likewise. Also make some locals const. (memory_xfer_partial_1): Make some locals const. * gdb/target.h (struct target_ops) <get_section_table>: Make return type const. (target_section_by_addr): Likewise. (target_get_section_table): Likewise. --- gdb/ChangeLog | 30 ++++++++++++++++++++++++++++++ gdb/bfd-target.c | 4 ++-- gdb/exec.c | 10 +++++----- gdb/exec.h | 2 +- gdb/ppc64-tdep.c | 2 +- gdb/record-btrace.c | 2 +- gdb/remote.c | 6 +++--- gdb/s390-tdep.c | 4 ++-- gdb/solib-dsbt.c | 3 ++- gdb/solib-svr4.c | 3 ++- gdb/target-debug.h | 2 +- gdb/target-delegates.c | 14 +++++++------- gdb/target.c | 17 ++++++++--------- gdb/target.h | 8 ++++---- 14 files changed, 69 insertions(+), 38 deletions(-) diff --git a/gdb/bfd-target.c b/gdb/bfd-target.c index 90d247a981a..09d9c978b05 100644 --- a/gdb/bfd-target.c +++ b/gdb/bfd-target.c @@ -50,7 +50,7 @@ class target_bfd : public target_ops ULONGEST offset, ULONGEST len, ULONGEST *xfered_len) override; - target_section_table *get_section_table () override; + const target_section_table *get_section_table () override; private: /* The BFD we're wrapping. */ @@ -82,7 +82,7 @@ target_bfd::xfer_partial (target_object object, } } -target_section_table * +const target_section_table * target_bfd::get_section_table () { return &m_table; diff --git a/gdb/exec.c b/gdb/exec.c index 68b35204068..1cac5fb5d3d 100644 --- a/gdb/exec.c +++ b/gdb/exec.c @@ -75,7 +75,7 @@ struct exec_target final : public target_ops const gdb_byte *writebuf, ULONGEST offset, ULONGEST len, ULONGEST *xfered_len) override; - target_section_table *get_section_table () override; + const target_section_table *get_section_table () override; void files_info () override; bool has_memory () override; @@ -775,7 +775,7 @@ enum target_xfer_status section_table_read_available_memory (gdb_byte *readbuf, ULONGEST offset, ULONGEST len, ULONGEST *xfered_len) { - target_section_table *table = target_get_section_table (&exec_ops); + const target_section_table *table = target_get_section_table (&exec_ops); std::vector<mem_range> available_memory = section_table_available_memory (offset, len, *table); @@ -884,7 +884,7 @@ section_table_xfer_memory_partial (gdb_byte *readbuf, const gdb_byte *writebuf, return TARGET_XFER_EOF; /* We can't help. */ } -target_section_table * +const target_section_table * exec_target::get_section_table () { return ¤t_program_space->target_sections; @@ -896,7 +896,7 @@ exec_target::xfer_partial (enum target_object object, const gdb_byte *writebuf, ULONGEST offset, ULONGEST len, ULONGEST *xfered_len) { - target_section_table *table = get_section_table (); + const target_section_table *table = target_get_section_table (this); if (object == TARGET_OBJECT_MEMORY) return section_table_xfer_memory_partial (readbuf, writebuf, @@ -908,7 +908,7 @@ exec_target::xfer_partial (enum target_object object, \f void -print_section_info (target_section_table *t, bfd *abfd) +print_section_info (const target_section_table *t, bfd *abfd) { struct gdbarch *gdbarch = gdbarch_from_bfd (abfd); /* FIXME: 16 is not wide enough when gdbarch_addr_bit > 64. */ diff --git a/gdb/exec.h b/gdb/exec.h index cccc0d0cfa7..1119953dc8f 100644 --- a/gdb/exec.h +++ b/gdb/exec.h @@ -96,7 +96,7 @@ extern void exec_set_section_address (const char *, int, CORE_ADDR); special cased --- it's filename is omitted; if it is the executable file, its entry point is printed. */ -extern void print_section_info (target_section_table *table, +extern void print_section_info (const target_section_table *table, bfd *abfd); /* Helper function that attempts to open the symbol file at EXEC_FILE_HOST. diff --git a/gdb/ppc64-tdep.c b/gdb/ppc64-tdep.c index f8f60994bae..74873a6999e 100644 --- a/gdb/ppc64-tdep.c +++ b/gdb/ppc64-tdep.c @@ -561,7 +561,7 @@ ppc64_convert_from_func_ptr_addr (struct gdbarch *gdbarch, struct target_ops *targ) { enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); - struct target_section *s = target_section_by_addr (targ, addr); + const struct target_section *s = target_section_by_addr (targ, addr); /* Check if ADDR points to a function descriptor. */ if (s && strcmp (s->the_bfd_section->name, ".opd") == 0) diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c index ac51ff2bf49..d9cc7a3b6d8 100644 --- a/gdb/record-btrace.c +++ b/gdb/record-btrace.c @@ -1439,7 +1439,7 @@ record_btrace_target::xfer_partial (enum target_object object, { case TARGET_OBJECT_MEMORY: { - struct target_section *section; + const struct target_section *section; /* We do not allow writing memory in general. */ if (writebuf != NULL) diff --git a/gdb/remote.c b/gdb/remote.c index 31c6e17a1c4..2c85bdcffbc 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -9044,7 +9044,7 @@ remote_target::remote_xfer_live_readonly_partial (gdb_byte *readbuf, int unit_size, ULONGEST *xfered_len) { - struct target_section *secp; + const struct target_section *secp; secp = target_section_by_addr (this, memaddr); if (secp != NULL @@ -9052,8 +9052,8 @@ remote_target::remote_xfer_live_readonly_partial (gdb_byte *readbuf, { ULONGEST memend = memaddr + len; - target_section_table *table = target_get_section_table (this); - for (target_section &p : *table) + const target_section_table *table = target_get_section_table (this); + for (const target_section &p : *table) { if (memaddr >= p.addr) { diff --git a/gdb/s390-tdep.c b/gdb/s390-tdep.c index 57ddd540609..39c8ee0450a 100644 --- a/gdb/s390-tdep.c +++ b/gdb/s390-tdep.c @@ -684,8 +684,8 @@ s390_load (struct s390_prologue_data *data, we're analyzing the code to unwind past that frame. */ if (pv_is_constant (addr)) { - struct target_section *secp; - secp = target_section_by_addr (current_top_target (), addr.k); + const struct target_section *secp + = target_section_by_addr (current_top_target (), addr.k); if (secp != NULL && (bfd_section_flags (secp->the_bfd_section) & SEC_READONLY)) return pv_constant (read_memory_integer (addr.k, size, diff --git a/gdb/solib-dsbt.c b/gdb/solib-dsbt.c index 71245b26ca2..6d218060343 100644 --- a/gdb/solib-dsbt.c +++ b/gdb/solib-dsbt.c @@ -424,7 +424,8 @@ scan_dyntag (int dyntag, bfd *abfd, CORE_ADDR *ptr) return 0; bool found = false; - for (target_section &target_section : current_program_space->target_sections) + for (const target_section &target_section + : current_program_space->target_sections) if (sect == target_section.the_bfd_section) { dyn_addr = target_section.addr; diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c index 0416faa17c3..92253a25f18 100644 --- a/gdb/solib-svr4.c +++ b/gdb/solib-svr4.c @@ -610,7 +610,8 @@ scan_dyntag (const int desired_dyntag, bfd *abfd, CORE_ADDR *ptr, return 0; bool found = false; - for (target_section &target_section : current_program_space->target_sections) + for (const target_section &target_section + : current_program_space->target_sections) if (sect == target_section.the_bfd_section) { dyn_addr = target_section.addr; diff --git a/gdb/target-debug.h b/gdb/target-debug.h index 69103388652..1cc82562095 100644 --- a/gdb/target-debug.h +++ b/gdb/target-debug.h @@ -104,7 +104,7 @@ target_debug_do_print (host_address_to_string (X)) #define target_debug_print_struct_ui_file_p(X) \ target_debug_do_print (host_address_to_string (X)) -#define target_debug_print_target_section_table_p(X) \ +#define target_debug_print_const_target_section_table_p(X) \ target_debug_do_print (host_address_to_string (X)) #define target_debug_print_void_p(X) \ target_debug_do_print (host_address_to_string (X)) diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c index 437b19b8581..69fbc0f3b23 100644 --- a/gdb/target-delegates.c +++ b/gdb/target-delegates.c @@ -77,7 +77,7 @@ struct dummy_target : public target_ops void rcmd (const char *arg0, struct ui_file *arg1) override; char *pid_to_exec_file (int arg0) override; void log_command (const char *arg0) override; - target_section_table *get_section_table () override; + const target_section_table *get_section_table () override; thread_control_capabilities get_thread_control_capabilities () override; bool attach_no_wait () override; bool can_async_p () override; @@ -248,7 +248,7 @@ struct debug_target : public target_ops void rcmd (const char *arg0, struct ui_file *arg1) override; char *pid_to_exec_file (int arg0) override; void log_command (const char *arg0) override; - target_section_table *get_section_table () override; + const target_section_table *get_section_table () override; thread_control_capabilities get_thread_control_capabilities () override; bool attach_no_wait () override; bool can_async_p () override; @@ -2021,27 +2021,27 @@ debug_target::log_command (const char *arg0) fputs_unfiltered (")\n", gdb_stdlog); } -target_section_table * +const target_section_table * target_ops::get_section_table () { return this->beneath ()->get_section_table (); } -target_section_table * +const target_section_table * dummy_target::get_section_table () { return NULL; } -target_section_table * +const target_section_table * debug_target::get_section_table () { - target_section_table * result; + const target_section_table * result; fprintf_unfiltered (gdb_stdlog, "-> %s->get_section_table (...)\n", this->beneath ()->shortname ()); result = this->beneath ()->get_section_table (); fprintf_unfiltered (gdb_stdlog, "<- %s->get_section_table (", this->beneath ()->shortname ()); fputs_unfiltered (") = ", gdb_stdlog); - target_debug_print_target_section_table_p (result); + target_debug_print_const_target_section_table_p (result); fputs_unfiltered ("\n", gdb_stdlog); return result; } diff --git a/gdb/target.c b/gdb/target.c index 06d7b4fbf8e..78535b89e58 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -812,7 +812,7 @@ target_read_string (CORE_ADDR memaddr, int len, int *bytes_read) return gdb::unique_xmalloc_ptr<char> ((char *) buffer.release ()); } -target_section_table * +const target_section_table * target_get_section_table (struct target_ops *target) { return target->get_section_table (); @@ -820,15 +820,15 @@ target_get_section_table (struct target_ops *target) /* Find a section containing ADDR. */ -struct target_section * +const struct target_section * target_section_by_addr (struct target_ops *target, CORE_ADDR addr) { - target_section_table *table = target_get_section_table (target); + const target_section_table *table = target_get_section_table (target); if (table == NULL) return NULL; - for (target_section &secp : *table) + for (const target_section &secp : *table) { if (addr >= secp.addr && addr < secp.endaddr) return &secp; @@ -965,7 +965,7 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object, if (pc_in_unmapped_range (memaddr, section)) { - target_section_table *table = target_get_section_table (ops); + const target_section_table *table = target_get_section_table (ops); const char *section_name = section->the_bfd_section->name; memaddr = overlay_mapped_address (memaddr, section); @@ -984,13 +984,12 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object, /* Try the executable files, if "trust-readonly-sections" is set. */ if (readbuf != NULL && trust_readonly) { - struct target_section *secp; - - secp = target_section_by_addr (ops, memaddr); + const struct target_section *secp + = target_section_by_addr (ops, memaddr); if (secp != NULL && (bfd_section_flags (secp->the_bfd_section) & SEC_READONLY)) { - target_section_table *table = target_get_section_table (ops); + const target_section_table *table = target_get_section_table (ops); return section_table_xfer_memory_partial (readbuf, writebuf, memaddr, len, xfered_len, *table); diff --git a/gdb/target.h b/gdb/target.h index 0de78075e9b..c11a8c91aa8 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -687,7 +687,7 @@ struct target_ops TARGET_DEFAULT_RETURN (NULL); virtual void log_command (const char *) TARGET_DEFAULT_IGNORE (); - virtual target_section_table *get_section_table () + virtual const target_section_table *get_section_table () TARGET_DEFAULT_RETURN (NULL); /* Provide default values for all "must have" methods. */ @@ -2413,13 +2413,13 @@ extern CORE_ADDR target_translate_tls_address (struct objfile *objfile, CORE_ADDR offset); /* Return the "section" containing the specified address. */ -struct target_section *target_section_by_addr (struct target_ops *target, - CORE_ADDR addr); +const struct target_section *target_section_by_addr (struct target_ops *target, + CORE_ADDR addr); /* Return the target section table this target (or the targets beneath) currently manipulate. */ -extern target_section_table *target_get_section_table +extern const target_section_table *target_get_section_table (struct target_ops *target); /* From mem-break.c */ -- 2.25.4 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/4] gdb: make the target_sections table private within program_space 2021-02-16 17:50 [PATCH 0/4] Allow access to target_sections even without an executable Andrew Burgess 2021-02-16 17:50 ` [PATCH 1/4] gdb: add a new 'maint info target-sections' command Andrew Burgess 2021-02-16 17:50 ` [PATCH 2/4] gdb: spread a little 'const' through the target_section_table code Andrew Burgess @ 2021-02-16 17:50 ` Andrew Burgess 2021-02-19 14:24 ` Tom Tromey 2021-02-16 17:50 ` [PATCH 4/4] gdb: move get_section_table from exec_target to dummy_target Andrew Burgess ` (2 subsequent siblings) 5 siblings, 1 reply; 19+ messages in thread From: Andrew Burgess @ 2021-02-16 17:50 UTC (permalink / raw) To: gdb-patches Following on from the previous commit which made access to the target_sections table more 'const', this commit makes the table private within the program_space class and provides member functions to access and update the table. There are just two places, from solib-*.c, where code outside of the program_space class actually modifies the target_sections table. To support this I added the program_space::foreach_target_section function. In all other cases access to the table is read-only, so the new program_space::target_sections function returns a const reference. There should be no user visible changes after this commit. gdb/ChangeLog: * exec.c (exec_target::close): Call new clear_target_sections function. (program_space::add_target_sections): Update name of member variable. (program_space::foreach_target_section): New function. (program_space::add_target_sections): Update name of member variable. (program_space::remove_target_sections): Likewise. (exec_one_fork): Use new target_sections member function. (exec_target::get_section_table): Likewise. (exec_target::files_info): Likewise. (set_section_command): Use new foreach_target_section member function. (exec_set_section_address): Likewise. (exec_target::has_memory): Use new target_sections member function. * progspace.h (program_space::clear_target_sections): New member function. (program_space::target_sections): Rename member variable to m_target_sections, replace with a new member function. (program_space::foreach_target_section): Declare new member function. (program_space::m_target_sections): New member variable. * solib-dsbt.c (scan_dyntag): Use new member function. * solib-svr4.c (scan_dyntag): Likewise. --- gdb/ChangeLog | 28 +++++++++++++++ gdb/exec.c | 90 ++++++++++++++++++++++++++++-------------------- gdb/progspace.h | 30 +++++++++++++--- gdb/solib-dsbt.c | 2 +- gdb/solib-svr4.c | 2 +- 5 files changed, 109 insertions(+), 43 deletions(-) diff --git a/gdb/exec.c b/gdb/exec.c index 1cac5fb5d3d..2ad89af4798 100644 --- a/gdb/exec.c +++ b/gdb/exec.c @@ -156,7 +156,7 @@ exec_target::close () { for (struct program_space *ss : program_spaces) { - ss->target_sections.clear (); + ss->clear_target_sections (); ss->exec_close (); } } @@ -599,8 +599,8 @@ program_space::add_target_sections (void *owner, { for (const target_section &s : sections) { - target_sections.push_back (s); - target_sections.back ().owner = owner; + m_target_sections.push_back (s); + m_target_sections.back ().owner = owner; } scoped_restore_current_pspace_and_thread restore_pspace_thread; @@ -622,6 +622,19 @@ program_space::add_target_sections (void *owner, } } +/* See progspace.h. */ + +void +program_space::foreach_target_section + (gdb::function_view<bool (target_section &sec)> callback) +{ + for (target_section &sec : m_target_sections) + { + if (!callback (sec)) + break; + } +} + /* Add the sections of OBJFILE to the current set of target sections. */ void @@ -637,9 +650,9 @@ program_space::add_target_sections (struct objfile *objfile) if (bfd_section_size (osect->the_bfd_section) == 0) continue; - target_sections.emplace_back (obj_section_addr (osect), - obj_section_endaddr (osect), - osect->the_bfd_section, (void *) objfile); + m_target_sections.emplace_back (obj_section_addr (osect), + obj_section_endaddr (osect), + osect->the_bfd_section, (void *) objfile); } } @@ -651,18 +664,18 @@ program_space::remove_target_sections (void *owner) { gdb_assert (owner != NULL); - auto it = std::remove_if (target_sections.begin (), - target_sections.end (), + auto it = std::remove_if (m_target_sections.begin (), + m_target_sections.end (), [&] (target_section §) { return sect.owner == owner; }); - target_sections.erase (it, target_sections.end ()); + m_target_sections.erase (it, m_target_sections.end ()); /* If we don't have any more sections to read memory from, remove the file_stratum target from the stack of each inferior sharing the program space. */ - if (target_sections.empty ()) + if (m_target_sections.empty ()) { scoped_restore_current_pspace_and_thread restore_pspace_thread; @@ -682,7 +695,7 @@ program_space::remove_target_sections (void *owner) void exec_on_vfork () { - if (!current_program_space->target_sections.empty ()) + if (!current_program_space->target_sections ().empty ()) push_target (&exec_ops); } @@ -887,7 +900,7 @@ section_table_xfer_memory_partial (gdb_byte *readbuf, const gdb_byte *writebuf, const target_section_table * exec_target::get_section_table () { - return ¤t_program_space->target_sections; + return ¤t_program_space->target_sections (); } enum target_xfer_status @@ -985,7 +998,7 @@ void exec_target::files_info () { if (current_program_space->exec_bfd ()) - print_section_info (¤t_program_space->target_sections, + print_section_info (¤t_program_space->target_sections (), current_program_space->exec_bfd ()); else puts_filtered (_("\t<no file loaded>\n")); @@ -1010,19 +1023,21 @@ set_section_command (const char *args, int from_tty) /* Parse out new virtual address. */ secaddr = parse_and_eval_address (args); - for (target_section &p : current_program_space->target_sections) - { - if (!strncmp (secname, bfd_section_name (p.the_bfd_section), seclen) - && bfd_section_name (p.the_bfd_section)[seclen] == '\0') - { - offset = secaddr - p.addr; - p.addr += offset; - p.endaddr += offset; - if (from_tty) - exec_ops.files_info (); - return; - } - } + current_program_space->foreach_target_section ([&] (target_section &p) + { + if (!strncmp (secname, bfd_section_name (p.the_bfd_section), seclen) + && bfd_section_name (p.the_bfd_section)[seclen] == '\0') + { + offset = secaddr - p.addr; + p.addr += offset; + p.endaddr += offset; + if (from_tty) + exec_ops.files_info (); + return false; + } + return true; + }); + if (seclen >= sizeof (secprint)) seclen = sizeof (secprint) - 1; strncpy (secprint, secname, seclen); @@ -1036,16 +1051,17 @@ set_section_command (const char *args, int from_tty) void exec_set_section_address (const char *filename, int index, CORE_ADDR address) { - for (target_section &p : current_program_space->target_sections) - { - if (filename_cmp (filename, - bfd_get_filename (p.the_bfd_section->owner)) == 0 - && index == p.the_bfd_section->index) - { - p.endaddr += address - p.addr; - p.addr = address; - } - } + current_program_space->foreach_target_section ([&] (target_section &p) -> bool + { + if (filename_cmp (filename, + bfd_get_filename (p.the_bfd_section->owner)) == 0 + && index == p.the_bfd_section->index) + { + p.endaddr += address - p.addr; + p.addr = address; + } + return true; + }); } bool @@ -1053,7 +1069,7 @@ exec_target::has_memory () { /* We can provide memory if we have any file/target sections to read from. */ - return !current_program_space->target_sections.empty (); + return !current_program_space->target_sections ().empty (); } gdb::unique_xmalloc_ptr<char> diff --git a/gdb/progspace.h b/gdb/progspace.h index 6ac8932cd60..2bf7331d34e 100644 --- a/gdb/progspace.h +++ b/gdb/progspace.h @@ -309,6 +309,27 @@ struct program_space sections. They are given OBJFILE as the "owner". */ void add_target_sections (struct objfile *objfile); + /* Clear all target sections from M_TARGET_SECTIONS table. */ + void clear_target_sections () + { + m_target_sections.clear (); + } + + /* Return a read-only reference to the M_TARGET_SECTIONS table. */ + const target_section_table &target_sections () const + { + return m_target_sections; + } + + /* Run CALLBACK on each target_section in M_TARGET_SECTIONS. For each + invocation of CALLBACK, if CALLBACK returns true then continue to the + next target_section, otherwise stop and immediately return. + + CALLBACK is passed a non-const reference to each target_section, and + is allowed to modify the section. */ + void foreach_target_section + (gdb::function_view<bool (target_section &sec)> callback); + /* Unique ID number. */ int num = 0; @@ -359,10 +380,6 @@ struct program_space /* All known objfiles are kept in a linked list. */ std::list<std::shared_ptr<objfile>> objfiles_list; - /* The set of target sections matching the sections mapped into - this program space. Managed by both exec_ops and solib.c. */ - target_section_table target_sections; - /* List of shared objects mapped into this space. Managed by solib.c. */ struct so_list *so_list = NULL; @@ -380,6 +397,11 @@ struct program_space /* Per pspace data-pointers required by other GDB modules. */ REGISTRY_FIELDS {}; + +private: + /* The set of target sections matching the sections mapped into + this program space. Managed by both exec_ops and solib.c. */ + target_section_table m_target_sections; }; /* An address space. It is used for comparing if diff --git a/gdb/solib-dsbt.c b/gdb/solib-dsbt.c index 6d218060343..4b1b7560e16 100644 --- a/gdb/solib-dsbt.c +++ b/gdb/solib-dsbt.c @@ -425,7 +425,7 @@ scan_dyntag (int dyntag, bfd *abfd, CORE_ADDR *ptr) bool found = false; for (const target_section &target_section - : current_program_space->target_sections) + : current_program_space->target_sections ()) if (sect == target_section.the_bfd_section) { dyn_addr = target_section.addr; diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c index 92253a25f18..6c0fe819a2f 100644 --- a/gdb/solib-svr4.c +++ b/gdb/solib-svr4.c @@ -611,7 +611,7 @@ scan_dyntag (const int desired_dyntag, bfd *abfd, CORE_ADDR *ptr, bool found = false; for (const target_section &target_section - : current_program_space->target_sections) + : current_program_space->target_sections ()) if (sect == target_section.the_bfd_section) { dyn_addr = target_section.addr; -- 2.25.4 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/4] gdb: make the target_sections table private within program_space 2021-02-16 17:50 ` [PATCH 3/4] gdb: make the target_sections table private within program_space Andrew Burgess @ 2021-02-19 14:24 ` Tom Tromey 0 siblings, 0 replies; 19+ messages in thread From: Tom Tromey @ 2021-02-19 14:24 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes: Andrew> program_space class actually modifies the target_sections table. To Andrew> support this I added the program_space::foreach_target_section Andrew> function. Normally I think we should prefer iterators to callbacks, but in this case there are only a couple of callers, and not likely to be many more, so it seems fine. Andrew> - for (target_section &p : current_program_space->target_sections) Andrew> - { Andrew> - if (!strncmp (secname, bfd_section_name (p.the_bfd_section), seclen) Andrew> - && bfd_section_name (p.the_bfd_section)[seclen] == '\0') Andrew> - { Andrew> - offset = secaddr - p.addr; Andrew> - p.addr += offset; Andrew> - p.endaddr += offset; Andrew> - if (from_tty) Andrew> - exec_ops.files_info (); Andrew> - return; Andrew> - } Andrew> - } Andrew> + current_program_space->foreach_target_section ([&] (target_section &p) Andrew> + { Andrew> + if (!strncmp (secname, bfd_section_name (p.the_bfd_section), seclen) Andrew> + && bfd_section_name (p.the_bfd_section)[seclen] == '\0') Andrew> + { Andrew> + offset = secaddr - p.addr; Andrew> + p.addr += offset; Andrew> + p.endaddr += offset; Andrew> + if (from_tty) Andrew> + exec_ops.files_info (); Andrew> + return false; Andrew> + } Andrew> + return true; Andrew> + }); Here the old code returned from the function, but the new code continues on to the following: Andrew> + Andrew> if (seclen >= sizeof (secprint)) Andrew> seclen = sizeof (secprint) - 1; Andrew> strncpy (secprint, secname, seclen); ... not sure if this is intended and/or harmless. Tom ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 4/4] gdb: move get_section_table from exec_target to dummy_target 2021-02-16 17:50 [PATCH 0/4] Allow access to target_sections even without an executable Andrew Burgess ` (2 preceding siblings ...) 2021-02-16 17:50 ` [PATCH 3/4] gdb: make the target_sections table private within program_space Andrew Burgess @ 2021-02-16 17:50 ` Andrew Burgess 2021-02-19 14:46 ` [PATCH 0/4] Allow access to target_sections even without an executable Tom Tromey 2021-02-19 20:07 ` [PATCHv2 0/6] " Andrew Burgess 5 siblings, 0 replies; 19+ messages in thread From: Andrew Burgess @ 2021-02-16 17:50 UTC (permalink / raw) To: gdb-patches The only target that implements target_ops::get_section_table in a meaningful way is exec_target. This target calls back into the program space to return the current global section_table. The global section table is populated whenever the user provides GDB with an executable, or when a symbol file is loaded, e.g. when a dynamic library is loaded, or when the user does add-symbol-file. I recently ran into a situation where a user, debugging a remote target, was not supplying GDB with a main executable at all. Instead the user attached to the target then did add-symbol-file, and then proceeded to debug the target. This works fine, but it was noticed that even when trust-readonly-sections was on GDB was still accessing the target to get the contents of readonly sections. The problem is that by not providing an executable there was no exec_target in the target stack, and so when GDB calls the target_ops::get_section_table function GDB ends up in dummy_target::get_section_table, which just returns NULL. What I want is that even when GDB doesn't have an exec_target in the target stack, a call to target_ops::get_section_table will still return the section_table from the current program space. When considering how to achieve this my first though was, why is the request for the section table going via the target stack at all? The set of sections loaded is a property of the program space, not the target. This is, after all, why the data is being stored in the program space. So I initially tried changing target_get_section_table so that, instead of calling into the target it just returns current_program_space->target_sections (). This would be fine except for one issue, target_bfd (from bfd-target.c). This code is used from solib-svr4.c to create a temporary target_ops structure that implements two functions target_bfd::xfer_partial and target_bfd::get_section_table. The purpose behind the code is to enable two targets, ppc64 and frv to decode function descriptors from the dynamic linker, based on the non-relocated addresses from within the dynamic linker bfd object. Both of the implemented functions in target_bfd rely on the target_bfd object holding a section table, and the ppc64 target requires that the target_bfd implement ::get_section_table. The frv target doesn't require ::get_section_table, instead it requires the ::xfer_partial. We could in theory change the ppc64 target to use the same approach as frv, however, this would be a bad idea. I believe that the frv target approach is broken. I'll explain: The frv target calls get_target_memory_unsigned to read the function descriptor. The address being read is the non-relocated address read from the dynamic linker in solib-srv4.c:enable_break. Calling get_target_memory_unsigned eventually ends up in target_xfer_partial with an object type of TARGET_OBJECT_RAW_MEMORY. This will then call memory_xfer_check_region. I believe that it is quite possible that a the non-relocated addresses pulled from the dynamic linker could be in a memory region that is not readable, while the relocated addresses are in a readable memory region. If this was ever the case for the frv target then GDB would reject the attempt to read the non-relocated function pointer. In contrast the ppc64 target calls target_section_by_addr, which calls target_get_section_table, which then calls the ::get_section_table function on the target. Thus, when reflecting on target_bfd we see two functions, ::xfer_partial and ::get_section_table. The former is required by the frv target, but that target is (I think) potentially broken. While the latter is required by the ppc64 target, but this forces ::get_section_table to exist as a target_ops member function. So my original plan, have target_get_section_table NOT call a target_ops member function appears to be flawed. My next idea was to remove exec_target::get_section_table, and instead move the implementation into dummy_target::get_section_table. Currently the dummy_target implementation always returns NULL indicating no section table, but plenty of other dummy_target member functions do more than just return null values. So now, dummy_target::get_section_table returns the section table from the current program space. This allows target_bfd to remain unchanged, so ppc64 and frv should not be affected. Making this change removes the requirement for the user to provide an executable, GDB can now always access the section_table, as the dummy_target always exists in the target stack. Finally, there's a test that the target_section table is not empty in the case where the user does add-symbol-file without providing an executable. gdb/ChangeLog: * exec.c (exec_target::get_section_table): Delete member function. (section_table_read_available_memory): Use current_top_target, not just the exec_ops target. * target-delegates.c: Regenerate. * target.c (default_get_section_table): New function. * target.h (target_ops::get_section_table): Change default behaviour to call default_get_section_table. (default_get_section_table): Declare. --- gdb/ChangeLog | 11 +++++++++++ gdb/exec.c | 10 ++-------- gdb/target-delegates.c | 2 +- gdb/target.c | 7 +++++++ gdb/target.h | 6 +++++- gdb/testsuite/gdb.base/maint-info-sections.exp | 4 +--- 6 files changed, 27 insertions(+), 13 deletions(-) diff --git a/gdb/exec.c b/gdb/exec.c index 2ad89af4798..e613409e8ef 100644 --- a/gdb/exec.c +++ b/gdb/exec.c @@ -75,7 +75,6 @@ struct exec_target final : public target_ops const gdb_byte *writebuf, ULONGEST offset, ULONGEST len, ULONGEST *xfered_len) override; - const target_section_table *get_section_table () override; void files_info () override; bool has_memory () override; @@ -788,7 +787,8 @@ enum target_xfer_status section_table_read_available_memory (gdb_byte *readbuf, ULONGEST offset, ULONGEST len, ULONGEST *xfered_len) { - const target_section_table *table = target_get_section_table (&exec_ops); + const target_section_table *table + = target_get_section_table (current_top_target ()); std::vector<mem_range> available_memory = section_table_available_memory (offset, len, *table); @@ -897,12 +897,6 @@ section_table_xfer_memory_partial (gdb_byte *readbuf, const gdb_byte *writebuf, return TARGET_XFER_EOF; /* We can't help. */ } -const target_section_table * -exec_target::get_section_table () -{ - return ¤t_program_space->target_sections (); -} - enum target_xfer_status exec_target::xfer_partial (enum target_object object, const char *annex, gdb_byte *readbuf, diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c index 69fbc0f3b23..1c7999724c7 100644 --- a/gdb/target-delegates.c +++ b/gdb/target-delegates.c @@ -2030,7 +2030,7 @@ target_ops::get_section_table () const target_section_table * dummy_target::get_section_table () { - return NULL; + return default_get_section_table (); } const target_section_table * diff --git a/gdb/target.c b/gdb/target.c index 78535b89e58..ba445e7fd34 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -836,6 +836,13 @@ target_section_by_addr (struct target_ops *target, CORE_ADDR addr) return NULL; } +/* See target.h. */ + +const target_section_table * +default_get_section_table () +{ + return ¤t_program_space->target_sections (); +} /* Helper for the memory xfer routines. Checks the attributes of the memory region of MEMADDR against the read or write being attempted. diff --git a/gdb/target.h b/gdb/target.h index c11a8c91aa8..45b92f7d83f 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -688,7 +688,7 @@ struct target_ops virtual void log_command (const char *) TARGET_DEFAULT_IGNORE (); virtual const target_section_table *get_section_table () - TARGET_DEFAULT_RETURN (NULL); + TARGET_DEFAULT_RETURN (default_get_section_table ()); /* Provide default values for all "must have" methods. */ virtual bool has_all_memory () { return false; } @@ -2422,6 +2422,10 @@ const struct target_section *target_section_by_addr (struct target_ops *target, extern const target_section_table *target_get_section_table (struct target_ops *target); +/* Default implementation of get_section_table for dummy_target. */ + +extern const target_section_table *default_get_section_table (); + /* From mem-break.c */ extern int memory_remove_breakpoint (struct target_ops *, diff --git a/gdb/testsuite/gdb.base/maint-info-sections.exp b/gdb/testsuite/gdb.base/maint-info-sections.exp index b86812e4f12..06508de366d 100644 --- a/gdb/testsuite/gdb.base/maint-info-sections.exp +++ b/gdb/testsuite/gdb.base/maint-info-sections.exp @@ -223,9 +223,7 @@ gdb_test_multiple "maint info sections -all-objects" "" { } } -# NOTE: We would like to check 'maint info target-sections' again -# here, but GDB currently doesn't display the target sections table in -# this case. This is a bug and fill be fixed shortly!! +check_maint_info_target_sections_output "with loaded symbol file" # Test the command line completion on 'maint info sections'. First # the command line flag. -- 2.25.4 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/4] Allow access to target_sections even without an executable 2021-02-16 17:50 [PATCH 0/4] Allow access to target_sections even without an executable Andrew Burgess ` (3 preceding siblings ...) 2021-02-16 17:50 ` [PATCH 4/4] gdb: move get_section_table from exec_target to dummy_target Andrew Burgess @ 2021-02-19 14:46 ` Tom Tromey 2021-02-19 20:07 ` [PATCHv2 0/6] " Andrew Burgess 5 siblings, 0 replies; 19+ messages in thread From: Tom Tromey @ 2021-02-19 14:46 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes: Andrew> The last patch in this series is the interesting one. Andrew> I ran into a situation where a user was using GDB against a remote Andrew> target without specifying an executable (either on the command line, Andrew> or using the file command). Instead, they loaded the executabe with Andrew> add-symbol-file and then continued to debug their remote target. Andrew> This works fine, except that 'set trust-readonly-sections' doesn't Andrew> work. I looked through these, and sent the one comment I had. The explanation in patch #4 was particularly helpful. Thanks for taking the time to write that. Tom ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCHv2 0/6] Allow access to target_sections even without an executable 2021-02-16 17:50 [PATCH 0/4] Allow access to target_sections even without an executable Andrew Burgess ` (4 preceding siblings ...) 2021-02-19 14:46 ` [PATCH 0/4] Allow access to target_sections even without an executable Tom Tromey @ 2021-02-19 20:07 ` Andrew Burgess 2021-02-19 20:07 ` [PATCHv2 1/6] gdb: add a new 'maint info target-sections' command Andrew Burgess ` (6 more replies) 5 siblings, 7 replies; 19+ messages in thread From: Andrew Burgess @ 2021-02-19 20:07 UTC (permalink / raw) To: gdb-patches Changes since v2: Patch #1 - Fixed typo in a comment that Christian pointed out. Patch #2 - Unchanged. Patch #3 - New in this version. Patch #4 - Removed the callback foreach_target_section and just made target_sections return a non-const reference instead. Fixed the bug that Tom pointed out. Patch #5 - Unchanged. Patch #6 - New in this version. Thanks, Andrew --- Andrew Burgess (6): gdb: add a new 'maint info target-sections' command gdb: spread a little 'const' through the target_section_table code gdb/testsuite: enable gdb.base/sect-cmd.exp test for all targets gdb: make the target_sections table private within program_space gdb: move get_section_table from exec_target to dummy_target gdb: use std::string instead of a fixed size buffer gdb/ChangeLog | 77 +++++++++ gdb/NEWS | 3 + gdb/bfd-target.c | 4 +- gdb/doc/ChangeLog | 5 + gdb/doc/gdb.texinfo | 8 + gdb/exec.c | 62 +++----- gdb/exec.h | 2 +- gdb/maint.c | 59 +++++++ gdb/ppc64-tdep.c | 2 +- gdb/progspace.h | 21 ++- gdb/record-btrace.c | 2 +- gdb/remote.c | 6 +- gdb/s390-tdep.c | 4 +- gdb/solib-dsbt.c | 3 +- gdb/solib-svr4.c | 3 +- gdb/target-debug.h | 2 +- gdb/target-delegates.c | 16 +- gdb/target.c | 24 +-- gdb/target.h | 14 +- gdb/testsuite/ChangeLog | 14 ++ .../gdb.base/maint-info-sections.exp | 66 +++++++- gdb/testsuite/gdb.base/sect-cmd.exp | 146 +++++++----------- 22 files changed, 377 insertions(+), 166 deletions(-) -- 2.25.4 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCHv2 1/6] gdb: add a new 'maint info target-sections' command 2021-02-19 20:07 ` [PATCHv2 0/6] " Andrew Burgess @ 2021-02-19 20:07 ` Andrew Burgess 2021-02-19 20:07 ` [PATCHv2 2/6] gdb: spread a little 'const' through the target_section_table code Andrew Burgess ` (5 subsequent siblings) 6 siblings, 0 replies; 19+ messages in thread From: Andrew Burgess @ 2021-02-19 20:07 UTC (permalink / raw) To: gdb-patches We already have a command 'maint info sections', this command prints all sections from all known object files. However, GDB maintains a second section table internally. This section table is used when GDB wants to read directly from an object file rather than actually reading memory on the target. As such only some sections (the allocatable ones) are added to this secondary section table. I recently ran into a situation where some of GDB's optimisations for reading directly from the files were not working. In 'maint info sections' I could see that GDB knew about the object file, and did know about the sections that it _should_ have been reading from. But I couldn't ask GDB which sections it had copied into its secondary section table. This commit adds a new command 'maint info target-sections' that fills this gap. This command lists only those sections that GDB has copied into its secondary table. You'll notice that the testsuite includes a comment indicating that there's a bug in GDB. Normally this is not something I would add to the testsuite, instead we should raise an actual bugzilla bug and then mark an xfail, however, a later patch in this series will remove this comment once the actual bug in GDB is fixed. gdb/ChangeLog: * NEWS: Mention new 'maint info target-sections' command. * maint.c (maintenance_info_target_sections): New function. (_initialize_maint_cmds): Register new command. gdb/doc/ChangeLog: * gdb.texinfo (Files): Document new 'maint info target-sections' command. gdb/testsuite/ChangeLog: * gdb.base/maint-info-sections.exp: Add new tests. (check_maint_info_target_sections_output): New proc. --- gdb/ChangeLog | 6 ++ gdb/NEWS | 3 + gdb/doc/ChangeLog | 5 ++ gdb/doc/gdb.texinfo | 8 +++ gdb/maint.c | 59 ++++++++++++++++ gdb/testsuite/ChangeLog | 5 ++ .../gdb.base/maint-info-sections.exp | 68 ++++++++++++++++++- 7 files changed, 153 insertions(+), 1 deletion(-) diff --git a/gdb/NEWS b/gdb/NEWS index 1dfbbc648eb..7f5a745d0c0 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -47,6 +47,9 @@ maintenance flush register-cache maintenance flush dcache A new command to flush the dcache. +maintenance info target-sections + Print GDB's internal target sections table. + * Changed commands break [PROBE_MODIFIER] [LOCATION] [thread THREADNUM] diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 90f0c7683f6..80ccf74a049 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -20668,6 +20668,14 @@ Section contains common symbols. @end table @end table + +@kindex maint info target-sections +@item maint info target-sections +This command prints @value{GDBN}'s internal section table. For each +target @value{GDBN} maintains a table containing the allocatable +sections from all currently mapped objects, along with information +about where the section is mapped. + @kindex set trust-readonly-sections @cindex read-only sections @item set trust-readonly-sections on diff --git a/gdb/maint.c b/gdb/maint.c index 707d156ec2a..bfdd1d5ca3e 100644 --- a/gdb/maint.c +++ b/gdb/maint.c @@ -464,6 +464,56 @@ maintenance_info_sections (const char *arg, int from_tty) maint_print_all_sections (_("Core file: "), core_bfd, nullptr, arg); } +/* Implement the "maintenance info target-sections" command. */ + +static void +maintenance_info_target_sections (const char *arg, int from_tty) +{ + bfd *abfd = nullptr; + int digits = 0; + const target_section_table *table + = target_get_section_table (current_top_target ()); + if (table == nullptr) + return; + + for (const target_section &sec : *table) + { + if (abfd == nullptr || sec.the_bfd_section->owner != abfd) + { + abfd = sec.the_bfd_section->owner; + digits = std::max (index_digits (gdb_bfd_count_sections (abfd)), + digits); + } + } + + struct gdbarch *gdbarch = nullptr; + int addr_size = 0; + abfd = nullptr; + for (const target_section &sec : *table) + { + if (sec.the_bfd_section->owner != abfd) + { + abfd = sec.the_bfd_section->owner; + gdbarch = gdbarch_from_bfd (abfd); + addr_size = gdbarch_addr_bit (gdbarch) / 8; + + printf_filtered (_("From '%s', file type %s:\n"), + bfd_get_filename (abfd), bfd_get_target (abfd)); + } + print_bfd_section_info (abfd, + sec.the_bfd_section, + nullptr, + digits); + /* The magic '8 + digits' here ensures that the 'Start' is aligned + with the output of print_bfd_section_info. */ + printf_filtered ("%*sStart: %s, End: %s, Owner token: %p\n", + (8 + digits), "", + hex_string_custom (sec.addr, addr_size), + hex_string_custom (sec.endaddr, addr_size), + sec.owner); + } +} + static void maintenance_print_statistics (const char *args, int from_tty) { @@ -1122,6 +1172,15 @@ Options:\n\ &maintenanceinfolist); set_cmd_completer_handle_brkchars (cmd, maint_info_sections_completer); + add_cmd ("target-sections", class_maintenance, + maintenance_info_target_sections, _("\ +List GDB's internal section table.\n\ +\n\ +Print the current targets section list. This is a sub-set of all\n\ +sections, from all objects currently loaded. Usually the ALLOC\n\ +sectoins."), + &maintenanceinfolist); + add_basic_prefix_cmd ("print", class_maintenance, _("Maintenance command for printing GDB internal state."), &maintenanceprintlist, "maintenance print ", 0, diff --git a/gdb/testsuite/gdb.base/maint-info-sections.exp b/gdb/testsuite/gdb.base/maint-info-sections.exp index 07c53b16177..17e38ebc416 100644 --- a/gdb/testsuite/gdb.base/maint-info-sections.exp +++ b/gdb/testsuite/gdb.base/maint-info-sections.exp @@ -13,7 +13,8 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see <http://www.gnu.org/licenses/>. -# Test just for the 'maintenance info sections' command. +# Test just for the 'maintenance info sections' command and the +# 'maintenance info target-sections' command. load_lib completion-support.exp @@ -29,6 +30,65 @@ if ![runto_main] then { return -1 } +# Check the output of 'maint info target-sections' command. +proc check_maint_info_target_sections_output {prefix} { + global hex gdb_prompt + + with_test_prefix $prefix { + # Check the output of the 'maint info target-sections' command. + # Ensures that all the lines have the expected format, and that we see + # an auxiliary information line after every section information line. + # + # We also check that every bfd section has the ALLOC flag. + set seen_header false + set seen_a_section false + set seen_aux_info false + set expecting_aux_info false + gdb_test_multiple "maint info target-sections" "" { + -re "^maint info target-sections\r\n" { + # Consume the command we sent to GDB that the terminal echo'd + # back. + exp_continue + } + -re "^From '\[^\r\n\]+', file type \[^\r\n\]+:\r\n" { + # There might be more than one header, but there should be at + # least one. + set seen_header true + if { $expecting_aux_info } { + fail $gdb_test_name + } else { + exp_continue + } + } + -re "^ \\\[\[0-9\]+\\\]\\s+$hex->$hex at $hex: \[^*\r\]+\\yALLOC\\y\[^*\r\]*\r\n" { + # A bfd section description line. + set seen_a_section true + if { $expecting_aux_info } { + fail $gdb_test_name + } else { + set expecting_aux_info true + exp_continue + } + } + -re "^\\s+Start: $hex, End: $hex, Owner token: $hex\r\n" { + # A target section auxiliary information line. + set seen_aux_info true + if { !$expecting_aux_info } { + fail $gdb_test_name + } else { + set expecting_aux_info false + exp_continue + } + } + -re "^$gdb_prompt $" { + gdb_assert { $seen_header && $seen_a_section && $seen_aux_info } + gdb_assert { !$expecting_aux_info } + pass $gdb_test_name + } + } + } +} + # Check that 'maint info sections' output looks correct. When # checking the lines for each section we reject section names starting # with a '*' character, the internal *COM*, *UND*, *ABS*, and *IND* @@ -128,6 +188,8 @@ gdb_test_multiple "maint info sections DATA" "" { } } +check_maint_info_target_sections_output "with executable" + # Restart GDB, but don't load the executable. clean_restart @@ -161,6 +223,10 @@ gdb_test_multiple "maint info sections -all-objects" "" { } } +# NOTE: We would like to check 'maint info target-sections' again +# here, but GDB currently doesn't display the target sections table in +# this case. This is a bug and will be fixed shortly!! + # Test the command line completion on 'maint info sections'. First # the command line flag. test_gdb_complete_unique \ -- 2.25.4 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCHv2 2/6] gdb: spread a little 'const' through the target_section_table code 2021-02-19 20:07 ` [PATCHv2 0/6] " Andrew Burgess 2021-02-19 20:07 ` [PATCHv2 1/6] gdb: add a new 'maint info target-sections' command Andrew Burgess @ 2021-02-19 20:07 ` Andrew Burgess 2021-02-19 20:07 ` [PATCHv2 3/6] gdb/testsuite: enable gdb.base/sect-cmd.exp test for all targets Andrew Burgess ` (4 subsequent siblings) 6 siblings, 0 replies; 19+ messages in thread From: Andrew Burgess @ 2021-02-19 20:07 UTC (permalink / raw) To: gdb-patches The code to access the target section table can be made more const, so lets do that. There should be no user visible changes after this commit. gdb/ChangeLog: * gdb/bfd-target.c (class target_bfd) <get_section_table>: Make return type const. * gdb/exec.c (struct exec_target) <get_section_table>: Likewise. (section_table_read_available_memory): Make local const. (exec_target::xfer_partial): Make local const. (print_section_info): Make parameter const. * gdb/exec.h (print_section_info): Likewise. * gdb/ppc64-tdep.c (ppc64_convert_from_func_ptr_addr): Make local const. * gdb/record-btrace.c (record_btrace_target::xfer_partial): Likewise. * gdb/remote.c (remote_target::remote_xfer_live_readonly_partial): Likewise. * gdb/s390-tdep.c (s390_load): Likewise. * gdb/solib-dsbt.c (scan_dyntag): Likewise. * gdb/solib-svr4.c (scan_dyntag): Likewise. * gdb/target-debug.h (target_debug_print_target_section_table_p): Rename to... (target_debug_print_const_target_section_table_p): ...this. * gdb/target-delegates.c: Regenerate. * gdb/target.c (target_get_section_table): Make return type const. (target_section_by_addr): Likewise. Also make some locals const. (memory_xfer_partial_1): Make some locals const. * gdb/target.h (struct target_ops) <get_section_table>: Make return type const. (target_section_by_addr): Likewise. (target_get_section_table): Likewise. --- gdb/ChangeLog | 30 ++++++++++++++++++++++++++++++ gdb/bfd-target.c | 4 ++-- gdb/exec.c | 10 +++++----- gdb/exec.h | 2 +- gdb/ppc64-tdep.c | 2 +- gdb/record-btrace.c | 2 +- gdb/remote.c | 6 +++--- gdb/s390-tdep.c | 4 ++-- gdb/solib-dsbt.c | 3 ++- gdb/solib-svr4.c | 3 ++- gdb/target-debug.h | 2 +- gdb/target-delegates.c | 14 +++++++------- gdb/target.c | 17 ++++++++--------- gdb/target.h | 8 ++++---- 14 files changed, 69 insertions(+), 38 deletions(-) diff --git a/gdb/bfd-target.c b/gdb/bfd-target.c index 90d247a981a..09d9c978b05 100644 --- a/gdb/bfd-target.c +++ b/gdb/bfd-target.c @@ -50,7 +50,7 @@ class target_bfd : public target_ops ULONGEST offset, ULONGEST len, ULONGEST *xfered_len) override; - target_section_table *get_section_table () override; + const target_section_table *get_section_table () override; private: /* The BFD we're wrapping. */ @@ -82,7 +82,7 @@ target_bfd::xfer_partial (target_object object, } } -target_section_table * +const target_section_table * target_bfd::get_section_table () { return &m_table; diff --git a/gdb/exec.c b/gdb/exec.c index 68b35204068..1cac5fb5d3d 100644 --- a/gdb/exec.c +++ b/gdb/exec.c @@ -75,7 +75,7 @@ struct exec_target final : public target_ops const gdb_byte *writebuf, ULONGEST offset, ULONGEST len, ULONGEST *xfered_len) override; - target_section_table *get_section_table () override; + const target_section_table *get_section_table () override; void files_info () override; bool has_memory () override; @@ -775,7 +775,7 @@ enum target_xfer_status section_table_read_available_memory (gdb_byte *readbuf, ULONGEST offset, ULONGEST len, ULONGEST *xfered_len) { - target_section_table *table = target_get_section_table (&exec_ops); + const target_section_table *table = target_get_section_table (&exec_ops); std::vector<mem_range> available_memory = section_table_available_memory (offset, len, *table); @@ -884,7 +884,7 @@ section_table_xfer_memory_partial (gdb_byte *readbuf, const gdb_byte *writebuf, return TARGET_XFER_EOF; /* We can't help. */ } -target_section_table * +const target_section_table * exec_target::get_section_table () { return ¤t_program_space->target_sections; @@ -896,7 +896,7 @@ exec_target::xfer_partial (enum target_object object, const gdb_byte *writebuf, ULONGEST offset, ULONGEST len, ULONGEST *xfered_len) { - target_section_table *table = get_section_table (); + const target_section_table *table = target_get_section_table (this); if (object == TARGET_OBJECT_MEMORY) return section_table_xfer_memory_partial (readbuf, writebuf, @@ -908,7 +908,7 @@ exec_target::xfer_partial (enum target_object object, \f void -print_section_info (target_section_table *t, bfd *abfd) +print_section_info (const target_section_table *t, bfd *abfd) { struct gdbarch *gdbarch = gdbarch_from_bfd (abfd); /* FIXME: 16 is not wide enough when gdbarch_addr_bit > 64. */ diff --git a/gdb/exec.h b/gdb/exec.h index cccc0d0cfa7..1119953dc8f 100644 --- a/gdb/exec.h +++ b/gdb/exec.h @@ -96,7 +96,7 @@ extern void exec_set_section_address (const char *, int, CORE_ADDR); special cased --- it's filename is omitted; if it is the executable file, its entry point is printed. */ -extern void print_section_info (target_section_table *table, +extern void print_section_info (const target_section_table *table, bfd *abfd); /* Helper function that attempts to open the symbol file at EXEC_FILE_HOST. diff --git a/gdb/ppc64-tdep.c b/gdb/ppc64-tdep.c index f8f60994bae..74873a6999e 100644 --- a/gdb/ppc64-tdep.c +++ b/gdb/ppc64-tdep.c @@ -561,7 +561,7 @@ ppc64_convert_from_func_ptr_addr (struct gdbarch *gdbarch, struct target_ops *targ) { enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); - struct target_section *s = target_section_by_addr (targ, addr); + const struct target_section *s = target_section_by_addr (targ, addr); /* Check if ADDR points to a function descriptor. */ if (s && strcmp (s->the_bfd_section->name, ".opd") == 0) diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c index ac51ff2bf49..d9cc7a3b6d8 100644 --- a/gdb/record-btrace.c +++ b/gdb/record-btrace.c @@ -1439,7 +1439,7 @@ record_btrace_target::xfer_partial (enum target_object object, { case TARGET_OBJECT_MEMORY: { - struct target_section *section; + const struct target_section *section; /* We do not allow writing memory in general. */ if (writebuf != NULL) diff --git a/gdb/remote.c b/gdb/remote.c index 31c6e17a1c4..2c85bdcffbc 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -9044,7 +9044,7 @@ remote_target::remote_xfer_live_readonly_partial (gdb_byte *readbuf, int unit_size, ULONGEST *xfered_len) { - struct target_section *secp; + const struct target_section *secp; secp = target_section_by_addr (this, memaddr); if (secp != NULL @@ -9052,8 +9052,8 @@ remote_target::remote_xfer_live_readonly_partial (gdb_byte *readbuf, { ULONGEST memend = memaddr + len; - target_section_table *table = target_get_section_table (this); - for (target_section &p : *table) + const target_section_table *table = target_get_section_table (this); + for (const target_section &p : *table) { if (memaddr >= p.addr) { diff --git a/gdb/s390-tdep.c b/gdb/s390-tdep.c index 57ddd540609..39c8ee0450a 100644 --- a/gdb/s390-tdep.c +++ b/gdb/s390-tdep.c @@ -684,8 +684,8 @@ s390_load (struct s390_prologue_data *data, we're analyzing the code to unwind past that frame. */ if (pv_is_constant (addr)) { - struct target_section *secp; - secp = target_section_by_addr (current_top_target (), addr.k); + const struct target_section *secp + = target_section_by_addr (current_top_target (), addr.k); if (secp != NULL && (bfd_section_flags (secp->the_bfd_section) & SEC_READONLY)) return pv_constant (read_memory_integer (addr.k, size, diff --git a/gdb/solib-dsbt.c b/gdb/solib-dsbt.c index 71245b26ca2..6d218060343 100644 --- a/gdb/solib-dsbt.c +++ b/gdb/solib-dsbt.c @@ -424,7 +424,8 @@ scan_dyntag (int dyntag, bfd *abfd, CORE_ADDR *ptr) return 0; bool found = false; - for (target_section &target_section : current_program_space->target_sections) + for (const target_section &target_section + : current_program_space->target_sections) if (sect == target_section.the_bfd_section) { dyn_addr = target_section.addr; diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c index 0416faa17c3..92253a25f18 100644 --- a/gdb/solib-svr4.c +++ b/gdb/solib-svr4.c @@ -610,7 +610,8 @@ scan_dyntag (const int desired_dyntag, bfd *abfd, CORE_ADDR *ptr, return 0; bool found = false; - for (target_section &target_section : current_program_space->target_sections) + for (const target_section &target_section + : current_program_space->target_sections) if (sect == target_section.the_bfd_section) { dyn_addr = target_section.addr; diff --git a/gdb/target-debug.h b/gdb/target-debug.h index 69103388652..1cc82562095 100644 --- a/gdb/target-debug.h +++ b/gdb/target-debug.h @@ -104,7 +104,7 @@ target_debug_do_print (host_address_to_string (X)) #define target_debug_print_struct_ui_file_p(X) \ target_debug_do_print (host_address_to_string (X)) -#define target_debug_print_target_section_table_p(X) \ +#define target_debug_print_const_target_section_table_p(X) \ target_debug_do_print (host_address_to_string (X)) #define target_debug_print_void_p(X) \ target_debug_do_print (host_address_to_string (X)) diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c index 437b19b8581..69fbc0f3b23 100644 --- a/gdb/target-delegates.c +++ b/gdb/target-delegates.c @@ -77,7 +77,7 @@ struct dummy_target : public target_ops void rcmd (const char *arg0, struct ui_file *arg1) override; char *pid_to_exec_file (int arg0) override; void log_command (const char *arg0) override; - target_section_table *get_section_table () override; + const target_section_table *get_section_table () override; thread_control_capabilities get_thread_control_capabilities () override; bool attach_no_wait () override; bool can_async_p () override; @@ -248,7 +248,7 @@ struct debug_target : public target_ops void rcmd (const char *arg0, struct ui_file *arg1) override; char *pid_to_exec_file (int arg0) override; void log_command (const char *arg0) override; - target_section_table *get_section_table () override; + const target_section_table *get_section_table () override; thread_control_capabilities get_thread_control_capabilities () override; bool attach_no_wait () override; bool can_async_p () override; @@ -2021,27 +2021,27 @@ debug_target::log_command (const char *arg0) fputs_unfiltered (")\n", gdb_stdlog); } -target_section_table * +const target_section_table * target_ops::get_section_table () { return this->beneath ()->get_section_table (); } -target_section_table * +const target_section_table * dummy_target::get_section_table () { return NULL; } -target_section_table * +const target_section_table * debug_target::get_section_table () { - target_section_table * result; + const target_section_table * result; fprintf_unfiltered (gdb_stdlog, "-> %s->get_section_table (...)\n", this->beneath ()->shortname ()); result = this->beneath ()->get_section_table (); fprintf_unfiltered (gdb_stdlog, "<- %s->get_section_table (", this->beneath ()->shortname ()); fputs_unfiltered (") = ", gdb_stdlog); - target_debug_print_target_section_table_p (result); + target_debug_print_const_target_section_table_p (result); fputs_unfiltered ("\n", gdb_stdlog); return result; } diff --git a/gdb/target.c b/gdb/target.c index 06d7b4fbf8e..78535b89e58 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -812,7 +812,7 @@ target_read_string (CORE_ADDR memaddr, int len, int *bytes_read) return gdb::unique_xmalloc_ptr<char> ((char *) buffer.release ()); } -target_section_table * +const target_section_table * target_get_section_table (struct target_ops *target) { return target->get_section_table (); @@ -820,15 +820,15 @@ target_get_section_table (struct target_ops *target) /* Find a section containing ADDR. */ -struct target_section * +const struct target_section * target_section_by_addr (struct target_ops *target, CORE_ADDR addr) { - target_section_table *table = target_get_section_table (target); + const target_section_table *table = target_get_section_table (target); if (table == NULL) return NULL; - for (target_section &secp : *table) + for (const target_section &secp : *table) { if (addr >= secp.addr && addr < secp.endaddr) return &secp; @@ -965,7 +965,7 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object, if (pc_in_unmapped_range (memaddr, section)) { - target_section_table *table = target_get_section_table (ops); + const target_section_table *table = target_get_section_table (ops); const char *section_name = section->the_bfd_section->name; memaddr = overlay_mapped_address (memaddr, section); @@ -984,13 +984,12 @@ memory_xfer_partial_1 (struct target_ops *ops, enum target_object object, /* Try the executable files, if "trust-readonly-sections" is set. */ if (readbuf != NULL && trust_readonly) { - struct target_section *secp; - - secp = target_section_by_addr (ops, memaddr); + const struct target_section *secp + = target_section_by_addr (ops, memaddr); if (secp != NULL && (bfd_section_flags (secp->the_bfd_section) & SEC_READONLY)) { - target_section_table *table = target_get_section_table (ops); + const target_section_table *table = target_get_section_table (ops); return section_table_xfer_memory_partial (readbuf, writebuf, memaddr, len, xfered_len, *table); diff --git a/gdb/target.h b/gdb/target.h index 0de78075e9b..c11a8c91aa8 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -687,7 +687,7 @@ struct target_ops TARGET_DEFAULT_RETURN (NULL); virtual void log_command (const char *) TARGET_DEFAULT_IGNORE (); - virtual target_section_table *get_section_table () + virtual const target_section_table *get_section_table () TARGET_DEFAULT_RETURN (NULL); /* Provide default values for all "must have" methods. */ @@ -2413,13 +2413,13 @@ extern CORE_ADDR target_translate_tls_address (struct objfile *objfile, CORE_ADDR offset); /* Return the "section" containing the specified address. */ -struct target_section *target_section_by_addr (struct target_ops *target, - CORE_ADDR addr); +const struct target_section *target_section_by_addr (struct target_ops *target, + CORE_ADDR addr); /* Return the target section table this target (or the targets beneath) currently manipulate. */ -extern target_section_table *target_get_section_table +extern const target_section_table *target_get_section_table (struct target_ops *target); /* From mem-break.c */ -- 2.25.4 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCHv2 3/6] gdb/testsuite: enable gdb.base/sect-cmd.exp test for all targets 2021-02-19 20:07 ` [PATCHv2 0/6] " Andrew Burgess 2021-02-19 20:07 ` [PATCHv2 1/6] gdb: add a new 'maint info target-sections' command Andrew Burgess 2021-02-19 20:07 ` [PATCHv2 2/6] gdb: spread a little 'const' through the target_section_table code Andrew Burgess @ 2021-02-19 20:07 ` Andrew Burgess 2021-02-19 20:07 ` [PATCHv2 4/6] gdb: make the target_sections table private within program_space Andrew Burgess ` (3 subsequent siblings) 6 siblings, 0 replies; 19+ messages in thread From: Andrew Burgess @ 2021-02-19 20:07 UTC (permalink / raw) To: gdb-patches During review of the next patch (which changes the 'section' command), a bug was pointed out. I wondered why no tests spotted this bug and I found that the 'section' command test (sect-cmd.exp) is only run on hppa targets! In this commit I have given this test script a bit of a spring clean, bringing it up to date with current testsuite style. I have made some of the patterns a little more robust, but in general my intention was not to change the underlying meaning of any of these tests. gdb/testsuite/ChangeLog: * gdb.base/sect-cmd.exp: Rewrite using modern testsuite techniques. Enable the test for all targets. --- gdb/testsuite/ChangeLog | 5 + gdb/testsuite/gdb.base/sect-cmd.exp | 136 +++++++++++----------------- 2 files changed, 56 insertions(+), 85 deletions(-) diff --git a/gdb/testsuite/gdb.base/sect-cmd.exp b/gdb/testsuite/gdb.base/sect-cmd.exp index 4dbbc09b39c..e42f46d38ca 100644 --- a/gdb/testsuite/gdb.base/sect-cmd.exp +++ b/gdb/testsuite/gdb.base/sect-cmd.exp @@ -13,99 +13,70 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see <http://www.gnu.org/licenses/>. */ -# -# test running programs -# +# Test the 'section NAME ADDRESS' command. -# This test exists solely to exercise the "section" command for -# code-coverage on HP-UX. (So far as I can tell, the "section" -# command isn't needed on HP-UX, but probably is for embedded -# apps.) -# -if ![istarget "hppa*-*-hpux*"] then { - return +if { [prepare_for_testing "failed to prepare" "sect-cmd" \ + {break.c break1.c} {debug nowarnings}] } { + return -1 } - -set testfile "sect-cmd" set srcfile break.c set srcfile1 break1.c -set binfile ${objdir}/${subdir}/${testfile} -if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}0.o" object {debug nowarnings}] != "" } { - untested "failed to compile" - return -1 +if ![runto_main] then { + fail "couldn't run to main" + return } -if { [gdb_compile "${srcdir}/${subdir}/${srcfile1}" "${binfile}1.o" object {debug nowarnings}] != "" } { - untested "failed to compile" - return -1 -} +# Get the address of an executable section. This test was originally +# written for (and only run on) hppa targets. For PA32 programs using +# the SOM file format the code section is (apparently) called $CODE$, +# hence why the patterns here include that as a choice. +# +set address1 "" +set address2 "" +set section_name "" -if { [gdb_compile "${binfile}0.o ${binfile}1.o" "${binfile}" executable {debug nowarnings}] != "" } { - untested "failed to compile" - return -1 +gdb_test_multiple "info files" "" { + -re -wrap "\\s+($hex) - ($hex) is (\\\$CODE\\\$|\\.text\\S*) in .*" { + set address1 $expect_out(1,string) + set address2 $expect_out(2,string) + set section_name $expect_out(3,string) + pass $gdb_test_name + } } -gdb_exit -gdb_start -gdb_reinitialize_dir $srcdir/$subdir -gdb_load ${binfile} - -if ![runto_main] then { fail "section command tests suppressed" } - -# Get the $CODE$ section's starting address. +# If we don't have the details we need then we can't continue. # -# (Note that this works for PA32 programs, which use the SOM file -# format. PA64 uses ELF, and when support for that is added, it's -# not clear that there'll be a section named "$CODE$" in such -# programs.) -# - -set address1 "" -set address2 "" -send_gdb "info files\n" -gdb_expect { - -re ".*(0x\[0-9a-fA-F\]*) - (0x\[0-9a-fA-F\]*) is .(CODE|text).*$gdb_prompt $"\ - {pass "info files" - set address1 $expect_out(1,string) - set address2 $expect_out(2,string)} - -re "$gdb_prompt $"\ - {fail "info files"} - timeout {fail "(timeout) info files"} +if { $address1 == "" || $address2 == "" || $section_name == "" } { + unresolved "failed to find required section details" + return } # Reset the section to that same starting address, which should be # harmless (i.e., we just want to exercise the section command). # -if [istarget "hppa2.0w-*-*"] then { - send_gdb "section \.text $address1\n" - gdb_expect { - -re ".*$address1 \- $address2 is .text.*$gdb_prompt $"\ - {pass "set section command"} - -re "$gdb_prompt $"\ - {fail "set section command"} - timeout {fail "(timeout) set section command"} - } -} else { - send_gdb "section \$CODE\$ $address1\n" - gdb_expect { - -re ".*$address1 \- $address2 is .CODE..*$gdb_prompt $"\ - {pass "set section command"} - -re "$gdb_prompt $"\ - {fail "set section command"} - timeout {fail "(timeout) set section command"} - } -} +set saw_section_address_line false +gdb_test_multiple "section $section_name $address1" \ + "set section $section_name to original address" { + -re ".*$address1 \- $address2 is $section_name.*" { + set saw_section_address_line true + exp_continue + } + -re "Section \[^\r\n\]+ not found\r\n" { + fail "$gdb_test_name, saw not found marker" + exp_continue + } + -re "$gdb_prompt $" { + gdb_assert { $saw_section_address_line } $gdb_test_name + } + } # Verify that GDB responds gracefully to a non-existent section name. # -send_gdb "section FOOBARBAZ 0x1234\n" -gdb_expect { - -re "Section FOOBARBAZ not found\r\n$gdb_prompt $"\ - {pass "non-existent section disallowed"} - -re "$gdb_prompt $"\ - {fail "non-existent section disallowed"} - timeout {fail "(timeout) non-existent section disallowed"} +gdb_test_multiple "section FOOBARBAZ 0x1234" "" { + -re -wrap "Section FOOBARBAZ not found" { + pass $gdb_test_name + } } # We "happen to know" that GDB uses a fixed size character buffer to @@ -113,14 +84,9 @@ gdb_expect { # characters in length. Verify that GDB gracefully handles section # names longer than that. (The section is also non-existent.) # -send_gdb "section A234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123 0x1234\n" -gdb_expect { - -re "Section A23456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789 not found\r\n$gdb_prompt $"\ - {pass "non-existent too-long section disallowed"} - -re "$gdb_prompt $"\ - {fail "non-existent too-long section disallowed"} - timeout {fail "(timeout) non-existent too-long section disallowed"} -} - -gdb_exit -return 0 +gdb_test_multiple "section A234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123 0x1234" \ + "non-existent too-long section disallowed" { + -re -wrap "Section A23456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789 not found" { + pass $gdb_test_name + } + } -- 2.25.4 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCHv2 4/6] gdb: make the target_sections table private within program_space 2021-02-19 20:07 ` [PATCHv2 0/6] " Andrew Burgess ` (2 preceding siblings ...) 2021-02-19 20:07 ` [PATCHv2 3/6] gdb/testsuite: enable gdb.base/sect-cmd.exp test for all targets Andrew Burgess @ 2021-02-19 20:07 ` Andrew Burgess 2021-02-19 20:07 ` [PATCHv2 5/6] gdb: move get_section_table from exec_target to dummy_target Andrew Burgess ` (2 subsequent siblings) 6 siblings, 0 replies; 19+ messages in thread From: Andrew Burgess @ 2021-02-19 20:07 UTC (permalink / raw) To: gdb-patches Following on from earlier commits which made access to the target_sections table more 'const', this commit makes the table private within the program_space class and provides member functions to access the table. Ideally I would have liked for the new target_sections member function (on program_space) to return a 'const' reference to the table within the program_space. Unfortunately, there are two places in solib-*.c, where code outside of the program_space class modifies the target_sections table, and so to support this we need to return a non-const reference. There should be no user visible changes after this commit. gdb/ChangeLog: * exec.c (exec_target::close): Call new clear_target_sections function. (program_space::add_target_sections): Update name of member variable. (program_space::foreach_target_section): New function. (program_space::add_target_sections): Update name of member variable. (program_space::remove_target_sections): Likewise. (exec_one_fork): Use new target_sections member function. (exec_target::get_section_table): Likewise. (exec_target::files_info): Likewise. (set_section_command): Use new foreach_target_section member function. (exec_set_section_address): Likewise. (exec_target::has_memory): Use new target_sections member function. * progspace.h (program_space::clear_target_sections): New member function. (program_space::target_sections): Rename member variable to m_target_sections, replace with a new member function. (program_space::foreach_target_section): Declare new member function. (program_space::m_target_sections): New member variable. * solib-dsbt.c (scan_dyntag): Use new member function. * solib-svr4.c (scan_dyntag): Likewise. --- gdb/ChangeLog | 24 ++++++++++++++++++++++++ gdb/exec.c | 32 ++++++++++++++++---------------- gdb/progspace.h | 21 +++++++++++++++++---- gdb/solib-dsbt.c | 2 +- gdb/solib-svr4.c | 2 +- 5 files changed, 59 insertions(+), 22 deletions(-) diff --git a/gdb/exec.c b/gdb/exec.c index 1cac5fb5d3d..c55a41aa8a1 100644 --- a/gdb/exec.c +++ b/gdb/exec.c @@ -156,7 +156,7 @@ exec_target::close () { for (struct program_space *ss : program_spaces) { - ss->target_sections.clear (); + ss->clear_target_sections (); ss->exec_close (); } } @@ -599,8 +599,8 @@ program_space::add_target_sections (void *owner, { for (const target_section &s : sections) { - target_sections.push_back (s); - target_sections.back ().owner = owner; + m_target_sections.push_back (s); + m_target_sections.back ().owner = owner; } scoped_restore_current_pspace_and_thread restore_pspace_thread; @@ -637,9 +637,9 @@ program_space::add_target_sections (struct objfile *objfile) if (bfd_section_size (osect->the_bfd_section) == 0) continue; - target_sections.emplace_back (obj_section_addr (osect), - obj_section_endaddr (osect), - osect->the_bfd_section, (void *) objfile); + m_target_sections.emplace_back (obj_section_addr (osect), + obj_section_endaddr (osect), + osect->the_bfd_section, (void *) objfile); } } @@ -651,18 +651,18 @@ program_space::remove_target_sections (void *owner) { gdb_assert (owner != NULL); - auto it = std::remove_if (target_sections.begin (), - target_sections.end (), + auto it = std::remove_if (m_target_sections.begin (), + m_target_sections.end (), [&] (target_section §) { return sect.owner == owner; }); - target_sections.erase (it, target_sections.end ()); + m_target_sections.erase (it, m_target_sections.end ()); /* If we don't have any more sections to read memory from, remove the file_stratum target from the stack of each inferior sharing the program space. */ - if (target_sections.empty ()) + if (m_target_sections.empty ()) { scoped_restore_current_pspace_and_thread restore_pspace_thread; @@ -682,7 +682,7 @@ program_space::remove_target_sections (void *owner) void exec_on_vfork () { - if (!current_program_space->target_sections.empty ()) + if (!current_program_space->target_sections ().empty ()) push_target (&exec_ops); } @@ -887,7 +887,7 @@ section_table_xfer_memory_partial (gdb_byte *readbuf, const gdb_byte *writebuf, const target_section_table * exec_target::get_section_table () { - return ¤t_program_space->target_sections; + return ¤t_program_space->target_sections (); } enum target_xfer_status @@ -985,7 +985,7 @@ void exec_target::files_info () { if (current_program_space->exec_bfd ()) - print_section_info (¤t_program_space->target_sections, + print_section_info (¤t_program_space->target_sections (), current_program_space->exec_bfd ()); else puts_filtered (_("\t<no file loaded>\n")); @@ -1010,7 +1010,7 @@ set_section_command (const char *args, int from_tty) /* Parse out new virtual address. */ secaddr = parse_and_eval_address (args); - for (target_section &p : current_program_space->target_sections) + for (target_section &p : current_program_space->target_sections ()) { if (!strncmp (secname, bfd_section_name (p.the_bfd_section), seclen) && bfd_section_name (p.the_bfd_section)[seclen] == '\0') @@ -1036,7 +1036,7 @@ set_section_command (const char *args, int from_tty) void exec_set_section_address (const char *filename, int index, CORE_ADDR address) { - for (target_section &p : current_program_space->target_sections) + for (target_section &p : current_program_space->target_sections ()) { if (filename_cmp (filename, bfd_get_filename (p.the_bfd_section->owner)) == 0 @@ -1053,7 +1053,7 @@ exec_target::has_memory () { /* We can provide memory if we have any file/target sections to read from. */ - return !current_program_space->target_sections.empty (); + return !current_program_space->target_sections ().empty (); } gdb::unique_xmalloc_ptr<char> diff --git a/gdb/progspace.h b/gdb/progspace.h index 6ac8932cd60..790684743d8 100644 --- a/gdb/progspace.h +++ b/gdb/progspace.h @@ -309,6 +309,18 @@ struct program_space sections. They are given OBJFILE as the "owner". */ void add_target_sections (struct objfile *objfile); + /* Clear all target sections from M_TARGET_SECTIONS table. */ + void clear_target_sections () + { + m_target_sections.clear (); + } + + /* Return a reference to the M_TARGET_SECTIONS table. */ + target_section_table &target_sections () + { + return m_target_sections; + } + /* Unique ID number. */ int num = 0; @@ -359,10 +371,6 @@ struct program_space /* All known objfiles are kept in a linked list. */ std::list<std::shared_ptr<objfile>> objfiles_list; - /* The set of target sections matching the sections mapped into - this program space. Managed by both exec_ops and solib.c. */ - target_section_table target_sections; - /* List of shared objects mapped into this space. Managed by solib.c. */ struct so_list *so_list = NULL; @@ -380,6 +388,11 @@ struct program_space /* Per pspace data-pointers required by other GDB modules. */ REGISTRY_FIELDS {}; + +private: + /* The set of target sections matching the sections mapped into + this program space. Managed by both exec_ops and solib.c. */ + target_section_table m_target_sections; }; /* An address space. It is used for comparing if diff --git a/gdb/solib-dsbt.c b/gdb/solib-dsbt.c index 6d218060343..4b1b7560e16 100644 --- a/gdb/solib-dsbt.c +++ b/gdb/solib-dsbt.c @@ -425,7 +425,7 @@ scan_dyntag (int dyntag, bfd *abfd, CORE_ADDR *ptr) bool found = false; for (const target_section &target_section - : current_program_space->target_sections) + : current_program_space->target_sections ()) if (sect == target_section.the_bfd_section) { dyn_addr = target_section.addr; diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c index 92253a25f18..6c0fe819a2f 100644 --- a/gdb/solib-svr4.c +++ b/gdb/solib-svr4.c @@ -611,7 +611,7 @@ scan_dyntag (const int desired_dyntag, bfd *abfd, CORE_ADDR *ptr, bool found = false; for (const target_section &target_section - : current_program_space->target_sections) + : current_program_space->target_sections ()) if (sect == target_section.the_bfd_section) { dyn_addr = target_section.addr; -- 2.25.4 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCHv2 5/6] gdb: move get_section_table from exec_target to dummy_target 2021-02-19 20:07 ` [PATCHv2 0/6] " Andrew Burgess ` (3 preceding siblings ...) 2021-02-19 20:07 ` [PATCHv2 4/6] gdb: make the target_sections table private within program_space Andrew Burgess @ 2021-02-19 20:07 ` Andrew Burgess 2021-02-19 20:07 ` [PATCHv2 6/6] gdb: use std::string instead of a fixed size buffer Andrew Burgess 2021-02-23 18:01 ` [PATCHv2 0/6] Allow access to target_sections even without an executable Tom Tromey 6 siblings, 0 replies; 19+ messages in thread From: Andrew Burgess @ 2021-02-19 20:07 UTC (permalink / raw) To: gdb-patches The only target that implements target_ops::get_section_table in a meaningful way is exec_target. This target calls back into the program space to return the current global section_table. The global section table is populated whenever the user provides GDB with an executable, or when a symbol file is loaded, e.g. when a dynamic library is loaded, or when the user does add-symbol-file. I recently ran into a situation where a user, debugging a remote target, was not supplying GDB with a main executable at all. Instead the user attached to the target then did add-symbol-file, and then proceeded to debug the target. This works fine, but it was noticed that even when trust-readonly-sections was on GDB was still accessing the target to get the contents of readonly sections. The problem is that by not providing an executable there was no exec_target in the target stack, and so when GDB calls the target_ops::get_section_table function GDB ends up in dummy_target::get_section_table, which just returns NULL. What I want is that even when GDB doesn't have an exec_target in the target stack, a call to target_ops::get_section_table will still return the section_table from the current program space. When considering how to achieve this my first though was, why is the request for the section table going via the target stack at all? The set of sections loaded is a property of the program space, not the target. This is, after all, why the data is being stored in the program space. So I initially tried changing target_get_section_table so that, instead of calling into the target it just returns current_program_space->target_sections (). This would be fine except for one issue, target_bfd (from bfd-target.c). This code is used from solib-svr4.c to create a temporary target_ops structure that implements two functions target_bfd::xfer_partial and target_bfd::get_section_table. The purpose behind the code is to enable two targets, ppc64 and frv to decode function descriptors from the dynamic linker, based on the non-relocated addresses from within the dynamic linker bfd object. Both of the implemented functions in target_bfd rely on the target_bfd object holding a section table, and the ppc64 target requires that the target_bfd implement ::get_section_table. The frv target doesn't require ::get_section_table, instead it requires the ::xfer_partial. We could in theory change the ppc64 target to use the same approach as frv, however, this would be a bad idea. I believe that the frv target approach is broken. I'll explain: The frv target calls get_target_memory_unsigned to read the function descriptor. The address being read is the non-relocated address read from the dynamic linker in solib-srv4.c:enable_break. Calling get_target_memory_unsigned eventually ends up in target_xfer_partial with an object type of TARGET_OBJECT_RAW_MEMORY. This will then call memory_xfer_check_region. I believe that it is quite possible that a the non-relocated addresses pulled from the dynamic linker could be in a memory region that is not readable, while the relocated addresses are in a readable memory region. If this was ever the case for the frv target then GDB would reject the attempt to read the non-relocated function pointer. In contrast the ppc64 target calls target_section_by_addr, which calls target_get_section_table, which then calls the ::get_section_table function on the target. Thus, when reflecting on target_bfd we see two functions, ::xfer_partial and ::get_section_table. The former is required by the frv target, but that target is (I think) potentially broken. While the latter is required by the ppc64 target, but this forces ::get_section_table to exist as a target_ops member function. So my original plan, have target_get_section_table NOT call a target_ops member function appears to be flawed. My next idea was to remove exec_target::get_section_table, and instead move the implementation into dummy_target::get_section_table. Currently the dummy_target implementation always returns NULL indicating no section table, but plenty of other dummy_target member functions do more than just return null values. So now, dummy_target::get_section_table returns the section table from the current program space. This allows target_bfd to remain unchanged, so ppc64 and frv should not be affected. Making this change removes the requirement for the user to provide an executable, GDB can now always access the section_table, as the dummy_target always exists in the target stack. Finally, there's a test that the target_section table is not empty in the case where the user does add-symbol-file without providing an executable. gdb/ChangeLog: * exec.c (exec_target::get_section_table): Delete member function. (section_table_read_available_memory): Use current_top_target, not just the exec_ops target. * target-delegates.c: Regenerate. * target.c (default_get_section_table): New function. * target.h (target_ops::get_section_table): Change default behaviour to call default_get_section_table. (default_get_section_table): Declare. --- gdb/ChangeLog | 11 +++++++++++ gdb/exec.c | 10 ++-------- gdb/target-delegates.c | 2 +- gdb/target.c | 7 +++++++ gdb/target.h | 6 +++++- gdb/testsuite/gdb.base/maint-info-sections.exp | 4 +--- 6 files changed, 27 insertions(+), 13 deletions(-) diff --git a/gdb/exec.c b/gdb/exec.c index c55a41aa8a1..8e3c19ec272 100644 --- a/gdb/exec.c +++ b/gdb/exec.c @@ -75,7 +75,6 @@ struct exec_target final : public target_ops const gdb_byte *writebuf, ULONGEST offset, ULONGEST len, ULONGEST *xfered_len) override; - const target_section_table *get_section_table () override; void files_info () override; bool has_memory () override; @@ -775,7 +774,8 @@ enum target_xfer_status section_table_read_available_memory (gdb_byte *readbuf, ULONGEST offset, ULONGEST len, ULONGEST *xfered_len) { - const target_section_table *table = target_get_section_table (&exec_ops); + const target_section_table *table + = target_get_section_table (current_top_target ()); std::vector<mem_range> available_memory = section_table_available_memory (offset, len, *table); @@ -884,12 +884,6 @@ section_table_xfer_memory_partial (gdb_byte *readbuf, const gdb_byte *writebuf, return TARGET_XFER_EOF; /* We can't help. */ } -const target_section_table * -exec_target::get_section_table () -{ - return ¤t_program_space->target_sections (); -} - enum target_xfer_status exec_target::xfer_partial (enum target_object object, const char *annex, gdb_byte *readbuf, diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c index 69fbc0f3b23..1c7999724c7 100644 --- a/gdb/target-delegates.c +++ b/gdb/target-delegates.c @@ -2030,7 +2030,7 @@ target_ops::get_section_table () const target_section_table * dummy_target::get_section_table () { - return NULL; + return default_get_section_table (); } const target_section_table * diff --git a/gdb/target.c b/gdb/target.c index 78535b89e58..ba445e7fd34 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -836,6 +836,13 @@ target_section_by_addr (struct target_ops *target, CORE_ADDR addr) return NULL; } +/* See target.h. */ + +const target_section_table * +default_get_section_table () +{ + return ¤t_program_space->target_sections (); +} /* Helper for the memory xfer routines. Checks the attributes of the memory region of MEMADDR against the read or write being attempted. diff --git a/gdb/target.h b/gdb/target.h index c11a8c91aa8..45b92f7d83f 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -688,7 +688,7 @@ struct target_ops virtual void log_command (const char *) TARGET_DEFAULT_IGNORE (); virtual const target_section_table *get_section_table () - TARGET_DEFAULT_RETURN (NULL); + TARGET_DEFAULT_RETURN (default_get_section_table ()); /* Provide default values for all "must have" methods. */ virtual bool has_all_memory () { return false; } @@ -2422,6 +2422,10 @@ const struct target_section *target_section_by_addr (struct target_ops *target, extern const target_section_table *target_get_section_table (struct target_ops *target); +/* Default implementation of get_section_table for dummy_target. */ + +extern const target_section_table *default_get_section_table (); + /* From mem-break.c */ extern int memory_remove_breakpoint (struct target_ops *, diff --git a/gdb/testsuite/gdb.base/maint-info-sections.exp b/gdb/testsuite/gdb.base/maint-info-sections.exp index 17e38ebc416..06508de366d 100644 --- a/gdb/testsuite/gdb.base/maint-info-sections.exp +++ b/gdb/testsuite/gdb.base/maint-info-sections.exp @@ -223,9 +223,7 @@ gdb_test_multiple "maint info sections -all-objects" "" { } } -# NOTE: We would like to check 'maint info target-sections' again -# here, but GDB currently doesn't display the target sections table in -# this case. This is a bug and will be fixed shortly!! +check_maint_info_target_sections_output "with loaded symbol file" # Test the command line completion on 'maint info sections'. First # the command line flag. -- 2.25.4 ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCHv2 6/6] gdb: use std::string instead of a fixed size buffer 2021-02-19 20:07 ` [PATCHv2 0/6] " Andrew Burgess ` (4 preceding siblings ...) 2021-02-19 20:07 ` [PATCHv2 5/6] gdb: move get_section_table from exec_target to dummy_target Andrew Burgess @ 2021-02-19 20:07 ` Andrew Burgess 2021-02-23 18:00 ` Tom Tromey 2021-02-23 18:01 ` [PATCHv2 0/6] Allow access to target_sections even without an executable Tom Tromey 6 siblings, 1 reply; 19+ messages in thread From: Andrew Burgess @ 2021-02-19 20:07 UTC (permalink / raw) To: gdb-patches The 'section' command uses a fixed size buffer into which a section name is copied. This commit replaces this with a use of std::string so we can now display very long section names. The expected results of one test need to be updated. gdb/ChangeLog: * exec.c (set_section_command): Move variable declarations into the function body, and use std::string instead of a fixed size buffer. gdb/testsuite/ChangeLog: * gdb.base/sect-cmd.exp: Update expected results. --- gdb/ChangeLog | 6 ++++++ gdb/exec.c | 18 ++++++------------ gdb/testsuite/ChangeLog | 4 ++++ gdb/testsuite/gdb.base/sect-cmd.exp | 14 ++++++++------ 4 files changed, 24 insertions(+), 18 deletions(-) diff --git a/gdb/exec.c b/gdb/exec.c index 8e3c19ec272..544a05873f1 100644 --- a/gdb/exec.c +++ b/gdb/exec.c @@ -989,27 +989,23 @@ static void set_section_command (const char *args, int from_tty) { const char *secname; - unsigned seclen; - unsigned long secaddr; - char secprint[100]; - long offset; if (args == 0) error (_("Must specify section name and its virtual address")); /* Parse out section name. */ for (secname = args; !isspace (*args); args++); - seclen = args - secname; + unsigned seclen = args - secname; /* Parse out new virtual address. */ - secaddr = parse_and_eval_address (args); + CORE_ADDR secaddr = parse_and_eval_address (args); for (target_section &p : current_program_space->target_sections ()) { if (!strncmp (secname, bfd_section_name (p.the_bfd_section), seclen) && bfd_section_name (p.the_bfd_section)[seclen] == '\0') { - offset = secaddr - p.addr; + long offset = secaddr - p.addr; p.addr += offset; p.endaddr += offset; if (from_tty) @@ -1017,11 +1013,9 @@ set_section_command (const char *args, int from_tty) return; } } - if (seclen >= sizeof (secprint)) - seclen = sizeof (secprint) - 1; - strncpy (secprint, secname, seclen); - secprint[seclen] = '\0'; - error (_("Section %s not found"), secprint); + + std::string secprint (secname, seclen); + error (_("Section %s not found"), secprint.c_str ()); } /* If we can find a section in FILENAME with BFD index INDEX, adjust diff --git a/gdb/testsuite/gdb.base/sect-cmd.exp b/gdb/testsuite/gdb.base/sect-cmd.exp index e42f46d38ca..7aa24ca615f 100644 --- a/gdb/testsuite/gdb.base/sect-cmd.exp +++ b/gdb/testsuite/gdb.base/sect-cmd.exp @@ -79,14 +79,16 @@ gdb_test_multiple "section FOOBARBAZ 0x1234" "" { } } -# We "happen to know" that GDB uses a fixed size character buffer to -# parse the section name into, and the buffer is declared to be 100 -# characters in length. Verify that GDB gracefully handles section -# names longer than that. (The section is also non-existent.) +# Check that GDB can still print the error message when the section +# name is very long. It used to be the case that GDB could only print +# (up to) 100 character section names in this error message, but that +# is no longer the case. # -gdb_test_multiple "section A234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123 0x1234" \ +set long_sect_name \ + "A234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123" +gdb_test_multiple "section $long_sect_name 0x1234" \ "non-existent too-long section disallowed" { - -re -wrap "Section A23456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789 not found" { + -re -wrap "Section $long_sect_name not found" { pass $gdb_test_name } } -- 2.25.4 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCHv2 6/6] gdb: use std::string instead of a fixed size buffer 2021-02-19 20:07 ` [PATCHv2 6/6] gdb: use std::string instead of a fixed size buffer Andrew Burgess @ 2021-02-23 18:00 ` Tom Tromey 0 siblings, 0 replies; 19+ messages in thread From: Tom Tromey @ 2021-02-23 18:00 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes: Andrew> The 'section' command uses a fixed size buffer into which a section Andrew> name is copied. This commit replaces this with a use of std::string Andrew> so we can now display very long section names. Andrew> The expected results of one test need to be updated. Andrew> gdb/ChangeLog: Andrew> * exec.c (set_section_command): Move variable declarations into Andrew> the function body, and use std::string instead of a fixed size Andrew> buffer. Andrew> gdb/testsuite/ChangeLog: Andrew> * gdb.base/sect-cmd.exp: Update expected results. Looks reasonable to me. Tom ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCHv2 0/6] Allow access to target_sections even without an executable 2021-02-19 20:07 ` [PATCHv2 0/6] " Andrew Burgess ` (5 preceding siblings ...) 2021-02-19 20:07 ` [PATCHv2 6/6] gdb: use std::string instead of a fixed size buffer Andrew Burgess @ 2021-02-23 18:01 ` Tom Tromey 6 siblings, 0 replies; 19+ messages in thread From: Tom Tromey @ 2021-02-23 18:01 UTC (permalink / raw) To: Andrew Burgess; +Cc: gdb-patches >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes: Andrew> Changes since v2: Thanks, I looked at the new ones & this looks ok to me. Tom ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2021-02-23 18:01 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-02-16 17:50 [PATCH 0/4] Allow access to target_sections even without an executable Andrew Burgess 2021-02-16 17:50 ` [PATCH 1/4] gdb: add a new 'maint info target-sections' command Andrew Burgess 2021-02-16 17:55 ` Eli Zaretskii 2021-02-17 16:19 ` Christian Biesinger 2021-02-18 16:25 ` Andrew Burgess 2021-02-16 17:50 ` [PATCH 2/4] gdb: spread a little 'const' through the target_section_table code Andrew Burgess 2021-02-16 17:50 ` [PATCH 3/4] gdb: make the target_sections table private within program_space Andrew Burgess 2021-02-19 14:24 ` Tom Tromey 2021-02-16 17:50 ` [PATCH 4/4] gdb: move get_section_table from exec_target to dummy_target Andrew Burgess 2021-02-19 14:46 ` [PATCH 0/4] Allow access to target_sections even without an executable Tom Tromey 2021-02-19 20:07 ` [PATCHv2 0/6] " Andrew Burgess 2021-02-19 20:07 ` [PATCHv2 1/6] gdb: add a new 'maint info target-sections' command Andrew Burgess 2021-02-19 20:07 ` [PATCHv2 2/6] gdb: spread a little 'const' through the target_section_table code Andrew Burgess 2021-02-19 20:07 ` [PATCHv2 3/6] gdb/testsuite: enable gdb.base/sect-cmd.exp test for all targets Andrew Burgess 2021-02-19 20:07 ` [PATCHv2 4/6] gdb: make the target_sections table private within program_space Andrew Burgess 2021-02-19 20:07 ` [PATCHv2 5/6] gdb: move get_section_table from exec_target to dummy_target Andrew Burgess 2021-02-19 20:07 ` [PATCHv2 6/6] gdb: use std::string instead of a fixed size buffer Andrew Burgess 2021-02-23 18:00 ` Tom Tromey 2021-02-23 18:01 ` [PATCHv2 0/6] Allow access to target_sections even without an executable Tom Tromey
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).