Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU, for  vDSO32)
@ 2007-10-09 18:17 Jan Kratochvil
  2007-10-09 18:22 ` Daniel Jacobowitz
  0 siblings, 1 reply; 46+ messages in thread
From: Jan Kratochvil @ 2007-10-09 18:17 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 2737 bytes --]

Hi,

currently on GNU/Linux running i386 inferior on x86_64 host you do not see some
of the debug info for the vDSO32 provided by the kernel.

Unpatched:
------------------------------------------------------------------------------
0xffffe410 in __kernel_vsyscall ()
(gdb) bt
#0  0xffffe410 in __kernel_vsyscall ()
#1  0x00c7a853 in __read_nocancel () from /lib/libc.so.6
#2  0x0804b263 in ?? ()
#3  0x0804941f in ?? ()
#4  0x00bcd3a0 in __libc_start_main () from /lib/libc.so.6
#5  0x08048c61 in ?? ()
(gdb) info line *0xffffe410
No line number information available for address 0xffffe410 <__kernel_vsyscall+16>
------------------------------------------------------------------------------

Patched:
------------------------------------------------------------------------------
__kernel_vsyscall () at arch/x86_64/ia32/vsyscall-sysenter.S:26
26              pop     %ebp
Current language:  auto; currently asm
(gdb) bt
#0  __kernel_vsyscall () at arch/x86_64/ia32/vsyscall-sysenter.S:26
#1  0x00c7a853 in __read_nocancel () from /lib/libc.so.6
#2  0x0804b263 in ?? ()
#3  0x0804941f in ?? ()
#4  0x00bcd3a0 in __libc_start_main () from /lib/libc.so.6
#5  0x08048c61 in ?? ()
(gdb) info line $eip
Convenience variables used in line specs must have integer values.
(gdb) info line $rip
Convenience variables used in line specs must have integer values.
(gdb) p/x $eip
$1 = 0xffffe410
(gdb) info line *0xffffe410
Line 26 of "arch/x86_64/ia32/vsyscall-sysenter.S" starts at address 0xffffe410 <__kernel_vsyscall+16>
   and ends at 0xffffe411 <__kernel_vsyscall+17>.
------------------------------------------------------------------------------

The problem is this vDSO is in assembly language, therefore its
 <0><b>: Abbrev Number: 1 (DW_TAG_compile_unit)
  < c>     DW_AT_stmt_list   : 0
  <10>     DW_AT_ranges      : 0
  <14>     DW_AT_name        : arch/x86_64/ia32/vsyscall-sysenter.S
  <39>     DW_AT_comp_dir    : /usr/src/debug////////kernel-2.6.22/linux-2.6.22.x86_64
  <71>     DW_AT_producer    : GNU AS 2.17.50.0.18
  <85>     DW_AT_language    : 32769    (MIPS assembler)

Contents of the .debug_ranges section:

    Offset   Begin    End
    00000000 ffffffff 00000000 (start > end)
    00000000 ffffe400 ffffe414
    00000000 ffffe500 ffffe508
    00000000 ffffe600 ffffe607
    00000000 <End of list>

DW_TAG_compile_unit has no children and it has neither DW_AT_low_pc nor
DW_AT_high_pc but it has DW_AT_ranges instead.  So far DW_AT_ranges was not
parsed into PSYMTABS and so this file was ignored during the debug info search
later.  Reasons for the discontinuous vDSO32 layout/hack are in the Linux
kernel sources.

Thanks to Roland McGrath for pointing out this .debug_info is not broken for
`.S'.


Regards,
Jan

[-- Attachment #2: gdb-6.6-cu-ranges.patch --]
[-- Type: text/plain, Size: 13309 bytes --]

2007-10-09  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* dwarf2read.c (dwarf2_get_pc_bounds): Moved the `DW_AT_ranges' parsing
	code with its variables OBJFILE, CU_HEADER and OBFD into ...
	(dwarf2_ranges_read): ... a new function.
	(read_partial_die): Implemented the parsing of `DW_AT_ranges'.

2007-10-09  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.dwarf2/dw2-ranges.S, gdb.dwarf2/dw2-ranges.exp,
	gdb.dwarf2/dw2-ranges.lds: New files.

--- ./gdb/dwarf2read.c	26 Sep 2007 13:59:54 -0000	1.232
+++ ./gdb/dwarf2read.c	9 Oct 2007 15:03:09 -0000
@@ -3075,6 +3075,124 @@ read_lexical_block_scope (struct die_inf
   local_symbols = new->locals;
 }
 
+/* Get low and high pc attributes from DW_AT_ranges attribute value OFFSET.
+   Return 1 if the attributes are present and valid, otherwise, return 0.  */
+
+static int
+dwarf2_ranges_read (unsigned offset, CORE_ADDR *low_return,
+		    CORE_ADDR *high_return, struct dwarf2_cu *cu)
+{
+  struct objfile *objfile = cu->objfile;
+  struct comp_unit_head *cu_header = &cu->header;
+  bfd *obfd = objfile->obfd;
+  unsigned int addr_size = cu_header->addr_size;
+  CORE_ADDR mask = ~(~(CORE_ADDR)1 << (addr_size * 8 - 1));
+  /* Base address selection entry.  */
+  CORE_ADDR base;
+  int found_base;
+  unsigned int dummy;
+  gdb_byte *buffer;
+  CORE_ADDR marker;
+  int low_set;
+  CORE_ADDR low = 0;
+  CORE_ADDR high = 0;
+
+  found_base = cu_header->base_known;
+  base = cu_header->base_address;
+
+  if (offset >= dwarf2_per_objfile->ranges_size)
+    {
+      complaint (&symfile_complaints,
+		 _("Offset %d out of bounds for DW_AT_ranges attribute"),
+		 offset);
+      return 0;
+    }
+  buffer = dwarf2_per_objfile->ranges_buffer + offset;
+
+  /* Read in the largest possible address.  */
+  marker = read_address (obfd, buffer, cu, &dummy);
+  if ((marker & mask) == mask)
+    {
+      /* If we found the largest possible address, then
+	 read the base address.  */
+      base = read_address (obfd, buffer + addr_size, cu, &dummy);
+      buffer += 2 * addr_size;
+      offset += 2 * addr_size;
+      found_base = 1;
+    }
+
+  low_set = 0;
+
+  while (1)
+    {
+      CORE_ADDR range_beginning, range_end;
+
+      range_beginning = read_address (obfd, buffer, cu, &dummy);
+      buffer += addr_size;
+      range_end = read_address (obfd, buffer, cu, &dummy);
+      buffer += addr_size;
+      offset += 2 * addr_size;
+
+      /* An end of list marker is a pair of zero addresses.  */
+      if (range_beginning == 0 && range_end == 0)
+	/* Found the end of list entry.  */
+	break;
+
+      /* Each base address selection entry is a pair of 2 values.
+	 The first is the largest possible address, the second is
+	 the base address.  Check for a base address here.  */
+      if ((range_beginning & mask) == mask)
+	{
+	  /* If we found the largest possible address, then
+	     read the base address.  */
+	  base = read_address (obfd, buffer + addr_size, cu, &dummy);
+	  found_base = 1;
+	  continue;
+	}
+
+      if (!found_base)
+	{
+	  /* We have no valid base address for the ranges
+	     data.  */
+	  complaint (&symfile_complaints,
+		     _("Invalid .debug_ranges data (no base address)"));
+	  return 0;
+	}
+
+      range_beginning += base;
+      range_end += base;
+
+      /* FIXME: This is recording everything as a low-high
+	 segment of consecutive addresses.  We should have a
+	 data structure for discontiguous block ranges
+	 instead.  */
+      if (! low_set)
+	{
+	  low = range_beginning;
+	  high = range_end;
+	  low_set = 1;
+	}
+      else
+	{
+	  if (range_beginning < low)
+	    low = range_beginning;
+	  if (range_end > high)
+	    high = range_end;
+	}
+    }
+
+  if (! low_set)
+    /* If the first entry is an end-of-list marker, the range
+       describes an empty scope, i.e. no instructions.  */
+    return 0;
+
+  if (low_return)
+    *low_return = low;
+  if (high_return)
+    *high_return = high;
+  return 1;
+}
+
 /* Get low and high pc attributes from a die.  Return 1 if the attributes
    are present and valid, otherwise, return 0.  Return -1 if the range is
    discontinuous, i.e. derived from DW_AT_ranges information.  */
@@ -3082,10 +3200,7 @@ static int
 dwarf2_get_pc_bounds (struct die_info *die, CORE_ADDR *lowpc,
 		      CORE_ADDR *highpc, struct dwarf2_cu *cu)
 {
-  struct objfile *objfile = cu->objfile;
-  struct comp_unit_head *cu_header = &cu->header;
   struct attribute *attr;
-  bfd *obfd = objfile->obfd;
   CORE_ADDR low = 0;
   CORE_ADDR high = 0;
   int ret = 0;
@@ -3109,108 +3224,11 @@ dwarf2_get_pc_bounds (struct die_info *d
       attr = dwarf2_attr (die, DW_AT_ranges, cu);
       if (attr != NULL)
 	{
-	  unsigned int addr_size = cu_header->addr_size;
-	  CORE_ADDR mask = ~(~(CORE_ADDR)1 << (addr_size * 8 - 1));
 	  /* Value of the DW_AT_ranges attribute is the offset in the
 	     .debug_ranges section.  */
-	  unsigned int offset = DW_UNSND (attr);
-	  /* Base address selection entry.  */
-	  CORE_ADDR base;
-	  int found_base;
-	  unsigned int dummy;
-	  gdb_byte *buffer;
-	  CORE_ADDR marker;
-	  int low_set;
- 
-	  found_base = cu_header->base_known;
-	  base = cu_header->base_address;
-
-	  if (offset >= dwarf2_per_objfile->ranges_size)
-	    {
-	      complaint (&symfile_complaints,
-	                 _("Offset %d out of bounds for DW_AT_ranges attribute"),
-			 offset);
-	      return 0;
-	    }
-	  buffer = dwarf2_per_objfile->ranges_buffer + offset;
-
-	  /* Read in the largest possible address.  */
-	  marker = read_address (obfd, buffer, cu, &dummy);
-	  if ((marker & mask) == mask)
-	    {
-	      /* If we found the largest possible address, then
-		 read the base address.  */
-	      base = read_address (obfd, buffer + addr_size, cu, &dummy);
-	      buffer += 2 * addr_size;
-	      offset += 2 * addr_size;
-	      found_base = 1;
-	    }
-
-	  low_set = 0;
-
-	  while (1)
-	    {
-	      CORE_ADDR range_beginning, range_end;
-
-	      range_beginning = read_address (obfd, buffer, cu, &dummy);
-	      buffer += addr_size;
-	      range_end = read_address (obfd, buffer, cu, &dummy);
-	      buffer += addr_size;
-	      offset += 2 * addr_size;
-
-	      /* An end of list marker is a pair of zero addresses.  */
-	      if (range_beginning == 0 && range_end == 0)
-		/* Found the end of list entry.  */
-		break;
-
-	      /* Each base address selection entry is a pair of 2 values.
-		 The first is the largest possible address, the second is
-		 the base address.  Check for a base address here.  */
-	      if ((range_beginning & mask) == mask)
-		{
-		  /* If we found the largest possible address, then
-		     read the base address.  */
-		  base = read_address (obfd, buffer + addr_size, cu, &dummy);
-		  found_base = 1;
-		  continue;
-		}
-
-	      if (!found_base)
-		{
-		  /* We have no valid base address for the ranges
-		     data.  */
-		  complaint (&symfile_complaints,
-			     _("Invalid .debug_ranges data (no base address)"));
-		  return 0;
-		}
-
-	      range_beginning += base;
-	      range_end += base;
-
-	      /* FIXME: This is recording everything as a low-high
-		 segment of consecutive addresses.  We should have a
-		 data structure for discontiguous block ranges
-		 instead.  */
-	      if (! low_set)
-		{
-		  low = range_beginning;
-		  high = range_end;
-		  low_set = 1;
-		}
-	      else
-		{
-		  if (range_beginning < low)
-		    low = range_beginning;
-		  if (range_end > high)
-		    high = range_end;
-		}
-	    }
-
-	  if (! low_set)
-	    /* If the first entry is an end-of-list marker, the range
-	       describes an empty scope, i.e. no instructions.  */
+	  if (!dwarf2_ranges_read (DW_UNSND (attr), &low, &high, cu))
 	    return 0;
-
+	  /* Found discontinuous range of addresses.  */
 	  ret = -1;
 	}
     }
@@ -5571,6 +5589,11 @@ read_partial_die (struct partial_die_inf
 	  has_high_pc_attr = 1;
 	  part_die->highpc = DW_ADDR (&attr);
 	  break;
+	case DW_AT_ranges:
+	  if (dwarf2_ranges_read (DW_UNSND (&attr), &part_die->lowpc,
+				  &part_die->highpc, cu))
+	    has_low_pc_attr = has_high_pc_attr = 1;
+	  break;
 	case DW_AT_location:
           /* Support the .debug_loc offsets */
           if (attr_form_is_block (&attr))
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ ./gdb/testsuite/gdb.dwarf2/dw2-ranges.S	9 Oct 2007 15:03:10 -0000
@@ -0,0 +1,33 @@
+/*
+   Copyright 2007 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/>.
+ */
+
+	.text
+
+	.section	.text._start, "ax", @progbits
+	.globl	_start
+	.func	_start
+_start:	call	func
+	.endfunc
+	.size	_start, . - _start
+
+	.section	.text.func, "ax", @progbits
+	.globl	func
+	.func	func
+func:	int3
+	nop
+	.endfunc
+	.size	func, . - func
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ ./gdb/testsuite/gdb.dwarf2/dw2-ranges.exp	9 Oct 2007 15:03:10 -0000
@@ -0,0 +1,82 @@
+# Copyright 2007 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/>.
+
+# Test DW_TAG_compile_unit with no children and with neither DW_AT_low_pc nor
+# DW_AT_high_pc but with DW_AT_ranges instead.  We created the hole there for
+# DW_AT_ranges by the linker script.
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+# For now pick a sampling of likely targets.
+if {![istarget *-*-linux*]
+    && ![istarget *-*-gnu*]
+    && ![istarget *-*-elf*]
+    && ![istarget *-*-openbsd*]
+    && ![istarget arm-*-eabi*]
+    && ![istarget powerpc-*-eabi*]} {
+    verbose "Skipping i386/amd64 DW_AT_ranges test."
+    return 0  
+}
+if { ![istarget "i?86-*-*"] && ![istarget "x86_64-*-*"] } then {
+    verbose "Skipping i386/amd64 DW_AT_ranges test."
+    return 0
+}
+
+set testfile "dw2-ranges"
+set srcfile ${testfile}.S
+set ldsfile ${testfile}.lds
+set binfile ${objdir}/${subdir}/${testfile}
+
+# Avoid `-lm' from `lib/ada.exp' as it would fail with out `-nostdlib'.
+# Provide BOARD for SET_BOARD_INFO.
+set board [target_info name]
+set_board_info mathlib ""
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug "additional_flags=-Wl,--script=${srcdir}/${subdir}/${ldsfile} -nostdlib"]] != "" } {
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+# Former wrong output:
+# 	Program received signal SIGTRAP, Trace/breakpoint trap.
+# 	0x000000000000002b in func ()
+# 	(gdb) bt
+# 	#0  0x000000000000002b in func ()
+# 	#1  0x0000000000000029 in _start ()
+# 	(gdb) _
+# Correct output:
+# 	Program received signal SIGTRAP, Trace/breakpoint trap.
+# 	func () at dw2-ranges.S:14
+# 	14              nop
+# 	Current language:  auto; currently asm
+# 	(gdb) bt
+# 	#0  func () at dw2-ranges.S:14
+# 	#1  0x0000000000000029 in _start () at dw2-ranges.S:6
+# 	(gdb) _
+# The entry #1 is missing on i386.
+# 	"#0 +func \\(\\) at .*dw2-ranges.S:\[0-9\]+\r\n#1 .*in _start \\(\\) at .*dw2-ranges.S:\[0-9\]+"
+
+gdb_run_cmd
+set test "run"
+gdb_test_multiple "" "$test" {
+    -re "Program received signal SIGTRAP,.*$gdb_prompt $" {
+	pass $test
+    }
+}
+
+gdb_test "backtrace" "#0 +func \\(\\) at .*dw2-ranges.S:\[0-9\]+"
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ ./gdb/testsuite/gdb.dwarf2/dw2-ranges.lds	9 Oct 2007 15:03:10 -0000
@@ -0,0 +1,25 @@
+/*
+   Copyright 2007 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/>.
+ */
+
+SECTIONS
+{
+	.text._start : { *(.text._start) }
+	/* Create the hole.  */
+	. = . + 1;
+	.text.func : { *(.text.func) }
+}
+ENTRY(_start)

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU,  for  vDSO32)
  2007-10-09 18:17 [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU, for vDSO32) Jan Kratochvil
@ 2007-10-09 18:22 ` Daniel Jacobowitz
  2007-10-09 18:59   ` Jan Kratochvil
  0 siblings, 1 reply; 46+ messages in thread
From: Daniel Jacobowitz @ 2007-10-09 18:22 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

On Tue, Oct 09, 2007 at 08:02:46PM +0200, Jan Kratochvil wrote:
> DW_TAG_compile_unit has no children and it has neither DW_AT_low_pc nor
> DW_AT_high_pc but it has DW_AT_ranges instead.  So far DW_AT_ranges was not
> parsed into PSYMTABS and so this file was ignored during the debug info search
> later.  Reasons for the discontinuous vDSO32 layout/hack are in the Linux
> kernel sources.

I'm curious, why is this?  I couldn't find them in my kernel tree.

> 2007-10-09  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* dwarf2read.c (dwarf2_get_pc_bounds): Moved the `DW_AT_ranges' parsing
> 	code with its variables OBJFILE, CU_HEADER and OBFD into ...
> 	(dwarf2_ranges_read): ... a new function.
> 	(read_partial_die): Implemented the parsing of `DW_AT_ranges'.

This is fine.

> 2007-10-09  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* gdb.dwarf2/dw2-ranges.S, gdb.dwarf2/dw2-ranges.exp,
> 	gdb.dwarf2/dw2-ranges.lds: New files.

This is ugly, but mostly fine.  This bit is not:

> +# Avoid `-lm' from `lib/ada.exp' as it would fail with out `-nostdlib'.
> +# Provide BOARD for SET_BOARD_INFO.
> +set board [target_info name]
> +set_board_info mathlib ""

Actually this is from DejaGNU.  And it's global state, so you have
just made other tests behave differently depending on whether you run
this one first or not.

I bet you don't need a custom linker script and -nostdlib to test
this.  Gas must be generating the DW_AT_ranges itself regardless of
whether there's a gap.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU,  for  vDSO32)
  2007-10-09 18:22 ` Daniel Jacobowitz
@ 2007-10-09 18:59   ` Jan Kratochvil
  2007-10-09 19:13     ` Daniel Jacobowitz
  0 siblings, 1 reply; 46+ messages in thread
From: Jan Kratochvil @ 2007-10-09 18:59 UTC (permalink / raw)
  To: gdb-patches; +Cc: Daniel Jacobowitz

[-- Attachment #1: Type: text/plain, Size: 1585 bytes --]

On Tue, 09 Oct 2007 20:17:01 +0200, Daniel Jacobowitz wrote:
> On Tue, Oct 09, 2007 at 08:02:46PM +0200, Jan Kratochvil wrote:
> > DW_TAG_compile_unit has no children and it has neither DW_AT_low_pc nor
> > DW_AT_high_pc but it has DW_AT_ranges instead.  So far DW_AT_ranges was not
> > parsed into PSYMTABS and so this file was ignored during the debug info search
> > later.  Reasons for the discontinuous vDSO32 layout/hack are in the Linux
> > kernel sources.
> 
> I'm curious, why is this?  I couldn't find them in my kernel tree.

linux-2.6.22/arch/x86_64/ia32/vsyscall.lds
  /* This is an 32bit object and we cannot easily get the offsets
     into the 64bit kernel. Just hardcode them here. This assumes
     that all the stubs don't need more than 0x100 bytes. */
  . = VSYSCALL_BASE + 0x500;


> > +# Avoid `-lm' from `lib/ada.exp' as it would fail with out `-nostdlib'.
> > +# Provide BOARD for SET_BOARD_INFO.
> > +set board [target_info name]
> > +set_board_info mathlib ""
> 
> Actually this is from DejaGNU.  And it's global state, so you have
> just made other tests behave differently depending on whether you run
> this one first or not.

Overlooked it, thanks.  Saved/restored it in the attached updated patch.
I understand it is very ugly hack, it would be probably more appropriate to fix
`lib/ada.exp' instead.


> I bet you don't need a custom linker script and -nostdlib to test
> this.  Gas must be generating the DW_AT_ranges itself regardless of
> whether there's a gap.

Which one, .space, .org +x? These all fill the space with flesh bytes.


Regards,
Jan

[-- Attachment #2: gdb-6.6-cu-ranges.patch --]
[-- Type: text/plain, Size: 13755 bytes --]

2007-10-09  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* dwarf2read.c (dwarf2_get_pc_bounds): Moved the `DW_AT_ranges' parsing
	code with its variables OBJFILE, CU_HEADER and OBFD into ...
	(dwarf2_ranges_read): ... a new function.
	(read_partial_die): Implemented the parsing of `DW_AT_ranges'.

2007-10-09  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.dwarf2/dw2-ranges.S, gdb.dwarf2/dw2-ranges.exp,
	gdb.dwarf2/dw2-ranges.lds: New files.

--- ./gdb/dwarf2read.c	26 Sep 2007 13:59:54 -0000	1.232
+++ ./gdb/dwarf2read.c	9 Oct 2007 15:03:09 -0000
@@ -3075,6 +3075,124 @@ read_lexical_block_scope (struct die_inf
   local_symbols = new->locals;
 }
 
+/* Get low and high pc attributes from DW_AT_ranges attribute value OFFSET.
+   Return 1 if the attributes are present and valid, otherwise, return 0.  */
+
+static int
+dwarf2_ranges_read (unsigned offset, CORE_ADDR *low_return,
+		    CORE_ADDR *high_return, struct dwarf2_cu *cu)
+{
+  struct objfile *objfile = cu->objfile;
+  struct comp_unit_head *cu_header = &cu->header;
+  bfd *obfd = objfile->obfd;
+  unsigned int addr_size = cu_header->addr_size;
+  CORE_ADDR mask = ~(~(CORE_ADDR)1 << (addr_size * 8 - 1));
+  /* Base address selection entry.  */
+  CORE_ADDR base;
+  int found_base;
+  unsigned int dummy;
+  gdb_byte *buffer;
+  CORE_ADDR marker;
+  int low_set;
+  CORE_ADDR low = 0;
+  CORE_ADDR high = 0;
+
+  found_base = cu_header->base_known;
+  base = cu_header->base_address;
+
+  if (offset >= dwarf2_per_objfile->ranges_size)
+    {
+      complaint (&symfile_complaints,
+		 _("Offset %d out of bounds for DW_AT_ranges attribute"),
+		 offset);
+      return 0;
+    }
+  buffer = dwarf2_per_objfile->ranges_buffer + offset;
+
+  /* Read in the largest possible address.  */
+  marker = read_address (obfd, buffer, cu, &dummy);
+  if ((marker & mask) == mask)
+    {
+      /* If we found the largest possible address, then
+	 read the base address.  */
+      base = read_address (obfd, buffer + addr_size, cu, &dummy);
+      buffer += 2 * addr_size;
+      offset += 2 * addr_size;
+      found_base = 1;
+    }
+
+  low_set = 0;
+
+  while (1)
+    {
+      CORE_ADDR range_beginning, range_end;
+
+      range_beginning = read_address (obfd, buffer, cu, &dummy);
+      buffer += addr_size;
+      range_end = read_address (obfd, buffer, cu, &dummy);
+      buffer += addr_size;
+      offset += 2 * addr_size;
+
+      /* An end of list marker is a pair of zero addresses.  */
+      if (range_beginning == 0 && range_end == 0)
+	/* Found the end of list entry.  */
+	break;
+
+      /* Each base address selection entry is a pair of 2 values.
+	 The first is the largest possible address, the second is
+	 the base address.  Check for a base address here.  */
+      if ((range_beginning & mask) == mask)
+	{
+	  /* If we found the largest possible address, then
+	     read the base address.  */
+	  base = read_address (obfd, buffer + addr_size, cu, &dummy);
+	  found_base = 1;
+	  continue;
+	}
+
+      if (!found_base)
+	{
+	  /* We have no valid base address for the ranges
+	     data.  */
+	  complaint (&symfile_complaints,
+		     _("Invalid .debug_ranges data (no base address)"));
+	  return 0;
+	}
+
+      range_beginning += base;
+      range_end += base;
+
+      /* FIXME: This is recording everything as a low-high
+	 segment of consecutive addresses.  We should have a
+	 data structure for discontiguous block ranges
+	 instead.  */
+      if (! low_set)
+	{
+	  low = range_beginning;
+	  high = range_end;
+	  low_set = 1;
+	}
+      else
+	{
+	  if (range_beginning < low)
+	    low = range_beginning;
+	  if (range_end > high)
+	    high = range_end;
+	}
+    }
+
+  if (! low_set)
+    /* If the first entry is an end-of-list marker, the range
+       describes an empty scope, i.e. no instructions.  */
+    return 0;
+
+  if (low_return)
+    *low_return = low;
+  if (high_return)
+    *high_return = high;
+  return 1;
+}
+
 /* Get low and high pc attributes from a die.  Return 1 if the attributes
    are present and valid, otherwise, return 0.  Return -1 if the range is
    discontinuous, i.e. derived from DW_AT_ranges information.  */
@@ -3082,10 +3200,7 @@ static int
 dwarf2_get_pc_bounds (struct die_info *die, CORE_ADDR *lowpc,
 		      CORE_ADDR *highpc, struct dwarf2_cu *cu)
 {
-  struct objfile *objfile = cu->objfile;
-  struct comp_unit_head *cu_header = &cu->header;
   struct attribute *attr;
-  bfd *obfd = objfile->obfd;
   CORE_ADDR low = 0;
   CORE_ADDR high = 0;
   int ret = 0;
@@ -3109,108 +3224,11 @@ dwarf2_get_pc_bounds (struct die_info *d
       attr = dwarf2_attr (die, DW_AT_ranges, cu);
       if (attr != NULL)
 	{
-	  unsigned int addr_size = cu_header->addr_size;
-	  CORE_ADDR mask = ~(~(CORE_ADDR)1 << (addr_size * 8 - 1));
 	  /* Value of the DW_AT_ranges attribute is the offset in the
 	     .debug_ranges section.  */
-	  unsigned int offset = DW_UNSND (attr);
-	  /* Base address selection entry.  */
-	  CORE_ADDR base;
-	  int found_base;
-	  unsigned int dummy;
-	  gdb_byte *buffer;
-	  CORE_ADDR marker;
-	  int low_set;
- 
-	  found_base = cu_header->base_known;
-	  base = cu_header->base_address;
-
-	  if (offset >= dwarf2_per_objfile->ranges_size)
-	    {
-	      complaint (&symfile_complaints,
-	                 _("Offset %d out of bounds for DW_AT_ranges attribute"),
-			 offset);
-	      return 0;
-	    }
-	  buffer = dwarf2_per_objfile->ranges_buffer + offset;
-
-	  /* Read in the largest possible address.  */
-	  marker = read_address (obfd, buffer, cu, &dummy);
-	  if ((marker & mask) == mask)
-	    {
-	      /* If we found the largest possible address, then
-		 read the base address.  */
-	      base = read_address (obfd, buffer + addr_size, cu, &dummy);
-	      buffer += 2 * addr_size;
-	      offset += 2 * addr_size;
-	      found_base = 1;
-	    }
-
-	  low_set = 0;
-
-	  while (1)
-	    {
-	      CORE_ADDR range_beginning, range_end;
-
-	      range_beginning = read_address (obfd, buffer, cu, &dummy);
-	      buffer += addr_size;
-	      range_end = read_address (obfd, buffer, cu, &dummy);
-	      buffer += addr_size;
-	      offset += 2 * addr_size;
-
-	      /* An end of list marker is a pair of zero addresses.  */
-	      if (range_beginning == 0 && range_end == 0)
-		/* Found the end of list entry.  */
-		break;
-
-	      /* Each base address selection entry is a pair of 2 values.
-		 The first is the largest possible address, the second is
-		 the base address.  Check for a base address here.  */
-	      if ((range_beginning & mask) == mask)
-		{
-		  /* If we found the largest possible address, then
-		     read the base address.  */
-		  base = read_address (obfd, buffer + addr_size, cu, &dummy);
-		  found_base = 1;
-		  continue;
-		}
-
-	      if (!found_base)
-		{
-		  /* We have no valid base address for the ranges
-		     data.  */
-		  complaint (&symfile_complaints,
-			     _("Invalid .debug_ranges data (no base address)"));
-		  return 0;
-		}
-
-	      range_beginning += base;
-	      range_end += base;
-
-	      /* FIXME: This is recording everything as a low-high
-		 segment of consecutive addresses.  We should have a
-		 data structure for discontiguous block ranges
-		 instead.  */
-	      if (! low_set)
-		{
-		  low = range_beginning;
-		  high = range_end;
-		  low_set = 1;
-		}
-	      else
-		{
-		  if (range_beginning < low)
-		    low = range_beginning;
-		  if (range_end > high)
-		    high = range_end;
-		}
-	    }
-
-	  if (! low_set)
-	    /* If the first entry is an end-of-list marker, the range
-	       describes an empty scope, i.e. no instructions.  */
+	  if (!dwarf2_ranges_read (DW_UNSND (attr), &low, &high, cu))
 	    return 0;
-
+	  /* Found discontinuous range of addresses.  */
 	  ret = -1;
 	}
     }
@@ -5571,6 +5589,11 @@ read_partial_die (struct partial_die_inf
 	  has_high_pc_attr = 1;
 	  part_die->highpc = DW_ADDR (&attr);
 	  break;
+	case DW_AT_ranges:
+	  if (dwarf2_ranges_read (DW_UNSND (&attr), &part_die->lowpc,
+				  &part_die->highpc, cu))
+	    has_low_pc_attr = has_high_pc_attr = 1;
+	  break;
 	case DW_AT_location:
           /* Support the .debug_loc offsets */
           if (attr_form_is_block (&attr))
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ ./gdb/testsuite/gdb.dwarf2/dw2-ranges.S	9 Oct 2007 15:03:10 -0000
@@ -0,0 +1,33 @@
+/*
+   Copyright 2007 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/>.
+ */
+
+	.text
+
+	.section	.text._start, "ax", @progbits
+	.globl	_start
+	.func	_start
+_start:	call	func
+	.endfunc
+	.size	_start, . - _start
+
+	.section	.text.func, "ax", @progbits
+	.globl	func
+	.func	func
+func:	int3
+	nop
+	.endfunc
+	.size	func, . - func
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ ./gdb/testsuite/gdb.dwarf2/dw2-ranges.exp	9 Oct 2007 15:03:10 -0000
@@ -0,0 +1,98 @@
+# Copyright 2007 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/>.
+
+# Test DW_TAG_compile_unit with no children and with neither DW_AT_low_pc nor
+# DW_AT_high_pc but with DW_AT_ranges instead.  We created the hole there for
+# DW_AT_ranges by the linker script.
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+# For now pick a sampling of likely targets.
+if {![istarget *-*-linux*]
+    && ![istarget *-*-gnu*]
+    && ![istarget *-*-elf*]
+    && ![istarget *-*-openbsd*]
+    && ![istarget arm-*-eabi*]
+    && ![istarget powerpc-*-eabi*]} {
+    verbose "Skipping i386/amd64 DW_AT_ranges test."
+    return 0  
+}
+if { ![istarget "i?86-*-*"] && ![istarget "x86_64-*-*"] } then {
+    verbose "Skipping i386/amd64 DW_AT_ranges test."
+    return 0
+}
+
+set testfile "dw2-ranges"
+set srcfile ${testfile}.S
+set ldsfile ${testfile}.lds
+set binfile ${objdir}/${subdir}/${testfile}
+
+# Avoid `-lm' from `lib/ada.exp' as it would fail with our `-nostdlib'.
+# Provide BOARD for SET_BOARD_INFO.
+set board [target_info name]
+if [board_info $board exists mathlib] {
+    set mathlib_orig [board_info $board mathlib]
+} else {
+    unset -nocomplain mathlib_orig
+}
+set_board_info mathlib ""
+
+set compiled [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable [list debug "additional_flags=-Wl,--script=${srcdir}/${subdir}/${ldsfile} -nostdlib"]]
+
+# Restore the board info.
+if [info exists mathlib_orig] {
+    # SET_BOARD_INFO does not change an already set value.
+    set board_info($board,mathlib) $mathlib_orig
+} else {
+    # There is no deleting function for the board info.
+    unset board_info($board,mathlib)
+}
+
+if  { $compiled != "" } {
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+# Former wrong output:
+# 	Program received signal SIGTRAP, Trace/breakpoint trap.
+# 	0x000000000000002b in func ()
+# 	(gdb) bt
+# 	#0  0x000000000000002b in func ()
+# 	#1  0x0000000000000029 in _start ()
+# 	(gdb) _
+# Correct output:
+# 	Program received signal SIGTRAP, Trace/breakpoint trap.
+# 	func () at dw2-ranges.S:14
+# 	14              nop
+# 	Current language:  auto; currently asm
+# 	(gdb) bt
+# 	#0  func () at dw2-ranges.S:14
+# 	#1  0x0000000000000029 in _start () at dw2-ranges.S:6
+# 	(gdb) _
+# The entry #1 is missing on i386.
+# 	"#0 +func \\(\\) at .*dw2-ranges.S:\[0-9\]+\r\n#1 .*in _start \\(\\) at .*dw2-ranges.S:\[0-9\]+"
+
+gdb_run_cmd
+set test "run"
+gdb_test_multiple "" "$test" {
+    -re "Program received signal SIGTRAP,.*$gdb_prompt $" {
+	pass $test
+    }
+}
+
+gdb_test "backtrace" "#0 +func \\(\\) at .*dw2-ranges.S:\[0-9\]+"
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ ./gdb/testsuite/gdb.dwarf2/dw2-ranges.lds	9 Oct 2007 15:03:10 -0000
@@ -0,0 +1,25 @@
+/*
+   Copyright 2007 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/>.
+ */
+
+SECTIONS
+{
+	.text._start : { *(.text._start) }
+	/* Create the hole.  */
+	. = . + 1;
+	.text.func : { *(.text.func) }
+}
+ENTRY(_start)

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU,  for  vDSO32)
  2007-10-09 18:59   ` Jan Kratochvil
@ 2007-10-09 19:13     ` Daniel Jacobowitz
  2007-11-24 15:43       ` Jan Kratochvil
  0 siblings, 1 reply; 46+ messages in thread
From: Daniel Jacobowitz @ 2007-10-09 19:13 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

On Tue, Oct 09, 2007 at 08:54:34PM +0200, Jan Kratochvil wrote:
> > I bet you don't need a custom linker script and -nostdlib to test
> > this.  Gas must be generating the DW_AT_ranges itself regardless of
> > whether there's a gap.
> 
> Which one, .space, .org +x? These all fill the space with flesh bytes.

No, you just need the two sections.  Does that work with the default
linker script and without -nostdlib?  Just call the entry point main
instead of _start.

At that point it doesn't need to be i386 specific, either (see any of
the other tests in the same directory).

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU,  for vDSO32)
  2007-10-09 19:13     ` Daniel Jacobowitz
@ 2007-11-24 15:43       ` Jan Kratochvil
  2007-11-25 14:48         ` Daniel Jacobowitz
  2007-11-30  7:42         ` Vladimir Prus
  0 siblings, 2 replies; 46+ messages in thread
From: Jan Kratochvil @ 2007-11-24 15:43 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 219 bytes --]

On Tue, 09 Oct 2007 20:59:31 +0200, Daniel Jacobowitz wrote:
...
> Does that work with the default linker script and without -nostdlib?

Attached the patch with the testcase no longer using its own .lds.


Regards,
Jan

[-- Attachment #2: gdb-6.6-cu-ranges3.patch --]
[-- Type: text/plain, Size: 12123 bytes --]

2007-11-24  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* dwarf2read.c (dwarf2_get_pc_bounds): Moved the `DW_AT_ranges' parsing
	code with its variables OBJFILE, CU_HEADER and OBFD into ...
	(dwarf2_ranges_read): ... a new function.
	(read_partial_die): Implemented the parsing of `DW_AT_ranges'.

2007-11-24  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.dwarf2/dw2-ranges.S, gdb.dwarf2/dw2-ranges.exp: New files.

Index: gdb/dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.236
diff -u -p -r1.236 dwarf2read.c
--- gdb/dwarf2read.c	25 Oct 2007 20:54:27 -0000	1.236
+++ gdb/dwarf2read.c	24 Nov 2007 15:41:32 -0000
@@ -3077,6 +3077,124 @@ read_lexical_block_scope (struct die_inf
   local_symbols = new->locals;
 }
 
+/* Get low and high pc attributes from DW_AT_ranges attribute value OFFSET.
+   Return 1 if the attributes are present and valid, otherwise, return 0.  */
+
+static int
+dwarf2_ranges_read (unsigned offset, CORE_ADDR *low_return,
+		    CORE_ADDR *high_return, struct dwarf2_cu *cu)
+{
+  struct objfile *objfile = cu->objfile;
+  struct comp_unit_head *cu_header = &cu->header;
+  bfd *obfd = objfile->obfd;
+  unsigned int addr_size = cu_header->addr_size;
+  CORE_ADDR mask = ~(~(CORE_ADDR)1 << (addr_size * 8 - 1));
+  /* Base address selection entry.  */
+  CORE_ADDR base;
+  int found_base;
+  unsigned int dummy;
+  gdb_byte *buffer;
+  CORE_ADDR marker;
+  int low_set;
+  CORE_ADDR low = 0;
+  CORE_ADDR high = 0;
+
+  found_base = cu_header->base_known;
+  base = cu_header->base_address;
+
+  if (offset >= dwarf2_per_objfile->ranges_size)
+    {
+      complaint (&symfile_complaints,
+		 _("Offset %d out of bounds for DW_AT_ranges attribute"),
+		 offset);
+      return 0;
+    }
+  buffer = dwarf2_per_objfile->ranges_buffer + offset;
+
+  /* Read in the largest possible address.  */
+  marker = read_address (obfd, buffer, cu, &dummy);
+  if ((marker & mask) == mask)
+    {
+      /* If we found the largest possible address, then
+	 read the base address.  */
+      base = read_address (obfd, buffer + addr_size, cu, &dummy);
+      buffer += 2 * addr_size;
+      offset += 2 * addr_size;
+      found_base = 1;
+    }
+
+  low_set = 0;
+
+  while (1)
+    {
+      CORE_ADDR range_beginning, range_end;
+
+      range_beginning = read_address (obfd, buffer, cu, &dummy);
+      buffer += addr_size;
+      range_end = read_address (obfd, buffer, cu, &dummy);
+      buffer += addr_size;
+      offset += 2 * addr_size;
+
+      /* An end of list marker is a pair of zero addresses.  */
+      if (range_beginning == 0 && range_end == 0)
+	/* Found the end of list entry.  */
+	break;
+
+      /* Each base address selection entry is a pair of 2 values.
+	 The first is the largest possible address, the second is
+	 the base address.  Check for a base address here.  */
+      if ((range_beginning & mask) == mask)
+	{
+	  /* If we found the largest possible address, then
+	     read the base address.  */
+	  base = read_address (obfd, buffer + addr_size, cu, &dummy);
+	  found_base = 1;
+	  continue;
+	}
+
+      if (!found_base)
+	{
+	  /* We have no valid base address for the ranges
+	     data.  */
+	  complaint (&symfile_complaints,
+		     _("Invalid .debug_ranges data (no base address)"));
+	  return 0;
+	}
+
+      range_beginning += base;
+      range_end += base;
+
+      /* FIXME: This is recording everything as a low-high
+	 segment of consecutive addresses.  We should have a
+	 data structure for discontiguous block ranges
+	 instead.  */
+      if (! low_set)
+	{
+	  low = range_beginning;
+	  high = range_end;
+	  low_set = 1;
+	}
+      else
+	{
+	  if (range_beginning < low)
+	    low = range_beginning;
+	  if (range_end > high)
+	    high = range_end;
+	}
+    }
+
+  if (! low_set)
+    /* If the first entry is an end-of-list marker, the range
+       describes an empty scope, i.e. no instructions.  */
+    return 0;
+
+  if (low_return)
+    *low_return = low;
+  if (high_return)
+    *high_return = high;
+  return 1;
+}
+
 /* Get low and high pc attributes from a die.  Return 1 if the attributes
    are present and valid, otherwise, return 0.  Return -1 if the range is
    discontinuous, i.e. derived from DW_AT_ranges information.  */
@@ -3084,10 +3202,7 @@ static int
 dwarf2_get_pc_bounds (struct die_info *die, CORE_ADDR *lowpc,
 		      CORE_ADDR *highpc, struct dwarf2_cu *cu)
 {
-  struct objfile *objfile = cu->objfile;
-  struct comp_unit_head *cu_header = &cu->header;
   struct attribute *attr;
-  bfd *obfd = objfile->obfd;
   CORE_ADDR low = 0;
   CORE_ADDR high = 0;
   int ret = 0;
@@ -3111,108 +3226,11 @@ dwarf2_get_pc_bounds (struct die_info *d
       attr = dwarf2_attr (die, DW_AT_ranges, cu);
       if (attr != NULL)
 	{
-	  unsigned int addr_size = cu_header->addr_size;
-	  CORE_ADDR mask = ~(~(CORE_ADDR)1 << (addr_size * 8 - 1));
 	  /* Value of the DW_AT_ranges attribute is the offset in the
 	     .debug_ranges section.  */
-	  unsigned int offset = DW_UNSND (attr);
-	  /* Base address selection entry.  */
-	  CORE_ADDR base;
-	  int found_base;
-	  unsigned int dummy;
-	  gdb_byte *buffer;
-	  CORE_ADDR marker;
-	  int low_set;
- 
-	  found_base = cu_header->base_known;
-	  base = cu_header->base_address;
-
-	  if (offset >= dwarf2_per_objfile->ranges_size)
-	    {
-	      complaint (&symfile_complaints,
-	                 _("Offset %d out of bounds for DW_AT_ranges attribute"),
-			 offset);
-	      return 0;
-	    }
-	  buffer = dwarf2_per_objfile->ranges_buffer + offset;
-
-	  /* Read in the largest possible address.  */
-	  marker = read_address (obfd, buffer, cu, &dummy);
-	  if ((marker & mask) == mask)
-	    {
-	      /* If we found the largest possible address, then
-		 read the base address.  */
-	      base = read_address (obfd, buffer + addr_size, cu, &dummy);
-	      buffer += 2 * addr_size;
-	      offset += 2 * addr_size;
-	      found_base = 1;
-	    }
-
-	  low_set = 0;
-
-	  while (1)
-	    {
-	      CORE_ADDR range_beginning, range_end;
-
-	      range_beginning = read_address (obfd, buffer, cu, &dummy);
-	      buffer += addr_size;
-	      range_end = read_address (obfd, buffer, cu, &dummy);
-	      buffer += addr_size;
-	      offset += 2 * addr_size;
-
-	      /* An end of list marker is a pair of zero addresses.  */
-	      if (range_beginning == 0 && range_end == 0)
-		/* Found the end of list entry.  */
-		break;
-
-	      /* Each base address selection entry is a pair of 2 values.
-		 The first is the largest possible address, the second is
-		 the base address.  Check for a base address here.  */
-	      if ((range_beginning & mask) == mask)
-		{
-		  /* If we found the largest possible address, then
-		     read the base address.  */
-		  base = read_address (obfd, buffer + addr_size, cu, &dummy);
-		  found_base = 1;
-		  continue;
-		}
-
-	      if (!found_base)
-		{
-		  /* We have no valid base address for the ranges
-		     data.  */
-		  complaint (&symfile_complaints,
-			     _("Invalid .debug_ranges data (no base address)"));
-		  return 0;
-		}
-
-	      range_beginning += base;
-	      range_end += base;
-
-	      /* FIXME: This is recording everything as a low-high
-		 segment of consecutive addresses.  We should have a
-		 data structure for discontiguous block ranges
-		 instead.  */
-	      if (! low_set)
-		{
-		  low = range_beginning;
-		  high = range_end;
-		  low_set = 1;
-		}
-	      else
-		{
-		  if (range_beginning < low)
-		    low = range_beginning;
-		  if (range_end > high)
-		    high = range_end;
-		}
-	    }
-
-	  if (! low_set)
-	    /* If the first entry is an end-of-list marker, the range
-	       describes an empty scope, i.e. no instructions.  */
+	  if (!dwarf2_ranges_read (DW_UNSND (attr), &low, &high, cu))
 	    return 0;
-
+	  /* Found discontinuous range of addresses.  */
 	  ret = -1;
 	}
     }
@@ -5569,6 +5587,11 @@ read_partial_die (struct partial_die_inf
 	  has_high_pc_attr = 1;
 	  part_die->highpc = DW_ADDR (&attr);
 	  break;
+	case DW_AT_ranges:
+	  if (dwarf2_ranges_read (DW_UNSND (&attr), &part_die->lowpc,
+				  &part_die->highpc, cu))
+	    has_low_pc_attr = has_high_pc_attr = 1;
+	  break;
 	case DW_AT_location:
           /* Support the .debug_loc offsets */
           if (attr_form_is_block (&attr))
Index: gdb/testsuite/gdb.dwarf2/dw2-ranges.S
===================================================================
RCS file: gdb/testsuite/gdb.dwarf2/dw2-ranges.S
diff -N gdb/testsuite/gdb.dwarf2/dw2-ranges.S
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.dwarf2/dw2-ranges.S	24 Nov 2007 15:41:33 -0000
@@ -0,0 +1,40 @@
+/*
+   Copyright 2007 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/>.
+ */
+
+/* Despite the sections below will be adjacent the assembler has to produce
+   DW_AT_ranges as the linker could place both sections at arbitrary locations.
+   */
+
+	/* Without this directive GAS will not emit DWARF2 unless we provide an
+	   instruction to assemble.  We want to avoid any instructions to
+	   remain architecture independent.  */
+	.loc_mark_labels	1
+
+	.text
+
+	.globl	main
+	.func	main
+main:	.int	0
+	.endfunc
+	.size	main, . - main
+
+	.section	.text.func, "ax", @progbits
+	.globl	func
+	.func	func
+func:	.int	0
+	.endfunc
+	.size	func, . - func
Index: gdb/testsuite/gdb.dwarf2/dw2-ranges.exp
===================================================================
RCS file: gdb/testsuite/gdb.dwarf2/dw2-ranges.exp
diff -N gdb/testsuite/gdb.dwarf2/dw2-ranges.exp
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.dwarf2/dw2-ranges.exp	24 Nov 2007 15:41:33 -0000
@@ -0,0 +1,49 @@
+# Copyright 2007 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/>.
+
+# Test DW_TAG_compile_unit with no children and with neither DW_AT_low_pc nor
+# DW_AT_high_pc but with DW_AT_ranges instead.
+
+# This test can only be run on targets which support DWARF-2 and use gas.
+# For now pick a sampling of likely targets.
+if {![istarget *-*-linux*]
+    && ![istarget *-*-gnu*]
+    && ![istarget *-*-elf*]
+    && ![istarget *-*-openbsd*]
+    && ![istarget arm-*-eabi*]
+    && ![istarget powerpc-*-eabi*]} {
+    verbose "Skipping DW_AT_ranges test."
+    return 0  
+}
+
+set testfile "dw2-ranges"
+set srcfile ${testfile}.S
+set binfile ${objdir}/${subdir}/${testfile}.o
+
+if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" object debug] != "" } {
+    return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+# Correct output:
+# 	Line 39 of "../.././gdb/testsuite/gdb.dwarf2/dw2-ranges.S" starts at address 0x4 and ends at 0x8.
+# Wrong output:
+# 	No line number information available for address 0x4
+
+gdb_test "info line func" "Line \[0-9\]* of .* starts at address .* and ends at .*"

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU,  for vDSO32)
  2007-11-24 15:43       ` Jan Kratochvil
@ 2007-11-25 14:48         ` Daniel Jacobowitz
  2007-11-30  7:42         ` Vladimir Prus
  1 sibling, 0 replies; 46+ messages in thread
From: Daniel Jacobowitz @ 2007-11-25 14:48 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

On Sat, Nov 24, 2007 at 04:43:39PM +0100, Jan Kratochvil wrote:
> On Tue, 09 Oct 2007 20:59:31 +0200, Daniel Jacobowitz wrote:
> ...
> > Does that work with the default linker script and without -nostdlib?
> 
> Attached the patch with the testcase no longer using its own .lds.

Thanks.  This version is OK.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU,  for vDSO32)
  2007-11-24 15:43       ` Jan Kratochvil
  2007-11-25 14:48         ` Daniel Jacobowitz
@ 2007-11-30  7:42         ` Vladimir Prus
  2007-11-30 11:10           ` Jan Kratochvil
  1 sibling, 1 reply; 46+ messages in thread
From: Vladimir Prus @ 2007-11-30  7:42 UTC (permalink / raw)
  To: Jan Kratochvil, gdb-patches

[-- Attachment #1: Type: text/plain, Size: 564 bytes --]

Jan Kratochvil wrote:

> On Tue, 09 Oct 2007 20:59:31 +0200, Daniel Jacobowitz wrote:
> ...
>> Does that work with the default linker script and without -nostdlib?
> 
> Attached the patch with the testcase no longer using its own .lds.

Jan,
I'm afraid this patch causes the following regression for me:

        FAIL: gdb.threads/thread_check.exp: breakpoint at tf

This FAIL disappears as soon as I revert to revision 1.236 of 
gdb/dwarf2read.c -- right before your changes. Do you see the same
failure?

I attach the gdb.log, if that helps.

Thanks,
Volodya




[-- Attachment #2: gdb.log --]
[-- Type: text/plain, Size: 4669 bytes --]

Test Run By ghost on Fri Nov 30 10:39:14 2007
Native configuration is i686-pc-linux-gnu

		=== gdb tests ===

Schedule of variations:
    unix

Running target unix
Using /usr/share/dejagnu/baseboards/unix.exp as board description file for target.
Using /usr/share/dejagnu/config/unix.exp as generic interface file for target.
Using /home/ghost/Work/CodeSourcery/Projects/egdb/gdb/gdb/testsuite/config/unix.exp as tool-and-target-specific interface file.
Running /home/ghost/Work/CodeSourcery/Projects/egdb/gdb/gdb/testsuite/gdb.threads/thread_check.exp ...
Executing on host: gcc /home/ghost/Work/CodeSourcery/Projects/egdb/gdb/gdb/testsuite/gdb.threads/thread_check.c  -I/home/ghost/Build/gdb_ericsson/gdb/testsuite -g  -lpthreads -lm   -o /home/ghost/Build/gdb_ericsson/gdb/testsuite/gdb.threads/thread_check    (timeout = 300)
/usr/bin/ld: cannot find -lpthreads

collect2: ld returned 1 exit status

compiler exited with status 1
output is:
/usr/bin/ld: cannot find -lpthreads

collect2: ld returned 1 exit status


Executing on host: gcc /home/ghost/Work/CodeSourcery/Projects/egdb/gdb/gdb/testsuite/gdb.threads/thread_check.c  -I/home/ghost/Build/gdb_ericsson/gdb/testsuite -g  -lpthread -lm   -o /home/ghost/Build/gdb_ericsson/gdb/testsuite/gdb.threads/thread_check    (timeout = 300)
PASS: gdb.threads/thread_check.exp: successfully compiled posix threads test case
GNU gdb 6.7.50.20071127-cvs

Copyright (C) 2007 Free Software Foundation, Inc.

License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>

This is free software: you are free to change and redistribute it.

There is NO WARRANTY, to the extent permitted by law.  Type "show copying"

and "show warranty" for details.

This GDB was configured as "i686-pc-linux-gnu".

(gdb) set height 0

(gdb) set width 0

(gdb) dir

Reinitialize source path to empty? (y or n) y

Source directories searched: $cdir:$cwd

(gdb) dir /home/ghost/Work/CodeSourcery/Projects/egdb/gdb/gdb/testsuite/gdb.threads

Source directories searched: /home/ghost/Work/CodeSourcery/Projects/egdb/gdb/gdb/testsuite/gdb.threads:$cdir:$cwd

(gdb) kill

The program is not being run.

(gdb) file /home/ghost/Build/gdb_ericsson/gdb/testsuite/gdb.threads/thread_check

Reading symbols from /home/ghost/Build/gdb_ericsson/gdb/testsuite/gdb.threads/thread_check...done.

(gdb) delete breakpoints

(gdb) info breakpoints

No breakpoints or watchpoints.

(gdb) break main

Breakpoint 1 at 0x804863b: file /home/ghost/Work/CodeSourcery/Projects/egdb/gdb/gdb/testsuite/gdb.threads/thread_check.c, line 50.

(gdb) run 

Starting program: /home/ghost/Build/gdb_ericsson/gdb/testsuite/gdb.threads/thread_check 

[Thread debugging using libthread_db enabled]

[New Thread 0x401b76c0 (LWP 11072)]

[Switching to Thread 0x401b76c0 (LWP 11072)]



Breakpoint 1, main () at /home/ghost/Work/CodeSourcery/Projects/egdb/gdb/gdb/testsuite/gdb.threads/thread_check.c:50

50	    int unslept = 2;

(gdb) break tf

Breakpoint 2 at 0x804856d

(gdb) FAIL: gdb.threads/thread_check.exp: breakpoint at tf
continue

Continuing.

[New Thread 0x403b8b90 (LWP 11075)]

[Switching to Thread 0x403b8b90 (LWP 11075)]



Breakpoint 2, 0x0804856d in tf (arg=0xa8428197) at /home/ghost/Work/CodeSourcery/Projects/egdb/gdb/gdb/testsuite/gdb.threads/thread_check.c:31

31	{

(gdb) PASS: gdb.threads/thread_check.exp: continue to tf
backtrace

#0  0x0804856d in tf (arg=0xa8428197) at /home/ghost/Work/CodeSourcery/Projects/egdb/gdb/gdb/testsuite/gdb.threads/thread_check.c:31

#1  0x4003c31b in start_thread () from /lib/tls/i686/cmov/libpthread.so.0

#2  0x4014557e in clone () from /lib/tls/i686/cmov/libc.so.6

(gdb) PASS: gdb.threads/thread_check.exp: backtrace from thread function
delete breakpoints

Delete all breakpoints? (y or n) y

(gdb) info breakpoints

No breakpoints or watchpoints.

(gdb) testcase /home/ghost/Work/CodeSourcery/Projects/egdb/gdb/gdb/testsuite/gdb.threads/thread_check.exp completed in 1 seconds

		=== gdb Summary ===

# of expected passes		3
# of unexpected failures	1
Executing on host: /home/ghost/Build/gdb_ericsson/gdb/testsuite/../../gdb/gdb -nw --command gdb_cmd    (timeout = 300)
GNU gdb 6.7.50.20071127-cvs

Copyright (C) 2007 Free Software Foundation, Inc.

License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>

This is free software: you are free to change and redistribute it.

There is NO WARRANTY, to the extent permitted by law.  Type "show copying"

and "show warranty" for details.

This GDB was configured as "i686-pc-linux-gnu".

Hi

/home/ghost/Build/gdb_ericsson/gdb/testsuite/../../gdb/gdb version  6.7.50.20071127-cvs -nx

runtest completed at Fri Nov 30 10:39:16 2007


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU,  for vDSO32)
  2007-11-30  7:42         ` Vladimir Prus
@ 2007-11-30 11:10           ` Jan Kratochvil
  2007-11-30 14:56             ` Daniel Jacobowitz
  0 siblings, 1 reply; 46+ messages in thread
From: Jan Kratochvil @ 2007-11-30 11:10 UTC (permalink / raw)
  To: Vladimir Prus; +Cc: gdb-patches

On Fri, 30 Nov 2007 08:42:00 +0100, Vladimir Prus wrote:
> Jan,
> I'm afraid this patch causes the following regression for me:
> 
>         FAIL: gdb.threads/thread_check.exp: breakpoint at tf
> 
> This FAIL disappears as soon as I revert to revision 1.236 of 
> gdb/dwarf2read.c -- right before your changes. Do you see the same
> failure?

It was not reproducible for me but the problem is Vladimir's i386 crti.S has
DW_AT_ranges which overlap the main code (due to its .fini part).  The main
code full-symbols get ignored now due to it.

Going to post a fix (try to load symtab for each matching psymtab?) and an
updated testcase.


Thanks,
Jan


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU,  for vDSO32)
  2007-11-30 11:10           ` Jan Kratochvil
@ 2007-11-30 14:56             ` Daniel Jacobowitz
  2007-11-30 15:09               ` Jan Kratochvil
                                 ` (2 more replies)
  0 siblings, 3 replies; 46+ messages in thread
From: Daniel Jacobowitz @ 2007-11-30 14:56 UTC (permalink / raw)
  To: Jan Kratochvil, Jim Blandy; +Cc: Vladimir Prus, gdb-patches

On Fri, Nov 30, 2007 at 12:10:21PM +0100, Jan Kratochvil wrote:
> It was not reproducible for me but the problem is Vladimir's i386 crti.S has
> DW_AT_ranges which overlap the main code (due to its .fini part).  The main
> code full-symbols get ignored now due to it.
> 
> Going to post a fix (try to load symtab for each matching psymtab?) and an
> updated testcase.

Jim, can we get your addrmap changes in as they are, instead of
working on a representation change (which was the state when they were
briefly discussed, in October)?  After that, it's simple to solve this
problem more accurately by using addrmaps for symtabs too, not just
blocks.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU,  for vDSO32)
  2007-11-30 14:56             ` Daniel Jacobowitz
@ 2007-11-30 15:09               ` Jan Kratochvil
  2007-12-01  0:55               ` Jim Blandy
  2007-12-09 20:40               ` [patch] Discontiguous PSYMTABs [Re: [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU, for vDSO32)] Jan Kratochvil
  2 siblings, 0 replies; 46+ messages in thread
From: Jan Kratochvil @ 2007-11-30 15:09 UTC (permalink / raw)
  To: gdb-patches; +Cc: Daniel Jacobowitz, Jim Blandy, Vladimir Prus

[-- Attachment #1: Type: text/plain, Size: 566 bytes --]

On Fri, 30 Nov 2007 15:56:13 +0100, Daniel Jacobowitz wrote:
...
> Jim, can we get your addrmap changes in as they are, instead of
> working on a representation change (which was the state when they were
> briefly discussed, in October)?  After that, it's simple to solve this
> problem more accurately by using addrmaps for symtabs too, not just
> blocks.

Providing at least the updated testcase for now.  It fails now on:
	FAIL: gdb.dwarf2/dw2-ranges.exp: info line func

It would get IMO solved by a right patch for FIND_PC_SECT_SYMTAB otherwise.


Regards,
Jan

[-- Attachment #2: dw2ranges-overlap.patch --]
[-- Type: text/plain, Size: 3358 bytes --]

2007-11-30  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.dwarf2/dw2-ranges2.S: New file.
	* gdb.dwarf2/dw2-ranges.S: Merge the secondary section with `.fini'.
	* gdb.dwarf2/dw2-ranges.exp: Compile also "dw2-ranges2.S" and test also
	its MAIN2 and FUNC2 symbols.

--- ./gdb/testsuite/gdb.dwarf2/dw2-ranges.S	25 Nov 2007 21:40:39 -0000	1.1
+++ ./gdb/testsuite/gdb.dwarf2/dw2-ranges.S	30 Nov 2007 15:01:49 -0000
@@ -32,7 +32,10 @@ main:	.int	0
 	.endfunc
 	.size	main, . - main
 
-	.section	.text.func, "ax", @progbits
+	/* `.fini' section is here to make sure `dw2-ranges.S'
+	   vs. `dw2-ranges2.S' overlap their DW_AT_ranges with eac other.  */
+	.section	.fini, "ax", @progbits
+
 	.globl	func
 	.func	func
 func:	.int	0
--- ./gdb/testsuite/gdb.dwarf2/dw2-ranges.exp	25 Nov 2007 21:40:39 -0000	1.1
+++ ./gdb/testsuite/gdb.dwarf2/dw2-ranges.exp	30 Nov 2007 15:01:49 -0000
@@ -30,9 +30,10 @@ if {![istarget *-*-linux*]
 
 set testfile "dw2-ranges"
 set srcfile ${testfile}.S
-set binfile ${objdir}/${subdir}/${testfile}.o
+set srcfile2 ${testfile}2.S
+set binfile ${objdir}/${subdir}/${testfile}
 
-if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" object debug] != "" } {
+if {[gdb_compile "${srcdir}/${subdir}/${srcfile} ${srcdir}/${subdir}/${srcfile2}" "${binfile}" executable debug] != "" } {
     return -1
 }
 
@@ -46,4 +47,7 @@ gdb_load ${binfile}
 # Wrong output:
 # 	No line number information available for address 0x4
 
+gdb_test "info line main" "Line \[0-9\]* of .* starts at address .* and ends at .*"
 gdb_test "info line func" "Line \[0-9\]* of .* starts at address .* and ends at .*"
+gdb_test "info line main2" "Line \[0-9\]* of .* starts at address .* and ends at .*"
+gdb_test "info line func2" "Line \[0-9\]* of .* starts at address .* and ends at .*"
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ ./gdb/testsuite/gdb.dwarf2/dw2-ranges2.S	30 Nov 2007 15:01:49 -0000
@@ -0,0 +1,43 @@
+/*
+   Copyright 2007 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/>.
+ */
+
+/* Despite the sections below will be adjacent the assembler has to produce
+   DW_AT_ranges as the linker could place both sections at arbitrary locations.
+   */
+
+	/* Without this directive GAS will not emit DWARF2 unless we provide an
+	   instruction to assemble.  We want to avoid any instructions to
+	   remain architecture independent.  */
+	.loc_mark_labels	1
+
+	.text
+
+	.globl	main2
+	.func	main2
+main2:	.int	0
+	.endfunc
+	.size	main2, . - main2
+
+	/* `.fini' section is here to make sure `dw2-ranges.S'
+	   vs. `dw2-ranges2.S' overlap their DW_AT_ranges with eac other.  */
+	.section	.fini, "ax", @progbits
+
+	.globl	func2
+	.func	func2
+func2:	.int	0
+	.endfunc
+	.size	func2, . - func2

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU,  for vDSO32)
  2007-11-30 14:56             ` Daniel Jacobowitz
  2007-11-30 15:09               ` Jan Kratochvil
@ 2007-12-01  0:55               ` Jim Blandy
  2007-12-01 17:30                 ` Joel Brobecker
  2007-12-09 20:40               ` [patch] Discontiguous PSYMTABs [Re: [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU, for vDSO32)] Jan Kratochvil
  2 siblings, 1 reply; 46+ messages in thread
From: Jim Blandy @ 2007-12-01  0:55 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: Vladimir Prus, gdb-patches, Joel Brobecker


Daniel Jacobowitz <drow at false.org> writes:
> On Fri, Nov 30, 2007 at 12:10:21PM +0100, Jan Kratochvil wrote:
>> It was not reproducible for me but the problem is Vladimir's i386 crti.S has
>> DW_AT_ranges which overlap the main code (due to its .fini part).  The main
>> code full-symbols get ignored now due to it.
>> 
>> Going to post a fix (try to load symtab for each matching psymtab?) and an
>> updated testcase.
>
> Jim, can we get your addrmap changes in as they are, instead of
> working on a representation change (which was the state when they were
> briefly discussed, in October)?  After that, it's simple to solve this
> problem more accurately by using addrmaps for symtabs too, not just
> blocks.

Sure, if Joel doesn't object.


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU,  for vDSO32)
  2007-12-01  0:55               ` Jim Blandy
@ 2007-12-01 17:30                 ` Joel Brobecker
  0 siblings, 0 replies; 46+ messages in thread
From: Joel Brobecker @ 2007-12-01 17:30 UTC (permalink / raw)
  To: Jim Blandy; +Cc: Jan Kratochvil, a, Vladimir Prus, gdb-patches

> > Jim, can we get your addrmap changes in as they are, instead of
> > working on a representation change (which was the state when they were
> > briefly discussed, in October)?  After that, it's simple to solve this
> > problem more accurately by using addrmaps for symtabs too, not just
> > blocks.
> 
> Sure, if Joel doesn't object.

Not at all, I remember having some questions, but I liked the idea.

-- 
Joel


^ permalink raw reply	[flat|nested] 46+ messages in thread

* [patch] Discontiguous PSYMTABs  [Re: [patch] Parse DW_AT_ranges  into PSYMTABS (for childless CU, for vDSO32)]
  2007-11-30 14:56             ` Daniel Jacobowitz
  2007-11-30 15:09               ` Jan Kratochvil
  2007-12-01  0:55               ` Jim Blandy
@ 2007-12-09 20:40               ` Jan Kratochvil
  2007-12-10  0:21                 ` [patch] Removal of the FIND_PC_SECT_PSYMTAB search [Re: [patch] Discontiguous PSYMTABs] Jan Kratochvil
                                   ` (2 more replies)
  2 siblings, 3 replies; 46+ messages in thread
From: Jan Kratochvil @ 2007-12-09 20:40 UTC (permalink / raw)
  To: gdb-patches; +Cc: Daniel Jacobowitz, Jim Blandy, Vladimir Prus

[-- Attachment #1: Type: text/plain, Size: 1397 bytes --]

On Fri, 30 Nov 2007 15:56:13 +0100, Daniel Jacobowitz wrote:
> On Fri, Nov 30, 2007 at 12:10:21PM +0100, Jan Kratochvil wrote:
> > It was not reproducible for me but the problem is Vladimir's i386 crti.S has
> > DW_AT_ranges which overlap the main code (due to its .fini part).  The main
> > code full-symbols get ignored now due to it.
...
> Jim, can we get your addrmap changes in as they are,
...
> After that, it's simple to solve this problem more accurately by using
> addrmaps for symtabs too, not just blocks.

Attaching the fix for discontiguous psymtabs based on the addrmap framework.

This one is a conservative one - it tries to just fix it with minimal changes.
It is bidirectionally compatible:
 * Producer (dwarf2read.c) still tries to set the bounds TEXTLOW and TEXTHIGH.
 * Consumer (symtab.c) deals with both set and unset PSYMTABS_ADDRMAP.

#1 With the new OBJFILE->PSYMTABS_ADDRMAP I believe the whole
   PARTIAL_SYMTAB->{TEXTLOW,TEXTHIGH} can be removed.  It requires updating of all
   the producers and consumers.  Dumb producers may even just set contiguous
   ranges in PSYMTABS_ADDRMAP.

#2 My folowup mail will offer a future removal of FIND_PC_SECT_PSYMTAB_IS_VALID.


The included testcase (FAIL->PASS by this patch) is the one posted here as:
	http://sources.redhat.com/ml/gdb-patches/2007-11/msg00565.html

Testsuite has been run on x86_64 Fedora 8.


Regards,
Jan

[-- Attachment #2: gdb-cvs-psymtab-ranges.patch --]
[-- Type: text/plain, Size: 19507 bytes --]

2007-12-09  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* Makefile.in: Update dependencies.
	* dwarf2read.c: Include "addrmap.h"
	(struct dwarf2_cu): New fields RANGES_OFFSET and HAS_RANGES_OFFSET.
	(dwarf2_ranges_read): New prototype.
	(dwarf2_build_psymtabs_hard): Initialize and prepare PSYMTABS_ADDRMAP.
	Add discontiguous range to PSYMTABS_ADDRMAP by DWARF2_RANGES_READ on
	HAS_RANGES_OFFSET, otherwise add there the contiguous range.
	(dwarf2_ranges_read): New parameter RANGES_PST, update the function
	comment for it.  Add the found ranges to RANGES_PST.  New variable
	BASEADDR, initialize it the common way.
	(dwarf2_get_pc_bounds): Update the caller for the new parameter.
	(read_partial_die): `DW_AT_ranges' now only sets RANGES_OFFSET and
	HAS_RANGES_OFFSET for the later processing.
	* objfiles.h (struct objfile): New field PSYMTABS_ADDRMAP.
	* symtab.c: Include "addrmap.h"
	(find_pc_sect_psymtab): Support reading the field PSYMTABS_ADDRMAP.
	Move the psymtab locator into ...
	(find_pc_sect_psymtab_is_valid): ... a new function.

2007-11-30  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.dwarf2/dw2-ranges2.S: New file.
	* gdb.dwarf2/dw2-ranges.S: Merge the secondary section with `.fini'.
	* gdb.dwarf2/dw2-ranges.exp: Compile also "dw2-ranges2.S" and test also
	its MAIN2 and FUNC2 symbols.

--- ./gdb/Makefile.in	6 Dec 2007 11:17:03 -0000	1.965
+++ ./gdb/Makefile.in	9 Dec 2007 19:45:11 -0000
@@ -2057,7 +2057,8 @@ dwarf2-frame.o: dwarf2-frame.c $(defs_h)
 dwarf2loc.o: dwarf2loc.c $(defs_h) $(ui_out_h) $(value_h) $(frame_h) \
 	$(gdbcore_h) $(target_h) $(inferior_h) $(ax_h) $(ax_gdb_h) \
 	$(regcache_h) $(objfiles_h) $(exceptions_h) $(elf_dwarf2_h) \
-	$(dwarf2expr_h) $(dwarf2loc_h) $(gdb_string_h) $(gdb_assert_h)
+	$(dwarf2expr_h) $(dwarf2loc_h) $(gdb_string_h) $(gdb_assert_h) \
+	$(addrmap_h)
 dwarf2read.o: dwarf2read.c $(defs_h) $(bfd_h) $(symtab_h) $(gdbtypes_h) \
 	$(objfiles_h) $(elf_dwarf2_h) $(buildsym_h) $(demangle_h) \
 	$(expression_h) $(filenames_h) $(macrotab_h) $(language_h) \
@@ -2856,7 +2857,7 @@ symtab.o: symtab.c $(defs_h) $(symtab_h)
 	$(filenames_h) $(objc_lang_h) $(ada_lang_h) $(hashtab_h) \
 	$(gdb_obstack_h) $(block_h) $(dictionary_h) $(gdb_string_h) \
 	$(gdb_stat_h) $(cp_abi_h) $(observer_h) $(gdb_assert_h) \
-	$(solist_h) $(p_lang_h)
+	$(solist_h) $(p_lang_h) $(addrmap_h)
 target.o: target.c $(defs_h) $(gdb_string_h) $(target_h) $(gdbcmd_h) \
 	$(symtab_h) $(inferior_h) $(bfd_h) $(symfile_h) $(objfiles_h) \
 	$(gdb_wait_h) $(dcache_h) $(regcache_h) $(gdb_assert_h) $(gdbcore_h) \
--- ./gdb/dwarf2read.c	4 Dec 2007 23:43:57 -0000	1.239
+++ ./gdb/dwarf2read.c	9 Dec 2007 19:45:23 -0000
@@ -45,6 +45,7 @@
 #include "hashtab.h"
 #include "command.h"
 #include "gdbcmd.h"
+#include "addrmap.h"
 
 #include <fcntl.h>
 #include "gdb_string.h"
@@ -297,6 +298,9 @@ struct dwarf2_cu
   /* Hash table holding all the loaded partial DIEs.  */
   htab_t partial_dies;
 
+  /* `.debug_ranges' offset for this `DW_TAG_compile_unit' DIE.  */
+  unsigned long ranges_offset;
+
   /* Storage for things with the same lifetime as this read-in compilation
      unit, including partial DIEs.  */
   struct obstack comp_unit_obstack;
@@ -339,6 +343,9 @@ struct dwarf2_cu
      DIEs for namespaces, we don't need to try to infer them
      from mangled names.  */
   unsigned int has_namespace_info : 1;
+
+  /* Field `ranges_offset' is filled in; flag as the value may be zero.  */
+  unsigned int has_ranges_offset : 1;
 };
 
 /* Persistent data held for a compilation unit, even when not
@@ -888,6 +895,9 @@ static void read_func_scope (struct die_
 
 static void read_lexical_block_scope (struct die_info *, struct dwarf2_cu *);
 
+static int dwarf2_ranges_read (unsigned, CORE_ADDR *, CORE_ADDR *,
+			       struct dwarf2_cu *, struct partial_symtab *);
+
 static int dwarf2_get_pc_bounds (struct die_info *,
 				 CORE_ADDR *, CORE_ADDR *, struct dwarf2_cu *);
 
@@ -1416,6 +1426,9 @@ dwarf2_build_psymtabs_hard (struct objfi
 
   create_all_comp_units (objfile);
 
+  objfile->psymtabs_addrmap = addrmap_create_mutable
+						    (&objfile->objfile_obstack);
+
   /* Since the objects we're extracting from .debug_info vary in
      length, only the individual functions to extract them (like
      read_comp_unit_head and load_partial_die) can really know whether
@@ -1481,7 +1494,8 @@ dwarf2_build_psymtabs_hard (struct objfi
       /* Allocate a new partial symbol table structure */
       pst = start_psymtab_common (objfile, objfile->section_offsets,
 				  comp_unit_die.name ? comp_unit_die.name : "",
-				  comp_unit_die.lowpc,
+				  /* TEXTLOW and TEXTHIGH are set below.  */
+				  0,
 				  objfile->global_psymbols.next,
 				  objfile->static_psymbols.next);
 
@@ -1514,6 +1528,15 @@ dwarf2_build_psymtabs_hard (struct objfi
 
       this_cu->psymtab = pst;
 
+      /* Possibly set the default values of LOWPC and HIGHPC from
+         `DW_AT_ranges'.  */
+      if (cu.has_ranges_offset)
+	{
+	  if (dwarf2_ranges_read (cu.ranges_offset, &comp_unit_die.lowpc,
+				  &comp_unit_die.highpc, &cu, pst))
+	    comp_unit_die.has_pc_info = 1;
+	}
+
       /* Check if comp unit has_children.
          If so, read the rest of the partial symbols from this comp unit.
          If not, there's no more debug_info for this comp unit. */
@@ -1544,6 +1567,11 @@ dwarf2_build_psymtabs_hard (struct objfi
       pst->textlow = comp_unit_die.lowpc + baseaddr;
       pst->texthigh = comp_unit_die.highpc + baseaddr;
 
+      /* Store the contiguous range; `DW_AT_ranges' range is stored above.  */
+      if (!cu.has_ranges_offset)
+	addrmap_set_empty (objfile->psymtabs_addrmap, pst->textlow,
+			   pst->texthigh - 1, pst);
+
       pst->n_global_syms = objfile->global_psymbols.next -
 	(objfile->global_psymbols.list + pst->globals_offset);
       pst->n_static_syms = objfile->static_psymbols.next -
@@ -1567,6 +1595,10 @@ dwarf2_build_psymtabs_hard (struct objfi
 
       do_cleanups (back_to_inner);
     }
+
+  objfile->psymtabs_addrmap = addrmap_create_fixed (objfile->psymtabs_addrmap,
+						    &objfile->objfile_obstack);
+
   do_cleanups (back_to);
 }
 
@@ -3078,11 +3110,13 @@ read_lexical_block_scope (struct die_inf
 }
 
 /* Get low and high pc attributes from DW_AT_ranges attribute value OFFSET.
-   Return 1 if the attributes are present and valid, otherwise, return 0.  */
+   Return 1 if the attributes are present and valid, otherwise, return 0.
+   If RANGES_PST is not NULL we should setup `objfile->psymtabs_addrmap'.  */
 
 static int
 dwarf2_ranges_read (unsigned offset, CORE_ADDR *low_return,
-		    CORE_ADDR *high_return, struct dwarf2_cu *cu)
+		    CORE_ADDR *high_return, struct dwarf2_cu *cu,
+		    struct partial_symtab *ranges_pst)
 {
   struct objfile *objfile = cu->objfile;
   struct comp_unit_head *cu_header = &cu->header;
@@ -3098,6 +3132,7 @@ dwarf2_ranges_read (unsigned offset, COR
   int low_set;
   CORE_ADDR low = 0;
   CORE_ADDR high = 0;
+  CORE_ADDR baseaddr;
 
   found_base = cu_header->base_known;
   base = cu_header->base_address;
@@ -3125,6 +3160,9 @@ dwarf2_ranges_read (unsigned offset, COR
 
   low_set = 0;
 
+  if (ranges_pst != NULL)
+    baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
+
   while (1)
     {
       CORE_ADDR range_beginning, range_end;
@@ -3164,6 +3202,11 @@ dwarf2_ranges_read (unsigned offset, COR
       range_beginning += base;
       range_end += base;
 
+      if (ranges_pst != NULL)
+	addrmap_set_empty (objfile->psymtabs_addrmap,
+			   range_beginning + baseaddr, range_end - 1 + baseaddr,
+			   ranges_pst);
+
       /* FIXME: This is recording everything as a low-high
 	 segment of consecutive addresses.  We should have a
 	 data structure for discontiguous block ranges
@@ -3228,7 +3271,7 @@ dwarf2_get_pc_bounds (struct die_info *d
 	{
 	  /* Value of the DW_AT_ranges attribute is the offset in the
 	     .debug_ranges section.  */
-	  if (!dwarf2_ranges_read (DW_UNSND (attr), &low, &high, cu))
+	  if (!dwarf2_ranges_read (DW_UNSND (attr), &low, &high, cu, NULL))
 	    return 0;
 	  /* Found discontinuous range of addresses.  */
 	  ret = -1;
@@ -5663,9 +5706,11 @@ read_partial_die (struct partial_die_inf
 	  part_die->highpc = DW_ADDR (&attr);
 	  break;
 	case DW_AT_ranges:
-	  if (dwarf2_ranges_read (DW_UNSND (&attr), &part_die->lowpc,
-				  &part_die->highpc, cu))
-	    has_low_pc_attr = has_high_pc_attr = 1;
+	  if (part_die->tag == DW_TAG_compile_unit)
+	    {
+	      cu->ranges_offset = DW_UNSND (&attr);
+	      cu->has_ranges_offset = 1;
+	    }
 	  break;
 	case DW_AT_location:
           /* Support the .debug_loc offsets */
--- ./gdb/objfiles.h	4 Dec 2007 23:33:00 -0000	1.47
+++ ./gdb/objfiles.h	9 Dec 2007 19:45:27 -0000
@@ -220,6 +220,12 @@ struct objfile
 
     struct partial_symtab *psymtabs;
 
+    /* Map addresses to the entries of PSYMTABS.  It would be more efficient to
+       have a map per the whole process but ADDRMAP cannot selectively remove
+       its items during FREE_OBJFILE.  */
+
+    struct addrmap *psymtabs_addrmap;
+
     /* List of freed partial symtabs, available for re-use */
 
     struct partial_symtab *free_psymtabs;
--- ./gdb/symtab.c	24 Oct 2007 13:25:16 -0000	1.167
+++ ./gdb/symtab.c	9 Dec 2007 19:45:38 -0000
@@ -41,6 +41,7 @@
 #include "objc-lang.h"
 #include "ada-lang.h"
 #include "p-lang.h"
+#include "addrmap.h"
 
 #include "hashtab.h"
 
@@ -759,6 +760,83 @@ matching_bfd_sections (asection *first, 
   return 0;
 }
 
+/* Find which partial symtab contains PC and SECTION starting at psymtab PST.
+   We may find a different psymtab than PST.  See FIND_PC_SECT_PSYMTAB.  */
+
+struct partial_symtab *
+find_pc_sect_psymtab_is_valid (CORE_ADDR pc, asection *section,
+			       struct partial_symtab *pst,
+			       struct minimal_symbol *msymbol)
+{
+  struct objfile *objfile = pst->objfile;
+  struct partial_symtab *tpst;
+  struct partial_symtab *best_pst = pst;
+  CORE_ADDR best_addr = pst->textlow;
+
+  /* An objfile that has its functions reordered might have
+     many partial symbol tables containing the PC, but
+     we want the partial symbol table that contains the
+     function containing the PC.  */
+  if (!(objfile->flags & OBJF_REORDERED) &&
+      section == 0)	/* can't validate section this way */
+    return pst;
+
+  if (msymbol == NULL)
+    return (pst);
+
+  /* The code range of partial symtabs sometimes overlap, so, in
+     the loop below, we need to check all partial symtabs and
+     find the one that fits better for the given PC address. We
+     select the partial symtab that contains a symbol whose
+     address is closest to the PC address.  By closest we mean
+     that find_pc_sect_symbol returns the symbol with address
+     that is closest and still less than the given PC.  */
+  for (tpst = pst; tpst != NULL; tpst = tpst->next)
+    {
+      if (pc >= tpst->textlow && pc < tpst->texthigh)
+	{
+	  struct partial_symbol *p;
+	  CORE_ADDR this_addr;
+
+	  /* NOTE: This assumes that every psymbol has a
+	     corresponding msymbol, which is not necessarily
+	     true; the debug info might be much richer than the
+	     object's symbol table.  */
+	  p = find_pc_sect_psymbol (tpst, pc, section);
+	  if (p != NULL
+	      && SYMBOL_VALUE_ADDRESS (p)
+	      == SYMBOL_VALUE_ADDRESS (msymbol))
+	    return tpst;
+
+	  /* Also accept the textlow value of a psymtab as a
+	     "symbol", to provide some support for partial
+	     symbol tables with line information but no debug
+	     symbols (e.g. those produced by an assembler).  */
+	  if (p != NULL)
+	    this_addr = SYMBOL_VALUE_ADDRESS (p);
+	  else
+	    this_addr = tpst->textlow;
+
+	  /* Check whether it is closer than our current
+	     BEST_ADDR.  Since this symbol address is
+	     necessarily lower or equal to PC, the symbol closer
+	     to PC is the symbol which address is the highest.
+	     This way we return the psymtab which contains such
+	     best match symbol. This can help in cases where the
+	     symbol information/debuginfo is not complete, like
+	     for instance on IRIX6 with gcc, where no debug info
+	     is emitted for statics. (See also the nodebug.exp
+	     testcase.) */
+	  if (this_addr > best_addr)
+	    {
+	      best_addr = this_addr;
+	      best_pst = tpst;
+	    }
+	}
+    }
+  return best_pst;
+}
+
 /* Find which partial symtab contains PC and SECTION.  Return 0 if
    none.  We return the psymtab that contains a symbol whose address
    exactly matches PC, or, if we cannot find an exact match, the
@@ -766,7 +844,6 @@ matching_bfd_sections (asection *first, 
 struct partial_symtab *
 find_pc_sect_psymtab (CORE_ADDR pc, asection *section)
 {
-  struct partial_symtab *pst;
   struct objfile *objfile;
   struct minimal_symbol *msymbol;
 
@@ -782,79 +859,43 @@ find_pc_sect_psymtab (CORE_ADDR pc, asec
 	  || msymbol->type == mst_file_bss))
     return NULL;
 
-  ALL_PSYMTABS (objfile, pst)
-  {
-    if (pc >= pst->textlow && pc < pst->texthigh)
-      {
-	struct partial_symtab *tpst;
-	struct partial_symtab *best_pst = pst;
-	CORE_ADDR best_addr = pst->textlow;
-
-	/* An objfile that has its functions reordered might have
-	   many partial symbol tables containing the PC, but
-	   we want the partial symbol table that contains the
-	   function containing the PC.  */
-	if (!(objfile->flags & OBJF_REORDERED) &&
-	    section == 0)	/* can't validate section this way */
-	  return (pst);
-
-	if (msymbol == NULL)
-	  return (pst);
-
-	/* The code range of partial symtabs sometimes overlap, so, in
-	   the loop below, we need to check all partial symtabs and
-	   find the one that fits better for the given PC address. We
-	   select the partial symtab that contains a symbol whose
-	   address is closest to the PC address.  By closest we mean
-	   that find_pc_sect_symbol returns the symbol with address
-	   that is closest and still less than the given PC.  */
-	for (tpst = pst; tpst != NULL; tpst = tpst->next)
-	  {
-	    if (pc >= tpst->textlow && pc < tpst->texthigh)
+  ALL_OBJFILES (objfile)
+    {
+      if (objfile->psymtabs_addrmap != NULL)
+        {
+	  struct partial_symtab *pst;
+
+	  pst = addrmap_find (objfile->psymtabs_addrmap, pc);
+	  if (pst != NULL)
+	    {
+	      struct partial_symtab *best_pst;
+
+	      best_pst = find_pc_sect_psymtab_is_valid (pc, section, pst,
+							msymbol);
+	      if (best_pst != NULL)
+		return best_pst;
+	    }
+	  /* Existing PSYMTABS_ADDRMAP should cover all the PSYMTABS of
+	     OBJFILE, there is no need to scan the remaining ones by hand.  */
+	}
+      else
+        {
+	  struct partial_symtab *pst;
+
+	  ALL_OBJFILE_PSYMTABS (objfile, pst)
+	    if (pc >= pst->textlow && pc < pst->texthigh)
 	      {
-		struct partial_symbol *p;
-		CORE_ADDR this_addr;
+		struct partial_symtab *best_pst;
 
-		/* NOTE: This assumes that every psymbol has a
-		   corresponding msymbol, which is not necessarily
-		   true; the debug info might be much richer than the
-		   object's symbol table.  */
-		p = find_pc_sect_psymbol (tpst, pc, section);
-		if (p != NULL
-		    && SYMBOL_VALUE_ADDRESS (p)
-		    == SYMBOL_VALUE_ADDRESS (msymbol))
-		  return (tpst);
-
-		/* Also accept the textlow value of a psymtab as a
-		   "symbol", to provide some support for partial
-		   symbol tables with line information but no debug
-		   symbols (e.g. those produced by an assembler).  */
-		if (p != NULL)
-		  this_addr = SYMBOL_VALUE_ADDRESS (p);
-		else
-		  this_addr = tpst->textlow;
-
-		/* Check whether it is closer than our current
-		   BEST_ADDR.  Since this symbol address is
-		   necessarily lower or equal to PC, the symbol closer
-		   to PC is the symbol which address is the highest.
-		   This way we return the psymtab which contains such
-		   best match symbol. This can help in cases where the
-		   symbol information/debuginfo is not complete, like
-		   for instance on IRIX6 with gcc, where no debug info
-		   is emitted for statics. (See also the nodebug.exp
-		   testcase.) */
-		if (this_addr > best_addr)
-		  {
-		    best_addr = this_addr;
-		    best_pst = tpst;
-		  }
+	        best_pst = find_pc_sect_psymtab_is_valid (pc, section, pst,
+							  msymbol);
+		if (best_pst != NULL)
+		  return best_pst;
 	      }
-	  }
-	return (best_pst);
-      }
-  }
-  return (NULL);
+	}
+    }
+
+  return NULL;
 }
 
 /* Find which partial symtab contains PC.  Return 0 if none. 
--- ./gdb/testsuite/gdb.dwarf2/dw2-ranges.S	25 Nov 2007 21:40:39 -0000	1.1
+++ ./gdb/testsuite/gdb.dwarf2/dw2-ranges.S	30 Nov 2007 15:01:49 -0000
@@ -32,7 +32,10 @@ main:	.int	0
 	.endfunc
 	.size	main, . - main
 
-	.section	.text.func, "ax", @progbits
+	/* `.fini' section is here to make sure `dw2-ranges.S'
+	   vs. `dw2-ranges2.S' overlap their DW_AT_ranges with eac other.  */
+	.section	.fini, "ax", @progbits
+
 	.globl	func
 	.func	func
 func:	.int	0
--- ./gdb/testsuite/gdb.dwarf2/dw2-ranges.exp	25 Nov 2007 21:40:39 -0000	1.1
+++ ./gdb/testsuite/gdb.dwarf2/dw2-ranges.exp	30 Nov 2007 15:01:49 -0000
@@ -30,9 +30,10 @@ if {![istarget *-*-linux*]
 
 set testfile "dw2-ranges"
 set srcfile ${testfile}.S
-set binfile ${objdir}/${subdir}/${testfile}.o
+set srcfile2 ${testfile}2.S
+set binfile ${objdir}/${subdir}/${testfile}
 
-if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" object debug] != "" } {
+if {[gdb_compile "${srcdir}/${subdir}/${srcfile} ${srcdir}/${subdir}/${srcfile2}" "${binfile}" executable debug] != "" } {
     return -1
 }
 
@@ -46,4 +47,7 @@ gdb_load ${binfile}
 # Wrong output:
 # 	No line number information available for address 0x4
 
+gdb_test "info line main" "Line \[0-9\]* of .* starts at address .* and ends at .*"
 gdb_test "info line func" "Line \[0-9\]* of .* starts at address .* and ends at .*"
+gdb_test "info line main2" "Line \[0-9\]* of .* starts at address .* and ends at .*"
+gdb_test "info line func2" "Line \[0-9\]* of .* starts at address .* and ends at .*"
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ ./gdb/testsuite/gdb.dwarf2/dw2-ranges2.S	30 Nov 2007 15:01:49 -0000
@@ -0,0 +1,43 @@
+/*
+   Copyright 2007 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/>.
+ */
+
+/* Despite the sections below will be adjacent the assembler has to produce
+   DW_AT_ranges as the linker could place both sections at arbitrary locations.
+   */
+
+	/* Without this directive GAS will not emit DWARF2 unless we provide an
+	   instruction to assemble.  We want to avoid any instructions to
+	   remain architecture independent.  */
+	.loc_mark_labels	1
+
+	.text
+
+	.globl	main2
+	.func	main2
+main2:	.int	0
+	.endfunc
+	.size	main2, . - main2
+
+	/* `.fini' section is here to make sure `dw2-ranges.S'
+	   vs. `dw2-ranges2.S' overlap their DW_AT_ranges with eac other.  */
+	.section	.fini, "ax", @progbits
+
+	.globl	func2
+	.func	func2
+func2:	.int	0
+	.endfunc
+	.size	func2, . - func2

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [patch] Removal of the FIND_PC_SECT_PSYMTAB search  [Re: [patch]  Discontiguous PSYMTABs]
  2007-12-09 20:40               ` [patch] Discontiguous PSYMTABs [Re: [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU, for vDSO32)] Jan Kratochvil
@ 2007-12-10  0:21                 ` Jan Kratochvil
  2007-12-17  1:02                 ` [patch] Discontiguous PSYMTABs [Re: [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU, for vDSO32)] Daniel Jacobowitz
  2008-04-23 21:31                 ` [patch] Discontiguous PSYMTABs [Re: [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU, for vDSO32)] Doug Evans
  2 siblings, 0 replies; 46+ messages in thread
From: Jan Kratochvil @ 2007-12-10  0:21 UTC (permalink / raw)
  To: gdb-patches; +Cc: Daniel Jacobowitz, Jim Blandy, Vladimir Prus

[-- Attachment #1: Type: text/plain, Size: 641 bytes --]

On Sun, 09 Dec 2007 21:31:38 +0100, Jan Kratochvil wrote:
...
> Attaching the fix for discontiguous psymtabs based on the addrmap framework.
...
> #2 My folowup mail will offer a future removal of FIND_PC_SECT_PSYMTAB_IS_VALID.

This is the promised simplification.  It has no testsuite regressions on x86_64
GNU/Linux but I feel such test is not sufficient.

I guess the heuristic complicated search there was just to deal with
overlapping PSYMTABs - these are now solved the clean way by ADDRMAP.

But some compilers may not produce the right CU DW_AT_ranges and/or the debug
info formats may not support it, I do not know.


Regards,
Jan

[-- Attachment #2: gdb-cvs-psymtab-ranges-radical.patch --]
[-- Type: text/plain, Size: 4445 bytes --]

2007-12-09  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* symtab.c (find_pc_sect_psymtab_is_valid): Remoev the function.
	(find_pc_sect_psymtab): Remove the FIND_PC_SECT_PSYMTAB_IS_VALID call,
	assume the found PSYMTAB is always valid.

diff -u -rup gdb-addrmap-conservative/symtab.c gdb/symtab.c
--- ./gdb-addrmap-conservative/symtab.c	2007-12-09 17:59:30.000000000 +0100
+++ ./gdb/symtab.c	2007-12-09 20:42:33.000000000 +0100
@@ -760,83 +760,6 @@ matching_bfd_sections (asection *first, 
   return 0;
 }
 
-/* Find which partial symtab contains PC and SECTION starting at psymtab PST.
-   We may find a different psymtab than PST.  See FIND_PC_SECT_PSYMTAB.  */
-
-struct partial_symtab *
-find_pc_sect_psymtab_is_valid (CORE_ADDR pc, asection *section,
-			       struct partial_symtab *pst,
-			       struct minimal_symbol *msymbol)
-{
-  struct objfile *objfile = pst->objfile;
-  struct partial_symtab *tpst;
-  struct partial_symtab *best_pst = pst;
-  CORE_ADDR best_addr = pst->textlow;
-
-  /* An objfile that has its functions reordered might have
-     many partial symbol tables containing the PC, but
-     we want the partial symbol table that contains the
-     function containing the PC.  */
-  if (!(objfile->flags & OBJF_REORDERED) &&
-      section == 0)	/* can't validate section this way */
-    return pst;
-
-  if (msymbol == NULL)
-    return (pst);
-
-  /* The code range of partial symtabs sometimes overlap, so, in
-     the loop below, we need to check all partial symtabs and
-     find the one that fits better for the given PC address. We
-     select the partial symtab that contains a symbol whose
-     address is closest to the PC address.  By closest we mean
-     that find_pc_sect_symbol returns the symbol with address
-     that is closest and still less than the given PC.  */
-  for (tpst = pst; tpst != NULL; tpst = tpst->next)
-    {
-      if (pc >= tpst->textlow && pc < tpst->texthigh)
-	{
-	  struct partial_symbol *p;
-	  CORE_ADDR this_addr;
-
-	  /* NOTE: This assumes that every psymbol has a
-	     corresponding msymbol, which is not necessarily
-	     true; the debug info might be much richer than the
-	     object's symbol table.  */
-	  p = find_pc_sect_psymbol (tpst, pc, section);
-	  if (p != NULL
-	      && SYMBOL_VALUE_ADDRESS (p)
-	      == SYMBOL_VALUE_ADDRESS (msymbol))
-	    return tpst;
-
-	  /* Also accept the textlow value of a psymtab as a
-	     "symbol", to provide some support for partial
-	     symbol tables with line information but no debug
-	     symbols (e.g. those produced by an assembler).  */
-	  if (p != NULL)
-	    this_addr = SYMBOL_VALUE_ADDRESS (p);
-	  else
-	    this_addr = tpst->textlow;
-
-	  /* Check whether it is closer than our current
-	     BEST_ADDR.  Since this symbol address is
-	     necessarily lower or equal to PC, the symbol closer
-	     to PC is the symbol which address is the highest.
-	     This way we return the psymtab which contains such
-	     best match symbol. This can help in cases where the
-	     symbol information/debuginfo is not complete, like
-	     for instance on IRIX6 with gcc, where no debug info
-	     is emitted for statics. (See also the nodebug.exp
-	     testcase.) */
-	  if (this_addr > best_addr)
-	    {
-	      best_addr = this_addr;
-	      best_pst = tpst;
-	    }
-	}
-    }
-  return best_pst;
-}
-
 /* Find which partial symtab contains PC and SECTION.  Return 0 if
    none.  We return the psymtab that contains a symbol whose address
    exactly matches PC, or, if we cannot find an exact match, the
@@ -867,14 +790,7 @@ find_pc_sect_psymtab (CORE_ADDR pc, asec
 
 	  pst = addrmap_find (objfile->psymtabs_addrmap, pc);
 	  if (pst != NULL)
-	    {
-	      struct partial_symtab *best_pst;
-
-	      best_pst = find_pc_sect_psymtab_is_valid (pc, section, pst,
-							msymbol);
-	      if (best_pst != NULL)
-		return best_pst;
-	    }
+	    return pst;
 	  /* Existing PSYMTABS_ADDRMAP should cover all the PSYMTABS of
 	     OBJFILE, there is no need to scan the remaining ones by hand.  */
 	}
@@ -884,14 +800,7 @@ find_pc_sect_psymtab (CORE_ADDR pc, asec
 
 	  ALL_OBJFILE_PSYMTABS (objfile, pst)
 	    if (pc >= pst->textlow && pc < pst->texthigh)
-	      {
-		struct partial_symtab *best_pst;
-
-	        best_pst = find_pc_sect_psymtab_is_valid (pc, section, pst,
-							  msymbol);
-		if (best_pst != NULL)
-		  return best_pst;
-	      }
+	      return pst;
 	}
     }
 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [patch] Discontiguous PSYMTABs  [Re: [patch] Parse  DW_AT_ranges into PSYMTABS (for childless CU, for vDSO32)]
  2007-12-09 20:40               ` [patch] Discontiguous PSYMTABs [Re: [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU, for vDSO32)] Jan Kratochvil
  2007-12-10  0:21                 ` [patch] Removal of the FIND_PC_SECT_PSYMTAB search [Re: [patch] Discontiguous PSYMTABs] Jan Kratochvil
@ 2007-12-17  1:02                 ` Daniel Jacobowitz
  2007-12-17  1:03                   ` Daniel Jacobowitz
  2008-04-23 21:31                 ` [patch] Discontiguous PSYMTABs [Re: [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU, for vDSO32)] Doug Evans
  2 siblings, 1 reply; 46+ messages in thread
From: Daniel Jacobowitz @ 2007-12-17  1:02 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches, Jim Blandy, Vladimir Prus

On Sun, Dec 09, 2007 at 09:31:38PM +0100, Jan Kratochvil wrote:
> Attaching the fix for discontiguous psymtabs based on the addrmap framework.
> 
> This one is a conservative one - it tries to just fix it with minimal changes.
> It is bidirectionally compatible:
>  * Producer (dwarf2read.c) still tries to set the bounds TEXTLOW and TEXTHIGH.
>  * Consumer (symtab.c) deals with both set and unset PSYMTABS_ADDRMAP.

This looks OK to me.  Thanks for doing it!

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [patch] Discontiguous PSYMTABs  [Re: [patch] Parse  DW_AT_ranges into PSYMTABS (for childless CU, for vDSO32)]
  2007-12-17  1:02                 ` [patch] Discontiguous PSYMTABs [Re: [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU, for vDSO32)] Daniel Jacobowitz
@ 2007-12-17  1:03                   ` Daniel Jacobowitz
  2007-12-17  2:41                     ` [patch] Discontiguous PSYMTABs Jan Kratochvil
                                       ` (3 more replies)
  0 siblings, 4 replies; 46+ messages in thread
From: Daniel Jacobowitz @ 2007-12-17  1:03 UTC (permalink / raw)
  To: Jan Kratochvil, gdb-patches, Jim Blandy, Vladimir Prus

On Sun, Dec 16, 2007 at 07:56:41PM -0500, Daniel Jacobowitz wrote:
> On Sun, Dec 09, 2007 at 09:31:38PM +0100, Jan Kratochvil wrote:
> > Attaching the fix for discontiguous psymtabs based on the addrmap framework.
> > 
> > This one is a conservative one - it tries to just fix it with minimal changes.
> > It is bidirectionally compatible:
> >  * Producer (dwarf2read.c) still tries to set the bounds TEXTLOW and TEXTHIGH.
> >  * Consumer (symtab.c) deals with both set and unset PSYMTABS_ADDRMAP.
> 
> This looks OK to me.  Thanks for doing it!

Oops, maybe not.  I've just thought of another case where we might
have trouble.  What if one file contains DW_AT_ranges and another file
only contains stabs?

If we trust the addrmap, will we still find the file with stabs?  We
may need to fall through.

I'm sure there are some old versions of GCC which generated
DW_AT_high_pc / DW_AT_low_pc when they should have generated
DW_AT_ranges.  Hopefully we do not need to cater to that.  However,
stabs has no way to represent range information.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [patch] Discontiguous PSYMTABs
  2007-12-17  1:03                   ` Daniel Jacobowitz
@ 2007-12-17  2:41                     ` Jan Kratochvil
  2007-12-17  3:41                       ` Daniel Jacobowitz
  2008-04-23 22:15                     ` [patch] [0/2] " Jan Kratochvil
                                       ` (2 subsequent siblings)
  3 siblings, 1 reply; 46+ messages in thread
From: Jan Kratochvil @ 2007-12-17  2:41 UTC (permalink / raw)
  To: gdb-patches; +Cc: Daniel Jacobowitz

On Mon, 17 Dec 2007 02:02:17 +0100, Daniel Jacobowitz wrote:
> On Sun, Dec 16, 2007 at 07:56:41PM -0500, Daniel Jacobowitz wrote:
> > On Sun, Dec 09, 2007 at 09:31:38PM +0100, Jan Kratochvil wrote:
> > > Attaching the fix for discontiguous psymtabs based on the addrmap framework.
> > > 
> > > This one is a conservative one - it tries to just fix it with minimal changes.
> > > It is bidirectionally compatible:
> > >  * Producer (dwarf2read.c) still tries to set the bounds TEXTLOW and TEXTHIGH.
> > >  * Consumer (symtab.c) deals with both set and unset PSYMTABS_ADDRMAP.
> > 
> > This looks OK to me.  Thanks for doing it!
> 
> Oops, maybe not.  I've just thought of another case where we might
> have trouble.  What if one file contains DW_AT_ranges and another file
> only contains stabs?

"file" here is OBJFILE?  In such case the patch behaves right - it search each
OBJFILE by ALL_OBJFILES separately.

If "file" is CU it is IMO not possible to combine multiple debug formats inside
one OBJFILE, right?


> If we trust the addrmap, will we still find the file with stabs?  We
> may need to fall through.

Existing PSYMTABS_ADDRMAP will catch only the really present addresses, it has
no false positives.  In other cases it should be IMO backward compatible as it
will fallback to the code emulating the old one.


Could you please give a more specific counterexample?



Thanks,
Jan


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [patch] Discontiguous PSYMTABs
  2007-12-17  2:41                     ` [patch] Discontiguous PSYMTABs Jan Kratochvil
@ 2007-12-17  3:41                       ` Daniel Jacobowitz
  0 siblings, 0 replies; 46+ messages in thread
From: Daniel Jacobowitz @ 2007-12-17  3:41 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

On Mon, Dec 17, 2007 at 02:49:31AM +0100, Jan Kratochvil wrote:
> "file" here is OBJFILE?  In such case the patch behaves right - it search each
> OBJFILE by ALL_OBJFILES separately.
> 
> If "file" is CU it is IMO not possible to combine multiple debug formats inside
> one OBJFILE, right?

I meant CU.  It's easy:

gcc -gstabs+ -c break1.c
gcc -gdwarf-2 -c break.c
gcc -o break break1.o break.o

Then both the stabs reader and the DWARF reader will be used for that
binary.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [patch] Discontiguous PSYMTABs [Re: [patch] Parse DW_AT_ranges  into PSYMTABS (for childless CU, for vDSO32)]
  2008-04-23 21:31                 ` [patch] Discontiguous PSYMTABs [Re: [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU, for vDSO32)] Doug Evans
@ 2008-04-23 21:31                   ` Jan Kratochvil
  0 siblings, 0 replies; 46+ messages in thread
From: Jan Kratochvil @ 2008-04-23 21:31 UTC (permalink / raw)
  To: Doug Evans; +Cc: gdb-patches

On Wed, 23 Apr 2008 23:17:41 +0200, Doug Evans wrote:
> Hi.  What's the status of this work?  I ask because I'm tripping over
> a problem here that is fixed by this.

I have prepared the fix with AFAIK no regressions so far but I was going to do
some more testing (not reviewing) before posting it upstream.  But sure it
would be great if more people could check it more.

Posting separately the prepared mails along.


Regards,
Jan


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [patch] Discontiguous PSYMTABs [Re: [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU, for vDSO32)]
  2007-12-09 20:40               ` [patch] Discontiguous PSYMTABs [Re: [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU, for vDSO32)] Jan Kratochvil
  2007-12-10  0:21                 ` [patch] Removal of the FIND_PC_SECT_PSYMTAB search [Re: [patch] Discontiguous PSYMTABs] Jan Kratochvil
  2007-12-17  1:02                 ` [patch] Discontiguous PSYMTABs [Re: [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU, for vDSO32)] Daniel Jacobowitz
@ 2008-04-23 21:31                 ` Doug Evans
  2008-04-23 21:31                   ` Jan Kratochvil
  2 siblings, 1 reply; 46+ messages in thread
From: Doug Evans @ 2008-04-23 21:31 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

Hi.  What's the status of this work?  I ask because I'm tripping over
a problem here that is fixed by this.


On Sun, Dec 9, 2007 at 1:31 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Fri, 30 Nov 2007 15:56:13 +0100, Daniel Jacobowitz wrote:
>  > On Fri, Nov 30, 2007 at 12:10:21PM +0100, Jan Kratochvil wrote:
>  > > It was not reproducible for me but the problem is Vladimir's i386 crti.S has
>  > > DW_AT_ranges which overlap the main code (due to its .fini part).  The main
>  > > code full-symbols get ignored now due to it.
>  ...
>  > Jim, can we get your addrmap changes in as they are,
>  ...
>  > After that, it's simple to solve this problem more accurately by using
>  > addrmaps for symtabs too, not just blocks.
>
>  Attaching the fix for discontiguous psymtabs based on the addrmap framework.
>
>  This one is a conservative one - it tries to just fix it with minimal changes.
>  It is bidirectionally compatible:
>   * Producer (dwarf2read.c) still tries to set the bounds TEXTLOW and TEXTHIGH.
>   * Consumer (symtab.c) deals with both set and unset PSYMTABS_ADDRMAP.
>
>  #1 With the new OBJFILE->PSYMTABS_ADDRMAP I believe the whole
>    PARTIAL_SYMTAB->{TEXTLOW,TEXTHIGH} can be removed.  It requires updating of all
>    the producers and consumers.  Dumb producers may even just set contiguous
>    ranges in PSYMTABS_ADDRMAP.
>
>  #2 My folowup mail will offer a future removal of FIND_PC_SECT_PSYMTAB_IS_VALID.
>
>
>  The included testcase (FAIL->PASS by this patch) is the one posted here as:
>         http://sources.redhat.com/ml/gdb-patches/2007-11/msg00565.html
>
>  Testsuite has been run on x86_64 Fedora 8.
>
>
>  Regards,
>  Jan
>
> 2007-12-09  Jan Kratochvil  <jan.kratochvil@redhat.com>
>
>         * Makefile.in: Update dependencies.
>         * dwarf2read.c: Include "addrmap.h"
>         (struct dwarf2_cu): New fields RANGES_OFFSET and HAS_RANGES_OFFSET.
>         (dwarf2_ranges_read): New prototype.
>         (dwarf2_build_psymtabs_hard): Initialize and prepare PSYMTABS_ADDRMAP.
>         Add discontiguous range to PSYMTABS_ADDRMAP by DWARF2_RANGES_READ on
>         HAS_RANGES_OFFSET, otherwise add there the contiguous range.
>         (dwarf2_ranges_read): New parameter RANGES_PST, update the function
>         comment for it.  Add the found ranges to RANGES_PST.  New variable
>         BASEADDR, initialize it the common way.
>         (dwarf2_get_pc_bounds): Update the caller for the new parameter.
>         (read_partial_die): `DW_AT_ranges' now only sets RANGES_OFFSET and
>         HAS_RANGES_OFFSET for the later processing.
>         * objfiles.h (struct objfile): New field PSYMTABS_ADDRMAP.
>         * symtab.c: Include "addrmap.h"
>         (find_pc_sect_psymtab): Support reading the field PSYMTABS_ADDRMAP.
>         Move the psymtab locator into ...
>         (find_pc_sect_psymtab_is_valid): ... a new function.
>
>  2007-11-30  Jan Kratochvil  <jan.kratochvil@redhat.com>
>
>         * gdb.dwarf2/dw2-ranges2.S: New file.
>         * gdb.dwarf2/dw2-ranges.S: Merge the secondary section with `.fini'.
>         * gdb.dwarf2/dw2-ranges.exp: Compile also "dw2-ranges2.S" and test also
>         its MAIN2 and FUNC2 symbols.
>
>  --- ./gdb/Makefile.in   6 Dec 2007 11:17:03 -0000       1.965
>  +++ ./gdb/Makefile.in   9 Dec 2007 19:45:11 -0000
>  @@ -2057,7 +2057,8 @@ dwarf2-frame.o: dwarf2-frame.c $(defs_h)
>   dwarf2loc.o: dwarf2loc.c $(defs_h) $(ui_out_h) $(value_h) $(frame_h) \
>         $(gdbcore_h) $(target_h) $(inferior_h) $(ax_h) $(ax_gdb_h) \
>         $(regcache_h) $(objfiles_h) $(exceptions_h) $(elf_dwarf2_h) \
>  -       $(dwarf2expr_h) $(dwarf2loc_h) $(gdb_string_h) $(gdb_assert_h)
>  +       $(dwarf2expr_h) $(dwarf2loc_h) $(gdb_string_h) $(gdb_assert_h) \
>  +       $(addrmap_h)
>   dwarf2read.o: dwarf2read.c $(defs_h) $(bfd_h) $(symtab_h) $(gdbtypes_h) \
>         $(objfiles_h) $(elf_dwarf2_h) $(buildsym_h) $(demangle_h) \
>         $(expression_h) $(filenames_h) $(macrotab_h) $(language_h) \
>  @@ -2856,7 +2857,7 @@ symtab.o: symtab.c $(defs_h) $(symtab_h)
>         $(filenames_h) $(objc_lang_h) $(ada_lang_h) $(hashtab_h) \
>         $(gdb_obstack_h) $(block_h) $(dictionary_h) $(gdb_string_h) \
>         $(gdb_stat_h) $(cp_abi_h) $(observer_h) $(gdb_assert_h) \
>  -       $(solist_h) $(p_lang_h)
>  +       $(solist_h) $(p_lang_h) $(addrmap_h)
>   target.o: target.c $(defs_h) $(gdb_string_h) $(target_h) $(gdbcmd_h) \
>         $(symtab_h) $(inferior_h) $(bfd_h) $(symfile_h) $(objfiles_h) \
>         $(gdb_wait_h) $(dcache_h) $(regcache_h) $(gdb_assert_h) $(gdbcore_h) \
>  --- ./gdb/dwarf2read.c  4 Dec 2007 23:43:57 -0000       1.239
>  +++ ./gdb/dwarf2read.c  9 Dec 2007 19:45:23 -0000
>  @@ -45,6 +45,7 @@
>   #include "hashtab.h"
>   #include "command.h"
>   #include "gdbcmd.h"
>  +#include "addrmap.h"
>
>   #include <fcntl.h>
>   #include "gdb_string.h"
>  @@ -297,6 +298,9 @@ struct dwarf2_cu
>    /* Hash table holding all the loaded partial DIEs.  */
>    htab_t partial_dies;
>
>  +  /* `.debug_ranges' offset for this `DW_TAG_compile_unit' DIE.  */
>  +  unsigned long ranges_offset;
>  +
>    /* Storage for things with the same lifetime as this read-in compilation
>       unit, including partial DIEs.  */
>    struct obstack comp_unit_obstack;
>  @@ -339,6 +343,9 @@ struct dwarf2_cu
>       DIEs for namespaces, we don't need to try to infer them
>       from mangled names.  */
>    unsigned int has_namespace_info : 1;
>  +
>  +  /* Field `ranges_offset' is filled in; flag as the value may be zero.  */
>  +  unsigned int has_ranges_offset : 1;
>   };
>
>   /* Persistent data held for a compilation unit, even when not
>  @@ -888,6 +895,9 @@ static void read_func_scope (struct die_
>
>   static void read_lexical_block_scope (struct die_info *, struct dwarf2_cu *);
>
>  +static int dwarf2_ranges_read (unsigned, CORE_ADDR *, CORE_ADDR *,
>  +                              struct dwarf2_cu *, struct partial_symtab *);
>  +
>   static int dwarf2_get_pc_bounds (struct die_info *,
>                                  CORE_ADDR *, CORE_ADDR *, struct dwarf2_cu *);
>
>  @@ -1416,6 +1426,9 @@ dwarf2_build_psymtabs_hard (struct objfi
>
>    create_all_comp_units (objfile);
>
>  +  objfile->psymtabs_addrmap = addrmap_create_mutable
>  +                                                   (&objfile->objfile_obstack);
>  +
>    /* Since the objects we're extracting from .debug_info vary in
>       length, only the individual functions to extract them (like
>       read_comp_unit_head and load_partial_die) can really know whether
>  @@ -1481,7 +1494,8 @@ dwarf2_build_psymtabs_hard (struct objfi
>        /* Allocate a new partial symbol table structure */
>        pst = start_psymtab_common (objfile, objfile->section_offsets,
>                                   comp_unit_die.name ? comp_unit_die.name : "",
>  -                                 comp_unit_die.lowpc,
>  +                                 /* TEXTLOW and TEXTHIGH are set below.  */
>  +                                 0,
>                                   objfile->global_psymbols.next,
>                                   objfile->static_psymbols.next);
>
>  @@ -1514,6 +1528,15 @@ dwarf2_build_psymtabs_hard (struct objfi
>
>        this_cu->psymtab = pst;
>
>  +      /* Possibly set the default values of LOWPC and HIGHPC from
>  +         `DW_AT_ranges'.  */
>  +      if (cu.has_ranges_offset)
>  +       {
>  +         if (dwarf2_ranges_read (cu.ranges_offset, &comp_unit_die.lowpc,
>  +                                 &comp_unit_die.highpc, &cu, pst))
>  +           comp_unit_die.has_pc_info = 1;
>  +       }
>  +
>        /* Check if comp unit has_children.
>           If so, read the rest of the partial symbols from this comp unit.
>           If not, there's no more debug_info for this comp unit. */
>  @@ -1544,6 +1567,11 @@ dwarf2_build_psymtabs_hard (struct objfi
>        pst->textlow = comp_unit_die.lowpc + baseaddr;
>        pst->texthigh = comp_unit_die.highpc + baseaddr;
>
>  +      /* Store the contiguous range; `DW_AT_ranges' range is stored above.  */
>  +      if (!cu.has_ranges_offset)
>  +       addrmap_set_empty (objfile->psymtabs_addrmap, pst->textlow,
>  +                          pst->texthigh - 1, pst);
>  +
>        pst->n_global_syms = objfile->global_psymbols.next -
>         (objfile->global_psymbols.list + pst->globals_offset);
>        pst->n_static_syms = objfile->static_psymbols.next -
>  @@ -1567,6 +1595,10 @@ dwarf2_build_psymtabs_hard (struct objfi
>
>        do_cleanups (back_to_inner);
>      }
>  +
>  +  objfile->psymtabs_addrmap = addrmap_create_fixed (objfile->psymtabs_addrmap,
>  +                                                   &objfile->objfile_obstack);
>  +
>    do_cleanups (back_to);
>   }
>
>  @@ -3078,11 +3110,13 @@ read_lexical_block_scope (struct die_inf
>   }
>
>   /* Get low and high pc attributes from DW_AT_ranges attribute value OFFSET.
>  -   Return 1 if the attributes are present and valid, otherwise, return 0.  */
>  +   Return 1 if the attributes are present and valid, otherwise, return 0.
>  +   If RANGES_PST is not NULL we should setup `objfile->psymtabs_addrmap'.  */
>
>   static int
>   dwarf2_ranges_read (unsigned offset, CORE_ADDR *low_return,
>  -                   CORE_ADDR *high_return, struct dwarf2_cu *cu)
>  +                   CORE_ADDR *high_return, struct dwarf2_cu *cu,
>  +                   struct partial_symtab *ranges_pst)
>   {
>    struct objfile *objfile = cu->objfile;
>    struct comp_unit_head *cu_header = &cu->header;
>  @@ -3098,6 +3132,7 @@ dwarf2_ranges_read (unsigned offset, COR
>    int low_set;
>    CORE_ADDR low = 0;
>    CORE_ADDR high = 0;
>  +  CORE_ADDR baseaddr;
>
>    found_base = cu_header->base_known;
>    base = cu_header->base_address;
>  @@ -3125,6 +3160,9 @@ dwarf2_ranges_read (unsigned offset, COR
>
>    low_set = 0;
>
>  +  if (ranges_pst != NULL)
>  +    baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
>  +
>    while (1)
>      {
>        CORE_ADDR range_beginning, range_end;
>  @@ -3164,6 +3202,11 @@ dwarf2_ranges_read (unsigned offset, COR
>        range_beginning += base;
>        range_end += base;
>
>  +      if (ranges_pst != NULL)
>  +       addrmap_set_empty (objfile->psymtabs_addrmap,
>  +                          range_beginning + baseaddr, range_end - 1 + baseaddr,
>  +                          ranges_pst);
>  +
>        /* FIXME: This is recording everything as a low-high
>          segment of consecutive addresses.  We should have a
>          data structure for discontiguous block ranges
>  @@ -3228,7 +3271,7 @@ dwarf2_get_pc_bounds (struct die_info *d
>         {
>           /* Value of the DW_AT_ranges attribute is the offset in the
>              .debug_ranges section.  */
>  -         if (!dwarf2_ranges_read (DW_UNSND (attr), &low, &high, cu))
>  +         if (!dwarf2_ranges_read (DW_UNSND (attr), &low, &high, cu, NULL))
>             return 0;
>           /* Found discontinuous range of addresses.  */
>           ret = -1;
>  @@ -5663,9 +5706,11 @@ read_partial_die (struct partial_die_inf
>           part_die->highpc = DW_ADDR (&attr);
>           break;
>         case DW_AT_ranges:
>  -         if (dwarf2_ranges_read (DW_UNSND (&attr), &part_die->lowpc,
>  -                                 &part_die->highpc, cu))
>  -           has_low_pc_attr = has_high_pc_attr = 1;
>  +         if (part_die->tag == DW_TAG_compile_unit)
>  +           {
>  +             cu->ranges_offset = DW_UNSND (&attr);
>  +             cu->has_ranges_offset = 1;
>  +           }
>           break;
>         case DW_AT_location:
>            /* Support the .debug_loc offsets */
>  --- ./gdb/objfiles.h    4 Dec 2007 23:33:00 -0000       1.47
>  +++ ./gdb/objfiles.h    9 Dec 2007 19:45:27 -0000
>  @@ -220,6 +220,12 @@ struct objfile
>
>      struct partial_symtab *psymtabs;
>
>  +    /* Map addresses to the entries of PSYMTABS.  It would be more efficient to
>  +       have a map per the whole process but ADDRMAP cannot selectively remove
>  +       its items during FREE_OBJFILE.  */
>  +
>  +    struct addrmap *psymtabs_addrmap;
>  +
>      /* List of freed partial symtabs, available for re-use */
>
>      struct partial_symtab *free_psymtabs;
>  --- ./gdb/symtab.c      24 Oct 2007 13:25:16 -0000      1.167
>  +++ ./gdb/symtab.c      9 Dec 2007 19:45:38 -0000
>  @@ -41,6 +41,7 @@
>   #include "objc-lang.h"
>   #include "ada-lang.h"
>   #include "p-lang.h"
>  +#include "addrmap.h"
>
>   #include "hashtab.h"
>
>  @@ -759,6 +760,83 @@ matching_bfd_sections (asection *first,
>    return 0;
>   }
>
>  +/* Find which partial symtab contains PC and SECTION starting at psymtab PST.
>  +   We may find a different psymtab than PST.  See FIND_PC_SECT_PSYMTAB.  */
>  +
>  +struct partial_symtab *
>  +find_pc_sect_psymtab_is_valid (CORE_ADDR pc, asection *section,
>  +                              struct partial_symtab *pst,
>  +                              struct minimal_symbol *msymbol)
>  +{
>  +  struct objfile *objfile = pst->objfile;
>  +  struct partial_symtab *tpst;
>  +  struct partial_symtab *best_pst = pst;
>  +  CORE_ADDR best_addr = pst->textlow;
>  +
>  +  /* An objfile that has its functions reordered might have
>  +     many partial symbol tables containing the PC, but
>  +     we want the partial symbol table that contains the
>  +     function containing the PC.  */
>  +  if (!(objfile->flags & OBJF_REORDERED) &&
>  +      section == 0)    /* can't validate section this way */
>  +    return pst;
>  +
>  +  if (msymbol == NULL)
>  +    return (pst);
>  +
>  +  /* The code range of partial symtabs sometimes overlap, so, in
>  +     the loop below, we need to check all partial symtabs and
>  +     find the one that fits better for the given PC address. We
>  +     select the partial symtab that contains a symbol whose
>  +     address is closest to the PC address.  By closest we mean
>  +     that find_pc_sect_symbol returns the symbol with address
>  +     that is closest and still less than the given PC.  */
>  +  for (tpst = pst; tpst != NULL; tpst = tpst->next)
>  +    {
>  +      if (pc >= tpst->textlow && pc < tpst->texthigh)
>  +       {
>  +         struct partial_symbol *p;
>  +         CORE_ADDR this_addr;
>  +
>  +         /* NOTE: This assumes that every psymbol has a
>  +            corresponding msymbol, which is not necessarily
>  +            true; the debug info might be much richer than the
>  +            object's symbol table.  */
>  +         p = find_pc_sect_psymbol (tpst, pc, section);
>  +         if (p != NULL
>  +             && SYMBOL_VALUE_ADDRESS (p)
>  +             == SYMBOL_VALUE_ADDRESS (msymbol))
>  +           return tpst;
>  +
>  +         /* Also accept the textlow value of a psymtab as a
>  +            "symbol", to provide some support for partial
>  +            symbol tables with line information but no debug
>  +            symbols (e.g. those produced by an assembler).  */
>  +         if (p != NULL)
>  +           this_addr = SYMBOL_VALUE_ADDRESS (p);
>  +         else
>  +           this_addr = tpst->textlow;
>  +
>  +         /* Check whether it is closer than our current
>  +            BEST_ADDR.  Since this symbol address is
>  +            necessarily lower or equal to PC, the symbol closer
>  +            to PC is the symbol which address is the highest.
>  +            This way we return the psymtab which contains such
>  +            best match symbol. This can help in cases where the
>  +            symbol information/debuginfo is not complete, like
>  +            for instance on IRIX6 with gcc, where no debug info
>  +            is emitted for statics. (See also the nodebug.exp
>  +            testcase.) */
>  +         if (this_addr > best_addr)
>  +           {
>  +             best_addr = this_addr;
>  +             best_pst = tpst;
>  +           }
>  +       }
>  +    }
>  +  return best_pst;
>  +}
>  +
>   /* Find which partial symtab contains PC and SECTION.  Return 0 if
>     none.  We return the psymtab that contains a symbol whose address
>     exactly matches PC, or, if we cannot find an exact match, the
>  @@ -766,7 +844,6 @@ matching_bfd_sections (asection *first,
>   struct partial_symtab *
>   find_pc_sect_psymtab (CORE_ADDR pc, asection *section)
>   {
>  -  struct partial_symtab *pst;
>    struct objfile *objfile;
>    struct minimal_symbol *msymbol;
>
>  @@ -782,79 +859,43 @@ find_pc_sect_psymtab (CORE_ADDR pc, asec
>           || msymbol->type == mst_file_bss))
>      return NULL;
>
>  -  ALL_PSYMTABS (objfile, pst)
>  -  {
>  -    if (pc >= pst->textlow && pc < pst->texthigh)
>  -      {
>  -       struct partial_symtab *tpst;
>  -       struct partial_symtab *best_pst = pst;
>  -       CORE_ADDR best_addr = pst->textlow;
>  -
>  -       /* An objfile that has its functions reordered might have
>  -          many partial symbol tables containing the PC, but
>  -          we want the partial symbol table that contains the
>  -          function containing the PC.  */
>  -       if (!(objfile->flags & OBJF_REORDERED) &&
>  -           section == 0)       /* can't validate section this way */
>  -         return (pst);
>  -
>  -       if (msymbol == NULL)
>  -         return (pst);
>  -
>  -       /* The code range of partial symtabs sometimes overlap, so, in
>  -          the loop below, we need to check all partial symtabs and
>  -          find the one that fits better for the given PC address. We
>  -          select the partial symtab that contains a symbol whose
>  -          address is closest to the PC address.  By closest we mean
>  -          that find_pc_sect_symbol returns the symbol with address
>  -          that is closest and still less than the given PC.  */
>  -       for (tpst = pst; tpst != NULL; tpst = tpst->next)
>  -         {
>  -           if (pc >= tpst->textlow && pc < tpst->texthigh)
>  +  ALL_OBJFILES (objfile)
>  +    {
>  +      if (objfile->psymtabs_addrmap != NULL)
>  +        {
>  +         struct partial_symtab *pst;
>  +
>  +         pst = addrmap_find (objfile->psymtabs_addrmap, pc);
>  +         if (pst != NULL)
>  +           {
>  +             struct partial_symtab *best_pst;
>  +
>  +             best_pst = find_pc_sect_psymtab_is_valid (pc, section, pst,
>  +                                                       msymbol);
>  +             if (best_pst != NULL)
>  +               return best_pst;
>  +           }
>  +         /* Existing PSYMTABS_ADDRMAP should cover all the PSYMTABS of
>  +            OBJFILE, there is no need to scan the remaining ones by hand.  */
>  +       }
>  +      else
>  +        {
>  +         struct partial_symtab *pst;
>  +
>  +         ALL_OBJFILE_PSYMTABS (objfile, pst)
>  +           if (pc >= pst->textlow && pc < pst->texthigh)
>               {
>  -               struct partial_symbol *p;
>  -               CORE_ADDR this_addr;
>  +               struct partial_symtab *best_pst;
>
>  -               /* NOTE: This assumes that every psymbol has a
>  -                  corresponding msymbol, which is not necessarily
>  -                  true; the debug info might be much richer than the
>  -                  object's symbol table.  */
>  -               p = find_pc_sect_psymbol (tpst, pc, section);
>  -               if (p != NULL
>  -                   && SYMBOL_VALUE_ADDRESS (p)
>  -                   == SYMBOL_VALUE_ADDRESS (msymbol))
>  -                 return (tpst);
>  -
>  -               /* Also accept the textlow value of a psymtab as a
>  -                  "symbol", to provide some support for partial
>  -                  symbol tables with line information but no debug
>  -                  symbols (e.g. those produced by an assembler).  */
>  -               if (p != NULL)
>  -                 this_addr = SYMBOL_VALUE_ADDRESS (p);
>  -               else
>  -                 this_addr = tpst->textlow;
>  -
>  -               /* Check whether it is closer than our current
>  -                  BEST_ADDR.  Since this symbol address is
>  -                  necessarily lower or equal to PC, the symbol closer
>  -                  to PC is the symbol which address is the highest.
>  -                  This way we return the psymtab which contains such
>  -                  best match symbol. This can help in cases where the
>  -                  symbol information/debuginfo is not complete, like
>  -                  for instance on IRIX6 with gcc, where no debug info
>  -                  is emitted for statics. (See also the nodebug.exp
>  -                  testcase.) */
>  -               if (this_addr > best_addr)
>  -                 {
>  -                   best_addr = this_addr;
>  -                   best_pst = tpst;
>  -                 }
>  +               best_pst = find_pc_sect_psymtab_is_valid (pc, section, pst,
>  +                                                         msymbol);
>  +               if (best_pst != NULL)
>  +                 return best_pst;
>               }
>  -         }
>  -       return (best_pst);
>  -      }
>  -  }
>  -  return (NULL);
>  +       }
>  +    }
>  +
>  +  return NULL;
>   }
>
>   /* Find which partial symtab contains PC.  Return 0 if none.
>  --- ./gdb/testsuite/gdb.dwarf2/dw2-ranges.S     25 Nov 2007 21:40:39 -0000      1.1
>  +++ ./gdb/testsuite/gdb.dwarf2/dw2-ranges.S     30 Nov 2007 15:01:49 -0000
>  @@ -32,7 +32,10 @@ main:        .int    0
>         .endfunc
>         .size   main, . - main
>
>  -       .section        .text.func, "ax", @progbits
>  +       /* `.fini' section is here to make sure `dw2-ranges.S'
>  +          vs. `dw2-ranges2.S' overlap their DW_AT_ranges with eac other.  */
>  +       .section        .fini, "ax", @progbits
>  +
>         .globl  func
>         .func   func
>   func:  .int    0
>  --- ./gdb/testsuite/gdb.dwarf2/dw2-ranges.exp   25 Nov 2007 21:40:39 -0000      1.1
>  +++ ./gdb/testsuite/gdb.dwarf2/dw2-ranges.exp   30 Nov 2007 15:01:49 -0000
>  @@ -30,9 +30,10 @@ if {![istarget *-*-linux*]
>
>   set testfile "dw2-ranges"
>   set srcfile ${testfile}.S
>  -set binfile ${objdir}/${subdir}/${testfile}.o
>  +set srcfile2 ${testfile}2.S
>  +set binfile ${objdir}/${subdir}/${testfile}
>
>  -if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" object debug] != "" } {
>  +if {[gdb_compile "${srcdir}/${subdir}/${srcfile} ${srcdir}/${subdir}/${srcfile2}" "${binfile}" executable debug] != "" } {
>      return -1
>   }
>
>  @@ -46,4 +47,7 @@ gdb_load ${binfile}
>   # Wrong output:
>   #      No line number information available for address 0x4
>
>  +gdb_test "info line main" "Line \[0-9\]* of .* starts at address .* and ends at .*"
>   gdb_test "info line func" "Line \[0-9\]* of .* starts at address .* and ends at .*"
>  +gdb_test "info line main2" "Line \[0-9\]* of .* starts at address .* and ends at .*"
>  +gdb_test "info line func2" "Line \[0-9\]* of .* starts at address .* and ends at .*"
>  --- /dev/null   1 Jan 1970 00:00:00 -0000
>  +++ ./gdb/testsuite/gdb.dwarf2/dw2-ranges2.S    30 Nov 2007 15:01:49 -0000
>  @@ -0,0 +1,43 @@
>  +/*
>  +   Copyright 2007 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/>.
>  + */
>  +
>  +/* Despite the sections below will be adjacent the assembler has to produce
>  +   DW_AT_ranges as the linker could place both sections at arbitrary locations.
>  +   */
>  +
>  +       /* Without this directive GAS will not emit DWARF2 unless we provide an
>  +          instruction to assemble.  We want to avoid any instructions to
>  +          remain architecture independent.  */
>  +       .loc_mark_labels        1
>  +
>  +       .text
>  +
>  +       .globl  main2
>  +       .func   main2
>  +main2: .int    0
>  +       .endfunc
>  +       .size   main2, . - main2
>  +
>  +       /* `.fini' section is here to make sure `dw2-ranges.S'
>  +          vs. `dw2-ranges2.S' overlap their DW_AT_ranges with eac other.  */
>  +       .section        .fini, "ax", @progbits
>  +
>  +       .globl  func2
>  +       .func   func2
>  +func2: .int    0
>  +       .endfunc
>  +       .size   func2, . - func2
>
>


^ permalink raw reply	[flat|nested] 46+ messages in thread

* [patch] [0/2] Discontiguous PSYMTABs
  2007-12-17  1:03                   ` Daniel Jacobowitz
  2007-12-17  2:41                     ` [patch] Discontiguous PSYMTABs Jan Kratochvil
@ 2008-04-23 22:15                     ` Jan Kratochvil
  2008-04-23 22:18                     ` [patch] [1/2] Discontiguous PSYMTABs (partial DIEs base address) Jan Kratochvil
  2008-04-23 22:24                     ` [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap) Jan Kratochvil
  3 siblings, 0 replies; 46+ messages in thread
From: Jan Kratochvil @ 2008-04-23 22:15 UTC (permalink / raw)
  To: gdb-patches

Hi,

The original patch
	[patch] Parse DW_AT_ranges into PSYMTABS (for childless CU, for vDSO32)
	http://sourceware.org/ml/gdb-patches/2007-10/msg00207.html
is already committed but it caused a regression
	http://sourceware.org/ml/gdb-patches/2007-11/msg00560.html
and the later patch to fix the regression was non-DWARF2 incompatible
	http://sourceware.org/ml/gdb-patches/2007-12/msg00248.html

Follow two fixes which have no regressions to any previous version.


Regards,
Jan


^ permalink raw reply	[flat|nested] 46+ messages in thread

* [patch] [1/2] Discontiguous PSYMTABs (partial DIEs base address)
  2007-12-17  1:03                   ` Daniel Jacobowitz
  2007-12-17  2:41                     ` [patch] Discontiguous PSYMTABs Jan Kratochvil
  2008-04-23 22:15                     ` [patch] [0/2] " Jan Kratochvil
@ 2008-04-23 22:18                     ` Jan Kratochvil
  2008-05-01 19:43                       ` Daniel Jacobowitz
  2008-04-23 22:24                     ` [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap) Jan Kratochvil
  3 siblings, 1 reply; 46+ messages in thread
From: Jan Kratochvil @ 2008-04-23 22:18 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 781 bytes --]

Hi,

It fixes this testcase run (when `complaints' is on):

-Reading symbols from /home/jkratoch/redhat/sources/gdb/testsuite/gdb.cp/m-static...Invalid .debug_ranges data (no base address)...done.
+Reading symbols from /home/jkratoch/redhat/sources/gdb/testsuite/gdb.cp/m-static...done.

Currently the base address required for the CU DW_AT_ranges resolution is set
only during full symbols reading which is too late for the secondary patch
which uses OBJFILE->PSYMTABS_ADDRMAP built from these ranges to find which
psymtabs expand to the full symtabs (chicken-egg problem).

It has no real benefits but it is a non-regression standalone patch required by
the next patch [2/2].

Going to post the results of some more verifications, posting as Doug Evans has
asked.


Regards,
Jan

[-- Attachment #2: gdb-cvs-psymtab-base_address.patch --]
[-- Type: text/plain, Size: 2138 bytes --]

2008-04-21  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Set CU BASE_ADDRESS already from partial DIEs.
	* dwarf2read.c (read_partial_die): New variables BASE_ADDRESS and
	BASE_ADDRESS_TYPE.  Set these variables from DW_AT_LOW_PC and
	DW_AT_ENTRY_PC.  Set CU->HEADER.BASE_KNOWN and CU->HEADER.BASE_ADDRESS
	from these variables if it was still unset.

--- ./gdb/dwarf2read.c	19 Apr 2008 05:06:54 -0000	1.255
+++ ./gdb/dwarf2read.c	21 Apr 2008 13:49:11 -0000
@@ -5808,6 +5851,15 @@ read_partial_die (struct partial_die_inf
   struct attribute attr;
   int has_low_pc_attr = 0;
   int has_high_pc_attr = 0;
+  CORE_ADDR base_address;
+  enum
+    {
+      base_address_none,
+      base_address_low_pc,
+      /* Overrides BASE_ADDRESS_LOW_PC.  */
+      base_address_entry_pc
+    }
+  base_address_type = base_address_none;
 
   memset (part_die, 0, sizeof (struct partial_die_info));
 
@@ -5845,11 +5897,25 @@ read_partial_die (struct partial_die_inf
 	case DW_AT_low_pc:
 	  has_low_pc_attr = 1;
 	  part_die->lowpc = DW_ADDR (&attr);
+	  if (part_die->tag == DW_TAG_compile_unit
+	      && base_address_type < base_address_low_pc)
+	    {
+	      base_address = DW_ADDR (&attr);
+	      base_address_type = base_address_low_pc;
+	    }
 	  break;
 	case DW_AT_high_pc:
 	  has_high_pc_attr = 1;
 	  part_die->highpc = DW_ADDR (&attr);
 	  break;
+	case DW_AT_entry_pc:
+	  if (part_die->tag == DW_TAG_compile_unit
+	      && base_address_type < base_address_entry_pc)
+	    {
+	      base_address = DW_ADDR (&attr);
+	      base_address_type = base_address_entry_pc;
+	    }
+	  break;
 	case DW_AT_ranges:
 	  if (dwarf2_ranges_read (DW_UNSND (&attr), &part_die->lowpc,
 				  &part_die->highpc, cu))
@@ -5942,6 +6010,14 @@ read_partial_die (struct partial_die_inf
       && (part_die->lowpc != 0
 	  || dwarf2_per_objfile->has_section_at_zero))
     part_die->has_pc_info = 1;
+
+  if (base_address_type != base_address_none && !cu->header.base_known)
+    {
+      gdb_assert (part_die->tag == DW_TAG_compile_unit);
+      cu->header.base_known = 1;
+      cu->header.base_address = base_address;
+    }
+
   return info_ptr;
 }
 

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap)
  2007-12-17  1:03                   ` Daniel Jacobowitz
                                       ` (2 preceding siblings ...)
  2008-04-23 22:18                     ` [patch] [1/2] Discontiguous PSYMTABs (partial DIEs base address) Jan Kratochvil
@ 2008-04-23 22:24                     ` Jan Kratochvil
  2008-05-01 19:46                       ` Daniel Jacobowitz
  2008-05-12 22:24                       ` Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap)) Ulrich Weigand
  3 siblings, 2 replies; 46+ messages in thread
From: Jan Kratochvil @ 2008-04-23 22:24 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 317 bytes --]

Hi,

fix + a testcase for DWARF/non-DWARF debuginfos with continuous sections for
childless CUs.

Going to post the results of some more verifications, posting as Doug Evans has
asked.

It is only a minor update against its previous version:
	http://sourceware.org/ml/gdb-patches/2007-12/msg00143.html


Regards,
Jan

[-- Attachment #2: gdb-cvs-psymtab-ranges-stabs.patch --]
[-- Type: text/plain, Size: 22662 bytes --]

2008-04-23  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* Makefile.in: Update dependencies.
	* dwarf2read.c: Include "addrmap.h"
	(struct dwarf2_cu): New fields RANGES_OFFSET and HAS_RANGES_OFFSET.
	(dwarf2_ranges_read): New prototype.
	(dwarf2_build_psymtabs_hard): Initialize and prepare PSYMTABS_ADDRMAP.
	Add discontiguous range to PSYMTABS_ADDRMAP by DWARF2_RANGES_READ on
	HAS_RANGES_OFFSET, otherwise add there the contiguous range.
	(dwarf2_ranges_read): New parameter RANGES_PST, update the function
	comment for it.  Add the found ranges to RANGES_PST.  New variable
	BASEADDR, initialize it the common way.
	(dwarf2_get_pc_bounds): Update the caller for the new parameter.
	(read_partial_die): `DW_AT_ranges' now only sets RANGES_OFFSET and
	HAS_RANGES_OFFSET for the later processing.
	* objfiles.h (struct objfile): New field PSYMTABS_ADDRMAP.
	* symtab.c: Include "addrmap.h"
	(find_pc_sect_psymtab): Support reading the field PSYMTABS_ADDRMAP.
	Move the psymtab locator into ...
	(find_pc_sect_psymtab_closer): ... a new function.

2008-04-23  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.dwarf2/dw2-ranges.S: Merge the secondary section with `.fini'.
	* gdb.dwarf2/dw2-ranges.exp: Compile also `dw2-ranges2.S' and
	`dw2-ranges3.S' and test also their MAIN2, FUNC2 and MAIN3 symbols.
	* gdb.dwarf2/dw2-ranges2.S, gdb.dwarf2/dw2-ranges3.S: New files.

--- ./gdb/Makefile.in	2008-04-20 16:37:51.000000000 +0200
+++ ./gdb/Makefile.in	2008-04-23 10:28:05.000000000 +0200
@@ -2085,7 +2085,8 @@ dwarf2-frame.o: dwarf2-frame.c $(defs_h)
 dwarf2loc.o: dwarf2loc.c $(defs_h) $(ui_out_h) $(value_h) $(frame_h) \
 	$(gdbcore_h) $(target_h) $(inferior_h) $(ax_h) $(ax_gdb_h) \
 	$(regcache_h) $(objfiles_h) $(exceptions_h) $(elf_dwarf2_h) \
-	$(dwarf2expr_h) $(dwarf2loc_h) $(gdb_string_h) $(gdb_assert_h)
+	$(dwarf2expr_h) $(dwarf2loc_h) $(gdb_string_h) $(gdb_assert_h) \
+	$(addrmap_h)
 dwarf2read.o: dwarf2read.c $(defs_h) $(bfd_h) $(symtab_h) $(gdbtypes_h) \
 	$(objfiles_h) $(elf_dwarf2_h) $(buildsym_h) $(demangle_h) \
 	$(expression_h) $(filenames_h) $(macrotab_h) $(language_h) \
@@ -2893,7 +2894,7 @@ symtab.o: symtab.c $(defs_h) $(symtab_h)
 	$(filenames_h) $(objc_lang_h) $(ada_lang_h) $(hashtab_h) \
 	$(gdb_obstack_h) $(block_h) $(dictionary_h) $(gdb_string_h) \
 	$(gdb_stat_h) $(cp_abi_h) $(observer_h) $(gdb_assert_h) \
-	$(solist_h) $(p_lang_h)
+	$(solist_h) $(p_lang_h) $(addrmap_h)
 target.o: target.c $(defs_h) $(gdb_string_h) $(target_h) $(gdbcmd_h) \
 	$(symtab_h) $(inferior_h) $(bfd_h) $(symfile_h) $(objfiles_h) \
 	$(gdb_wait_h) $(dcache_h) $(regcache_h) $(gdb_assert_h) $(gdbcore_h) \
--- ./gdb/dwarf2read.c	2008-04-23 00:16:47.000000000 +0200
+++ ./gdb/dwarf2read.c	2008-04-23 11:47:02.000000000 +0200
@@ -45,6 +45,7 @@
 #include "hashtab.h"
 #include "command.h"
 #include "gdbcmd.h"
+#include "addrmap.h"
 
 #include <fcntl.h>
 #include "gdb_string.h"
@@ -303,6 +304,9 @@ struct dwarf2_cu
   /* Hash table holding all the loaded partial DIEs.  */
   htab_t partial_dies;
 
+  /* `.debug_ranges' offset for this `DW_TAG_compile_unit' DIE.  */
+  unsigned long ranges_offset;
+
   /* Storage for things with the same lifetime as this read-in compilation
      unit, including partial DIEs.  */
   struct obstack comp_unit_obstack;
@@ -345,6 +349,9 @@ struct dwarf2_cu
      DIEs for namespaces, we don't need to try to infer them
      from mangled names.  */
   unsigned int has_namespace_info : 1;
+
+  /* Field `ranges_offset' is filled in; flag as the value may be zero.  */
+  unsigned int has_ranges_offset : 1;
 };
 
 /* Persistent data held for a compilation unit, even when not
@@ -894,6 +901,9 @@ static void read_func_scope (struct die_
 
 static void read_lexical_block_scope (struct die_info *, struct dwarf2_cu *);
 
+static int dwarf2_ranges_read (unsigned, CORE_ADDR *, CORE_ADDR *,
+			       struct dwarf2_cu *, struct partial_symtab *);
+
 static int dwarf2_get_pc_bounds (struct die_info *,
 				 CORE_ADDR *, CORE_ADDR *, struct dwarf2_cu *);
 
@@ -1472,6 +1482,9 @@ dwarf2_build_psymtabs_hard (struct objfi
 
   create_all_comp_units (objfile);
 
+  objfile->psymtabs_addrmap = addrmap_create_mutable
+						    (&objfile->objfile_obstack);
+
   /* Since the objects we're extracting from .debug_info vary in
      length, only the individual functions to extract them (like
      read_comp_unit_head and load_partial_die) can really know whether
@@ -1537,7 +1550,8 @@ dwarf2_build_psymtabs_hard (struct objfi
       /* Allocate a new partial symbol table structure */
       pst = start_psymtab_common (objfile, objfile->section_offsets,
 				  comp_unit_die.name ? comp_unit_die.name : "",
-				  comp_unit_die.lowpc,
+				  /* TEXTLOW and TEXTHIGH are set below.  */
+				  0,
 				  objfile->global_psymbols.next,
 				  objfile->static_psymbols.next);
 
@@ -1570,6 +1584,15 @@ dwarf2_build_psymtabs_hard (struct objfi
 
       this_cu->psymtab = pst;
 
+      /* Possibly set the default values of LOWPC and HIGHPC from
+         `DW_AT_ranges'.  */
+      if (cu.has_ranges_offset)
+	{
+	  if (dwarf2_ranges_read (cu.ranges_offset, &comp_unit_die.lowpc,
+				  &comp_unit_die.highpc, &cu, pst))
+	    comp_unit_die.has_pc_info = 1;
+	}
+
       /* Check if comp unit has_children.
          If so, read the rest of the partial symbols from this comp unit.
          If not, there's no more debug_info for this comp unit. */
@@ -1600,6 +1623,12 @@ dwarf2_build_psymtabs_hard (struct objfi
       pst->textlow = comp_unit_die.lowpc + baseaddr;
       pst->texthigh = comp_unit_die.highpc + baseaddr;
 
+      /* Store the contiguous range; `DW_AT_ranges' range is stored above.  The
+         range can be also empty for CUs with no code.  */
+      if (!cu.has_ranges_offset && pst->textlow < pst->texthigh)
+	addrmap_set_empty (objfile->psymtabs_addrmap, pst->textlow,
+			   pst->texthigh - 1, pst);
+
       pst->n_global_syms = objfile->global_psymbols.next -
 	(objfile->global_psymbols.list + pst->globals_offset);
       pst->n_static_syms = objfile->static_psymbols.next -
@@ -1623,6 +1652,10 @@ dwarf2_build_psymtabs_hard (struct objfi
 
       do_cleanups (back_to_inner);
     }
+
+  objfile->psymtabs_addrmap = addrmap_create_fixed (objfile->psymtabs_addrmap,
+						    &objfile->objfile_obstack);
+
   do_cleanups (back_to);
 }
 
@@ -3143,11 +3176,13 @@ read_lexical_block_scope (struct die_inf
 }
 
 /* Get low and high pc attributes from DW_AT_ranges attribute value OFFSET.
-   Return 1 if the attributes are present and valid, otherwise, return 0.  */
+   Return 1 if the attributes are present and valid, otherwise, return 0.
+   If RANGES_PST is not NULL we should setup `objfile->psymtabs_addrmap'.  */
 
 static int
 dwarf2_ranges_read (unsigned offset, CORE_ADDR *low_return,
-		    CORE_ADDR *high_return, struct dwarf2_cu *cu)
+		    CORE_ADDR *high_return, struct dwarf2_cu *cu,
+		    struct partial_symtab *ranges_pst)
 {
   struct objfile *objfile = cu->objfile;
   struct comp_unit_head *cu_header = &cu->header;
@@ -3163,6 +3198,7 @@ dwarf2_ranges_read (unsigned offset, COR
   int low_set;
   CORE_ADDR low = 0;
   CORE_ADDR high = 0;
+  CORE_ADDR baseaddr;
 
   found_base = cu_header->base_known;
   base = cu_header->base_address;
@@ -3190,6 +3226,9 @@ dwarf2_ranges_read (unsigned offset, COR
 
   low_set = 0;
 
+  if (ranges_pst != NULL)
+    baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
+
   while (1)
     {
       CORE_ADDR range_beginning, range_end;
@@ -3229,6 +3268,11 @@ dwarf2_ranges_read (unsigned offset, COR
       range_beginning += base;
       range_end += base;
 
+      if (ranges_pst != NULL && range_beginning < range_end)
+	addrmap_set_empty (objfile->psymtabs_addrmap,
+			   range_beginning + baseaddr, range_end - 1 + baseaddr,
+			   ranges_pst);
+
       /* FIXME: This is recording everything as a low-high
 	 segment of consecutive addresses.  We should have a
 	 data structure for discontiguous block ranges
@@ -3293,7 +3337,7 @@ dwarf2_get_pc_bounds (struct die_info *d
 	{
 	  /* Value of the DW_AT_ranges attribute is the offset in the
 	     .debug_ranges section.  */
-	  if (!dwarf2_ranges_read (DW_UNSND (attr), &low, &high, cu))
+	  if (!dwarf2_ranges_read (DW_UNSND (attr), &low, &high, cu, NULL))
 	    return 0;
 	  /* Found discontinuous range of addresses.  */
 	  ret = -1;
@@ -5857,9 +5901,11 @@ read_partial_die (struct partial_die_inf
 	  part_die->highpc = DW_ADDR (&attr);
 	  break;
 	case DW_AT_ranges:
-	  if (dwarf2_ranges_read (DW_UNSND (&attr), &part_die->lowpc,
-				  &part_die->highpc, cu))
-	    has_low_pc_attr = has_high_pc_attr = 1;
+	  if (part_die->tag == DW_TAG_compile_unit)
+	    {
+	      cu->ranges_offset = DW_UNSND (&attr);
+	      cu->has_ranges_offset = 1;
+	    }
 	  break;
 	case DW_AT_location:
           /* Support the .debug_loc offsets */
--- ./gdb/objfiles.h	2008-04-20 16:37:53.000000000 +0200
+++ ./gdb/objfiles.h	2008-04-23 10:28:05.000000000 +0200
@@ -220,6 +220,13 @@ struct objfile
 
     struct partial_symtab *psymtabs;
 
+    /* Map addresses to the entries of PSYMTABS.  It would be more efficient to
+       have a map per the whole process but ADDRMAP cannot selectively remove
+       its items during FREE_OBJFILE.  This mapping is already present even for
+       PARTIAL_SYMTABs which still have no corresponding full SYMTABs read.  */
+
+    struct addrmap *psymtabs_addrmap;
+
     /* List of freed partial symtabs, available for re-use */
 
     struct partial_symtab *free_psymtabs;
--- ./gdb/symtab.c	2008-04-20 16:37:53.000000000 +0200
+++ ./gdb/symtab.c	2008-04-23 10:28:05.000000000 +0200
@@ -41,6 +41,7 @@
 #include "objc-lang.h"
 #include "ada-lang.h"
 #include "p-lang.h"
+#include "addrmap.h"
 
 #include "hashtab.h"
 
@@ -801,6 +802,83 @@ matching_bfd_sections (asection *first, 
   return 0;
 }
 
+/* Find which partial symtab contains PC and SECTION starting at psymtab PST.
+   We may find a different psymtab than PST.  See FIND_PC_SECT_PSYMTAB.  */
+
+struct partial_symtab *
+find_pc_sect_psymtab_closer (CORE_ADDR pc, asection *section,
+			     struct partial_symtab *pst,
+			     struct minimal_symbol *msymbol)
+{
+  struct objfile *objfile = pst->objfile;
+  struct partial_symtab *tpst;
+  struct partial_symtab *best_pst = pst;
+  CORE_ADDR best_addr = pst->textlow;
+
+  /* An objfile that has its functions reordered might have
+     many partial symbol tables containing the PC, but
+     we want the partial symbol table that contains the
+     function containing the PC.  */
+  if (!(objfile->flags & OBJF_REORDERED) &&
+      section == 0)	/* can't validate section this way */
+    return pst;
+
+  if (msymbol == NULL)
+    return (pst);
+
+  /* The code range of partial symtabs sometimes overlap, so, in
+     the loop below, we need to check all partial symtabs and
+     find the one that fits better for the given PC address. We
+     select the partial symtab that contains a symbol whose
+     address is closest to the PC address.  By closest we mean
+     that find_pc_sect_symbol returns the symbol with address
+     that is closest and still less than the given PC.  */
+  for (tpst = pst; tpst != NULL; tpst = tpst->next)
+    {
+      if (pc >= tpst->textlow && pc < tpst->texthigh)
+	{
+	  struct partial_symbol *p;
+	  CORE_ADDR this_addr;
+
+	  /* NOTE: This assumes that every psymbol has a
+	     corresponding msymbol, which is not necessarily
+	     true; the debug info might be much richer than the
+	     object's symbol table.  */
+	  p = find_pc_sect_psymbol (tpst, pc, section);
+	  if (p != NULL
+	      && SYMBOL_VALUE_ADDRESS (p)
+	      == SYMBOL_VALUE_ADDRESS (msymbol))
+	    return tpst;
+
+	  /* Also accept the textlow value of a psymtab as a
+	     "symbol", to provide some support for partial
+	     symbol tables with line information but no debug
+	     symbols (e.g. those produced by an assembler).  */
+	  if (p != NULL)
+	    this_addr = SYMBOL_VALUE_ADDRESS (p);
+	  else
+	    this_addr = tpst->textlow;
+
+	  /* Check whether it is closer than our current
+	     BEST_ADDR.  Since this symbol address is
+	     necessarily lower or equal to PC, the symbol closer
+	     to PC is the symbol which address is the highest.
+	     This way we return the psymtab which contains such
+	     best match symbol. This can help in cases where the
+	     symbol information/debuginfo is not complete, like
+	     for instance on IRIX6 with gcc, where no debug info
+	     is emitted for statics. (See also the nodebug.exp
+	     testcase.) */
+	  if (this_addr > best_addr)
+	    {
+	      best_addr = this_addr;
+	      best_pst = tpst;
+	    }
+	}
+    }
+  return best_pst;
+}
+
 /* Find which partial symtab contains PC and SECTION.  Return 0 if
    none.  We return the psymtab that contains a symbol whose address
    exactly matches PC, or, if we cannot find an exact match, the
@@ -808,7 +886,6 @@ matching_bfd_sections (asection *first, 
 struct partial_symtab *
 find_pc_sect_psymtab (CORE_ADDR pc, asection *section)
 {
-  struct partial_symtab *pst;
   struct objfile *objfile;
   struct minimal_symbol *msymbol;
 
@@ -824,79 +901,53 @@ find_pc_sect_psymtab (CORE_ADDR pc, asec
 	  || msymbol->type == mst_file_bss))
     return NULL;
 
-  ALL_PSYMTABS (objfile, pst)
-  {
-    if (pc >= pst->textlow && pc < pst->texthigh)
+  /* Try just the PSYMTABS_ADDRMAP mapping first as it has better granularity
+     than the later used TEXTLOW/TEXTHIGH one.  */
+
+  ALL_OBJFILES (objfile)
+    if (objfile->psymtabs_addrmap != NULL)
       {
-	struct partial_symtab *tpst;
-	struct partial_symtab *best_pst = pst;
-	CORE_ADDR best_addr = pst->textlow;
-
-	/* An objfile that has its functions reordered might have
-	   many partial symbol tables containing the PC, but
-	   we want the partial symbol table that contains the
-	   function containing the PC.  */
-	if (!(objfile->flags & OBJF_REORDERED) &&
-	    section == 0)	/* can't validate section this way */
-	  return (pst);
-
-	if (msymbol == NULL)
-	  return (pst);
-
-	/* The code range of partial symtabs sometimes overlap, so, in
-	   the loop below, we need to check all partial symtabs and
-	   find the one that fits better for the given PC address. We
-	   select the partial symtab that contains a symbol whose
-	   address is closest to the PC address.  By closest we mean
-	   that find_pc_sect_symbol returns the symbol with address
-	   that is closest and still less than the given PC.  */
-	for (tpst = pst; tpst != NULL; tpst = tpst->next)
+	struct partial_symtab *pst;
+
+	pst = addrmap_find (objfile->psymtabs_addrmap, pc);
+	if (pst != NULL)
 	  {
-	    if (pc >= tpst->textlow && pc < tpst->texthigh)
-	      {
-		struct partial_symbol *p;
-		CORE_ADDR this_addr;
+	    /* We do not try to call FIND_PC_SECT_PSYMTAB_CLOSER as
+	       PSYMTABS_ADDRMAP we used has already the best 1-byte
+	       granularity and FIND_PC_SECT_PSYMTAB_CLOSER may mislead us into
+	       a worse chosen section due to the TEXTLOW/TEXTHIGH ranges
+	       overlap.  */
 
-		/* NOTE: This assumes that every psymbol has a
-		   corresponding msymbol, which is not necessarily
-		   true; the debug info might be much richer than the
-		   object's symbol table.  */
-		p = find_pc_sect_psymbol (tpst, pc, section);
-		if (p != NULL
-		    && SYMBOL_VALUE_ADDRESS (p)
-		    == SYMBOL_VALUE_ADDRESS (msymbol))
-		  return (tpst);
-
-		/* Also accept the textlow value of a psymtab as a
-		   "symbol", to provide some support for partial
-		   symbol tables with line information but no debug
-		   symbols (e.g. those produced by an assembler).  */
-		if (p != NULL)
-		  this_addr = SYMBOL_VALUE_ADDRESS (p);
-		else
-		  this_addr = tpst->textlow;
-
-		/* Check whether it is closer than our current
-		   BEST_ADDR.  Since this symbol address is
-		   necessarily lower or equal to PC, the symbol closer
-		   to PC is the symbol which address is the highest.
-		   This way we return the psymtab which contains such
-		   best match symbol. This can help in cases where the
-		   symbol information/debuginfo is not complete, like
-		   for instance on IRIX6 with gcc, where no debug info
-		   is emitted for statics. (See also the nodebug.exp
-		   testcase.) */
-		if (this_addr > best_addr)
-		  {
-		    best_addr = this_addr;
-		    best_pst = tpst;
-		  }
-	      }
+	    return pst;
 	  }
-	return (best_pst);
       }
-  }
-  return (NULL);
+
+  /* Existing PSYMTABS_ADDRMAP mapping is present even for PARTIAL_SYMTABs
+     which still have no corresponding full SYMTABs read.  But it is not
+     present for non-DWARF2 debug infos not supporting PSYMTABS_ADDRMAP in GDB
+     so far.  */
+
+  ALL_OBJFILES (objfile)
+    {
+      struct partial_symtab *pst;
+
+      /* Check even OBJFILE with non-zero PSYMTABS_ADDRMAP as only several of
+	 its CUs may be missing in PSYMTABS_ADDRMAP as they may be varying
+	 debug info type in single OBJFILE.  */
+
+      ALL_OBJFILE_PSYMTABS (objfile, pst)
+	if (pc >= pst->textlow && pc < pst->texthigh)
+	  {
+	    struct partial_symtab *best_pst;
+
+	    best_pst = find_pc_sect_psymtab_closer (pc, section, pst,
+						    msymbol);
+	    if (best_pst != NULL)
+	      return best_pst;
+	  }
+    }
+
+  return NULL;
 }
 
 /* Find which partial symtab contains PC.  Return 0 if none. 
--- ./gdb/testsuite/gdb.dwarf2/dw2-ranges.S	2008-04-20 16:38:19.000000000 +0200
+++ ./gdb/testsuite/gdb.dwarf2/dw2-ranges.S	2008-04-23 10:28:05.000000000 +0200
@@ -35,7 +35,10 @@ main:	.int	0
 	.endfunc
 	.size	main, . - main
 
-	.section	.text.func, "ax", @progbits
+	/* `.fini' section is here to make sure `dw2-ranges.S'
+	   vs. `dw2-ranges2.S' overlap their DW_AT_ranges with each other.  */
+	.section	.fini, "ax", @progbits
+
 	.globl	func
 	.func	func
 func:	.int	0
--- ./gdb/testsuite/gdb.dwarf2/dw2-ranges.exp	2008-04-20 16:38:19.000000000 +0200
+++ ./gdb/testsuite/gdb.dwarf2/dw2-ranges.exp	2008-04-23 10:28:05.000000000 +0200
@@ -30,9 +30,23 @@ if {![istarget *-*-linux*]
 
 set testfile "dw2-ranges"
 set srcfile ${testfile}.S
-set binfile ${objdir}/${subdir}/${testfile}.o
+set srcfile2 ${testfile}2.S
+set srcfile3 ${testfile}3.S
+set objfile ${objdir}/${subdir}/${testfile}.o
+set objfile2 ${objdir}/${subdir}/${testfile}2.o
+set objfile3 ${objdir}/${subdir}/${testfile}3.o
+set binfile ${objdir}/${subdir}/${testfile}
 
-if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" object debug] != "" } {
+if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${objfile}" object {additional_flags=-gdwarf-2}] != "" } {
+    return -1
+}
+if {[gdb_compile "${srcdir}/${subdir}/${srcfile2}" "${objfile2}" object {additional_flags=-gdwarf-2}] != "" } {
+    return -1
+}
+if {[gdb_compile "${srcdir}/${subdir}/${srcfile3}" "${objfile3}" object {additional_flags=-gstabs}] != "" } {
+    return -1
+}
+if {[gdb_compile "${objfile} ${objfile2} ${objfile3}" "${binfile}" executable {}] != "" } {
     return -1
 }
 
@@ -46,4 +60,8 @@ gdb_load ${binfile}
 # Wrong output:
 # 	No line number information available for address 0x4
 
+gdb_test "info line main" "Line \[0-9\]* of .* starts at address .* and ends at .*"
 gdb_test "info line func" "Line \[0-9\]* of .* starts at address .* and ends at .*"
+gdb_test "info line main2" "Line \[0-9\]* of .* starts at address .* and ends at .*"
+gdb_test "info line func2" "Line \[0-9\]* of .* starts at address .* and ends at .*"
+gdb_test "info line main3" "Line \[0-9\]* of .* starts at address .* and ends at .*"
--- ./gdb/testsuite/gdb.dwarf2/dw2-ranges2.S	1970-01-01 01:00:00.000000000 +0100
+++ ./gdb/testsuite/gdb.dwarf2/dw2-ranges2.S	2008-04-23 10:28:05.000000000 +0200
@@ -0,0 +1,46 @@
+/*
+   Copyright 2007 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/>.
+ */
+
+/* Despite the sections below will be adjacent the assembler has to produce
+   DW_AT_ranges as the linker could place both sections at arbitrary locations.
+   */
+
+	/* Such directive is required by GAS for builds without `-g'.  */
+	.file	1 "dw2-ranges2.S"
+
+	/* Without this directive GAS will not emit DWARF2 unless we provide an
+	   instruction to assemble.  We want to avoid any instructions to
+	   remain architecture independent.  */
+	.loc_mark_labels	1
+
+	.text
+
+	.globl	main2
+	.func	main2
+main2:	.int	0
+	.endfunc
+	.size	main2, . - main2
+
+	/* `.fini' section is here to make sure `dw2-ranges.S'
+	   vs. `dw2-ranges2.S' overlap their DW_AT_ranges with each other.  */
+	.section	.fini, "ax", @progbits
+
+	.globl	func2
+	.func	func2
+func2:	.int	0
+	.endfunc
+	.size	func2, . - func2
--- ./gdb/testsuite/gdb.dwarf2/dw2-ranges3.S	1970-01-01 01:00:00.000000000 +0100
+++ ./gdb/testsuite/gdb.dwarf2/dw2-ranges3.S	2008-04-23 10:28:05.000000000 +0200
@@ -0,0 +1,36 @@
+/*
+   Copyright 2007, 2008 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/>.
+ */
+
+/* Despite the sections below will be adjacent the assembler has to produce
+   DW_AT_ranges as the linker could place both sections at arbitrary locations.
+   */
+
+	/* Such directive is required by GAS for builds without `-g'.  */
+	.file	1 "dw2-ranges3.S"
+
+	/* Without this directive GAS will not emit DWARF2 unless we provide an
+	   instruction to assemble.  We want to avoid any instructions to
+	   remain architecture independent.  */
+	.loc_mark_labels	1
+
+	.text
+
+	.globl	main3
+	.func	main3
+main3:	.int	0
+	.endfunc
+	.size	main3, . - main3

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [patch] [1/2] Discontiguous PSYMTABs (partial DIEs base  address)
  2008-04-23 22:18                     ` [patch] [1/2] Discontiguous PSYMTABs (partial DIEs base address) Jan Kratochvil
@ 2008-05-01 19:43                       ` Daniel Jacobowitz
  0 siblings, 0 replies; 46+ messages in thread
From: Daniel Jacobowitz @ 2008-05-01 19:43 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

On Wed, Apr 23, 2008 at 11:30:55PM +0200, Jan Kratochvil wrote:
> 2008-04-21  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Set CU BASE_ADDRESS already from partial DIEs.
> 	* dwarf2read.c (read_partial_die): New variables BASE_ADDRESS and
> 	BASE_ADDRESS_TYPE.  Set these variables from DW_AT_LOW_PC and
> 	DW_AT_ENTRY_PC.  Set CU->HEADER.BASE_KNOWN and CU->HEADER.BASE_ADDRESS
> 	from these variables if it was still unset.

This looks OK to me, thanks.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by  addrmap)
  2008-04-23 22:24                     ` [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap) Jan Kratochvil
@ 2008-05-01 19:46                       ` Daniel Jacobowitz
  2008-05-04 17:38                         ` Jan Kratochvil
  2008-05-12 22:24                       ` Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap)) Ulrich Weigand
  1 sibling, 1 reply; 46+ messages in thread
From: Daniel Jacobowitz @ 2008-05-01 19:46 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

On Wed, Apr 23, 2008 at 11:30:59PM +0200, Jan Kratochvil wrote:
> Hi,
> 
> fix + a testcase for DWARF/non-DWARF debuginfos with continuous sections for
> childless CUs.
> 
> Going to post the results of some more verifications, posting as Doug Evans has
> asked.
> 
> It is only a minor update against its previous version:
> 	http://sourceware.org/ml/gdb-patches/2007-12/msg00143.html

This looks good to me.  If your testing hasn't found any trouble,
please check it in.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by  addrmap)
  2008-05-01 19:46                       ` Daniel Jacobowitz
@ 2008-05-04 17:38                         ` Jan Kratochvil
  0 siblings, 0 replies; 46+ messages in thread
From: Jan Kratochvil @ 2008-05-04 17:38 UTC (permalink / raw)
  To: gdb-patches; +Cc: Daniel Jacobowitz

On Thu, 01 May 2008 21:46:22 +0200, Daniel Jacobowitz wrote:
> On Wed, Apr 23, 2008 at 11:30:59PM +0200, Jan Kratochvil wrote:
> > fix + a testcase for DWARF/non-DWARF debuginfos with continuous sections for
> > childless CUs.
...
> This looks good to me.  If your testing hasn't found any trouble,
> please check it in.

Committed (no regressions on the i386/x86_64 native/biarch testsuite).


Regards,
Jan


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))
  2008-04-23 22:24                     ` [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap) Jan Kratochvil
  2008-05-01 19:46                       ` Daniel Jacobowitz
@ 2008-05-12 22:24                       ` Ulrich Weigand
  2008-05-12 22:37                         ` Michael Snyder
  2008-05-12 23:52                         ` Jan Kratochvil
  1 sibling, 2 replies; 46+ messages in thread
From: Ulrich Weigand @ 2008-05-12 22:24 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches, drow

Jan Kratochvil wrote:

> 	* symtab.c: Include "addrmap.h"
> 	(find_pc_sect_psymtab): Support reading the field PSYMTABS_ADDRMAP.
> 	Move the psymtab locator into ...
> 	(find_pc_sect_psymtab_closer): ... a new function.

It appears this change broke overlay support.  In fact, it looks like the
main idea of this patch is fundamentally incompatible with overlays:

> +    /* Map addresses to the entries of PSYMTABS.  It would be more efficient to
> +       have a map per the whole process but ADDRMAP cannot selectively remove
> +       its items during FREE_OBJFILE.  This mapping is already present even for
> +       PARTIAL_SYMTABs which still have no corresponding full SYMTABs read.  */
> +
> +    struct addrmap *psymtabs_addrmap;

In the presence of overlays, a single address can map to *multiple* PSYMTABS
corresponding to multiple overlay segments optionally loaded at that address.

The old code chose between them using the "section" argument to 
find_pc_sect_psymtab; it seems the new code just ignores this argument:

  /* Try just the PSYMTABS_ADDRMAP mapping first as it has better granularity
     than the later used TEXTLOW/TEXTHIGH one.  */

  ALL_OBJFILES (objfile)
    if (objfile->psymtabs_addrmap != NULL)
      {
        struct partial_symtab *pst;

        pst = addrmap_find (objfile->psymtabs_addrmap, pc);
        if (pst != NULL)
          {
            /* We do not try to call FIND_PC_SECT_PSYMTAB_CLOSER as
               PSYMTABS_ADDRMAP we used has already the best 1-byte
               granularity and FIND_PC_SECT_PSYMTAB_CLOSER may mislead us into
               a worse chosen section due to the TEXTLOW/TEXTHIGH ranges
               overlap.  */

            return pst;
          }
      }


Any suggestions how to fix this?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Overlay support broken (Re: [patch] [2/2] Discontiguous  PSYMTABs (psymtabs->symtabs by addrmap))
  2008-05-12 22:24                       ` Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap)) Ulrich Weigand
@ 2008-05-12 22:37                         ` Michael Snyder
  2008-05-13  1:39                           ` Daniel Jacobowitz
  2008-05-13 15:31                           ` Doug Evans
  2008-05-12 23:52                         ` Jan Kratochvil
  1 sibling, 2 replies; 46+ messages in thread
From: Michael Snyder @ 2008-05-12 22:37 UTC (permalink / raw)
  To: gdb-patches

On Mon, 2008-05-12 at 21:45 +0200, Ulrich Weigand wrote:
> Jan Kratochvil wrote:
> 
> > 	* symtab.c: Include "addrmap.h"
> > 	(find_pc_sect_psymtab): Support reading the field PSYMTABS_ADDRMAP.
> > 	Move the psymtab locator into ...
> > 	(find_pc_sect_psymtab_closer): ... a new function.
> 
> It appears this change broke overlay support.  

Thanks for noticing.   ;-)

I wrote that overlay code over 10 years ago.
I wonder whether anyone still uses it?

Michael




^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Overlay support broken (Re: [patch] [2/2] Discontiguous  PSYMTABs (psymtabs->symtabs by addrmap))
  2008-05-12 22:24                       ` Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap)) Ulrich Weigand
  2008-05-12 22:37                         ` Michael Snyder
@ 2008-05-12 23:52                         ` Jan Kratochvil
  2008-05-13 18:45                           ` Ulrich Weigand
  1 sibling, 1 reply; 46+ messages in thread
From: Jan Kratochvil @ 2008-05-12 23:52 UTC (permalink / raw)
  To: Ulrich Weigand; +Cc: gdb-patches, drow

On Mon, 12 May 2008 21:45:39 +0200, Ulrich Weigand wrote:
...
> It appears this change broke overlay support.

I generally considered the overlays to be no longer supported and just still
not removed from the code.  While reading other GDB code I was feeling overlays
are no longer supported on multiple places but sure I may be wrong.

Could you please name which current arch is dependent on overlaying?


Sorry,
Jan


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Overlay support broken (Re: [patch] [2/2] Discontiguous  PSYMTABs (psymtabs->symtabs by addrmap))
  2008-05-12 22:37                         ` Michael Snyder
@ 2008-05-13  1:39                           ` Daniel Jacobowitz
  2008-05-13  3:17                             ` Jan Kratochvil
                                               ` (2 more replies)
  2008-05-13 15:31                           ` Doug Evans
  1 sibling, 3 replies; 46+ messages in thread
From: Daniel Jacobowitz @ 2008-05-13  1:39 UTC (permalink / raw)
  To: Michael Snyder, Jan Kratochvil; +Cc: gdb-patches, Ulrich Weigand

On Mon, May 12, 2008 at 01:06:44PM -0700, Michael Snyder wrote:
> I wrote that overlay code over 10 years ago.
> I wonder whether anyone still uses it?

On Mon, May 12, 2008 at 10:32:32PM +0200, Jan Kratochvil wrote:
> I generally considered the overlays to be no longer supported and just still
> not removed from the code.  While reading other GDB code I was feeling overlays
> are no longer supported on multiple places but sure I may be wrong.
> 
> Could you please name which current arch is dependent on overlaying?

As you may be able to guess from context, the answer is SPU :-)

Each SPE has only a 256K local store which it uses for code, stack,
and heap.  That's not very much space, so overlays are frequently
required.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Overlay support broken (Re: [patch] [2/2] Discontiguous  PSYMTABs (psymtabs->symtabs by addrmap))
  2008-05-13  1:39                           ` Daniel Jacobowitz
@ 2008-05-13  3:17                             ` Jan Kratochvil
  2008-05-13 15:37                             ` Doug Evans
  2008-05-13 15:42                             ` Michael Snyder
  2 siblings, 0 replies; 46+ messages in thread
From: Jan Kratochvil @ 2008-05-13  3:17 UTC (permalink / raw)
  To: Ulrich Weigand; +Cc: Daniel Jacobowitz, Michael Snyder, gdb-patches

On Mon, 12 May 2008 22:39:34 +0200, Daniel Jacobowitz wrote:
...
> As you may be able to guess from context, the answer is SPU :-)

OK.  Going to provide some patch to be somehow sections-aware.


Regards,
Jan


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))
  2008-05-12 22:37                         ` Michael Snyder
  2008-05-13  1:39                           ` Daniel Jacobowitz
@ 2008-05-13 15:31                           ` Doug Evans
  1 sibling, 0 replies; 46+ messages in thread
From: Doug Evans @ 2008-05-13 15:31 UTC (permalink / raw)
  To: Michael Snyder; +Cc: gdb-patches, Ulrich Weigand

On Mon, May 12, 2008 at 1:06 PM, Michael Snyder <msnyder@specifix.com> wrote:
>  I wrote that overlay code over 10 years ago.
>  I wonder whether anyone still uses it?

I've been wondering if it'd be useful in the spu context.  Is what how
you discovered the breakage Ulrich?


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))
  2008-05-13  1:39                           ` Daniel Jacobowitz
  2008-05-13  3:17                             ` Jan Kratochvil
@ 2008-05-13 15:37                             ` Doug Evans
  2008-05-13 15:42                             ` Michael Snyder
  2 siblings, 0 replies; 46+ messages in thread
From: Doug Evans @ 2008-05-13 15:37 UTC (permalink / raw)
  To: Michael Snyder, Jan Kratochvil, gdb-patches, Ulrich Weigand

On Mon, May 12, 2008 at 1:39 PM, Daniel Jacobowitz <drow@false.org> wrote:
> On Mon, May 12, 2008 at 01:06:44PM -0700, Michael Snyder wrote:
>  > I wrote that overlay code over 10 years ago.
>  > I wonder whether anyone still uses it?
>
>
> On Mon, May 12, 2008 at 10:32:32PM +0200, Jan Kratochvil wrote:
>  > I generally considered the overlays to be no longer supported and just still
>  > not removed from the code.  While reading other GDB code I was feeling overlays
>  > are no longer supported on multiple places but sure I may be wrong.
>  >
>  > Could you please name which current arch is dependent on overlaying?
>
>  As you may be able to guess from context, the answer is SPU :-)

Ah hah! :-)

>  Each SPE has only a 256K local store which it uses for code, stack,
>  and heap.  That's not very much space, so overlays are frequently
>  required.

Yep.


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Overlay support broken (Re: [patch] [2/2] Discontiguous  PSYMTABs (psymtabs->symtabs by addrmap))
  2008-05-13  1:39                           ` Daniel Jacobowitz
  2008-05-13  3:17                             ` Jan Kratochvil
  2008-05-13 15:37                             ` Doug Evans
@ 2008-05-13 15:42                             ` Michael Snyder
  2 siblings, 0 replies; 46+ messages in thread
From: Michael Snyder @ 2008-05-13 15:42 UTC (permalink / raw)
  To: gdb-patches

On Mon, 2008-05-12 at 16:39 -0400, Daniel Jacobowitz wrote:
> On Mon, May 12, 2008 at 01:06:44PM -0700, Michael Snyder wrote:
> > I wrote that overlay code over 10 years ago.
> > I wonder whether anyone still uses it?
> 
> On Mon, May 12, 2008 at 10:32:32PM +0200, Jan Kratochvil wrote:
> > I generally considered the overlays to be no longer supported and just still
> > not removed from the code.  While reading other GDB code I was feeling overlays
> > are no longer supported on multiple places but sure I may be wrong.
> > 
> > Could you please name which current arch is dependent on overlaying?
> 
> As you may be able to guess from context, the answer is SPU :-)
> 
> Each SPE has only a 256K local store which it uses for code, stack,
> and heap.  That's not very much space, so overlays are frequently
> required.

Oho, well, Stan Shebs should be gratified.  He assured me
at the time that overlays would get more use than I thought
they would.  ;-)





^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Overlay support broken (Re: [patch] [2/2] Discontiguous     PSYMTABs (psymtabs->symtabs by addrmap))
  2008-05-12 23:52                         ` Jan Kratochvil
@ 2008-05-13 18:45                           ` Ulrich Weigand
  2008-05-13 19:08                             ` Pedro Alves
  2008-05-15 16:39                             ` Jan Kratochvil
  0 siblings, 2 replies; 46+ messages in thread
From: Ulrich Weigand @ 2008-05-13 18:45 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches, drow

Jan Kratochvil wrote:
> I generally considered the overlays to be no longer supported and just still
> not removed from the code.  While reading other GDB code I was feeling overlays
> are no longer supported on multiple places but sure I may be wrong.
> 
> Could you please name which current arch is dependent on overlaying?

As Daniel mentioned, I noticed this on spu-elf, where we do make use of
overlays (at least code overlays -- data overlays are not currently used).

Which parts of GDB do you think do not support overlays?  I'd be interested
in fixing such problems ...

For now, I'm using the patch below that simply falls back to the non-addrmap
case when debugging overlays and the addrmap returned the wrong section.


In testing this, I noticed an independent overlay regression introduced by
a patch of mine:
http://sourceware.org/ml/gdb-patches/2008-05/msg00120.html
which added code to fixup_section to verify the msymbol address.

This patch exposed a pre-existing bug in fixup_section: it uses
  ginfo->value.address
to access the address of a symbol or psymbol.  This is mostly correct,
but in one case it is not: when handling a LOC_BLOCK symbol, in which
case ginfo->value.address is undefined, and the start address needs to
be retrieved from the block at ginfo->value.block.

The following patch fixes this regression as well by having the callers
of fixup_section pass in the proper address.


Tested on spu-elf, powerpc-linux and powerpc64-linux with no regressions;
fixes overlays.exp on spu-elf.

If there is no objection, I'd like to commit this to fix the present
regression until we have a proper implementation of addrmaps for the
overlay case.

Bye,
Ulrich


ChangeLog:

	* symtab.c (fixup_section): Remove prototype.  Add ADDR parameter;
	use it instead of ginfo->value.address.
	(fixup_symbol_section): Pass in correct symbol address.
	(fixup_psymbol_section): Pass in correct symbol address.

	(find_pc_sect_psymtab): Fall back to non-addrmap case when debugging
	overlays and the addrmap returned the wrong section.


diff -urNp gdb-orig/gdb/symtab.c gdb-head/gdb/symtab.c
--- gdb-orig/gdb/symtab.c	2008-05-11 23:41:56.000000000 +0200
+++ gdb-head/gdb/symtab.c	2008-05-13 15:48:55.626975367 +0200
@@ -110,8 +110,6 @@ struct symbol *lookup_symbol_aux_psymtab
 					   const domain_enum domain,
 					   struct symtab **symtab);
 
-static void fixup_section (struct general_symbol_info *, struct objfile *);
-
 static int file_matches (char *, char **, int);
 
 static void print_symbol_info (domain_enum,
@@ -878,6 +876,23 @@ find_pc_sect_psymtab (CORE_ADDR pc, asec
 	pst = addrmap_find (objfile->psymtabs_addrmap, pc);
 	if (pst != NULL)
 	  {
+	    /* FIXME: addrmaps currently do not handle overlayed sections,
+	       so fall back to the non-addrmap case if we're debugging 
+	       overlays and the addrmap returned the wrong section.  */
+	    if (overlay_debugging && msymbol && section)
+	      {
+		struct partial_symbol *p;
+		/* NOTE: This assumes that every psymbol has a
+		   corresponding msymbol, which is not necessarily
+		   true; the debug info might be much richer than the
+		   object's symbol table.  */
+		p = find_pc_sect_psymbol (pst, pc, section);
+		if (!p
+		    || SYMBOL_VALUE_ADDRESS (p)
+		       != SYMBOL_VALUE_ADDRESS (msymbol))
+		  continue;
+	      }
+
 	    /* We do not try to call FIND_PC_SECT_PSYMTAB_CLOSER as
 	       PSYMTABS_ADDRMAP we used has already the best 1-byte
 	       granularity and FIND_PC_SECT_PSYMTAB_CLOSER may mislead us into
@@ -1010,7 +1025,8 @@ find_pc_psymbol (struct partial_symtab *
    out of the minimal symbols and stash that in the debug symbol.  */
 
 static void
-fixup_section (struct general_symbol_info *ginfo, struct objfile *objfile)
+fixup_section (struct general_symbol_info *ginfo, CORE_ADDR addr,
+	       struct objfile *objfile)
 {
   struct minimal_symbol *msym;
   msym = lookup_minimal_symbol (ginfo->name, NULL, objfile);
@@ -1020,8 +1036,7 @@ fixup_section (struct general_symbol_inf
      e.g. on PowerPC64, where the minimal symbol for a function will
      point to the function descriptor, while the debug symbol will
      point to the actual function code.  */
-  if (msym
-      && SYMBOL_VALUE_ADDRESS (msym) == ginfo->value.address)
+  if (msym && SYMBOL_VALUE_ADDRESS (msym) == addr)
     {
       ginfo->bfd_section = SYMBOL_BFD_SECTION (msym);
       ginfo->section = SYMBOL_SECTION (msym);
@@ -1064,11 +1079,8 @@ fixup_section (struct general_symbol_inf
 	 this reason, we still attempt a lookup by name prior to doing
 	 a search of the section table.  */
 	 
-      CORE_ADDR addr;
       struct obj_section *s;
 
-      addr = ginfo->value.address;
-
       ALL_OBJFILE_OSECTIONS (objfile, s)
 	{
 	  int idx = s->the_bfd_section->index;
@@ -1093,7 +1105,22 @@ fixup_symbol_section (struct symbol *sym
   if (SYMBOL_BFD_SECTION (sym))
     return sym;
 
-  fixup_section (&sym->ginfo, objfile);
+  switch (SYMBOL_CLASS (sym))
+    {
+    case LOC_LABEL:
+    case LOC_STATIC:
+    case LOC_INDIRECT:
+      fixup_section (&sym->ginfo, SYMBOL_VALUE_ADDRESS (sym), objfile);
+      break;
+
+    case LOC_BLOCK:
+      fixup_section (&sym->ginfo,
+		     BLOCK_START (SYMBOL_BLOCK_VALUE (sym)), objfile);
+      break;
+
+    default:
+      break;
+    }
 
   return sym;
 }
@@ -1107,7 +1134,18 @@ fixup_psymbol_section (struct partial_sy
   if (SYMBOL_BFD_SECTION (psym))
     return psym;
 
-  fixup_section (&psym->ginfo, objfile);
+  switch (SYMBOL_CLASS (psym))
+    {
+    case LOC_LABEL:
+    case LOC_STATIC:
+    case LOC_INDIRECT:
+    case LOC_BLOCK:
+      fixup_section (&psym->ginfo, SYMBOL_VALUE_ADDRESS (psym), objfile);
+      break;
+
+    default:
+      break;
+    }
 
   return psym;
 }

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Overlay support broken (Re: [patch] [2/2] Discontiguous     PSYMTABs (psymtabs->symtabs by addrmap))
  2008-05-13 19:08                             ` Pedro Alves
@ 2008-05-13 19:01                               ` Pedro Alves
  2008-05-13 19:11                               ` Michael Snyder
  1 sibling, 0 replies; 46+ messages in thread
From: Pedro Alves @ 2008-05-13 19:01 UTC (permalink / raw)
  To: gdb-patches; +Cc: Ulrich Weigand, Jan Kratochvil, gdb-patches, drow

[-- Attachment #1: Type: text/plain, Size: 7363 bytes --]

A Tuesday 13 May 2008 18:00:48, Ulrich Weigand wrote:
> Jan Kratochvil wrote:
> > I generally considered the overlays to be no longer supported and just
> > still not removed from the code.  While reading other GDB code I was
> > feeling overlays are no longer supported on multiple places but sure I
> > may be wrong.
> >
> > Could you please name which current arch is dependent on overlaying?
>
> As Daniel mentioned, I noticed this on spu-elf, where we do make use of
> overlays (at least code overlays -- data overlays are not currently used).
>
> Which parts of GDB do you think do not support overlays?  I'd be interested
> in fixing such problems ...
>
> For now, I'm using the patch below that simply falls back to the
> non-addrmap case when debugging overlays and the addrmap returned the wrong
> section.
>
>
> In testing this, I noticed an independent overlay regression introduced by
> a patch of mine:
> http://sourceware.org/ml/gdb-patches/2008-05/msg00120.html
> which added code to fixup_section to verify the msymbol address.
>
> This patch exposed a pre-existing bug in fixup_section: it uses
>   ginfo->value.address
> to access the address of a symbol or psymbol.  This is mostly correct,
> but in one case it is not: when handling a LOC_BLOCK symbol, in which
> case ginfo->value.address is undefined, and the start address needs to
> be retrieved from the block at ginfo->value.block.
>
> The following patch fixes this regression as well by having the callers
> of fixup_section pass in the proper address.
>

FWIW, we have been using something very similar in our trees that I
had never submitted because of not being able to test on an arch
that uses overlays.

>
> Tested on spu-elf, powerpc-linux and powerpc64-linux with no regressions;
> fixes overlays.exp on spu-elf.
>
> If there is no objection, I'd like to commit this to fix the present
> regression until we have a proper implementation of addrmaps for the
> overlay case.
>

Please also make sure that you're setting SYMBOL_CLASS
before calling fixup_*symbol_section everywhere.  This will break
dwarf2read.c, for example:

 if (attr_form_is_block (attr)
      && DW_BLOCK (attr)->size == 1 + cu_header->addr_size
      && DW_BLOCK (attr)->data[0] == DW_OP_addr)
    {
      unsigned int dummy;

      SYMBOL_VALUE_ADDRESS (sym) =
	read_address (objfile->obfd, DW_BLOCK (attr)->data + 1, cu, &dummy);
      fixup_symbol_section (sym, objfile);
      SYMBOL_VALUE_ADDRESS (sym) += ANOFFSET (objfile->section_offsets,
					      SYMBOL_SECTION (sym));
      SYMBOL_CLASS (sym) = LOC_STATIC;
      return;
    }

The breakage goes unnoticed on linux, as all loadable sections will
have the same ANOFFSET, but will break in systems loading .text and
.data with different offsets, as uclinux, for example.

Attached is what we're using, as an example.  The testcase no longer
fails on head, I suspect because of your other patch that checks
the address.  The fix was for GDB fixing up the wrong section, due
to the lookup by name finding the wrong minsym.

> Bye,
> Ulrich
>
>
> ChangeLog:
>
> 	* symtab.c (fixup_section): Remove prototype.  Add ADDR parameter;
> 	use it instead of ginfo->value.address.
> 	(fixup_symbol_section): Pass in correct symbol address.
> 	(fixup_psymbol_section): Pass in correct symbol address.
>
> 	(find_pc_sect_psymtab): Fall back to non-addrmap case when debugging
> 	overlays and the addrmap returned the wrong section.
>
>
> diff -urNp gdb-orig/gdb/symtab.c gdb-head/gdb/symtab.c
> --- gdb-orig/gdb/symtab.c	2008-05-11 23:41:56.000000000 +0200
> +++ gdb-head/gdb/symtab.c	2008-05-13 15:48:55.626975367 +0200
> @@ -110,8 +110,6 @@ struct symbol *lookup_symbol_aux_psymtab
>  					   const domain_enum domain,
>  					   struct symtab **symtab);
>
> -static void fixup_section (struct general_symbol_info *, struct objfile
> *); -
>  static int file_matches (char *, char **, int);
>
>  static void print_symbol_info (domain_enum,
> @@ -878,6 +876,23 @@ find_pc_sect_psymtab (CORE_ADDR pc, asec
>  	pst = addrmap_find (objfile->psymtabs_addrmap, pc);
>  	if (pst != NULL)
>  	  {
> +	    /* FIXME: addrmaps currently do not handle overlayed sections,
> +	       so fall back to the non-addrmap case if we're debugging
> +	       overlays and the addrmap returned the wrong section.  */
> +	    if (overlay_debugging && msymbol && section)
> +	      {
> +		struct partial_symbol *p;
> +		/* NOTE: This assumes that every psymbol has a
> +		   corresponding msymbol, which is not necessarily
> +		   true; the debug info might be much richer than the
> +		   object's symbol table.  */
> +		p = find_pc_sect_psymbol (pst, pc, section);
> +		if (!p
> +		    || SYMBOL_VALUE_ADDRESS (p)
> +		       != SYMBOL_VALUE_ADDRESS (msymbol))
> +		  continue;
> +	      }
> +
>  	    /* We do not try to call FIND_PC_SECT_PSYMTAB_CLOSER as
>  	       PSYMTABS_ADDRMAP we used has already the best 1-byte
>  	       granularity and FIND_PC_SECT_PSYMTAB_CLOSER may mislead us into
> @@ -1010,7 +1025,8 @@ find_pc_psymbol (struct partial_symtab *
>     out of the minimal symbols and stash that in the debug symbol.  */
>
>  static void
> -fixup_section (struct general_symbol_info *ginfo, struct objfile *objfile)
> +fixup_section (struct general_symbol_info *ginfo, CORE_ADDR addr,
> +	       struct objfile *objfile)
>  {
>    struct minimal_symbol *msym;
>    msym = lookup_minimal_symbol (ginfo->name, NULL, objfile);
> @@ -1020,8 +1036,7 @@ fixup_section (struct general_symbol_inf
>       e.g. on PowerPC64, where the minimal symbol for a function will
>       point to the function descriptor, while the debug symbol will
>       point to the actual function code.  */
> -  if (msym
> -      && SYMBOL_VALUE_ADDRESS (msym) == ginfo->value.address)
> +  if (msym && SYMBOL_VALUE_ADDRESS (msym) == addr)
>      {
>        ginfo->bfd_section = SYMBOL_BFD_SECTION (msym);
>        ginfo->section = SYMBOL_SECTION (msym);
> @@ -1064,11 +1079,8 @@ fixup_section (struct general_symbol_inf
>  	 this reason, we still attempt a lookup by name prior to doing
>  	 a search of the section table.  */
>
> -      CORE_ADDR addr;
>        struct obj_section *s;
>
> -      addr = ginfo->value.address;
> -
>        ALL_OBJFILE_OSECTIONS (objfile, s)
>  	{
>  	  int idx = s->the_bfd_section->index;
> @@ -1093,7 +1105,22 @@ fixup_symbol_section (struct symbol *sym
>    if (SYMBOL_BFD_SECTION (sym))
>      return sym;
>
> -  fixup_section (&sym->ginfo, objfile);
> +  switch (SYMBOL_CLASS (sym))
> +    {
> +    case LOC_LABEL:
> +    case LOC_STATIC:
> +    case LOC_INDIRECT:
> +      fixup_section (&sym->ginfo, SYMBOL_VALUE_ADDRESS (sym), objfile);
> +      break;
> +
> +    case LOC_BLOCK:
> +      fixup_section (&sym->ginfo,
> +		     BLOCK_START (SYMBOL_BLOCK_VALUE (sym)), objfile);
> +      break;
> +
> +    default:
> +      break;
> +    }
>
>    return sym;
>  }
> @@ -1107,7 +1134,18 @@ fixup_psymbol_section (struct partial_sy
>    if (SYMBOL_BFD_SECTION (psym))
>      return psym;
>
> -  fixup_section (&psym->ginfo, objfile);
> +  switch (SYMBOL_CLASS (psym))
> +    {
> +    case LOC_LABEL:
> +    case LOC_STATIC:
> +    case LOC_INDIRECT:
> +    case LOC_BLOCK:
> +      fixup_section (&psym->ginfo, SYMBOL_VALUE_ADDRESS (psym), objfile);
> +      break;
> +
> +    default:
> +      break;
> +    }
>
>    return psym;
>  }



-- 
Pedro Alves

[-- Attachment #2: fixup_sections.diff --]
[-- Type: text/x-diff, Size: 8895 bytes --]

2008-01-23  Pedro Alves  <pedro@codesourcery.com>

	Match symbol/msymbol/bfd_section by address.

	gdb/
	* symtab.c (fixup_section): Add addr parameter.  If possible,
	lookup the minimal symbol by address.  If that fails, fallback to
	the old by-name method.
	(fixup_symbol_section): Ensure we always have an objfile to look
	into.  Extract and pass to fixup_section the symbol's address that
	will match the minimal symbol's address.
	(fixup_psymbol_section): Likewise.
	* dwarf2read.c (var_decode_location): Set SYMBOL_CLASS before
	calling fixup_symbol_section.

	gdb/testsuite/
	* gdb.base/fixsection.exp: New file.
	* gdb.base/fixsection0.c: New file.
	* gdb.base/fixsection1.c: New file.
---
 gdb/dwarf2read.c                      |    2 
 gdb/symtab.c                          |   70 ++++++++++++++++++++++++++++++---
 gdb/testsuite/gdb.base/fixsection.c   |   35 ++++++++++++++++
 gdb/testsuite/gdb.base/fixsection.exp |   71 ++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/fixsectshr.c   |   10 ++++
 5 files changed, 180 insertions(+), 8 deletions(-)

Index: src/gdb/symtab.c
===================================================================
--- src.orig/gdb/symtab.c	2008-05-13 19:04:29.000000000 +0100
+++ src/gdb/symtab.c	2008-05-13 19:14:31.000000000 +0100
@@ -110,8 +110,6 @@ struct symbol *lookup_symbol_aux_psymtab
 					   const domain_enum domain,
 					   struct symtab **symtab);
 
-static void fixup_section (struct general_symbol_info *, struct objfile *);
-
 static int file_matches (char *, char **, int);
 
 static void print_symbol_info (domain_enum,
@@ -1010,10 +1008,20 @@ find_pc_psymbol (struct partial_symtab *
    out of the minimal symbols and stash that in the debug symbol.  */
 
 static void
-fixup_section (struct general_symbol_info *ginfo, struct objfile *objfile)
+fixup_section (struct general_symbol_info *ginfo,
+	       CORE_ADDR addr, struct objfile *objfile)
 {
-  struct minimal_symbol *msym;
-  msym = lookup_minimal_symbol (ginfo->name, NULL, objfile);
+  struct minimal_symbol *msym = NULL;
+  if (addr != ~(CORE_ADDR) 0)
+    /* If we have an address to lookup, use it.  */
+    msym = lookup_minimal_symbol_by_pc (addr);
+
+  if (!msym
+      || addr != SYMBOL_VALUE_ADDRESS (msym)
+      || strcmp (DEPRECATED_SYMBOL_NAME (msym), ginfo->name) != 0)
+    /* Try by looking up by name.  Not perfect, since it can match the
+       wrong symbol.  */
+    msym = lookup_minimal_symbol (ginfo->name, NULL, objfile);
 
   /* First, check whether a minimal symbol with the same name exists
      and points to the same address.  The address check is required
@@ -1087,13 +1095,45 @@ fixup_section (struct general_symbol_inf
 struct symbol *
 fixup_symbol_section (struct symbol *sym, struct objfile *objfile)
 {
+  CORE_ADDR addr;
+
   if (!sym)
     return NULL;
 
   if (SYMBOL_BFD_SECTION (sym))
     return sym;
 
-  fixup_section (&sym->ginfo, objfile);
+  /* We either have an OBJFILE, or we can get at it from the sym's
+     symtab.  Anything else is a bug.  */
+  gdb_assert (objfile || (sym->symtab && sym->symtab->objfile));
+
+  if (objfile == NULL)
+    objfile = sym->symtab->objfile;
+
+  /* We should have an objfile by now.  */
+  gdb_assert (objfile);
+
+  switch (SYMBOL_CLASS (sym))
+    {
+    case LOC_UNRESOLVED:
+      addr = ~(CORE_ADDR) 0;
+      break;
+    case LOC_STATIC:
+    case LOC_LABEL:
+    case LOC_INDIRECT:
+      addr = SYMBOL_VALUE_ADDRESS (sym);
+      break;
+    case LOC_BLOCK:
+      addr = BLOCK_START (SYMBOL_BLOCK_VALUE (sym));
+      break;
+
+    default:
+      /* Nothing else will be listed in the minsyms -- no use looking
+	 it up.  */
+      return sym;
+    }
+
+  fixup_section (&sym->ginfo, addr, objfile);
 
   return sym;
 }
@@ -1101,13 +1141,29 @@ fixup_symbol_section (struct symbol *sym
 struct partial_symbol *
 fixup_psymbol_section (struct partial_symbol *psym, struct objfile *objfile)
 {
+  CORE_ADDR addr;
+
   if (!psym)
     return NULL;
 
   if (SYMBOL_BFD_SECTION (psym))
     return psym;
 
-  fixup_section (&psym->ginfo, objfile);
+  gdb_assert (objfile);
+
+  switch (SYMBOL_CLASS (psym))
+    {
+    case LOC_STATIC:
+    case LOC_BLOCK:
+      addr = SYMBOL_VALUE_ADDRESS (psym);
+      break;
+    default:
+      /* Nothing else will be listed in the minsyms -- no use looking
+	 it up.  */
+      return psym;
+    }
+
+  fixup_section (&psym->ginfo, addr, objfile);
 
   return psym;
 }
Index: src/gdb/dwarf2read.c
===================================================================
--- src.orig/gdb/dwarf2read.c	2008-05-13 19:04:29.000000000 +0100
+++ src/gdb/dwarf2read.c	2008-05-13 19:14:31.000000000 +0100
@@ -7323,10 +7323,10 @@ var_decode_location (struct attribute *a
 
       SYMBOL_VALUE_ADDRESS (sym) =
 	read_address (objfile->obfd, DW_BLOCK (attr)->data + 1, cu, &dummy);
+      SYMBOL_CLASS (sym) = LOC_STATIC;
       fixup_symbol_section (sym, objfile);
       SYMBOL_VALUE_ADDRESS (sym) += ANOFFSET (objfile->section_offsets,
 					      SYMBOL_SECTION (sym));
-      SYMBOL_CLASS (sym) = LOC_STATIC;
       return;
     }
 
Index: src/gdb/testsuite/gdb.base/fixsectshr.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/gdb.base/fixsectshr.c	2008-05-13 19:14:31.000000000 +0100
@@ -0,0 +1,10 @@
+#include <stdio.h>
+#include <stdlib.h>
+
+static FILE *static_fun = NULL;
+
+FILE *
+force_static_fun (void)
+{
+  return static_fun;
+}
Index: src/gdb/testsuite/gdb.base/fixsection.exp
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/gdb.base/fixsection.exp	2008-05-13 19:14:31.000000000 +0100
@@ -0,0 +1,71 @@
+# Copyright (C) 2008 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/>.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+set prms_id 0
+set bug_id 0
+
+if {[skip_shlib_tests]} {
+    return 0
+}
+
+set testfile "fixsection"
+set srcfile ${srcdir}/${subdir}/${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+
+set libfile "fixsectshr"
+set libsrc ${srcdir}/${subdir}/${libfile}.c
+set lib_sl ${objdir}/${subdir}/${libfile}.sl
+
+set lib_opts [list debug nowarnings]
+set exec_opts [list debug nowarnings shlib=$lib_sl]
+
+if [get_compiler_info ${binfile}] {
+    return -1
+}
+
+if { [gdb_compile_shlib $libsrc $lib_sl $lib_opts] != ""
+     || [gdb_compile $srcfile $binfile executable $exec_opts] != ""} {
+    untested "Could not compile either $libsrc or $srcfile."
+    return -1
+}
+
+# Start with a fresh gdb.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+gdb_load_shlibs ${lib_sl}
+
+if ![runto_main] then {
+    fail "Can't run to main"
+    return 1;
+}
+
+#
+# set breakpoint at static function static_fun
+#
+gdb_test "break static_fun" \
+    "Breakpoint.*at.* file .*${srcfile}, line.*" \
+    "breakpoint at static_fun"
+
+#
+# exit gdb
+#
+gdb_exit
Index: src/gdb/testsuite/gdb.base/fixsection.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/gdb.base/fixsection.c	2008-05-13 19:14:31.000000000 +0100
@@ -0,0 +1,35 @@
+/* Copyright (C) 2008 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/>.
+
+   This file was written by Pedro Alves (pedro@codesourcery.com).  */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+extern FILE *force_static_fun (void);
+
+static void *
+static_fun (void *arg)
+{
+    return NULL;
+}
+
+int
+main (int argc, char **argv)
+{
+  static_fun (NULL);
+  force_static_fun ();
+  return 0;
+}

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Overlay support broken (Re: [patch] [2/2] Discontiguous     PSYMTABs (psymtabs->symtabs by addrmap))
  2008-05-13 18:45                           ` Ulrich Weigand
@ 2008-05-13 19:08                             ` Pedro Alves
  2008-05-13 19:01                               ` Pedro Alves
  2008-05-13 19:11                               ` Michael Snyder
  2008-05-15 16:39                             ` Jan Kratochvil
  1 sibling, 2 replies; 46+ messages in thread
From: Pedro Alves @ 2008-05-13 19:08 UTC (permalink / raw)
  To: gdb-patches; +Cc: Ulrich Weigand, Jan Kratochvil, gdb-patches, drow

[-- Attachment #1: Type: text/plain, Size: 7363 bytes --]

A Tuesday 13 May 2008 18:00:48, Ulrich Weigand wrote:
> Jan Kratochvil wrote:
> > I generally considered the overlays to be no longer supported and just
> > still not removed from the code.  While reading other GDB code I was
> > feeling overlays are no longer supported on multiple places but sure I
> > may be wrong.
> >
> > Could you please name which current arch is dependent on overlaying?
>
> As Daniel mentioned, I noticed this on spu-elf, where we do make use of
> overlays (at least code overlays -- data overlays are not currently used).
>
> Which parts of GDB do you think do not support overlays?  I'd be interested
> in fixing such problems ...
>
> For now, I'm using the patch below that simply falls back to the
> non-addrmap case when debugging overlays and the addrmap returned the wrong
> section.
>
>
> In testing this, I noticed an independent overlay regression introduced by
> a patch of mine:
> http://sourceware.org/ml/gdb-patches/2008-05/msg00120.html
> which added code to fixup_section to verify the msymbol address.
>
> This patch exposed a pre-existing bug in fixup_section: it uses
>   ginfo->value.address
> to access the address of a symbol or psymbol.  This is mostly correct,
> but in one case it is not: when handling a LOC_BLOCK symbol, in which
> case ginfo->value.address is undefined, and the start address needs to
> be retrieved from the block at ginfo->value.block.
>
> The following patch fixes this regression as well by having the callers
> of fixup_section pass in the proper address.
>

FWIW, we have been using something very similar in our trees that I
had never submitted because of not being able to test on an arch
that uses overlays.

>
> Tested on spu-elf, powerpc-linux and powerpc64-linux with no regressions;
> fixes overlays.exp on spu-elf.
>
> If there is no objection, I'd like to commit this to fix the present
> regression until we have a proper implementation of addrmaps for the
> overlay case.
>

Please also make sure that you're setting SYMBOL_CLASS
before calling fixup_*symbol_section everywhere.  This will break
dwarf2read.c, for example:

 if (attr_form_is_block (attr)
      && DW_BLOCK (attr)->size == 1 + cu_header->addr_size
      && DW_BLOCK (attr)->data[0] == DW_OP_addr)
    {
      unsigned int dummy;

      SYMBOL_VALUE_ADDRESS (sym) =
	read_address (objfile->obfd, DW_BLOCK (attr)->data + 1, cu, &dummy);
      fixup_symbol_section (sym, objfile);
      SYMBOL_VALUE_ADDRESS (sym) += ANOFFSET (objfile->section_offsets,
					      SYMBOL_SECTION (sym));
      SYMBOL_CLASS (sym) = LOC_STATIC;
      return;
    }

The breakage goes unnoticed on linux, as all loadable sections will
have the same ANOFFSET, but will break in systems loading .text and
.data with different offsets, as uclinux, for example.

Attached is what we're using, as an example.  The testcase no longer
fails on head, I suspect because of your other patch that checks
the address.  The fix was for GDB fixing up the wrong section, due
to the lookup by name finding the wrong minsym.

> Bye,
> Ulrich
>
>
> ChangeLog:
>
> 	* symtab.c (fixup_section): Remove prototype.  Add ADDR parameter;
> 	use it instead of ginfo->value.address.
> 	(fixup_symbol_section): Pass in correct symbol address.
> 	(fixup_psymbol_section): Pass in correct symbol address.
>
> 	(find_pc_sect_psymtab): Fall back to non-addrmap case when debugging
> 	overlays and the addrmap returned the wrong section.
>
>
> diff -urNp gdb-orig/gdb/symtab.c gdb-head/gdb/symtab.c
> --- gdb-orig/gdb/symtab.c	2008-05-11 23:41:56.000000000 +0200
> +++ gdb-head/gdb/symtab.c	2008-05-13 15:48:55.626975367 +0200
> @@ -110,8 +110,6 @@ struct symbol *lookup_symbol_aux_psymtab
>  					   const domain_enum domain,
>  					   struct symtab **symtab);
>
> -static void fixup_section (struct general_symbol_info *, struct objfile
> *); -
>  static int file_matches (char *, char **, int);
>
>  static void print_symbol_info (domain_enum,
> @@ -878,6 +876,23 @@ find_pc_sect_psymtab (CORE_ADDR pc, asec
>  	pst = addrmap_find (objfile->psymtabs_addrmap, pc);
>  	if (pst != NULL)
>  	  {
> +	    /* FIXME: addrmaps currently do not handle overlayed sections,
> +	       so fall back to the non-addrmap case if we're debugging
> +	       overlays and the addrmap returned the wrong section.  */
> +	    if (overlay_debugging && msymbol && section)
> +	      {
> +		struct partial_symbol *p;
> +		/* NOTE: This assumes that every psymbol has a
> +		   corresponding msymbol, which is not necessarily
> +		   true; the debug info might be much richer than the
> +		   object's symbol table.  */
> +		p = find_pc_sect_psymbol (pst, pc, section);
> +		if (!p
> +		    || SYMBOL_VALUE_ADDRESS (p)
> +		       != SYMBOL_VALUE_ADDRESS (msymbol))
> +		  continue;
> +	      }
> +
>  	    /* We do not try to call FIND_PC_SECT_PSYMTAB_CLOSER as
>  	       PSYMTABS_ADDRMAP we used has already the best 1-byte
>  	       granularity and FIND_PC_SECT_PSYMTAB_CLOSER may mislead us into
> @@ -1010,7 +1025,8 @@ find_pc_psymbol (struct partial_symtab *
>     out of the minimal symbols and stash that in the debug symbol.  */
>
>  static void
> -fixup_section (struct general_symbol_info *ginfo, struct objfile *objfile)
> +fixup_section (struct general_symbol_info *ginfo, CORE_ADDR addr,
> +	       struct objfile *objfile)
>  {
>    struct minimal_symbol *msym;
>    msym = lookup_minimal_symbol (ginfo->name, NULL, objfile);
> @@ -1020,8 +1036,7 @@ fixup_section (struct general_symbol_inf
>       e.g. on PowerPC64, where the minimal symbol for a function will
>       point to the function descriptor, while the debug symbol will
>       point to the actual function code.  */
> -  if (msym
> -      && SYMBOL_VALUE_ADDRESS (msym) == ginfo->value.address)
> +  if (msym && SYMBOL_VALUE_ADDRESS (msym) == addr)
>      {
>        ginfo->bfd_section = SYMBOL_BFD_SECTION (msym);
>        ginfo->section = SYMBOL_SECTION (msym);
> @@ -1064,11 +1079,8 @@ fixup_section (struct general_symbol_inf
>  	 this reason, we still attempt a lookup by name prior to doing
>  	 a search of the section table.  */
>
> -      CORE_ADDR addr;
>        struct obj_section *s;
>
> -      addr = ginfo->value.address;
> -
>        ALL_OBJFILE_OSECTIONS (objfile, s)
>  	{
>  	  int idx = s->the_bfd_section->index;
> @@ -1093,7 +1105,22 @@ fixup_symbol_section (struct symbol *sym
>    if (SYMBOL_BFD_SECTION (sym))
>      return sym;
>
> -  fixup_section (&sym->ginfo, objfile);
> +  switch (SYMBOL_CLASS (sym))
> +    {
> +    case LOC_LABEL:
> +    case LOC_STATIC:
> +    case LOC_INDIRECT:
> +      fixup_section (&sym->ginfo, SYMBOL_VALUE_ADDRESS (sym), objfile);
> +      break;
> +
> +    case LOC_BLOCK:
> +      fixup_section (&sym->ginfo,
> +		     BLOCK_START (SYMBOL_BLOCK_VALUE (sym)), objfile);
> +      break;
> +
> +    default:
> +      break;
> +    }
>
>    return sym;
>  }
> @@ -1107,7 +1134,18 @@ fixup_psymbol_section (struct partial_sy
>    if (SYMBOL_BFD_SECTION (psym))
>      return psym;
>
> -  fixup_section (&psym->ginfo, objfile);
> +  switch (SYMBOL_CLASS (psym))
> +    {
> +    case LOC_LABEL:
> +    case LOC_STATIC:
> +    case LOC_INDIRECT:
> +    case LOC_BLOCK:
> +      fixup_section (&psym->ginfo, SYMBOL_VALUE_ADDRESS (psym), objfile);
> +      break;
> +
> +    default:
> +      break;
> +    }
>
>    return psym;
>  }



-- 
Pedro Alves

[-- Attachment #2: fixup_sections.diff --]
[-- Type: text/x-diff, Size: 8895 bytes --]

2008-01-23  Pedro Alves  <pedro@codesourcery.com>

	Match symbol/msymbol/bfd_section by address.

	gdb/
	* symtab.c (fixup_section): Add addr parameter.  If possible,
	lookup the minimal symbol by address.  If that fails, fallback to
	the old by-name method.
	(fixup_symbol_section): Ensure we always have an objfile to look
	into.  Extract and pass to fixup_section the symbol's address that
	will match the minimal symbol's address.
	(fixup_psymbol_section): Likewise.
	* dwarf2read.c (var_decode_location): Set SYMBOL_CLASS before
	calling fixup_symbol_section.

	gdb/testsuite/
	* gdb.base/fixsection.exp: New file.
	* gdb.base/fixsection0.c: New file.
	* gdb.base/fixsection1.c: New file.
---
 gdb/dwarf2read.c                      |    2 
 gdb/symtab.c                          |   70 ++++++++++++++++++++++++++++++---
 gdb/testsuite/gdb.base/fixsection.c   |   35 ++++++++++++++++
 gdb/testsuite/gdb.base/fixsection.exp |   71 ++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/fixsectshr.c   |   10 ++++
 5 files changed, 180 insertions(+), 8 deletions(-)

Index: src/gdb/symtab.c
===================================================================
--- src.orig/gdb/symtab.c	2008-05-13 19:04:29.000000000 +0100
+++ src/gdb/symtab.c	2008-05-13 19:14:31.000000000 +0100
@@ -110,8 +110,6 @@ struct symbol *lookup_symbol_aux_psymtab
 					   const domain_enum domain,
 					   struct symtab **symtab);
 
-static void fixup_section (struct general_symbol_info *, struct objfile *);
-
 static int file_matches (char *, char **, int);
 
 static void print_symbol_info (domain_enum,
@@ -1010,10 +1008,20 @@ find_pc_psymbol (struct partial_symtab *
    out of the minimal symbols and stash that in the debug symbol.  */
 
 static void
-fixup_section (struct general_symbol_info *ginfo, struct objfile *objfile)
+fixup_section (struct general_symbol_info *ginfo,
+	       CORE_ADDR addr, struct objfile *objfile)
 {
-  struct minimal_symbol *msym;
-  msym = lookup_minimal_symbol (ginfo->name, NULL, objfile);
+  struct minimal_symbol *msym = NULL;
+  if (addr != ~(CORE_ADDR) 0)
+    /* If we have an address to lookup, use it.  */
+    msym = lookup_minimal_symbol_by_pc (addr);
+
+  if (!msym
+      || addr != SYMBOL_VALUE_ADDRESS (msym)
+      || strcmp (DEPRECATED_SYMBOL_NAME (msym), ginfo->name) != 0)
+    /* Try by looking up by name.  Not perfect, since it can match the
+       wrong symbol.  */
+    msym = lookup_minimal_symbol (ginfo->name, NULL, objfile);
 
   /* First, check whether a minimal symbol with the same name exists
      and points to the same address.  The address check is required
@@ -1087,13 +1095,45 @@ fixup_section (struct general_symbol_inf
 struct symbol *
 fixup_symbol_section (struct symbol *sym, struct objfile *objfile)
 {
+  CORE_ADDR addr;
+
   if (!sym)
     return NULL;
 
   if (SYMBOL_BFD_SECTION (sym))
     return sym;
 
-  fixup_section (&sym->ginfo, objfile);
+  /* We either have an OBJFILE, or we can get at it from the sym's
+     symtab.  Anything else is a bug.  */
+  gdb_assert (objfile || (sym->symtab && sym->symtab->objfile));
+
+  if (objfile == NULL)
+    objfile = sym->symtab->objfile;
+
+  /* We should have an objfile by now.  */
+  gdb_assert (objfile);
+
+  switch (SYMBOL_CLASS (sym))
+    {
+    case LOC_UNRESOLVED:
+      addr = ~(CORE_ADDR) 0;
+      break;
+    case LOC_STATIC:
+    case LOC_LABEL:
+    case LOC_INDIRECT:
+      addr = SYMBOL_VALUE_ADDRESS (sym);
+      break;
+    case LOC_BLOCK:
+      addr = BLOCK_START (SYMBOL_BLOCK_VALUE (sym));
+      break;
+
+    default:
+      /* Nothing else will be listed in the minsyms -- no use looking
+	 it up.  */
+      return sym;
+    }
+
+  fixup_section (&sym->ginfo, addr, objfile);
 
   return sym;
 }
@@ -1101,13 +1141,29 @@ fixup_symbol_section (struct symbol *sym
 struct partial_symbol *
 fixup_psymbol_section (struct partial_symbol *psym, struct objfile *objfile)
 {
+  CORE_ADDR addr;
+
   if (!psym)
     return NULL;
 
   if (SYMBOL_BFD_SECTION (psym))
     return psym;
 
-  fixup_section (&psym->ginfo, objfile);
+  gdb_assert (objfile);
+
+  switch (SYMBOL_CLASS (psym))
+    {
+    case LOC_STATIC:
+    case LOC_BLOCK:
+      addr = SYMBOL_VALUE_ADDRESS (psym);
+      break;
+    default:
+      /* Nothing else will be listed in the minsyms -- no use looking
+	 it up.  */
+      return psym;
+    }
+
+  fixup_section (&psym->ginfo, addr, objfile);
 
   return psym;
 }
Index: src/gdb/dwarf2read.c
===================================================================
--- src.orig/gdb/dwarf2read.c	2008-05-13 19:04:29.000000000 +0100
+++ src/gdb/dwarf2read.c	2008-05-13 19:14:31.000000000 +0100
@@ -7323,10 +7323,10 @@ var_decode_location (struct attribute *a
 
       SYMBOL_VALUE_ADDRESS (sym) =
 	read_address (objfile->obfd, DW_BLOCK (attr)->data + 1, cu, &dummy);
+      SYMBOL_CLASS (sym) = LOC_STATIC;
       fixup_symbol_section (sym, objfile);
       SYMBOL_VALUE_ADDRESS (sym) += ANOFFSET (objfile->section_offsets,
 					      SYMBOL_SECTION (sym));
-      SYMBOL_CLASS (sym) = LOC_STATIC;
       return;
     }
 
Index: src/gdb/testsuite/gdb.base/fixsectshr.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/gdb.base/fixsectshr.c	2008-05-13 19:14:31.000000000 +0100
@@ -0,0 +1,10 @@
+#include <stdio.h>
+#include <stdlib.h>
+
+static FILE *static_fun = NULL;
+
+FILE *
+force_static_fun (void)
+{
+  return static_fun;
+}
Index: src/gdb/testsuite/gdb.base/fixsection.exp
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/gdb.base/fixsection.exp	2008-05-13 19:14:31.000000000 +0100
@@ -0,0 +1,71 @@
+# Copyright (C) 2008 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/>.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+set prms_id 0
+set bug_id 0
+
+if {[skip_shlib_tests]} {
+    return 0
+}
+
+set testfile "fixsection"
+set srcfile ${srcdir}/${subdir}/${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+
+set libfile "fixsectshr"
+set libsrc ${srcdir}/${subdir}/${libfile}.c
+set lib_sl ${objdir}/${subdir}/${libfile}.sl
+
+set lib_opts [list debug nowarnings]
+set exec_opts [list debug nowarnings shlib=$lib_sl]
+
+if [get_compiler_info ${binfile}] {
+    return -1
+}
+
+if { [gdb_compile_shlib $libsrc $lib_sl $lib_opts] != ""
+     || [gdb_compile $srcfile $binfile executable $exec_opts] != ""} {
+    untested "Could not compile either $libsrc or $srcfile."
+    return -1
+}
+
+# Start with a fresh gdb.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+gdb_load_shlibs ${lib_sl}
+
+if ![runto_main] then {
+    fail "Can't run to main"
+    return 1;
+}
+
+#
+# set breakpoint at static function static_fun
+#
+gdb_test "break static_fun" \
+    "Breakpoint.*at.* file .*${srcfile}, line.*" \
+    "breakpoint at static_fun"
+
+#
+# exit gdb
+#
+gdb_exit
Index: src/gdb/testsuite/gdb.base/fixsection.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/gdb.base/fixsection.c	2008-05-13 19:14:31.000000000 +0100
@@ -0,0 +1,35 @@
+/* Copyright (C) 2008 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/>.
+
+   This file was written by Pedro Alves (pedro@codesourcery.com).  */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+extern FILE *force_static_fun (void);
+
+static void *
+static_fun (void *arg)
+{
+    return NULL;
+}
+
+int
+main (int argc, char **argv)
+{
+  static_fun (NULL);
+  force_static_fun ();
+  return 0;
+}

^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Overlay support broken (Re: [patch] [2/2] Discontiguous      PSYMTABs (psymtabs->symtabs by addrmap))
  2008-05-13 19:08                             ` Pedro Alves
  2008-05-13 19:01                               ` Pedro Alves
@ 2008-05-13 19:11                               ` Michael Snyder
  1 sibling, 0 replies; 46+ messages in thread
From: Michael Snyder @ 2008-05-13 19:11 UTC (permalink / raw)
  To: gdb-patches

On Tue, 2008-05-13 at 19:20 +0100, Pedro Alves wrote:

> FWIW, we have been using something very similar in our trees that I
> had never submitted because of not being able to test on an arch
> that uses overlays.

Try building the m32r target and testing with the simulator.




^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Overlay support broken (Re: [patch] [2/2] Discontiguous  PSYMTABs (psymtabs->symtabs by addrmap))
  2008-05-13 18:45                           ` Ulrich Weigand
  2008-05-13 19:08                             ` Pedro Alves
@ 2008-05-15 16:39                             ` Jan Kratochvil
  2008-05-15 18:16                               ` Ulrich Weigand
  2008-05-15 19:18                               ` Michael Snyder
  1 sibling, 2 replies; 46+ messages in thread
From: Jan Kratochvil @ 2008-05-15 16:39 UTC (permalink / raw)
  To: Ulrich Weigand; +Cc: gdb-patches, drow

On Tue, 13 May 2008 19:00:48 +0200, Ulrich Weigand wrote:
...
> Which parts of GDB do you think do not support overlays?  I'd be interested
> in fixing such problems ...

There is still a lot of stub functions X() just calling X_sect() using
find_pc_mapped_section(), these should get removed as otherwise one may find
a countercase where it fails for the overlayed sections.
[blockvector_for_pc, block_for_pc, find_pc_psymtab, find_pc_psymbol,
 find_pc_symtab, find_pc_section, find_pc_function]


> For now, I'm using the patch below that simply falls back to the non-addrmap
> case when debugging overlays and the addrmap returned the wrong section.

I started coding a similiar patch as IMO the overlayed sections have no use for
addrmap as they are not discontiguous, thanks for fixing it up this way.

...
> If there is no objection, I'd like to commit this to fix the present
> regression

> until we have a proper implementation of addrmaps for the overlay case.

It is not worth it as it would mean to either
* support multiple results from a single call of addrmap_find()
or
* to move `addrmap_find' from `struct objfile' to each `struct partial_symtab'
which is IMO not worth the performance in a general case as your patch only
stays at the same performance as before addrmap for the overlayed case.



Regards,
Jan


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Overlay support broken (Re: [patch] [2/2] Discontiguous     PSYMTABs (psymtabs->symtabs by addrmap))
  2008-05-15 16:39                             ` Jan Kratochvil
@ 2008-05-15 18:16                               ` Ulrich Weigand
  2008-05-15 18:44                                 ` Daniel Jacobowitz
  2008-05-15 19:18                               ` Michael Snyder
  1 sibling, 1 reply; 46+ messages in thread
From: Ulrich Weigand @ 2008-05-15 18:16 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches, drow

Jan Kratochvil wrote:
> On Tue, 13 May 2008 19:00:48 +0200, Ulrich Weigand wrote:
> ...
> > Which parts of GDB do you think do not support overlays?  I'd be interested
> > in fixing such problems ...
> 
> There is still a lot of stub functions X() just calling X_sect() using
> find_pc_mapped_section(), these should get removed as otherwise one may find
> a countercase where it fails for the overlayed sections.
> [blockvector_for_pc, block_for_pc, find_pc_psymtab, find_pc_psymbol,
>  find_pc_symtab, find_pc_section, find_pc_function]

I thought those remaining usages were generally OK -- but I guess you're
right that new uses may have crept in over time ...

> > For now, I'm using the patch below that simply falls back to the non-addrmap
> > case when debugging overlays and the addrmap returned the wrong section.
> 
> I started coding a similiar patch as IMO the overlayed sections have no use for
> addrmap as they are not discontiguous, thanks for fixing it up this way.

Hmm, OK.  However, even with overlay debugging, there might be some other
discontiguous sections, so I don't really like the
  if (overlay_debugging ...)
aspect of my patch.  But without that condition, one of your new test cases
would fail again.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Overlay support broken (Re: [patch] [2/2] Discontiguous  PSYMTABs (psymtabs->symtabs by addrmap))
  2008-05-15 18:16                               ` Ulrich Weigand
@ 2008-05-15 18:44                                 ` Daniel Jacobowitz
  2008-05-15 19:06                                   ` Ulrich Weigand
  2008-05-16 18:32                                   ` Ulrich Weigand
  0 siblings, 2 replies; 46+ messages in thread
From: Daniel Jacobowitz @ 2008-05-15 18:44 UTC (permalink / raw)
  To: Ulrich Weigand; +Cc: Jan Kratochvil, gdb-patches

On Thu, May 15, 2008 at 06:38:48PM +0200, Ulrich Weigand wrote:
> > > For now, I'm using the patch below that simply falls back to the non-addrmap
> > > case when debugging overlays and the addrmap returned the wrong section.
> > 
> > I started coding a similiar patch as IMO the overlayed sections have no use for
> > addrmap as they are not discontiguous, thanks for fixing it up this way.
> 
> Hmm, OK.  However, even with overlay debugging, there might be some other
> discontiguous sections, so I don't really like the
>   if (overlay_debugging ...)
> aspect of my patch.  But without that condition, one of your new test cases
> would fail again.

Yes, I don't like that part either.  I wonder if the memory usage
would be too bad if we kept an addrmap for each section and one
combined one for the non-overlay case?

I don't know what overlay debug info looks like.  Can we detect
overlays in any practical way when reading in symbols?

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Overlay support broken (Re: [patch] [2/2] Discontiguous     PSYMTABs (psymtabs->symtabs by addrmap))
  2008-05-15 18:44                                 ` Daniel Jacobowitz
@ 2008-05-15 19:06                                   ` Ulrich Weigand
  2008-05-16 18:32                                   ` Ulrich Weigand
  1 sibling, 0 replies; 46+ messages in thread
From: Ulrich Weigand @ 2008-05-15 19:06 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: Jan Kratochvil, gdb-patches

Daniel Jacobowitz wrote:

> Yes, I don't like that part either.  I wonder if the memory usage
> would be too bad if we kept an addrmap for each section and one
> combined one for the non-overlay case?
> 
> I don't know what overlay debug info looks like.  Can we detect
> overlays in any practical way when reading in symbols?

Not really, I don't think there is any indication in the debug
info that we have an overlay.  The address ranges just happen
to overlap ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Overlay support broken (Re: [patch] [2/2] Discontiguous  PSYMTABs (psymtabs->symtabs by addrmap))
  2008-05-15 16:39                             ` Jan Kratochvil
  2008-05-15 18:16                               ` Ulrich Weigand
@ 2008-05-15 19:18                               ` Michael Snyder
  1 sibling, 0 replies; 46+ messages in thread
From: Michael Snyder @ 2008-05-15 19:18 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

On Thu, 2008-05-15 at 12:11 +0200, Jan Kratochvil wrote:
> On Tue, 13 May 2008 19:00:48 +0200, Ulrich Weigand wrote:
> ...
> > Which parts of GDB do you think do not support overlays?  I'd be interested
> > in fixing such problems ...
> 
> There is still a lot of stub functions X() just calling X_sect() using
> find_pc_mapped_section(), these should get removed as otherwise one may find
> a countercase where it fails for the overlayed sections.
> [blockvector_for_pc, block_for_pc, find_pc_psymtab, find_pc_psymbol,
>  find_pc_symtab, find_pc_section, find_pc_function]

?  It's been 10 years, but I think that's the way it is
supposed to work.

Old code calls X(), without supplying a section.
Then X forwards the call to X_sect() after attempting
to lookup the appropriate section.

In most cases this should not be critical for overlays, 
because code that depends on overlays should be calling
X_sect() directly.  It was just a shortcut so that we
wouldn't have to change all the calls of X into X_sect
(and do the section lookup in many places instead of one).



^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Overlay support broken (Re: [patch] [2/2] Discontiguous     PSYMTABs (psymtabs->symtabs by addrmap))
  2008-05-15 18:44                                 ` Daniel Jacobowitz
  2008-05-15 19:06                                   ` Ulrich Weigand
@ 2008-05-16 18:32                                   ` Ulrich Weigand
  1 sibling, 0 replies; 46+ messages in thread
From: Ulrich Weigand @ 2008-05-16 18:32 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: Jan Kratochvil, gdb-patches

Daniel Jacobowitz wrote:

> Yes, I don't like that part either.  I wonder if the memory usage
> would be too bad if we kept an addrmap for each section and one
> combined one for the non-overlay case?

Hmm, maybe this would even save memory; a per-section addrmap would
not need to store any result except from the boolean "address in
section or not" flag ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))
  2008-05-14  1:11 Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap)) Ulrich Weigand
@ 2008-05-14  8:07 ` Pedro Alves
  0 siblings, 0 replies; 46+ messages in thread
From: Pedro Alves @ 2008-05-14  8:07 UTC (permalink / raw)
  To: gdb-patches; +Cc: Ulrich Weigand, jan.kratochvil, drow

A Tuesday 13 May 2008 22:12:20, Ulrich Weigand wrote:
> Pedro Alves wrote:
> > +  struct minimal_symbol *msym = NULL;
> > +  if (addr != ~(CORE_ADDR) 0)
> > +    /* If we have an address to lookup, use it.  */
> > +    msym = lookup_minimal_symbol_by_pc (addr);
> > +
> > +  if (!msym
> > +      || addr != SYMBOL_VALUE_ADDRESS (msym)
> > +      || strcmp (DEPRECATED_SYMBOL_NAME (msym), ginfo->name) != 0)
> > +    /* Try by looking up by name.  Not perfect, since it can match the
> > +       wrong symbol.  */
> > +    msym = lookup_minimal_symbol (ginfo->name, NULL, objfile);
>
> Hmm, I guess there is the possibility that even though there is
> a msymbol at ADDR with name NAME, both
>   lookup_minimal_symbol_by_pc (ADDR)
> and
>   lookup_minimal_symbol (NAME)
>
> might fail to find it ...   E.g. if there are minimal symbols
>   ADDR' NAME
>   ADDR  NAME'
>   ADDR  NAME
> lookup by pc might find NAME', but lookup by name might find ADDR'.
>
> Maybe we need a lookup_minimal_symbol_by_pc_name or so?
>

Ah, good point.  Looks like that would be the best practical
approach, yes.

> > +  /* We either have an OBJFILE, or we can get at it from the sym's
> > +     symtab.  Anything else is a bug.  */
> > +  gdb_assert (objfile || (sym->symtab && sym->symtab->objfile));
> > +
> > +  if (objfile == NULL)
> > +    objfile = sym->symtab->objfile;
>
> Huh.  If that's true, why does fixup_symbol_section even have an
> OBJFILE argument?  Is there ever a situation where we cannot use
> sym->symtab->objfile?
>

Yes, while still reading the debug info, you can get here with
a sym->symtab == NULL, but you'll have a valid objfile to pass
down.  The symtab is then set at the end of end_symtab.  E.g.:

(gdb) p sym->symtab
$2 = (struct symtab *) 0x0

(gdb) bt
#0  fixup_symbol_section (sym=0xb583e0, objfile=0xb336d0) 
at ../../src/gdb/symtab.c:1090
#1  0x0000000000552a1a in var_decode_location (attr=0xb51380, sym=0xb583e0, 
cu=0xb4c7a0)
    at ../../src/gdb/dwarf2read.c:7326
#2  0x00000000005530ce in new_symbol (die=0xb512f0, type=0x0, cu=0xb4c7a0) 
at ../../src/gdb/dwarf2read.c:7462
#3  0x00000000005491a7 in process_die (die=0xb512f0, cu=0xb4c7a0) 
at ../../src/gdb/dwarf2read.c:2777
#4  0x00000000005495a9 in read_file_scope (die=0xb4a630, cu=0xb4c7a0) 
at ../../src/gdb/dwarf2read.c:2895
#5  0x00000000005490c4 in process_die (die=0xb4a630, cu=0xb4c7a0) 
at ../../src/gdb/dwarf2read.c:2713
#6  0x0000000000548fcd in process_full_comp_unit (per_cu=0xb418d0) 
at ../../src/gdb/dwarf2read.c:2680
#7  0x0000000000548a3a in process_queue (objfile=0xb336d0) 
at ../../src/gdb/dwarf2read.c:2476
#8  0x0000000000548c56 in psymtab_to_symtab_1 (pst=0xb41cd0) 
at ../../src/gdb/dwarf2read.c:2556
...

That was reading the the symbol "main" while stopping at "main".

You can also get here with a NULL objfile, for example, by 
means of lookup_symbol_aux_block, when you pass
a NULL symtab output parameter around, fixup_symbol_section
is always called with a NULL objfile, but it's OK, since at
the time lookup_symbol* are called, the symbols should already
have a sym->symtab and a sym->symtab->objfile.

(I actually have no idea why that output *symtab arg is needed
in the lookup_symbol* functions, if a symbol has a symtab
pointer.  Legacy?)

> > +  switch (SYMBOL_CLASS (sym))
> > +    {
> > +    case LOC_UNRESOLVED:
> > +      addr = ~(CORE_ADDR) 0;
> > +      break;
>
> Why do we need to fixup the section for an LOC_UNRESOLVED symbol?
>
> I understand that every time we want to use the address of a
> LOC_UNRESOLVED, the user needs to look up the msymbol anyway.
> They should then use the section from the msymbol too, right?
>

Ah, you're right, from read_var_value:

    case LOC_UNRESOLVED:
      {
	struct minimal_symbol *msym;

	msym = lookup_minimal_symbol (DEPRECATED_SYMBOL_NAME (var), NULL, NULL);
	if (msym == NULL)
	  return 0;
	if (overlay_debugging)
	  addr = symbol_overlayed_address (SYMBOL_VALUE_ADDRESS (msym),
					   SYMBOL_BFD_SECTION (msym));
	else
	  addr = SYMBOL_VALUE_ADDRESS (msym);
      }
      break;


> > +  switch (SYMBOL_CLASS (psym))
> > +    {
> > +    case LOC_STATIC:
> > +    case LOC_BLOCK:
> > +      addr = SYMBOL_VALUE_ADDRESS (psym);
> > +      break;
> > +    default:
> > +      /* Nothing else will be listed in the minsyms -- no use looking
> > +	 it up.  */
> > +      return psym;
> > +    }
>
> Any reason for not supporting LOC_LABEL or LOC_INDIRECT for psymbols?
>
> (Well, except from the fact that apparently none of the symbol readers
> left in GDB will ever generate LOC_INDIRECT ...  But at least mdebugread.c
> will generate LOC_LABEL psymbols, it seems.)
>

But that comes from debug info.  I didn't think LOC_LABEL's would ever
be listed in the minsyms.  Can they?  There's certainly no harm in
adding it to the switch, though.
As for LOC_INDIRECT, I remember finding out no reader records it, and
meaning to garbage collect it instead, but defered that to
submission time.  :-/   If we want to keep it, it looks like, yes,
we should be fixing up its section too.

-- 
Pedro Alves


^ permalink raw reply	[flat|nested] 46+ messages in thread

* Re: Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap))
@ 2008-05-14  1:11 Ulrich Weigand
  2008-05-14  8:07 ` Pedro Alves
  0 siblings, 1 reply; 46+ messages in thread
From: Ulrich Weigand @ 2008-05-14  1:11 UTC (permalink / raw)
  To: pedro; +Cc: gdb-patches, jan.kratochvil, drow

Pedro Alves wrote:

> Please also make sure that you're setting SYMBOL_CLASS
> before calling fixup_*symbol_section everywhere.  This will break
> dwarf2read.c, for example:

Ah, thanks for pointing that out!

> +  struct minimal_symbol *msym = NULL;
> +  if (addr != ~(CORE_ADDR) 0)
> +    /* If we have an address to lookup, use it.  */
> +    msym = lookup_minimal_symbol_by_pc (addr);
> +
> +  if (!msym
> +      || addr != SYMBOL_VALUE_ADDRESS (msym)
> +      || strcmp (DEPRECATED_SYMBOL_NAME (msym), ginfo->name) != 0)
> +    /* Try by looking up by name.  Not perfect, since it can match the
> +       wrong symbol.  */
> +    msym = lookup_minimal_symbol (ginfo->name, NULL, objfile);

Hmm, I guess there is the possibility that even though there is
a msymbol at ADDR with name NAME, both 
  lookup_minimal_symbol_by_pc (ADDR)
and
  lookup_minimal_symbol (NAME)

might fail to find it ...   E.g. if there are minimal symbols
  ADDR' NAME
  ADDR  NAME'
  ADDR  NAME
lookup by pc might find NAME', but lookup by name might find ADDR'.

Maybe we need a lookup_minimal_symbol_by_pc_name or so?

> +  /* We either have an OBJFILE, or we can get at it from the sym's
> +     symtab.  Anything else is a bug.  */
> +  gdb_assert (objfile || (sym->symtab && sym->symtab->objfile));
> +
> +  if (objfile == NULL)
> +    objfile = sym->symtab->objfile;

Huh.  If that's true, why does fixup_symbol_section even have an
OBJFILE argument?  Is there ever a situation where we cannot use
sym->symtab->objfile?

> +  switch (SYMBOL_CLASS (sym))
> +    {
> +    case LOC_UNRESOLVED:
> +      addr = ~(CORE_ADDR) 0;
> +      break;

Why do we need to fixup the section for an LOC_UNRESOLVED symbol?

I understand that every time we want to use the address of a
LOC_UNRESOLVED, the user needs to look up the msymbol anyway.
They should then use the section from the msymbol too, right?


> +  switch (SYMBOL_CLASS (psym))
> +    {
> +    case LOC_STATIC:
> +    case LOC_BLOCK:
> +      addr = SYMBOL_VALUE_ADDRESS (psym);
> +      break;
> +    default:
> +      /* Nothing else will be listed in the minsyms -- no use looking
> +	 it up.  */
> +      return psym;
> +    }

Any reason for not supporting LOC_LABEL or LOC_INDIRECT for psymbols?

(Well, except from the fact that apparently none of the symbol readers
left in GDB will ever generate LOC_INDIRECT ...  But at least mdebugread.c
will generate LOC_LABEL psymbols, it seems.)

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com


^ permalink raw reply	[flat|nested] 46+ messages in thread

end of thread, other threads:[~2008-05-16 15:27 UTC | newest]

Thread overview: 46+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-10-09 18:17 [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU, for vDSO32) Jan Kratochvil
2007-10-09 18:22 ` Daniel Jacobowitz
2007-10-09 18:59   ` Jan Kratochvil
2007-10-09 19:13     ` Daniel Jacobowitz
2007-11-24 15:43       ` Jan Kratochvil
2007-11-25 14:48         ` Daniel Jacobowitz
2007-11-30  7:42         ` Vladimir Prus
2007-11-30 11:10           ` Jan Kratochvil
2007-11-30 14:56             ` Daniel Jacobowitz
2007-11-30 15:09               ` Jan Kratochvil
2007-12-01  0:55               ` Jim Blandy
2007-12-01 17:30                 ` Joel Brobecker
2007-12-09 20:40               ` [patch] Discontiguous PSYMTABs [Re: [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU, for vDSO32)] Jan Kratochvil
2007-12-10  0:21                 ` [patch] Removal of the FIND_PC_SECT_PSYMTAB search [Re: [patch] Discontiguous PSYMTABs] Jan Kratochvil
2007-12-17  1:02                 ` [patch] Discontiguous PSYMTABs [Re: [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU, for vDSO32)] Daniel Jacobowitz
2007-12-17  1:03                   ` Daniel Jacobowitz
2007-12-17  2:41                     ` [patch] Discontiguous PSYMTABs Jan Kratochvil
2007-12-17  3:41                       ` Daniel Jacobowitz
2008-04-23 22:15                     ` [patch] [0/2] " Jan Kratochvil
2008-04-23 22:18                     ` [patch] [1/2] Discontiguous PSYMTABs (partial DIEs base address) Jan Kratochvil
2008-05-01 19:43                       ` Daniel Jacobowitz
2008-04-23 22:24                     ` [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap) Jan Kratochvil
2008-05-01 19:46                       ` Daniel Jacobowitz
2008-05-04 17:38                         ` Jan Kratochvil
2008-05-12 22:24                       ` Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap)) Ulrich Weigand
2008-05-12 22:37                         ` Michael Snyder
2008-05-13  1:39                           ` Daniel Jacobowitz
2008-05-13  3:17                             ` Jan Kratochvil
2008-05-13 15:37                             ` Doug Evans
2008-05-13 15:42                             ` Michael Snyder
2008-05-13 15:31                           ` Doug Evans
2008-05-12 23:52                         ` Jan Kratochvil
2008-05-13 18:45                           ` Ulrich Weigand
2008-05-13 19:08                             ` Pedro Alves
2008-05-13 19:01                               ` Pedro Alves
2008-05-13 19:11                               ` Michael Snyder
2008-05-15 16:39                             ` Jan Kratochvil
2008-05-15 18:16                               ` Ulrich Weigand
2008-05-15 18:44                                 ` Daniel Jacobowitz
2008-05-15 19:06                                   ` Ulrich Weigand
2008-05-16 18:32                                   ` Ulrich Weigand
2008-05-15 19:18                               ` Michael Snyder
2008-04-23 21:31                 ` [patch] Discontiguous PSYMTABs [Re: [patch] Parse DW_AT_ranges into PSYMTABS (for childless CU, for vDSO32)] Doug Evans
2008-04-23 21:31                   ` Jan Kratochvil
2008-05-14  1:11 Overlay support broken (Re: [patch] [2/2] Discontiguous PSYMTABs (psymtabs->symtabs by addrmap)) Ulrich Weigand
2008-05-14  8:07 ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox