gdb: include the end address in in-memory bfd filenames

Message ID 20231016125928.3268165-1-markus.t.metzger@intel.com
State New
Headers
Series gdb: include the end address in in-memory bfd filenames |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed

Commit Message

Metzger, Markus T Oct. 16, 2023, 12:59 p.m. UTC
  Commit

    66984afd29e gdb: include the base address in in-memory bfd filenames

added the base address to in-memory bfd filenames.  Also add the end
address to allow dumping the in-memory bfd using the 'dump memory'
command.
---
 gdb/gdb_bfd.c                           | 7 ++++---
 gdb/testsuite/gdb.base/jit-bfd-name.exp | 2 +-
 2 files changed, 5 insertions(+), 4 deletions(-)
  

Comments

Andrew Burgess Oct. 16, 2023, 2:54 p.m. UTC | #1
Markus Metzger <markus.t.metzger@intel.com> writes:

> Commit
>
>     66984afd29e gdb: include the base address in in-memory bfd filenames
>
> added the base address to in-memory bfd filenames.  Also add the end
> address to allow dumping the in-memory bfd using the 'dump memory'
> command.
> ---
>  gdb/gdb_bfd.c                           | 7 ++++---
>  gdb/testsuite/gdb.base/jit-bfd-name.exp | 2 +-
>  2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
> index 217753cf914..56a4c5ecc91 100644
> --- a/gdb/gdb_bfd.c
> +++ b/gdb/gdb_bfd.c
> @@ -227,8 +227,9 @@ struct target_buffer : public gdb_bfd_iovec_base
>    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)))
> +      m_filename (xstrprintf ("<in-memory@%s-%s>",
> +			      core_addr_to_string_nz (m_base),
> +			      core_addr_to_string_nz (m_base + m_size)))
>    {
>    }
>  
> @@ -241,7 +242,7 @@ struct target_buffer : public gdb_bfd_iovec_base
>    { return m_base; }
>  
>    /* Return a generated filename for the in-memory BFD file.  The generated
> -     name will include the M_BASE value.  */
> +     name will include the begin and end address of the in-memory file.  */
>    const char *filename () const
>    { return m_filename.get (); }
>  
> diff --git a/gdb/testsuite/gdb.base/jit-bfd-name.exp b/gdb/testsuite/gdb.base/jit-bfd-name.exp
> index 80f06269524..f97f42f66f2 100644
> --- a/gdb/testsuite/gdb.base/jit-bfd-name.exp
> +++ b/gdb/testsuite/gdb.base/jit-bfd-name.exp
> @@ -102,7 +102,7 @@ gdb_test_multiple "maint info symtabs" "" {
>      -re "^\\\}\\s*\r\n" {
>  	exp_continue
>      }
> -    -re "^\\\{ objfile <in-memory@($hex)>\\s+\[^\r\n\]+\r\n" {
> +    -re "^\\\{ objfile <in-memory@($hex)-$hex>\\s+\[^\r\n\]+\r\n" {
>  	lappend bfd_name_addrs $expect_out(1,string)
>  	exp_continue
>      }

The diff below can be applied on top of your patch above, and extends
the testing to check the end address.  I think this, or something like
this, should be included in this commit.

Thoughts?

Thanks,
Andrew

---

diff --git a/gdb/testsuite/gdb.base/jit-bfd-name.exp b/gdb/testsuite/gdb.base/jit-bfd-name.exp
index f97f42f66f2..f4886a698ef 100644
--- a/gdb/testsuite/gdb.base/jit-bfd-name.exp
+++ b/gdb/testsuite/gdb.base/jit-bfd-name.exp
@@ -102,7 +102,7 @@ gdb_test_multiple "maint info symtabs" "" {
     -re "^\\\}\\s*\r\n" {
 	exp_continue
     }
-    -re "^\\\{ objfile <in-memory@($hex)-$hex>\\s+\[^\r\n\]+\r\n" {
+    -re "^\\\{ objfile <in-memory@($hex-$hex)>\\s+\[^\r\n\]+\r\n" {
 	lappend bfd_name_addrs $expect_out(1,string)
 	exp_continue
     }
@@ -133,12 +133,19 @@ foreach addr $symfile_addrs len $symfile_lengths {
 # Check that each of the expected jit symfile addresses was mentioned
 # in an in-memory BFD filename.
 set count 1
-foreach addr $symfile_addrs {
+foreach addr $symfile_addrs len $symfile_lengths {
     # Drop any loading zeros from the symfile address.
-    set addr [format 0x%x $addr]
+    set start [format 0x%x $addr]
+
+    # Calculate the end address.
+    set end [format 0x%x [expr $addr + $len]]
+
+    # This is what we expect the address range to look like in the BFD
+    # filename.
+    set rng "$start-$end"
 
     # Check there was a BFD with the expected address in its name.
-    gdb_assert { [expr [lsearch -exact $bfd_name_addrs $addr] != -1] } \
+    gdb_assert { [expr [lsearch -exact $bfd_name_addrs $rng] != -1] } \
 	"check for in-memory bfd $count"
     incr count
 }
  
Tom Tromey Oct. 16, 2023, 4:32 p.m. UTC | #2
>>>>> "Markus" == Markus Metzger <markus.t.metzger@intel.com> writes:

Markus> Commit
Markus>     66984afd29e gdb: include the base address in in-memory bfd filenames

Markus> added the base address to in-memory bfd filenames.  Also add the end
Markus> address to allow dumping the in-memory bfd using the 'dump memory'
Markus> command.

This seems fine to me.
Approved-By: Tom Tromey <tom@tromey.com>

Tom
  
Tom de Vries Oct. 18, 2023, 6 a.m. UTC | #3
On 10/16/23 16:54, Andrew Burgess wrote:
> - set addr [format 0x%x $addr]
> + set start [format 0x%x $addr]

It seems this bit didn't make it into the commit, so the test-case is 
failing:
...
ERROR: tcl error sourcing 
/data/vries/gdb/src/gdb/testsuite/gdb.base/jit-bfd-name.exp.
ERROR: can't read "start": no such variable
     while executing
"set rng "$start-$end""
...

This fixes it:
...
diff --git a/gdb/testsuite/gdb.base/jit-bfd-name.exp 
b/gdb/testsuite/gdb.base/jit-bfd-name.exp
index f6a6e765338..f4886a698ef 100644
--- a/gdb/testsuite/gdb.base/jit-bfd-name.exp
+++ b/gdb/testsuite/gdb.base/jit-bfd-name.exp
@@ -135,7 +135,7 @@ foreach addr $symfile_addrs len $symfile_lengths {
  set count 1
  foreach addr $symfile_addrs len $symfile_lengths {
      # Drop any loading zeros from the symfile address.
-    set addr [format 0x%x $addr]
+    set start [format 0x%x $addr]

      # Calculate the end address.
      set end [format 0x%x [expr $addr + $len]]
...

I'll push this.

Thanks,
- Tom
  
Metzger, Markus T Oct. 18, 2023, 6:47 a.m. UTC | #4
Thanks, Tom,

I did run the test before sending the v2 patch but my scripts only checked for

                === gdb Summary ===

# of expected passes            10

so I didn't notice the ERROR.

Sorry,
markus.

>-----Original Message-----
>From: Tom de Vries <tdevries@suse.de>
>Sent: Wednesday, October 18, 2023 8:00 AM
>To: Andrew Burgess <aburgess@redhat.com>; Metzger, Markus T
><markus.t.metzger@intel.com>; gdb-patches@sourceware.org
>Subject: Re: [PATCH] gdb: include the end address in in-memory bfd filenames
>
>On 10/16/23 16:54, Andrew Burgess wrote:
>> - set addr [format 0x%x $addr]
>> + set start [format 0x%x $addr]
>
>It seems this bit didn't make it into the commit, so the test-case is
>failing:
>...
>ERROR: tcl error sourcing
>/data/vries/gdb/src/gdb/testsuite/gdb.base/jit-bfd-name.exp.
>ERROR: can't read "start": no such variable
>     while executing
>"set rng "$start-$end""
>...
>
>This fixes it:
>...
>diff --git a/gdb/testsuite/gdb.base/jit-bfd-name.exp
>b/gdb/testsuite/gdb.base/jit-bfd-name.exp
>index f6a6e765338..f4886a698ef 100644
>--- a/gdb/testsuite/gdb.base/jit-bfd-name.exp
>+++ b/gdb/testsuite/gdb.base/jit-bfd-name.exp
>@@ -135,7 +135,7 @@ foreach addr $symfile_addrs len $symfile_lengths {
>  set count 1
>  foreach addr $symfile_addrs len $symfile_lengths {
>      # Drop any loading zeros from the symfile address.
>-    set addr [format 0x%x $addr]
>+    set start [format 0x%x $addr]
>
>      # Calculate the end address.
>      set end [format 0x%x [expr $addr + $len]]
>...
>
>I'll push this.
>
>Thanks,
>- Tom

Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
  

Patch

diff --git a/gdb/gdb_bfd.c b/gdb/gdb_bfd.c
index 217753cf914..56a4c5ecc91 100644
--- a/gdb/gdb_bfd.c
+++ b/gdb/gdb_bfd.c
@@ -227,8 +227,9 @@  struct target_buffer : public gdb_bfd_iovec_base
   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)))
+      m_filename (xstrprintf ("<in-memory@%s-%s>",
+			      core_addr_to_string_nz (m_base),
+			      core_addr_to_string_nz (m_base + m_size)))
   {
   }
 
@@ -241,7 +242,7 @@  struct target_buffer : public gdb_bfd_iovec_base
   { return m_base; }
 
   /* Return a generated filename for the in-memory BFD file.  The generated
-     name will include the M_BASE value.  */
+     name will include the begin and end address of the in-memory file.  */
   const char *filename () const
   { return m_filename.get (); }
 
diff --git a/gdb/testsuite/gdb.base/jit-bfd-name.exp b/gdb/testsuite/gdb.base/jit-bfd-name.exp
index 80f06269524..f97f42f66f2 100644
--- a/gdb/testsuite/gdb.base/jit-bfd-name.exp
+++ b/gdb/testsuite/gdb.base/jit-bfd-name.exp
@@ -102,7 +102,7 @@  gdb_test_multiple "maint info symtabs" "" {
     -re "^\\\}\\s*\r\n" {
 	exp_continue
     }
-    -re "^\\\{ objfile <in-memory@($hex)>\\s+\[^\r\n\]+\r\n" {
+    -re "^\\\{ objfile <in-memory@($hex)-$hex>\\s+\[^\r\n\]+\r\n" {
 	lappend bfd_name_addrs $expect_out(1,string)
 	exp_continue
     }