On Thursday 05 June 2008 20:16:02, Daniel Jacobowitz wrote:
> On Fri, May 16, 2008 at 05:21:15PM +0100, Pedro Alves wrote:
> > I've been meaning to test this on Cygwin/MinGW/Dwarf before
> > posting, but since the subject came up, here it goes anyway.
>
> I looked through the patch, and I can't see anything wrong with it.
> It's OK to commit, though you might want to do that Cygwin test
> first...

I've finally passed this through the testsuite on mingw32 + dwarf, and
found no regressions.

I checked it in after also retesting on x86_64-unknown-linux-gnu.

-- 
Pedro Alves

On Friday 16 May 2008 17:21:15, Pedro Alves wrote:
> Hi,
>
> I've been meaning to test this on Cygwin/MinGW/Dwarf before
> posting, but since the subject came up, here it goes anyway.
>
> GDB has currently several places where section addresses/offsets
> are stored: bfd, so_list, exec_ops, and objfile->section_offsets[],
> and obj_section.
>
> We should be consolidating most of these.  As a first step, the place
> that looks most redundant is the one in obj_section.
>
> This patch removes the addresses in obj_section, in objfiles.h:
> > /* Sections in an objfile.
> >
> >    It is strange that we have both this notion of "sections"
> >    and the one used by section_offsets.  Section as used
> >    here, (currently at least) means a BFD section, and the sections
> >    are set up from the BFD sections in allocate_objfile.
> >
> >    The sections in section_offsets have their meaning determined by
> >    the symbol format, and they are set up by the sym_offsets function
> >    for that symbol file format.
> >
> >    I'm not sure this could or should be changed, however.  */
> >
> > struct obj_section
> >   {
> >     CORE_ADDR addr;		/* lowest address in section */
> >     CORE_ADDR endaddr;		/* 1+highest address in section */
> >
> >     /* This field is being used for nefarious purposes by
> > syms_from_objfile. It is said to be redundant with section_offsets; it's
> > not really
>
> being
>
> >        used that way, however, it's some sort of hack I don't understand
> >        and am not going to try to eliminate (yet, anyway).  FIXME.
> >
> >        It was documented as "offset between (end)addr and actual memory
> >        addresses", but that's not true; addr & endaddr are actual memory
> >        addresses.  */
> >     CORE_ADDR offset;
> >
> >     struct bfd_section *the_bfd_section;	/* BFD section pointer */
> >
> >     /* Objfile this section is part of.  */
> >     struct objfile *objfile;
> >
> >     /* True if this "overlay section" is mapped into an "overlay region".
> > */ int ovly_mapped;
> >   };
>
> - When using a relocatable object as an exec_file, we adjust all
>   allocatable sections to not have overlapping vma's by adjusting the
> bfd_vma and the objfile->section_offsets[] and the exec_ops.to_sections. 
> The obj_section->(addr,endaddr,offset) are not.  Although this ends up
> being harmless, it is still bad for consistency.
>
> - There the hack in symfile.c guarded by the
>   DEPRECATED_IBM6000_TARGET macro, which sets the
>   objfile->section_offsets[] back to obj_section->(addr,endaddr,offset),
>   but even that looks wrong:
>
>   ADDR is the memory address (after relocation), OFFSET is the
>   relocation offset that was applied (comments mine),
>
>  s->addr -= s->offset; /* revert offset, so we get the VMA on file.  */
>  s->addr += s_addr;   /* (addr has been converted to offsets,
>                          so s_addr is now the new offset.)  */
>  s->endaddr -= s->offset;
>  s->endaddr += s_addr;
>
>  s->offset += s_addr;  /* This looks wrong.  It should be '=', not '+=' ?? 
> */
>
>   See patch for context.
>
> - The DEPRECATED_IBM6000_TARGET guard is there, to the best of my
>   knowledge, because xcoffread.c ignores the offsets that are passed to
>   xcoff_symfile_offsets (sym->fns->sym_offsets), and sets the
>   objfile->sections_offsets[] all to 0.  By merging the obj_section offsets
>   with objfile->section_offsets[], we just get rid of that block, and
> things should work.  I have no means to test it, though, so this worries be
> me.
>
>   xcoff_symfile_offsets:
>
>   for (i = 0; i < objfile->num_sections; ++i)
>     {
>       /* syms_from_objfile kindly subtracts from addr the
> 	 bfd_section_vma of the .text section.  This strikes me as
> 	 wrong--whether the offset to be applied to symbol reading is
> 	 relative to the start address of the section depends on the
> 	 symbol format.  In any event, this whole "addr" concept is
> 	 pretty broken (it doesn't handle any section but .text
> 	 sensibly), so just ignore the addr parameter and use 0.
> 	 rs6000-nat.c will set the correct section offsets via
> 	 objfile_relocate.  */
> 	(objfile->section_offsets)->offsets[i] = 0;
>     }
>
>   I seems the comment is a bit offbase.  The reader still applies
>   ANOFFSET to the symbols, so psymtab->symtab expansion time sees
>   the correct section offsets.  It feels like the xcoff reader
>   should cope with the section offsets != 0 internally, and not
>   do this ignoring, but since I don't have access to any AIX system,
>   I didn't investigate that further.  But I do think things should
>   still work as they used to.
>
> I just gave it another testsuite run on on x86-64-unknown-linux-gnu.
> No changes recorded.