[commit] fix symtab/12302

[commit] fix symtab/12302

[Doug Evans]

Hi.

fyi, I've committed this fix to symtab/12302.
It changes address table generation to be a simple dump of the current
addrmap.

There's been discussion of removing this table and using .debug_aranges
instead.  Until then, this was easy enough.

2010-12-08  Doug Evans  <dje@google.com>

	PR symtab/12302
	* dwarf2read.c (struct psymtab_cu_index_map): New struct.
	(hash_psymtab_cu_index, eq_psymtab_cu_index): New functions.
	(struct addrmap_index_data): New struct.
	(add_address_entry): Remove arg `pst', new args `start', `end'.
	(add_address_entry_worker, write_address_map): New functions.
	(write_psymtabs_to_index): Address table generation moved to
	write_address_map.  Build a table mapping psymtab to CU index
	to pass to it.

Index: dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.486
diff -u -p -r1.486 dwarf2read.c
--- dwarf2read.c	8 Dec 2010 18:26:40 -0000	1.486
+++ dwarf2read.c	8 Dec 2010 18:47:29 -0000
@@ -15137,31 +15137,129 @@ write_hash_table (struct mapped_symtab *
   htab_delete (symbol_hash_table);
 }
 
-/* Write an address entry to ADDR_OBSTACK.  The addresses are taken
-   from PST; CU_INDEX is the index of the CU in the vector of all
-   CUs.  */
+/* Struct to map psymtab to CU index in the index file.  */
+struct psymtab_cu_index_map
+{
+  struct partial_symtab *psymtab;
+  unsigned int cu_index;
+};
+
+static hashval_t
+hash_psymtab_cu_index (const void *item)
+{
+  const struct psymtab_cu_index_map *map = item;
+
+  return htab_hash_pointer (map->psymtab);
+}
+
+static int
+eq_psymtab_cu_index (const void *item_lhs, const void *item_rhs)
+{
+  const struct psymtab_cu_index_map *lhs = item_lhs;
+  const struct psymtab_cu_index_map *rhs = item_rhs;
+
+  return lhs->psymtab == rhs->psymtab;
+}
+
+/* Helper struct for building the address table.  */
+struct addrmap_index_data
+{
+  struct objfile *objfile;
+  struct obstack *addr_obstack;
+  htab_t cu_index_htab;
+
+  /* Non-zero if the previous_* fields are valid.
+     We can't write an entry until we see the next entry (since it is only then
+     that we know the end of the entry).  */
+  int previous_valid;
+  /* Index of the CU in the table of all CUs in the index file.  */
+  unsigned int previous_cu_index;
+  /* Start address of the CU. */
+  CORE_ADDR previous_cu_start;
+};
+
+/* Write an address entry to OBSTACK.  */
 
 static void
-add_address_entry (struct objfile *objfile,
-		   struct obstack *addr_obstack, struct partial_symtab *pst,
-		   unsigned int cu_index)
+add_address_entry (struct objfile *objfile, struct obstack *obstack,
+		   CORE_ADDR start, CORE_ADDR end, unsigned int cu_index)
 {
-  offset_type offset;
+  offset_type cu_index_to_write;
   char addr[8];
   CORE_ADDR baseaddr;
 
-  /* Don't bother recording empty ranges.  */
-  if (pst->textlow == pst->texthigh)
-    return;
-
   baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
 
-  store_unsigned_integer (addr, 8, BFD_ENDIAN_LITTLE, pst->textlow - baseaddr);
-  obstack_grow (addr_obstack, addr, 8);
-  store_unsigned_integer (addr, 8, BFD_ENDIAN_LITTLE, pst->texthigh - baseaddr);
-  obstack_grow (addr_obstack, addr, 8);
-  offset = MAYBE_SWAP (cu_index);
-  obstack_grow (addr_obstack, &offset, sizeof (offset_type));
+  store_unsigned_integer (addr, 8, BFD_ENDIAN_LITTLE, start - baseaddr);
+  obstack_grow (obstack, addr, 8);
+  store_unsigned_integer (addr, 8, BFD_ENDIAN_LITTLE, end - baseaddr);
+  obstack_grow (obstack, addr, 8);
+  cu_index_to_write = MAYBE_SWAP (cu_index);
+  obstack_grow (obstack, &cu_index_to_write, sizeof (offset_type));
+}
+
+/* Worker function for traversing an addrmap to build the address table.  */
+
+static int
+add_address_entry_worker (void *datap, CORE_ADDR start_addr, void *obj)
+{
+  struct addrmap_index_data *data = datap;
+  struct partial_symtab *pst = obj;
+  offset_type cu_index;
+  void **slot;
+
+  if (data->previous_valid)
+    add_address_entry (data->objfile, data->addr_obstack,
+		       data->previous_cu_start, start_addr,
+		       data->previous_cu_index);
+
+  data->previous_cu_start = start_addr;
+  if (pst != NULL)
+    {
+      struct psymtab_cu_index_map find_map, *map;
+      find_map.psymtab = pst;
+      map = htab_find (data->cu_index_htab, &find_map);
+      gdb_assert (map != NULL);
+      data->previous_cu_index = map->cu_index;
+      data->previous_valid = 1;
+    }
+  else
+      data->previous_valid = 0;
+
+  return 0;
+}
+
+/* Write OBJFILE's address map to OBSTACK.
+   CU_INDEX_HTAB is used to map addrmap entries to their CU indices
+   in the index file.  */
+
+static void
+write_address_map (struct objfile *objfile, struct obstack *obstack,
+		   htab_t cu_index_htab)
+{
+  struct addrmap_index_data addrmap_index_data;
+
+  /* When writing the address table, we have to cope with the fact that
+     the addrmap iterator only provides the start of a region; we have to
+     wait until the next invocation to get the start of the next region.  */
+
+  addrmap_index_data.objfile = objfile;
+  addrmap_index_data.addr_obstack = obstack;
+  addrmap_index_data.cu_index_htab = cu_index_htab;
+  addrmap_index_data.previous_valid = 0;
+
+  addrmap_foreach (objfile->psymtabs_addrmap, add_address_entry_worker,
+		   &addrmap_index_data);
+
+  /* It's highly unlikely the last entry (end address = 0xff...ff)
+     is valid, but we should still handle it.
+     The end address is recorded as the start of the next region, but that
+     doesn't work here.  To cope we pass 0xff...ff, this is a rare situation
+     anyway.  */
+  if (addrmap_index_data.previous_valid)
+    add_address_entry (objfile, obstack,
+		       addrmap_index_data.previous_cu_start, (CORE_ADDR) -1,
+		       addrmap_index_data.previous_cu_index);
 }
 
 /* Add a list of partial symbols to SYMTAB.  */
@@ -15294,6 +15392,8 @@ write_psymtabs_to_index (struct objfile 
   struct stat st;
   char buf[8];
   htab_t psyms_seen;
+  htab_t cu_index_htab;
+  struct psymtab_cu_index_map *psymtab_cu_index_map;
 
   if (!objfile->psymtabs)
     return;
@@ -15330,7 +15430,21 @@ write_psymtabs_to_index (struct objfile 
 				  NULL, xcalloc, xfree);
   make_cleanup (cleanup_htab, psyms_seen);
 
-  /* The list is already sorted, so we don't need to do additional
+  /* While we're scanning CU's create a table that maps a psymtab pointer
+     (which is what addrmap records) to its index (which is what is recorded
+     in the index file).  This will later be needed to write the address
+     table.  */
+  cu_index_htab = htab_create_alloc (100,
+				     hash_psymtab_cu_index,
+				     eq_psymtab_cu_index,
+				     NULL, xcalloc, xfree);
+  make_cleanup (cleanup_htab, cu_index_htab);
+  psymtab_cu_index_map = (struct psymtab_cu_index_map *)
+    xmalloc (sizeof (struct psymtab_cu_index_map)
+	     * dwarf2_per_objfile->n_comp_units);
+  make_cleanup (xfree, psymtab_cu_index_map);
+
+  /* The CU list is already sorted, so we don't need to do additional
      work here.  Also, the debug_types entries do not appear in
      all_comp_units, but only in their own hash table.  */
   for (i = 0; i < dwarf2_per_objfile->n_comp_units; ++i)
@@ -15338,6 +15452,8 @@ write_psymtabs_to_index (struct objfile 
       struct dwarf2_per_cu_data *per_cu = dwarf2_per_objfile->all_comp_units[i];
       struct partial_symtab *psymtab = per_cu->v.psymtab;
       gdb_byte val[8];
+      struct psymtab_cu_index_map *map;
+      void **slot;
 
       write_psymbols (symtab,
 		      psyms_seen,
@@ -15350,7 +15466,13 @@ write_psymtabs_to_index (struct objfile 
 		      psymtab->n_static_syms, i,
 		      1);
 
-      add_address_entry (objfile, &addr_obstack, psymtab, i);
+      map = &psymtab_cu_index_map[i];
+      map->psymtab = psymtab;
+      map->cu_index = i;
+      slot = htab_find_slot (cu_index_htab, map, INSERT);
+      gdb_assert (slot != NULL);
+      gdb_assert (*slot == NULL);
+      *slot = map;
 
       store_unsigned_integer (val, 8, BFD_ENDIAN_LITTLE, per_cu->offset);
       obstack_grow (&cu_list, val, 8);
@@ -15358,6 +15480,9 @@ write_psymtabs_to_index (struct objfile 
       obstack_grow (&cu_list, val, 8);
     }
 
+  /* Dump the address map.  */
+  write_address_map (objfile, &addr_obstack, cu_index_htab);
+
   /* Write out the .debug_type entries, if any.  */
   if (dwarf2_per_objfile->signatured_types)
     {

.gdb_index version 4? [Re: [commit] fix symtab/12302]

[Jan Kratochvil]

On Wed, 08 Dec 2010 20:15:49 +0100, Doug Evans wrote:
> fyi, I've committed this fix to symtab/12302.
> It changes address table generation to be a simple dump of the current
> addrmap.

This means there exist .gdb_indexes out there (such as whole Fedora 14) which
break GDB behavior (I could not list glibc sources today before -readnow).

Do you agree with upgrading the index version, Tom?

Also thanks for the fix, Doug.


Thanks,
Jan


gdb/
2011-01-21  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* dwarf2read.c (dwarf2_read_index, write_psymtabs_to_index)
	(save_gdb_index_command): Switch to .gdb_index version 4.

--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -2024,13 +2024,14 @@ dwarf2_read_index (struct objfile *objfile)
   /* Version check.  */
   version = MAYBE_SWAP (*(offset_type *) addr);
   /* Versions earlier than 3 emitted every copy of a psymbol.  This
-     causes the index to behave very poorly for certain requests.  So,
-     it seems better to just ignore such indices.  */
-  if (version < 3)
+     causes the index to behave very poorly for certain requests.  Version 4
+     contained incomplete addrmap.  So, it seems better to just ignore such
+     indices.  */
+  if (version < 4)
     return 0;
   /* Indexes with higher version than the one supported by GDB may be no
      longer backward compatible.  */
-  if (version > 3)
+  if (version > 4)
     return 0;
 
   map = OBSTACK_ZALLOC (&objfile->objfile_obstack, struct mapped_index);
@@ -15688,7 +15689,7 @@ write_psymtabs_to_index (struct objfile *objfile, const char *dir)
   total_len = size_of_contents;
 
   /* The version number.  */
-  val = MAYBE_SWAP (3);
+  val = MAYBE_SWAP (4);
   obstack_grow (&contents, &val, sizeof (val));
 
   /* The offset of the CU list from the start of the file.  */
@@ -15746,7 +15747,7 @@ write_psymtabs_to_index (struct objfile *objfile, const char *dir)
    1. The file header.  This is a sequence of values, of offset_type
    unless otherwise noted:
 
-   [0] The version number, currently 3.  Versions 1 and 2 are
+   [0] The version number, currently 4.  Versions 1, 2 and 3 are
    obsolete.
    [1] The offset, from the start of the file, of the CU list.
    [2] The offset, from the start of the file, of the types CU list.

Re: .gdb_index version 4? [Re: [commit] fix symtab/12302]

[Doug Evans]

On Fri, Jan 21, 2011 at 5:19 AM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> On Wed, 08 Dec 2010 20:15:49 +0100, Doug Evans wrote:
>> fyi, I've committed this fix to symtab/12302.
>> It changes address table generation to be a simple dump of the current
>> addrmap.
>
> This means there exist .gdb_indexes out there (such as whole Fedora 14) which
> break GDB behavior (I could not list glibc sources today before -readnow).
>
> Do you agree with upgrading the index version, Tom?
>
> Also thanks for the fix, Doug.

For reference sake,
you'll also need to update binutils/dwarf.c:display_gdb_index.

Re: .gdb_index version 4? [Re: [commit] fix symtab/12302]

[Jan Kratochvil]

On Fri, 21 Jan 2011 14:19:56 +0100, Jan Kratochvil wrote:
> This means there exist .gdb_indexes out there (such as whole Fedora 14) which
> break GDB behavior (I could not list glibc sources today before -readnow).
> 
> Do you agree with upgrading the index version, Tom?

Tom agreed off-list, therefore checked the version 4 in:
	http://sourceware.org/ml/gdb-cvs/2011-01/msg00169.html

Going to post the `readelf --debug-dump=gdb_index' fix.


Thanks,
Jan