gdb: move get_section_table from exec_target to dummy_target

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.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 841d51bc3ca42c8448f9e37bff15f2fe35b2d144..f50d89f25c1e663012bf9b22bced29b5c2fbf401 100644 (file)
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@
+2021-02-24  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+       * 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.
+
 2021-02-24  Andrew Burgess  <andrew.burgess@embecosm.com>
 
        * exec.c (exec_target::close): Call new clear_target_sections

diff --git a/gdb/exec.c b/gdb/exec.c
index c55a41aa8a19d44519bf0a92dadebcb89e4327a1..8e3c19ec272295a034620fbe5dd93f428b0271f8 100644 (file)
--- 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 &current_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 69fbc0f3b23c6bd08ac7e7ef1634e61def34f73e..1c7999724c7c2111e1803884d2acc20823f47e96 100644 (file)
--- 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 78535b89e581c7dc6eb3945f39ae922bab7b4e11..ba445e7fd34c9fe1aad86bc8bdda55eee4060ec3 100644 (file)
--- 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 &current_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 66d46e2facbc68acb83a88bec95a657e74fa1a43..ee93c5cf3959a1d8deadde821c38edb07a970e74 100644 (file)
--- 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; }
@@ -2436,6 +2436,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 17e38ebc41690e1409d4d08867a697fadbf6758d..06508de366d504e8aa0001430cb6b8d008c074b8 100644 (file)
--- 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.