[PATCH 0/2] More detailed filenames for in-memory BFD objects

[PATCH 0/2] More detailed filenames for in-memory BFD objects

[Andrew Burgess]

We currently generate a filename for every in-memory BFD object that
GDB created (as part of the JIT interface).  The generated name is
"<in-memory>" for every symfile, which can make it hard to tell
multiple such files apart.

This series changes the generated name to "<in-memory@ADDRESS>", wher
ADDRESS is, obviously, the actual address of the symfile in memory.

I believe that this filename is only ever exposed in the output of
maintenance commands, so I haven't mentioned this change in the NEWS
file.  Maybe I should though?

---

Andrew Burgess (2):
  gdb: remove filename arg from gdb_bfd_open_from_target_memory
  gdb: include the base address in in-memory bfd filenames

 gdb/gdb_bfd.c                           |  59 ++++++---
 gdb/gdb_bfd.h                           |   7 +-
 gdb/testsuite/gdb.base/jit-bfd-name.exp | 155 ++++++++++++++++++++++++
 gdb/testsuite/lib/gdb.exp               |  22 ++++
 gdb/testsuite/lib/jit-elf-helpers.exp   |  12 +-
 5 files changed, 234 insertions(+), 21 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/jit-bfd-name.exp

-- 
2.25.4

[PATCH 1/2] gdb: remove filename arg from gdb_bfd_open_from_target_memory

[Andrew Burgess]

The filename argument to gdb_bfd_open_from_target_memory was never
used; this argument had a default value of nullptr, and the only call
to this function, in jit.c, relied on the default value.

In the next commit I'm going to make some changes to the
gdb_bfd_open_from_target_memory function, and, though I could take
account of a filename parameter, it seems pointless to maintain an
unused argument.

This commit removes the filename argument.

There should be no user visible changes after this commit.
---
 gdb/gdb_bfd.c | 5 ++---
 gdb/gdb_bfd.h | 7 ++++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 7e743891aed..36ef5e1cc5a 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -288,14 +288,13 @@ mem_bfd_iovec_stat (struct bfd *abfd, void *stream, struct stat *sb)
 
 gdb_bfd_ref_ptr
 gdb_bfd_open_from_target_memory (CORE_ADDR addr, ULONGEST size,
-				 const char *target,
-				 const char *filename)
+				 const char *target)
 {
   struct target_buffer *buffer = XNEW (struct target_buffer);
 
   buffer->base = addr;
   buffer->size = size;
-  return gdb_bfd_openr_iovec (filename ? filename : "<in-memory>", target,
+  return gdb_bfd_openr_iovec ("<in-memory>", target,
 			      mem_bfd_iovec_open,
 			      buffer,
 			      mem_bfd_iovec_pread,
diff --git a/gdb/gdb_bfd.h b/gdb/gdb_bfd.h
index 86f7be85f20..6ea16ddc735 100644
--- a/gdb/gdb_bfd.h
+++ b/gdb/gdb_bfd.h
@@ -201,11 +201,12 @@ int gdb_bfd_requires_relocations (bfd *abfd);
 bool gdb_bfd_get_full_section_contents (bfd *abfd, asection *section,
 					gdb::byte_vector *contents);
 
-/* Create and initialize a BFD handle from a target in-memory range.  */
+/* Create and initialize a BFD handle from a target in-memory range.  The
+   BFD starts at ADDR and is SIZE bytes long.  TARGET is the BFD target
+   name as used in bfd_find_target.  */
 
 gdb_bfd_ref_ptr gdb_bfd_open_from_target_memory (CORE_ADDR addr, ULONGEST size,
-						 const char *target,
-						 const char *filename = nullptr);
+						 const char *target);
 
 /* Range adapter for a BFD's sections.
 
-- 
2.25.4

[PATCH 2/2] gdb: include the base address in in-memory bfd filenames

[Andrew Burgess]

The struct target_buffer (in gdb_bfd.c) is used to hold information
about an in-memory BFD object created by GDB.  For now this mechanism
is used by GDB when loading information about JIT symfiles.

This commit updates target_buffer (in gdb_bfd.c) to be more C++ like,
and, at the same time, adds the base address of the symfile into the
BFD filename.

Right now, every in-memory BFD is given the filename "<in-memory>".
This filename is visible in things like 'maint info symtabs' and
'maint info line-table'.  If there are multiple in-memory BFD objects
then it can be hard to match keep track if which BFD is which.  This
commit changes the name to be "<in-memory@ADDRESS>" where ADDRESS is
replaced with the base address for where the in-memory symbol file was
read from.

As an example of how this is useful, here's the output of 'maint info
jit' showing a single loaded JIT symfile:

  (gdb) maintenance info jit
  jit_code_entry address symfile address    symfile size
  0x00000000004056b0     0x0000000007000000 17320

And here's part of the output from 'maint info symtabs':

  (gdb) maintenance info symtabs
  ...snip...
  { objfile <in-memory@0x7000000> ((struct objfile *) 0x5258250)
    { ((struct compunit_symtab *) 0x4f0afb0)
      debugformat DWARF 4
      producer GNU C17 9.3.1 20200408 (Red Hat 9.3.1-2) -mtune=generic -march=x86-64 -g -fno-stack-protector -fpic
      name jit-elf-solib.c
      dirname /tmp/binutils-gdb/build/gdb/testsuite
      blockvector ((struct blockvector *) 0x5477850)
      user ((struct compunit_symtab *) (null))
  	{ symtab /tmp/binutils-gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/jit-elf-solib.c ((struct symtab *) 0x4f0b030)
  	  fullname (null)
  	  linetable ((struct linetable *) 0x5477880)
  	}
    }
  }

I've added a new test that checks the new in-memory file names are
generated correctly, and also checks that the in-memory JIT files can
be dumped back out using 'dump binary memory'.
---
 gdb/gdb_bfd.c                           |  56 +++++++--
 gdb/testsuite/gdb.base/jit-bfd-name.exp | 155 ++++++++++++++++++++++++
 gdb/testsuite/lib/gdb.exp               |  22 ++++
 gdb/testsuite/lib/jit-elf-helpers.exp   |  12 +-
 4 files changed, 229 insertions(+), 16 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/jit-bfd-name.exp

diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 36ef5e1cc5a..d2e18c74168 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -217,12 +217,43 @@ gdb_bfd_has_target_filename (struct bfd *abfd)
   return is_target_filename (bfd_get_filename (abfd));
 }
 
-/* For `gdb_bfd_open_from_target_memory`.  */
+/* For `gdb_bfd_open_from_target_memory`.  An object that manages the
+   details of a BFD in target memory.  */
 
 struct target_buffer
 {
-  CORE_ADDR base;
-  ULONGEST size;
+  /* Constructor.  BASE and SIZE define where the BFD can be found in
+     target memory.  */
+  target_buffer (CORE_ADDR base, ULONGEST size)
+    : m_base (base),
+      m_size (size)
+  {
+    m_filename
+      = xstrprintf ("<in-memory@%s>", core_addr_to_string_nz (m_base));
+  }
+
+  /* Return the size of the in-memory BFD file.  */
+  ULONGEST size () const
+  { return m_size; }
+
+  /* Return the base address of the in-memory BFD file.  */
+  CORE_ADDR base () const
+  { return m_base; }
+
+  /* Return a generated filename for the in-memory BFD file.  The generated
+     name will include the M_BASE value.  */
+  const char *filename () const
+  { return m_filename.get (); }
+
+private:
+  /* The base address of the in-memory BFD file.  */
+  CORE_ADDR m_base;
+
+  /* The size (in-bytes) of the in-memory BFD file.  */
+  ULONGEST m_size;
+
+  /* Holds the generated name of the in-memory BFD file.  */
+  gdb::unique_xmalloc_ptr<char> m_filename;
 };
 
 /* For `gdb_bfd_open_from_target_memory`.  Opening the file is a no-op.  */
@@ -239,7 +270,8 @@ mem_bfd_iovec_open (struct bfd *abfd, void *open_closure)
 static int
 mem_bfd_iovec_close (struct bfd *abfd, void *stream)
 {
-  xfree (stream);
+  struct target_buffer *buffer = (target_buffer *) stream;
+  delete buffer;
 
   /* Zero means success.  */
   return 0;
@@ -253,18 +285,18 @@ static file_ptr
 mem_bfd_iovec_pread (struct bfd *abfd, void *stream, void *buf,
 		     file_ptr nbytes, file_ptr offset)
 {
-  int err;
   struct target_buffer *buffer = (struct target_buffer *) stream;
 
   /* If this read will read all of the file, limit it to just the rest.  */
-  if (offset + nbytes > buffer->size)
-    nbytes = buffer->size - offset;
+  if (offset + nbytes > buffer->size ())
+    nbytes = buffer->size () - offset;
 
   /* If there are no more bytes left, we've reached EOF.  */
   if (nbytes == 0)
     return 0;
 
-  err = target_read_memory (buffer->base + offset, (gdb_byte *) buf, nbytes);
+  int err
+    = target_read_memory (buffer->base () + offset, (gdb_byte *) buf, nbytes);
   if (err)
     return -1;
 
@@ -280,7 +312,7 @@ mem_bfd_iovec_stat (struct bfd *abfd, void *stream, struct stat *sb)
   struct target_buffer *buffer = (struct target_buffer*) stream;
 
   memset (sb, 0, sizeof (struct stat));
-  sb->st_size = buffer->size;
+  sb->st_size = buffer->size ();
   return 0;
 }
 
@@ -290,11 +322,9 @@ gdb_bfd_ref_ptr
 gdb_bfd_open_from_target_memory (CORE_ADDR addr, ULONGEST size,
 				 const char *target)
 {
-  struct target_buffer *buffer = XNEW (struct target_buffer);
+  struct target_buffer *buffer = new target_buffer (addr, size);
 
-  buffer->base = addr;
-  buffer->size = size;
-  return gdb_bfd_openr_iovec ("<in-memory>", target,
+  return gdb_bfd_openr_iovec (buffer->filename (), target,
 			      mem_bfd_iovec_open,
 			      buffer,
 			      mem_bfd_iovec_pread,
diff --git a/gdb/testsuite/gdb.base/jit-bfd-name.exp b/gdb/testsuite/gdb.base/jit-bfd-name.exp
new file mode 100644
index 00000000000..7c8ad50d072
--- /dev/null
+++ b/gdb/testsuite/gdb.base/jit-bfd-name.exp
@@ -0,0 +1,155 @@
+# Copyright 2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check the BFD filename (as used in the symfile name) that is
+# automatically generated for in-memory BFD files, as used by the JIT
+# system within GDB.
+#
+# Additionally, check that GDB cau use 'dump binary memory' to write
+# out the in-memory JIT files.
+
+if {[skip_shlib_tests]} {
+    untested "skipping shared library tests"
+    return -1
+}
+
+load_lib jit-elf-helpers.exp
+
+# The main code that loads and registers JIT objects.
+set main_basename "jit-elf-main"
+set main_srcfile ${srcdir}/${subdir}/${main_basename}.c
+set main_binfile [standard_output_file ${main_basename}]
+
+# The shared library that gets loaded as JIT objects.
+set jit_solib_basename jit-elf-solib
+set jit_solib_srcfile ${srcdir}/${subdir}/${jit_solib_basename}.c
+
+# Compile two shared libraries to use as JIT objects.
+set jit_solibs_target [compile_and_download_n_jit_so \
+			   $jit_solib_basename $jit_solib_srcfile 2 \
+			   {debug}]
+if { $jit_solibs_target == -1 } {
+    return
+}
+
+# Compile the main code (which loads the JIT objects).
+if { [compile_jit_main ${main_srcfile} ${main_binfile} {}] != 0 } {
+    return
+}
+
+clean_restart $::main_binfile
+if { ![runto_main] } {
+    return
+}
+
+# Poke desired values directly into inferior instead of using "set
+# args" because "set args" does not work under gdbserver.
+set count [expr [llength $jit_solibs_target] + 1]
+gdb_test_no_output "set var argc=$count" "forging argc"
+gdb_test_no_output "set var argv=fake_argv" "forging argv"
+for {set i 1} {$i < $count} {incr i} {
+    set jit_solib_target [lindex $jit_solibs_target [expr $i-1]]
+    gdb_test_no_output "set var argv\[$i\]=\"${jit_solib_target}\"" \
+	"forging argv\[$i\]"
+}
+
+# Run until the JIT libraries are loaded.
+gdb_breakpoint [gdb_get_line_number "break here 1" $::main_srcfile]
+gdb_continue_to_breakpoint "break here 1"
+
+# Confirm that the two expected functions are available.
+gdb_test "info function ^jit_function" \
+    [multi_line \
+	 "File \[^\r\n\]+jit-elf-solib.c:" \
+	 "${decimal}:\\s+int jit_function_0001\\(\\);" \
+	 "${decimal}:\\s+int jit_function_0002\\(\\);"]
+
+# Capture the addresses of each JIT symfile.
+set symfile_addrs {}
+set symfile_lengths {}
+gdb_test_multiple "maint info jit" "" {
+    -re "^maint info jit\r\n" {
+	exp_continue
+    }
+    -re "^jit_code_entry address\\s+symfile address\\s+symfile size\\s*\r\n" {
+	exp_continue
+    }
+    -re "^${hex}\\s+(${hex})\\s+(${decimal})\\s*\r\n" {
+	lappend symfile_addrs $expect_out(1,string)
+	lappend symfile_lengths $expect_out(2,string)
+	exp_continue
+    }
+    -re "^$gdb_prompt $" {
+    }
+}
+
+# Now check the 'maint info symtabs' output to ensure that each
+# symfile is mentioned, and that the names are as expected.
+set bfd_name_addrs {}
+gdb_test_multiple "maint info symtabs" "" {
+    -re "^maint info symtabs\r\n" {
+	exp_continue
+    }
+    -re "^\\\}\\s*\r\n" {
+	exp_continue
+    }
+    -re "^\\\{ objfile <in-memory@($hex)>\\s+\[^\r\n\]+\r\n" {
+	lappend bfd_name_addrs $expect_out(1,string)
+	exp_continue
+    }
+    -re "^\\\{ objfile (\\S+)\\s+\[^\r\n\]+\r\n" {
+	exp_continue
+    }
+    -re "^\\s+\[^\r\n\]+\r\n" {
+	exp_continue
+    }
+    -re "^$gdb_prompt $" {
+    }
+}
+
+# Now dump each JIT solib using the 'dump binary memory' command.
+set count 0
+foreach addr $symfile_addrs len $symfile_lengths {
+    incr count
+    set output [standard_output_file "dump-elf-solib.${count}.so"]
+    set end [expr $addr + $len]
+    gdb_test_no_output "dump binary memory $output $addr $end" \
+	"dump jit solib $count"
+
+    gdb_assert { [cmp_binary_files $output [standard_output_file "jit-elf-solib.${count}.so"]] == 0} \
+	"check dump of jit solib $count is as expected"
+}
+
+
+# Check that each of the expected jit symfile addresses was mentioned
+# in an in-memory BFD filename.
+set count 1
+foreach addr $symfile_addrs {
+    # Drop any loading zeros from the symfile address.
+    set addr [format 0x%x $addr]
+
+    # Check there was a BFD with the expected address in its name.
+    gdb_assert { [expr [lsearch -exact $bfd_name_addrs $addr] != -1] } \
+	"check for in-memory bfd $count"
+    incr count
+}
+
+# Continue until the JIT libraries are unloaded.
+gdb_breakpoint [gdb_get_line_number "break here 2" $::main_srcfile]
+gdb_continue_to_breakpoint "break here 2"
+
+# All jit librares must have been unregistered.
+gdb_test "info function jit_function" \
+    "All functions matching regular expression \"jit_function\":"
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 432ed5e34ca..3e6daa2f60f 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -8152,6 +8152,28 @@ proc cmp_file_string { file str msg } {
     }
 }
 
+# Compare FILE1 and FILE2 as binary files.  Return 0 if the files are
+# equal, otherwise, return non-zero.
+
+proc cmp_binary_files { file1 file2 } {
+    set fd1 [open $file1]
+    fconfigure $fd1 -translation binary
+    set fd2 [open $file2]
+    fconfigure $fd2 -translation binary
+
+    set blk_size 1024
+    while {true} {
+	set blk1 [read $fd1 $blk_size]
+	set blk2 [read $fd2 $blk_size]
+	set diff [string compare $blk1 $blk2]
+	if {$diff != 0 || [eof $fd1] || [eof $fd2]} {
+	    close $fd1
+	    close $fd2
+	    return $diff
+	}
+    }
+}
+
 # Does the compiler support CTF debug output using '-gctf' compiler
 # flag?  If not then we should skip these tests.  We should also
 # skip them if libctf was explicitly disabled.
diff --git a/gdb/testsuite/lib/jit-elf-helpers.exp b/gdb/testsuite/lib/jit-elf-helpers.exp
index b699917f209..80ba769434c 100644
--- a/gdb/testsuite/lib/jit-elf-helpers.exp
+++ b/gdb/testsuite/lib/jit-elf-helpers.exp
@@ -74,9 +74,13 @@ proc compile_jit_elf_main_as_so {main_solib_srcfile main_solib_binfile options}
 # Compile jit-elf-solib.c as a shared library in multiple copies and
 # upload them to the target.
 #
+# OPTIONS_LIST is a list of additional options to pass through to
+# gdb_compile_shlib.
+#
 # On success, return a list of target path to the shared libraries.
 # On failure, return -1.
-proc compile_and_download_n_jit_so {jit_solib_basename jit_solib_srcfile count} {
+proc compile_and_download_n_jit_so {jit_solib_basename jit_solib_srcfile \
+					count {options_list {}}} {
     global jit_load_address jit_load_increment
     set binfiles_target {}
 
@@ -93,9 +97,11 @@ proc compile_and_download_n_jit_so {jit_solib_basename jit_solib_srcfile count}
 	# compiled shared library against a fixed base address.  Combined
 	# with mapping the resulting binary to the same fixed base it allows
 	# to dynamically execute functions from it without any further adjustments.
+	set fname [format "jit_function_%04d" $i]
 	set options [list \
-	    additional_flags=-DFUNCTION_NAME=[format "jit_function_%04d" $i] \
-	    text_segment=$addr]
+			 ${options_list} \
+			 additional_flags=-DFUNCTION_NAME=$fname \
+			 text_segment=$addr]
 	if { [gdb_compile_shlib ${jit_solib_srcfile} ${binfile} \
 		  $options] != "" } {
 	    set f [file tail $binfile]
-- 
2.25.4

Re: [PATCH 0/2] More detailed filenames for in-memory BFD objects

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

Andrew> We currently generate a filename for every in-memory BFD object that
Andrew> GDB created (as part of the JIT interface).  The generated name is
Andrew> "<in-memory>" for every symfile, which can make it hard to tell
Andrew> multiple such files apart.

Andrew> This series changes the generated name to "<in-memory@ADDRESS>", wher
Andrew> ADDRESS is, obviously, the actual address of the symfile in memory.

These both seem fine to me.

Andrew> I believe that this filename is only ever exposed in the output of
Andrew> maintenance commands, so I haven't mentioned this change in the NEWS
Andrew> file.  Maybe I should though?

I wouldn't bother.

Tom