[RFA] fix build failure in solib-som.c

[RFA] fix build failure in solib-som.c

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 733 bytes --]

Hello,

It looks like a change made a month ago that removed a field called
textsection from the so_list structure is the cause for a build failure
in solib-som.c.

I haven't tested this change, but wouldn't the attached patch make sense?
I don't think we need to keep track of which section is the text section
at this place, since this is done differently somewhere else (in the
current_sos method). So I just removed the associated piece of code.

2007-08-07  Joel Brobecker  <brobecker@adacore.com>

        * solib-som.c (som_relocate_section_addresses): Stop saving
        the $CODE$ section in the so_list structure.

This files compiles fine again after this change is applied.

Would that be OK to apply?

Thanks,
-- 
Joel

[-- Attachment #2: solib-som.c.diff --]
[-- Type: text/plain, Size: 746 bytes --]

Index: solib-som.c
===================================================================
RCS file: /cvs/src/src/gdb/solib-som.c,v
retrieving revision 1.11
diff -u -p -r1.11 solib-som.c
--- solib-som.c	2 Jul 2007 22:01:09 -0000	1.11
+++ solib-som.c	7 Aug 2007 20:35:36 -0000
@@ -113,13 +113,6 @@ som_relocate_section_addresses (struct s
 {
   flagword aflag = bfd_get_section_flags(so->abfd, sec->the_bfd_section);
 
-  /* solib.c does something similar, but it only recognizes ".text", SOM calls
-     the text section "$CODE$".  */
-  if (strcmp (sec->the_bfd_section->name, "$CODE$") == 0)
-    {
-      so->textsection = sec;
-    }
-
   if (aflag & SEC_CODE)
     {
       sec->addr    += so->lm_info->text_addr - so->lm_info->text_link_addr; 

Re: [RFA] fix build failure in solib-som.c

[Kevin Buettner]

On Tue, 7 Aug 2007 13:45:29 -0700
Joel Brobecker <brobecker@adacore.com> wrote:

> It looks like a change made a month ago that removed a field called
> textsection from the so_list structure is the cause for a build failure
> in solib-som.c.
> 
> I haven't tested this change, but wouldn't the attached patch make sense?
> I don't think we need to keep track of which section is the text section
> at this place, since this is done differently somewhere else (in the
> current_sos method). So I just removed the associated piece of code.
> 
> 2007-08-07  Joel Brobecker  <brobecker@adacore.com>
> 
>         * solib-som.c (som_relocate_section_addresses): Stop saving
>         the $CODE$ section in the so_list structure.
> 
> This files compiles fine again after this change is applied.
> 
> Would that be OK to apply?

I think it would be better to revise the code to set so->addr_low and
so->addr_high to sec->addr and sec->endaddr respectively.  You'll also
have to move this block of code to the end of the function.

Kevin

Re: [RFA] fix build failure in solib-som.c

[Joel Brobecker]

> > I haven't tested this change, but wouldn't the attached patch make sense?
> > I don't think we need to keep track of which section is the text section
> > at this place, since this is done differently somewhere else (in the
> > current_sos method). So I just removed the associated piece of code.
> > 
> > 2007-08-07  Joel Brobecker  <brobecker@adacore.com>
> > 
> >         * solib-som.c (som_relocate_section_addresses): Stop saving
> >         the $CODE$ section in the so_list structure.
> > 
> > This files compiles fine again after this change is applied.
> > 
> > Would that be OK to apply?
> 
> I think it would be better to revise the code to set so->addr_low and
> so->addr_high to sec->addr and sec->endaddr respectively.  You'll also
> have to move this block of code to the end of the function.

Isn't that already done in som_current_sos, though?

-- 
Joel

Re: [RFA] fix build failure in solib-som.c

[Kevin Buettner]

On Tue, 7 Aug 2007 15:43:48 -0700
Joel Brobecker <brobecker@adacore.com> wrote:

> > > I haven't tested this change, but wouldn't the attached patch make sense?
> > > I don't think we need to keep track of which section is the text section
> > > at this place, since this is done differently somewhere else (in the
> > > current_sos method). So I just removed the associated piece of code.
> > > 
> > > 2007-08-07  Joel Brobecker  <brobecker@adacore.com>
> > > 
> > >         * solib-som.c (som_relocate_section_addresses): Stop saving
> > >         the $CODE$ section in the so_list structure.
> > > 
> > > This files compiles fine again after this change is applied.
> > > 
> > > Would that be OK to apply?
> > 
> > I think it would be better to revise the code to set so->addr_low and
> > so->addr_high to sec->addr and sec->endaddr respectively.  You'll also
> > have to move this block of code to the end of the function.
> 
> Isn't that already done in som_current_sos, though?

It is, but those addresses won't be relocated.  In order to be
consistent with the other ports, I think these values should be
relocated.  Note that solib.c's solib_map_sections() sets `addr_low'
and `addr_high' for .text after the relocation has been done.  Clearly
this won't work for SOM since the .text section is named differently. 
However, given that `addr_low' and `addr_high' are set after the
relocation has been performed (for other ports with a .text), I think
it makes sense to arrange the SOM-specific code as suggested in my
earlier reply.  I'm beginning to think too that the assignments to
addr_low and addr_high ought to be removed from som_current_sos().

That said, it's not critical that `addr_low' and `addr_high' be
relocated since they're only used for display purposes in "info
sharedlibrary".  If it makes more sense for this port to display
unrelocated values, them I'm happy with your original patch.  I'll
leave it to you to decide the best course of action.

Kevin

Re: [RFA] fix build failure in solib-som.c

[Joel Brobecker]

Hi Kevin,

> > > > 2007-08-07  Joel Brobecker  <brobecker@adacore.com>
> > > > 
> > > >         * solib-som.c (som_relocate_section_addresses): Stop saving
> > > >         the $CODE$ section in the so_list structure.
> > > > 
> > > > This files compiles fine again after this change is applied.
> > > > 
> > > > Would that be OK to apply?
> > > 
> > > I think it would be better to revise the code to set so->addr_low and
> > > so->addr_high to sec->addr and sec->endaddr respectively.  You'll also
> > > have to move this block of code to the end of the function.
> > 
> > Isn't that already done in som_current_sos, though?
> 
> It is, but those addresses won't be relocated.  In order to be
> consistent with the other ports, I think these values should be
> relocated.  Note that solib.c's solib_map_sections() sets `addr_low'
> and `addr_high' for .text after the relocation has been done.  Clearly
> this won't work for SOM since the .text section is named differently. 
> However, given that `addr_low' and `addr_high' are set after the
> relocation has been performed (for other ports with a .text), I think
> it makes sense to arrange the SOM-specific code as suggested in my
> earlier reply.  I'm beginning to think too that the assignments to
> addr_low and addr_high ought to be removed from som_current_sos().

I had a closer look. I don't think we need to relocate the addr_low
and addr_high addresses, because they were extracted from the load
map info (which I would imagine would contain already relocated
addresses). This is in som_current_sos.

This is only by luck, but it looks like my original patch still
makes sense, no? I did a quick test by running any program, and
doing a "info sharedlibrary", and it looks likes the addresses
are indeed relocated (they don't correspond to the $CODE$ or $TEXT$
addresses printed by objdump).

-- 
Joel

Re: [RFA] fix build failure in solib-som.c

[Kevin Buettner]

On Wed, 8 Aug 2007 10:44:37 -0700
Joel Brobecker <brobecker@adacore.com> wrote:

> Hi Kevin,
> 
> > > > > 2007-08-07  Joel Brobecker  <brobecker@adacore.com>
> > > > > 
> > > > >         * solib-som.c (som_relocate_section_addresses): Stop saving
> > > > >         the $CODE$ section in the so_list structure.
> > > > > 
> > > > > This files compiles fine again after this change is applied.
> > > > > 
> > > > > Would that be OK to apply?
> > > > 
> > > > I think it would be better to revise the code to set so->addr_low and
> > > > so->addr_high to sec->addr and sec->endaddr respectively.  You'll also
> > > > have to move this block of code to the end of the function.
> > > 
> > > Isn't that already done in som_current_sos, though?
> > 
> > It is, but those addresses won't be relocated.  In order to be
> > consistent with the other ports, I think these values should be
> > relocated.  Note that solib.c's solib_map_sections() sets `addr_low'
> > and `addr_high' for .text after the relocation has been done.  Clearly
> > this won't work for SOM since the .text section is named differently. 
> > However, given that `addr_low' and `addr_high' are set after the
> > relocation has been performed (for other ports with a .text), I think
> > it makes sense to arrange the SOM-specific code as suggested in my
> > earlier reply.  I'm beginning to think too that the assignments to
> > addr_low and addr_high ought to be removed from som_current_sos().
> 
> I had a closer look. I don't think we need to relocate the addr_low
> and addr_high addresses, because they were extracted from the load
> map info (which I would imagine would contain already relocated
> addresses). This is in som_current_sos.

Ah, okay.

> This is only by luck, but it looks like my original patch still
> makes sense, no? I did a quick test by running any program, and
> doing a "info sharedlibrary", and it looks likes the addresses
> are indeed relocated (they don't correspond to the $CODE$ or $TEXT$
> addresses printed by objdump).

Sounds good.  Go ahead and check in your original patch.

Kevin

Re: [RFA] fix build failure in solib-som.c

[Joel Brobecker]

> > > > > > 2007-08-07  Joel Brobecker  <brobecker@adacore.com>
> > > > > > 
> > > > > >         * solib-som.c (som_relocate_section_addresses): Stop saving
> > > > > >         the $CODE$ section in the so_list structure.
> 
> Sounds good.  Go ahead and check in your original patch.

Thanks. Now checked in.

-- 
Joel