Re: [patch] Fix regression on prelinked executables

[Jan Kratochvil <jan.kratochvil@redhat.com>]

On Thu, 29 Jul 2010 18:36:48 +0200, Joel Brobecker wrote:
> > Curiously GDB already uses at
> > many places
> > 	baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
> > instead of using offset for the appropriate section at that place and nobody
> > complains.
> 
> It's something I actually noticed almost 10 years ago, now, while
> working on Tru64, I think. But I never really found a situation that
> required me to work on that, so...

OK, I will one day try to turn section_offsets array into a single variable.


> >  	  if (!(strcmp (sect_name, ".gnu.liblist") == 0
> >  		|| strcmp (sect_name, ".gnu.conflict") == 0
> > -		|| strcmp (sect_name, ".dynbss") == 0
> > -		|| strcmp (sect_name, ".sdynbss") == 0))
> > +		|| (strcmp (sect_name, ".bss") == 0
> > +		    && i > 0
> > +		    && strcmp (addrs->other[i - 1].name, ".dynbss") == 0
> > +		    && addrs_to_abfd_addrs[i - 1] != NULL)
> > +		|| (strcmp (sect_name, ".sbss") == 0
> > +		    && i > 0
> > +		    && strcmp (addrs->other[i - 1].name, ".sdynbss") == 0
> > +		    && addrs_to_abfd_addrs[i - 1] != NULL)))
            warning (_("section %s not found in %s"), sect_name,
                     bfd_get_filename (abfd));
> 
> I had to think over this for a while, and I think that the two new checks
> are correct.  However, I'm still trying to figure out why it's correct
> to also remove the simple check for .dynbss. In other words, if .dynbss
> is missing from the separate debug object file, shouldn't we emit a
> warning?

The meaning is reversed.  It is !(a || b || c).  Which I find more readable
than (!a && !b && !c).  But I will have to change my mind as it seems to not
everyone may consider my chosen format as more readable.

The code above says do NOT warn if section is either .gnu.liblist or
.gnu.conflict or etc.

That is it was very irresponsible from me to put .dynbss and .sdynbss there
before.  I should have examined what they really mean, when they appear etc.
I have seen them as added by prelink so I thought we can ignore them.  I did
not consider they contain the right address for .bss/.sbss as the real
.bss/.sbss address is invalid (from the GDB point of view) that time already.

That is the change above reducing the enumerated list in fact increases the
probability we give a warning.

Another issue is that we no longer suppress the warning for .dynbss/.sdynbss
as before and it works.  It is due to the new addr_section_name function,
originally .dynbss/.sdynbss got to this point while now .dynbss/.sdynbss get
processed but .bss/.sbss gets to this point instead.  Making the code
	  if (!(strcmp (sect_name, ".gnu.liblist") == 0
		|| strcmp (sect_name, ".gnu.conflict") == 0
		|| strcmp (sect_name, ".bss") == 0
		|| strcmp (sect_name, ".sbss") == 0))
is probably too obviously incorrect so we have to restrict the .bss/.sbss
entries warning suppression to the ones which only come exactly after the
prelink-generated .dynbss/.sdynbss sections.

I already got the approval but delaying it to resolving this issue.

OK to check-in?


Thanks,
Jan