[PATCH 0/2] Get gdb.base/overlays.exp passing on m32r

[PATCH 0/2] Get gdb.base/overlays.exp passing on m32r

[Andrew Burgess]

I have an interest in overlay debugging.

A good place to start would seem to be getting the existing overlay
test passing.

That's what this commit does.

Feedback welcome.

Thanks,
Andrew


---

Andrew Burgess (2):
  gdb/testsuite: allow gdb.base/overlays.exp to compile for m32r
  gdb: handle unmapped overlays in find_pc_line

 gdb/ChangeLog                       |  5 ++
 gdb/symtab.c                        | 15 ++++--
 gdb/testsuite/ChangeLog             |  8 +++
 gdb/testsuite/gdb.base/m32r.ld      | 84 ++++++++++++++++-------------
 gdb/testsuite/gdb.base/overlays.exp |  2 +-
 gdb/testsuite/gdb.base/ovlymgr.c    |  2 +
 6 files changed, 75 insertions(+), 41 deletions(-)

-- 
2.25.4

[PATCH 1/2] gdb/testsuite: allow gdb.base/overlays.exp to compile for m32r

[Andrew Burgess]

The gdb.base/overlays.exp test is only currently supported on m32r
baremetal targets, however, when I configure a toolchain for m32r-elf
the test does not compile.

This commit updates the linker script, fixes some TCL errors in the
exp file, and adds some missing includes to the source file so that
the test does compile.

With this test, when run against an m32r-elf toolchain the test mostly
passes, but there are a couple of failures, these are GDB issues and
will be addressed in a later commit.

gdb/testsuite/ChangeLog:

	* gdb.base/m32r.ld: Remove SEARCH_DIR line.  Add MEMORY regions,
	make use of regions throughout.
	* gdb.base/overlays.exp: Enclose string with variableds in "..",
	not {...}.
	* gdb.base/ovlymgr.c: Add 'string.h' and 'stdlib.h' includes.
---
 gdb/testsuite/ChangeLog             |  8 +++
 gdb/testsuite/gdb.base/m32r.ld      | 84 ++++++++++++++++-------------
 gdb/testsuite/gdb.base/overlays.exp |  2 +-
 gdb/testsuite/gdb.base/ovlymgr.c    |  2 +
 4 files changed, 58 insertions(+), 38 deletions(-)

diff --git a/gdb/testsuite/gdb.base/m32r.ld b/gdb/testsuite/gdb.base/m32r.ld
index b96077adb6a..3358f4ce7fa 100644
--- a/gdb/testsuite/gdb.base/m32r.ld
+++ b/gdb/testsuite/gdb.base/m32r.ld
@@ -2,34 +2,43 @@ OUTPUT_FORMAT("elf32-m32r", "elf32-m32r",
 	      "elf32-m32r")
 OUTPUT_ARCH(m32r)
 ENTRY(_start)
- SEARCH_DIR(/usr/cygnus/m32r-961018/H-sparc-sun-sunos4.1//lib);
+
+MEMORY
+{
+  RAM : ORIGIN = 0x208000, LENGTH = 0x100000
+  OVLY_1 : ORIGIN = 0x300000, LENGTH = 0x40000
+  OVLY_2 : ORIGIN = 0x340000, LENGTH = 0x40000
+  OVLY_3 : ORIGIN = 0x380000, LENGTH = 0x40000
+  OVLY_4 : ORIGIN = 0x3c0000, LENGTH = 0x40000
+  OVLY_STORAGE : ORIGIN = 0x400000, LENGTH = 0x100000
+}
+
 /* Do we need any of these for elf?
    __DYNAMIC = 0;    */
 SECTIONS
 {
-     OVERLAY 0x300000 : AT (0x400000)
-        {
-          .ovly0 { foo.o(.text) }
-          .ovly1 { bar.o(.text) }
-        }
-     OVERLAY 0x380000 : AT (0x480000)
-        {
-          .ovly2 { baz.o(.text) }
-          .ovly3 { grbx.o(.text) }
-        }
-     OVERLAY 0x340000 : AT (0x440000)
-        {
-          .data00 { foo.o(.data) }
-          .data01 { bar.o(.data) }
-        }
-     OVERLAY 0x3C0000 : AT (0x4C0000)
-        {
-          .data02 { baz.o(.data) }
-          .data03 { grbx.o(.data) }
-        }
+   OVERLAY :
+      {
+        .ovly0 { */overlays2.o(.text) }
+        .ovly1 { */overlays3.o(.text) }
+      } >OVLY_1 AT>OVLY_STORAGE
+   OVERLAY :
+      {
+        .ovly2 { */overlays4.o(.text) }
+        .ovly3 { */overlays5.o(.text) }
+      } >OVLY_3 AT>OVLY_STORAGE
+   OVERLAY :
+      {
+        .data00 { */overlays2.o(.data) }
+        .data01 { */overlays3.o(.data) }
+      } >OVLY_2 AT>OVLY_STORAGE
+   OVERLAY :
+      {
+        .data02 { */overlays4.o(.data) }
+        .data03 { */overlays5.o(.data) }
+      } >OVLY_4 AT>OVLY_STORAGE
 
   /* Read-only sections, merged into text segment: */
-  . = 0x208000;
   .interp        : { *(.interp) 	}
   .hash          : { *(.hash)		}
   .dynsym        : { *(.dynsym)		}
@@ -54,20 +63,21 @@ SECTIONS
   .rela.bss      : { *(.rela.bss)	}
   .rel.plt       : { *(.rel.plt)	}
   .rela.plt      : { *(.rela.plt)	}
-  .init          : { *(.init)		} =0
-  .plt           : { *(.plt)		}
+  .init          : { *(.init)		} >RAM AT>RAM =0
+  .plt           : { *(.plt)		} >RAM AT>RAM
+
   .text          :
   {
     *(.text)
     /* .gnu.warning sections are handled specially by elf32.em.  */
     *(.gnu.warning)
     *(.gnu.linkonce.t*)
-  } =0
+  } >RAM AT>RAM =0
   _etext = .;
   PROVIDE (etext = .);
-  .fini          : { *(.fini)		} =0
-  .rodata        : { *(.rodata) *(.gnu.linkonce.r*) }
-  .rodata1       : { *(.rodata1)	}
+  .fini          : { *(.fini)		} >RAM AT>RAM =0
+  .rodata        : { *(.rodata) *(.gnu.linkonce.r*) } >RAM AT>RAM
+  .rodata1       : { *(.rodata1)	} >RAM AT>RAM
   /* Adjust the address for the data segment.  We want to adjust up to
      the same address within the page on the next page up.  */
   . = ALIGN(32) + (ALIGN(8) & (32 - 1));
@@ -120,21 +130,21 @@ SECTIONS
 	LONG((_novlys - _ovly_table) / 16);
 
     CONSTRUCTORS
-  }
-  .data1         : { *(.data1)		}
-  .ctors         : { *(.ctors)		}
-  .dtors         : { *(.dtors)		}
-  .got           : { *(.got.plt) *(.got)}
-  .dynamic       : { *(.dynamic)	}
+  } >RAM AT>RAM
+  .data1         : { *(.data1)		} >RAM AT>RAM
+  .ctors         : { *(.ctors)		} >RAM AT>RAM
+  .dtors         : { *(.dtors)		} >RAM AT>RAM
+  .got           : { *(.got.plt) *(.got)} >RAM AT>RAM
+  .dynamic       : { *(.dynamic)	} >RAM AT>RAM
   /* We want the small data sections together, so single-instruction offsets
      can access them all, and initialized data all before uninitialized, so
      we can shorten the on-disk segment size.  */
-  .sdata         : { *(.sdata)		}
+  .sdata         : { *(.sdata)		} >RAM AT>RAM
   _edata  =  .;
   PROVIDE (edata = .);
   __bss_start = .;
-  .sbss          : { *(.sbss) *(.scommon) }
-  .bss           : { *(.dynbss) *(.bss) *(COMMON) }
+  .sbss          : { *(.sbss) *(.scommon) } >RAM AT>RAM
+  .bss           : { *(.dynbss) *(.bss) *(COMMON) } >RAM AT>RAM
   _end = . ;
   PROVIDE (end = .);
   /* Stabs debugging sections.  */
diff --git a/gdb/testsuite/gdb.base/overlays.exp b/gdb/testsuite/gdb.base/overlays.exp
index 7ebb77695b3..248379e25eb 100644
--- a/gdb/testsuite/gdb.base/overlays.exp
+++ b/gdb/testsuite/gdb.base/overlays.exp
@@ -38,7 +38,7 @@ standard_testfile overlays.c ovlymgr.c foo.c bar.c baz.c grbx.c
 
 if {[build_executable $testfile.exp $testfile \
 	 [list $srcfile $srcfile2 $srcfile3 $srcfile4 $srcfile5 $srcfile6] \
-	 {debug ldscript=-Wl,-T$linker_script}] == -1} {
+	 "debug ldscript=-Wl,-T$linker_script"] == -1} {
      untested "failed to compile"
      return -1
 }
diff --git a/gdb/testsuite/gdb.base/ovlymgr.c b/gdb/testsuite/gdb.base/ovlymgr.c
index 5d087ad90a2..262c2a61f02 100644
--- a/gdb/testsuite/gdb.base/ovlymgr.c
+++ b/gdb/testsuite/gdb.base/ovlymgr.c
@@ -4,6 +4,8 @@
  */
 
 #include "ovlymgr.h"
+#include <string.h>
+#include <stdlib.h>
 
 /* Local functions and data: */
 
-- 
2.25.4

[PATCH 2/2] gdb: handle unmapped overlays in find_pc_line

[Andrew Burgess]

I configured and build an m32r-elf toolchain, and ran the
gdb.base/overlays.exp test.  I saw a couple of errors where GDB would
place a breakpoint in the wrong place when placing a breakpoint using
a function name, for example in this function:

/* 1 */  int foo (int x)
/* 2 */  {
/* 3 */    if (x)
/* 4 */      return foox;
/* 5 */    else
/* 6 */      return 0;
/* 7 */  }

GDB would place the breakpoint on line 2 instead of line 3.  The issue
is that GDB was failing to skip the prologue correctly.

The reason for this is that in m32r-tdep.c:m32r_skip_prologue, we
first use find_pc_partial_function to find the functions start and end
addresses, then we use find_pc_line to find the start and end of the
first line of the function.

Currently, if the pc value passed to find_pc_partial_function is in an
unmapped overlay then the function start and end addresses that are
returned are also the unmapped addresses.

However, this is not the case for find_pc_line, here, if the address
passed in is in an unmapped overlay then we still get back a
symtab_and_line describing the mapped location.

What this means is that if a functions mapped location is 0x100 ->
0x120, and its unmapped locations is 0x400 -> 0x420 then we think that
the start/end is 0x400 and 0x420 respectively, but the first line
might run from 0x100 to 0x108.

GDB will then try to scan the prologue starting from 0x400 and ending
at 0x108, this immediately gives up as it thinks we have gone past the
end of the prologue and the breakpoint is placed at 0x400.

In this commit I propose that we change find_pc_line to return
addresses in the unmapped range if the address passed in is already in
the unmapped range.  Now the first line will appear to run from 0x400
to 0x408 and the prologue scanner will correctly find the end of the
prologue.

With this commit gdb.base/overlays.exp now completely passes with an
m32r-elf toolchain.

gdb/ChangeLog:

	* symtab.c (find_pc_line): Return unmapped addresses when the
	requested address is also unmapped.
---
 gdb/ChangeLog |  5 +++++
 gdb/symtab.c  | 15 ++++++++++++---
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/gdb/symtab.c b/gdb/symtab.c
index 04891c4a89b..a4f8239a8a1 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -3323,9 +3323,18 @@ find_pc_line (CORE_ADDR pc, int notcurrent)
   struct obj_section *section;
 
   section = find_pc_overlay (pc);
-  if (pc_in_unmapped_range (pc, section))
-    pc = overlay_mapped_address (pc, section);
-  return find_pc_sect_line (pc, section, notcurrent);
+  if (!pc_in_unmapped_range (pc, section))
+    return find_pc_sect_line (pc, section, notcurrent);
+
+  /* If the original PC was an unmapped address then we translate this to a
+     mapped address in order to lookup the sal.  However, as the user
+     passed us an unmapped address it makes more sense to return a result
+     that has the pc and end fields translated to unmapped addresses.  */
+  pc = overlay_mapped_address (pc, section);
+  symtab_and_line sal = find_pc_sect_line (pc, section, notcurrent);
+  sal.pc = overlay_unmapped_address (sal.pc, section);
+  sal.end = overlay_unmapped_address (sal.end, section);
+  return sal;
 }
 
 /* See symtab.h.  */
-- 
2.25.4

Re: [PATCH 2/2] gdb: handle unmapped overlays in find_pc_line

[Simon Marchi]

On 2020-09-18 2:17 p.m., Andrew Burgess wrote:
> I configured and build an m32r-elf toolchain, and ran the

build -> built

> gdb.base/overlays.exp test.  I saw a couple of errors where GDB would
> place a breakpoint in the wrong place when placing a breakpoint using
> a function name, for example in this function:
>
> /* 1 */  int foo (int x)
> /* 2 */  {
> /* 3 */    if (x)
> /* 4 */      return foox;

"foox" or "x"?

> /* 5 */    else
> /* 6 */      return 0;
> /* 7 */  }
>
> GDB would place the breakpoint on line 2 instead of line 3.  The issue
> is that GDB was failing to skip the prologue correctly.
>
> The reason for this is that in m32r-tdep.c:m32r_skip_prologue, we
> first use find_pc_partial_function to find the functions start and end
> addresses, then we use find_pc_line to find the start and end of the
> first line of the function.
>
> Currently, if the pc value passed to find_pc_partial_function is in an
> unmapped overlay then the function start and end addresses that are
> returned are also the unmapped addresses.
>
> However, this is not the case for find_pc_line, here, if the address
> passed in is in an unmapped overlay then we still get back a
> symtab_and_line describing the mapped location.
>
> What this means is that if a functions mapped location is 0x100 ->

"functions" -> "function's"

> 0x120, and its unmapped locations is 0x400 -> 0x420 then we think that
> the start/end is 0x400 and 0x420 respectively, but the first line
> might run from 0x100 to 0x108.
>
> GDB will then try to scan the prologue starting from 0x400 and ending
> at 0x108, this immediately gives up as it thinks we have gone past the
> end of the prologue and the breakpoint is placed at 0x400.
>
> In this commit I propose that we change find_pc_line to return
> addresses in the unmapped range if the address passed in is already in
> the unmapped range.  Now the first line will appear to run from 0x400
> to 0x408 and the prologue scanner will correctly find the end of the
> prologue.
>
> With this commit gdb.base/overlays.exp now completely passes with an
> m32r-elf toolchain.

I'm not too familiar with overlays (apart from understanding the general
concept), so I'm not sure how they are usually handled.  Do two pieces
of code in the same overlay have different unmapped addresses, which are
used to uniquely identify them?

In any case, what you did makes sense to me.  It makes the two
functions, find_pc_partial_function and find_pc_line behave in a more
consistent manner.

Simon

Re: [PATCH 2/2] gdb: handle unmapped overlays in find_pc_line

[Andrew Burgess]

* Simon Marchi <simark@simark.ca> [2020-09-20 17:08:39 -0400]:

> On 2020-09-18 2:17 p.m., Andrew Burgess wrote:
> > I configured and build an m32r-elf toolchain, and ran the
> 
> build -> built
> 
> > gdb.base/overlays.exp test.  I saw a couple of errors where GDB would
> > place a breakpoint in the wrong place when placing a breakpoint using
> > a function name, for example in this function:
> >
> > /* 1 */  int foo (int x)
> > /* 2 */  {
> > /* 3 */    if (x)
> > /* 4 */      return foox;
> 
> "foox" or "x"?

No, just me randomly cutting some code from a testcase
(gdb.base/foo.c).  I'll rename 'foox' to 'some_random_global_variable'
to make things clearer.

> 
> > /* 5 */    else
> > /* 6 */      return 0;
> > /* 7 */  }
> >
> > GDB would place the breakpoint on line 2 instead of line 3.  The issue
> > is that GDB was failing to skip the prologue correctly.
> >
> > The reason for this is that in m32r-tdep.c:m32r_skip_prologue, we
> > first use find_pc_partial_function to find the functions start and end
> > addresses, then we use find_pc_line to find the start and end of the
> > first line of the function.
> >
> > Currently, if the pc value passed to find_pc_partial_function is in an
> > unmapped overlay then the function start and end addresses that are
> > returned are also the unmapped addresses.
> >
> > However, this is not the case for find_pc_line, here, if the address
> > passed in is in an unmapped overlay then we still get back a
> > symtab_and_line describing the mapped location.
> >
> > What this means is that if a functions mapped location is 0x100 ->
> 
> "functions" -> "function's"
> 
> > 0x120, and its unmapped locations is 0x400 -> 0x420 then we think that
> > the start/end is 0x400 and 0x420 respectively, but the first line
> > might run from 0x100 to 0x108.
> >
> > GDB will then try to scan the prologue starting from 0x400 and ending
> > at 0x108, this immediately gives up as it thinks we have gone past the
> > end of the prologue and the breakpoint is placed at 0x400.
> >
> > In this commit I propose that we change find_pc_line to return
> > addresses in the unmapped range if the address passed in is already in
> > the unmapped range.  Now the first line will appear to run from 0x400
> > to 0x408 and the prologue scanner will correctly find the end of the
> > prologue.
> >
> > With this commit gdb.base/overlays.exp now completely passes with an
> > m32r-elf toolchain.
> 
> I'm not too familiar with overlays (apart from understanding the general
> concept), so I'm not sure how they are usually handled.  Do two pieces
> of code in the same overlay have different unmapped addresses, which are
> used to uniquely identify them?

Yes.

Under the currently supported scheme within GDB overlays are modelled
using sections.  Two different sections are assigned the same VMA, but
different LMAs.

There's then a table[1] that identifies which overlays are mapped in.

Based on which overlays are currently mapped GDB can take a VMA and
derive a section (or GDB can take an LMA and also derive a section).
Once we have a section we can ensure we are looking at the correct
debug info.

There's already bugs that the currently supported table format is not
the most optimal[2].

However, I'm currently working with a client who use a much more
powerful overlay system.  While under the current scheme each overlay
section can have just a single VMA, the system I'm supporting has an
overlay management engine that picks an available slot from a VMA
region into which the overlay code should be placed.  Of course, using
this system requires compiler support.  Within GDB I'm mostly
supporting this overlay scheme as a Python extension.

If time allows then my long term hope/goal is to upstream some of this
work, which basically involves refactoring the existing overlay
support so that the actual implementation is placed behind an
overlay_manager API, and then provide a Python interface to this API
so users can implement their own overlay managers.

Thanks,
Andrew


[1] https://sourceware.org/gdb/current/onlinedocs/gdb/Automatic-Overlay-Debugging.html#Automatic-Overlay-Debugging
[2] https://sourceware.org/bugzilla/show_bug.cgi?id=25408


> 
> In any case, what you did makes sense to me.  It makes the two
> functions, find_pc_partial_function and find_pc_line behave in a more
> consistent manner.
> 
> Simon

Re: [PATCH 0/2] Get gdb.base/overlays.exp passing on m32r

[Andrew Burgess]

I've now pushed this series with the fixes Simon suggested.

Thanks,
Andrew

* Andrew Burgess <andrew.burgess@embecosm.com> [2020-09-18 19:17:33 +0100]:

> I have an interest in overlay debugging.
> 
> A good place to start would seem to be getting the existing overlay
> test passing.
> 
> That's what this commit does.
> 
> Feedback welcome.
> 
> Thanks,
> Andrew
> 
> 
> ---
> 
> Andrew Burgess (2):
>   gdb/testsuite: allow gdb.base/overlays.exp to compile for m32r
>   gdb: handle unmapped overlays in find_pc_line
> 
>  gdb/ChangeLog                       |  5 ++
>  gdb/symtab.c                        | 15 ++++--
>  gdb/testsuite/ChangeLog             |  8 +++
>  gdb/testsuite/gdb.base/m32r.ld      | 84 ++++++++++++++++-------------
>  gdb/testsuite/gdb.base/overlays.exp |  2 +-
>  gdb/testsuite/gdb.base/ovlymgr.c    |  2 +
>  6 files changed, 75 insertions(+), 41 deletions(-)
> 
> -- 
> 2.25.4
>