Re: [patch] Fix false warning: section .gnu.liblist not found in ... [rediffed]

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

On Mon, 08 Mar 2010 08:23:17 +0100, Joel Brobecker wrote:
> > 2010-03-01  Jan Kratochvil  <jan.kratochvil@redhat.com>
> > 
> > 	* symfile.c (addr_info_make_relative): New variable sect_name, use it.
> > 	Do not warn on ".gnu.liblist" and ".gnu.conflict".
> 
> This looks reasonable. We can go with that at least for now,

> but I am wondering whether we might want to consider using a complaint
> instead if more sections like these keep popping up.

I have checked prelink sources and IIUC there are no other sections than
".gnu.liblist" and ".gnu.conflict" with this behavior/problem.  I do not agree
with this one sentence but I believe it is offtopic for this patch/thread.


> I have a small request:
> 
> > +	  /* These two sections are intentionally loaded into memory from
> > +	     the DYNAMIC segment and so they have both SEC_ALLOC and SEC_LOAD
> > +	     set in the main executable (not in the library files).  They
> > +	     are not present in the separate debug info file, though.  */
> > +
> > +	  if (!(strcmp (sect_name, ".gnu.liblist") == 0
> > +		|| strcmp (sect_name, ".gnu.conflict") == 0))
> > +	    warning (_("section %s not found in %s"), sect_name,
> > +		     bfd_get_filename (abfd));
> > +
> 
> I was a little confused at first by the comment, because it immediately
> mentioned "these two sections" without giving an idea of what these
> sections were.  May I suggest maybe something more detailed like so?
> 
>           /* This section does not exist in ABFD, which is normally
>              unexpected and we want to issue a warning.
> 
>              However, the ELF prelinker does create a couple of sections
>              (".gnu.liblist" and ".gnu.conflict") which are marked as
>              loadable (they are loaded in memory from the DYNAMIC segment)
>              and yet are not present in separate debug info files.  This
>              is fine, and should not cause a warning.  */

OK, thanks.

Just the main executable vs. shared library files difference was omitted there.
This difference was IIRC the main cost while investigating this problem
therefore I would like to keep the note there:

          /* This section does not exist in ABFD, which is normally
             unexpected and we want to issue a warning.

             However, the ELF prelinker does create a couple of sections
             (".gnu.liblist" and ".gnu.conflict") which are marked in the main
             executable as loadable (they are loaded in memory from the
             DYNAMIC segment) and yet are not present in separate debug info
             files.  This is fine, and should not cause a warning.  Shared
             libraries contain just the section ".gnu.liblist" but it is not
             marked as loadable there.  */

As the comment is already being discusses is approved even this variant?


Thanks for the review,
Jan