From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8855 invoked by alias); 21 Sep 2007 21:57:45 -0000 Received: (qmail 8846 invoked by uid 22791); 21 Sep 2007 21:57:44 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 21 Sep 2007 21:57:41 +0000 Received: (qmail 6568 invoked from network); 21 Sep 2007 21:57:39 -0000 Received: from unknown (HELO localhost) (jimb@127.0.0.2) by mail.codesourcery.com with ESMTPA; 21 Sep 2007 21:57:39 -0000 To: gdb-patches@sourceware.org Subject: RFC: Fix qOffsets handling From: Jim Blandy Date: Fri, 21 Sep 2007 21:57:00 -0000 Message-ID: User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.0.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2007-09/txt/msg00281.txt.bz2 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 * 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; };