RFC: Fix qOffsets handling

[Jim Blandy <jimb@codesourcery.com>]

This patch fixes a bug that causes GDB to misapply the offsets
recieved in response to a qOffsets packet.  It's a pretty serious bug
for any kind of remote target that uses that packet and returns the
older 'Text=xxx;Data=xxx;Bss=xxx' response, as opposed to the newer
'TextSeg=xxx;DataSeg=xxx' response.

Daniel Jacobowitz has approved this change off-list, and asked me to
suggest it be included on the 6.7 branch as well.  I'll wait for
further comments here, and commit to trunk on Monday; Joel, is this
okay for the branch?

gdb/ChangeLog:
2007-09-21  Jim Blandy  <jimb@codesourcery.com>

	* symfile.h (struct symfile_segment_data): Doc fixes.
	* symfile.c (symfile_map_offsets_to_segments): Doc fixes.
	Assert that we were passed some loaded segment addresses,
	and that sections' segment numbers are valid.
	Simplify offset calculation.
	* remote.c (get_offsets): Clarify selection of relocate-by-segment
	strategy, and set num_segments correctly.  Delete redundant
	assignments to do_sections.

diff -r 77afe7ffac2f gdb/remote.c
--- a/gdb/remote.c	Fri Sep 21 14:38:59 2007 -0700
+++ b/gdb/remote.c	Fri Sep 21 14:39:28 2007 -0700
@@ -2103,28 +2103,24 @@ get_offsets (void)
   do_segments = (data != NULL);
   do_sections = num_segments == 0;
 
-  /* Text= and Data= specify offsets for the text and data sections,
-     but symfile_map_offsets_to_segments expects base addresses
-     instead of offsets.  If we have two segments, we can still
-     try to relocate the whole segments instead of just ".text"
-     and ".data".  */
-  if (num_segments == 0)
-    {
-      do_sections = 1;
-      if (data == NULL || data->num_segments != 2)
-	do_segments = 0;
-      else
-	{
-	  segments[0] = data->segment_bases[0] + text_addr;
-	  segments[1] = data->segment_bases[1] + data_addr;
-	}
-    }
-  else
-    {
-      do_sections = 0;
+  if (num_segments > 0)
+    {
       segments[0] = text_addr;
       segments[1] = data_addr;
     }
+  /* If we have two segments, we can still try to relocate everything
+     by assuming that the .text and .data offsets apply to the whole
+     text and data segments.  Convert the offsets given in the packet
+     to base addresses for symfile_map_offsets_to_segments.  */
+  else if (data && data->num_segments == 2)
+    {
+      segments[0] = data->segment_bases[0] + text_addr;
+      segments[1] = data->segment_bases[1] + data_addr;
+      num_segments = 2;
+    }
+  /* There's no way to relocate by segment.  */
+  else
+    do_segments = 0;
 
   if (do_segments)
     {
diff -r 77afe7ffac2f gdb/symfile.c
--- a/gdb/symfile.c	Fri Sep 21 14:38:59 2007 -0700
+++ b/gdb/symfile.c	Fri Sep 21 14:39:28 2007 -0700
@@ -3987,6 +3987,22 @@ free_symfile_segment_data (struct symfil
   xfree (data);
 }
 
+
+/* Given:
+   - DATA, containing segment addresses from the object file ABFD, and
+     the mapping from ABFD's sections onto the segments that own them,
+     and
+   - SEGMENT_BASES[0 .. NUM_SEGMENT_BASES - 1], holding the actual
+     segment addresses reported by the target,
+   store the appropriate offsets for each section in OFFSETS.
+
+   If there are fewer entries in SEGMENT_BASES than there are segments
+   in DATA, then apply SEGMENT_BASES' last entry to all the segments.
+
+   If there are more, then verify that all the excess addresses are
+   the same as the last legitimate one, and then ignore them.  This
+   allows "TextSeg=X;DataSeg=X" qOffset replies for files which have
+   only a single segment.  */
 int
 symfile_map_offsets_to_segments (bfd *abfd, struct symfile_segment_data *data,
 				 struct section_offsets *offsets,
@@ -3996,15 +4012,16 @@ symfile_map_offsets_to_segments (bfd *ab
   int i;
   asection *sect;
 
+  /* It doesn't make sense to call this function unless you have some
+     segment base addresses.  */
+  gdb_assert (segment_bases > 0);
+
   /* If we do not have segment mappings for the object file, we
      can not relocate it by segments.  */
   gdb_assert (data != NULL);
   gdb_assert (data->num_segments > 0);
 
-  /* If more offsets are provided than we have segments, make sure the
-     excess offsets are all the same as the last segment's offset.
-     This allows "Text=X;Data=X" for files which have only a single
-     segment.  */
+  /* Check any extra SEGMENT_BASES entries.  */
   if (num_segment_bases > data->num_segments)
     for (i = data->num_segments; i < num_segment_bases; i++)
       if (segment_bases[i] != segment_bases[data->num_segments - 1])
@@ -4012,17 +4029,22 @@ symfile_map_offsets_to_segments (bfd *ab
 
   for (i = 0, sect = abfd->sections; sect != NULL; i++, sect = sect->next)
     {
-      CORE_ADDR vma;
       int which = data->segment_info[i];
 
+      gdb_assert (0 <= which && which <= data->num_segments);
+
+      /* Don't bother computing offsets for sections that aren't
+         loaded as part of any segment.  */
+      if (! which)
+        continue;
+
+      /* Use the last SEGMENT_BASES entry as the address of any extra
+         segments mentioned in DATA->segment_info.  */
       if (which > num_segment_bases)
-	offsets->offsets[i] = segment_bases[num_segment_bases - 1];
-      else if (which > 0)
-	offsets->offsets[i] = segment_bases[which - 1];
-      else
-	continue;
-
-      offsets->offsets[i] -= data->segment_bases[which - 1];
+        which = num_segment_bases;
+
+      offsets->offsets[i] = (segment_bases[which - 1]
+                             - data->segment_bases[which - 1]);
     }
 
   return 1;
diff -r 77afe7ffac2f gdb/symfile.h
--- a/gdb/symfile.h	Fri Sep 21 14:38:59 2007 -0700
+++ b/gdb/symfile.h	Fri Sep 21 14:39:28 2007 -0700
@@ -83,6 +83,9 @@ struct section_addr_info
   } other[1];
 };
 
+
+/* A table listing the load segments in a symfile, and which segment
+   each BFD section belongs to.  */
 struct symfile_segment_data
 {
   /* How many segments are present in this file.  If there are
@@ -99,9 +102,9 @@ struct symfile_segment_data
   CORE_ADDR *segment_sizes;
 
   /* If NUM_SEGMENTS is greater than zero, this is an array of entries
-     recording which segment contains each BFD section.  It is
-     ordered by section index.  A zero means that the section is not
-     in any segment.  */
+     recording which segment contains each BFD section.
+     SEGMENT_INFO[I] is S+1 if the I'th BFD section belongs to segment
+     S, or zero if it is not in any segment.  */
   int *segment_info;
 };