[RFC] gdb_bfd_count_sections snafu

[Doug Evans <dje@google.com>]

Hi.
I was getting different failures for jit.exp depending on whether
gdb was compiled with/without optimization.
That drove me to dig deeper.
It turned out that the objfile->msymbols array was being sorted differently
because several symbols like _DYNAMIC had been given different
addresses depending on whether gdb was compiled with -O0 or -O2.
[And if symbols moved in the resort then jit.c's cached minsyms would
get clobbered - but that's another bug. :-)]
After a bit more digging I found an out of bounds array access
in the section offset computation, and the offset was coming from the
stack, so that explained the difference I saw.

_DYNAMIC lives in the special bfd abs section, and thus
has a special section number computed by
gdb_bfd.c:gdb_bfd_section_index.
In my case bfd_count_sections was 32 and the section number for
_DYNAMIC was 35.

int
gdb_bfd_section_index (bfd *abfd, asection *section)
{
  if (section == NULL)
    return -1;
  else if (section == bfd_com_section_ptr)
    return bfd_count_sections (abfd) + 1;
  else if (section == bfd_und_section_ptr)
    return bfd_count_sections (abfd) + 2;
  else if (section == bfd_abs_section_ptr)
    return bfd_count_sections (abfd) + 3;
  else if (section == bfd_ind_section_ptr)
    return bfd_count_sections (abfd) + 4;
  return section->index;
}

There is a wrapper around bfd_count_sections to include these
extra sections: gdb_bfd.c:gdb_bfd_count_sections.

int
gdb_bfd_count_sections (bfd *abfd)
{
  return bfd_count_sections (abfd) + 4;
}

HOWEVER, objfile->num_sections is computed with bfd_count_sections
not gdb_bfd_count_sections.

I didn't dig deeper as I'm not familiar with all the file formats,
there are more calls to bfd_count_sections that perhaps should be
gdb_bfd_count_sections.

This patch is at least, I think(!), a step in the right direction.
There's some cleanups I'd like to do but I'll send those separately.
This patch does clean up one thing: AFAICT when syms_from_objfile_1
is passed NULL for both addrs and offsets, there's no point in
building local_addr to have more than one entry (zero would be fine
too I think but space needs to be allocated for at least one entry).

   if (! addrs && ! offsets)
     {
-      local_addr
-	= alloc_section_addr_info (bfd_count_sections (objfile->obfd));
+      local_addr = alloc_section_addr_info (1);
       make_cleanup (xfree, local_addr);
       addrs = local_addr;
     }

Anyways, here's the patch.
It fixes the jit.exp failure (but alas covers up another bug -
the minsym caching bug I reported earlier).
Regression tested on amd64-linux.

[I'm a bit concerned going forward.  I think we need to put in
some time to at least add more comments to alert the reader.
I can imagine one not always being clear when to use
bfd_count_sections and gdb_bfd_count_sections without such comments.
If you choose the wrong one the failure could be another bug like this.

My preference would be code changes to remove the possibility
for picking the wrong one, but that would, I think, be non-trivial.
Something for another day unless one wants to take it on now.
In the meantime we should at least alert the reader.]

2013-04-17  Doug Evans  <dje@google.com>

	* objfiles.c (objfile_relocate): Use gdb_bfd_count_sections instead
	of bfd_count_sections.
	* solib-target.c (solib_target_relocate_section_addresses): Ditto.
	* symfile.c (default_symfile_offsets): Ditto.
	(syms_from_objfile_1): Ditto.  Make dummy addrs list an array of
	one entry, not bfd_count_sections entries.

Index: objfiles.c
===================================================================
RCS file: /cvs/src/src/gdb/objfiles.c,v
retrieving revision 1.158
diff -u -p -r1.158 objfiles.c
--- objfiles.c	8 Apr 2013 20:04:42 -0000	1.158
+++ objfiles.c	17 Apr 2013 07:07:07 -0000
@@ -880,7 +880,7 @@ objfile_relocate (struct objfile *objfil
       addr_info_make_relative (objfile_addrs, debug_objfile->obfd);
 
       gdb_assert (debug_objfile->num_sections
-		  == bfd_count_sections (debug_objfile->obfd));
+		  == gdb_bfd_count_sections (debug_objfile->obfd));
       new_debug_offsets = 
 	xmalloc (SIZEOF_N_SECTION_OFFSETS (debug_objfile->num_sections));
       make_cleanup (xfree, new_debug_offsets);
Index: solib-target.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-target.c,v
retrieving revision 1.28
diff -u -p -r1.28 solib-target.c
--- solib-target.c	8 Apr 2013 20:04:42 -0000	1.28
+++ solib-target.c	17 Apr 2013 07:07:07 -0000
@@ -339,7 +339,7 @@ solib_target_relocate_section_addresses 
      it any earlier, since we need to open the file first.  */
   if (so->lm_info->offsets == NULL)
     {
-      int num_sections = bfd_count_sections (so->abfd);
+      int num_sections = gdb_bfd_count_sections (so->abfd);
 
       so->lm_info->offsets = xzalloc (SIZEOF_N_SECTION_OFFSETS (num_sections));
 
Index: symfile.c
===================================================================
RCS file: /cvs/src/src/gdb/symfile.c,v
retrieving revision 1.370
diff -u -p -r1.370 symfile.c
--- symfile.c	8 Apr 2013 20:04:42 -0000	1.370
+++ symfile.c	17 Apr 2013 07:07:07 -0000
@@ -678,7 +678,7 @@ void
 default_symfile_offsets (struct objfile *objfile,
 			 struct section_addr_info *addrs)
 {
-  objfile->num_sections = bfd_count_sections (objfile->obfd);
+  objfile->num_sections = gdb_bfd_count_sections (objfile->obfd);
   objfile->section_offsets = (struct section_offsets *)
     obstack_alloc (&objfile->objfile_obstack,
 		   SIZEOF_N_SECTION_OFFSETS (objfile->num_sections));
@@ -948,7 +948,7 @@ syms_from_objfile_1 (struct objfile *obj
     {
       /* No symbols to load, but we still need to make sure
 	 that the section_offsets table is allocated.  */
-      int num_sections = bfd_count_sections (objfile->obfd);
+      int num_sections = gdb_bfd_count_sections (objfile->obfd);
       size_t size = SIZEOF_N_SECTION_OFFSETS (num_offsets);
 
       objfile->num_sections = num_sections;
@@ -967,8 +967,7 @@ syms_from_objfile_1 (struct objfile *obj
      no load address was specified.  */
   if (! addrs && ! offsets)
     {
-      local_addr
-	= alloc_section_addr_info (bfd_count_sections (objfile->obfd));
+      local_addr = alloc_section_addr_info (1);
       make_cleanup (xfree, local_addr);
       addrs = local_addr;
     }