public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: "H. J. Lu" <hjl@lucon.org>
To: Nick Clifton <nickc@redhat.com>
Cc: Andrew STUBBS <andrew.stubbs@st.com>, binutils@sources.redhat.com
Subject: Re: Problem setting the CONTENTS flag
Date: Wed, 26 Apr 2006 00:39:00 -0000	[thread overview]
Message-ID: <20060425202007.GA31738@lucon.org> (raw)
In-Reply-To: <444E3E55.2080308@redhat.com>

On Tue, Apr 25, 2006 at 04:20:53PM +0100, Nick Clifton wrote:
> HI H.J.
> 
> >Here is an updated patch.
> 
> There are a couple of minor problems with this version of the patch:
> 
> >+  /* When we read a file or section BFD flags have been set, we don't
> >+     need section type and flags unless it is a linker created section.
> >+     They will be overridden in _bfd_elf_make_section_from_shdr
> >+     anyway.  */
> >+  if ((!sec->flags && abfd->direction != read_direction)
> >       || (sec->flags & SEC_LINKER_CREATED) != 0)
> 
> I think that the comment could be reworded to be clearer.  For example:
> 
>   A section without any flags or with the SEC_LINKER_CREATED flag
>   must have its type and other flags set.  Skip this for sections
>   with no flags that have been read in, as this state must have been
>   requested by the user, and it will be overridden by
>   _bfd_elf_make_section_from_shdr anyway.
> >-  /* FIXME: What if the output ELF section type has been set to
> >-     something different?  */
> >-  if (elf_section_type (osec) == SHT_NULL)
> >+  /* Don't copy the output ELF section type from input if it has been
> >+     set to something different.  */
> >+  if (osec->flags == isec->flags || !osec->flags)
> >     elf_section_type (osec) = elf_section_type (isec);
> 
> A section's type is not determined by its flags.  Therefore it seems to 

It is. See elf_fake_sections.

> be counter-intuitive to be testing the flags when we are concerned about 
> the type.   Also what happens if the output sections flags have been set 
> to something other than the input section's flags, but the output 
> section's type is SHT_NULL ?  According to the ELF spec this would mean 
> that the flags are undefined as well.
> 
> I think that for this part of the patch the comment ought to be reworded 
> and you ought to test for SHT_NULL and either change it or issue an 
> error message.  eg:
> 
>   /* Copy the output ELF section type from the input section type if
>      the output section flags were copied from the input, or if the
>      output section has no type or flags.  */
>   if (osec->flags == isec->flags
>       || osec->flags == 0
>       || elf_section_type (osec) == SHT_NULL)
>    elf_section_type (osec) = elf_section_type (isec);
> 
> 
> Cheers
>   Nick
> 


I updated the comments.


H.J.
----
2006-04-21  H.J. Lu  <hongjiu.lu@intel.com>

	PR binutils/2593
	* elf.c (_bfd_elf_new_section_hook): Don't set section ELF type
	and flags if its BFD flags have been set.
	(_bfd_elf_init_private_section_data): Don't copy the output ELF
	section type from input if it has been set to something
	different.

--- bfd/elf.c.flags	2006-04-24 20:11:58.000000000 -0700
+++ bfd/elf.c	2006-04-25 13:17:24.000000000 -0700
@@ -2487,10 +2487,13 @@ _bfd_elf_new_section_hook (bfd *abfd, as
   bed = get_elf_backend_data (abfd);
   sec->use_rela_p = bed->default_use_rela_p;
 
-  /* When we read a file, we don't need section type and flags unless
-     it is a linker created section.  They will be overridden in
-     _bfd_elf_make_section_from_shdr anyway.  */
-  if (abfd->direction != read_direction
+  /* When we read a file, we don't need to set ELF section type and
+     flags.  They will be overridden in _bfd_elf_make_section_from_shdr
+     anyway.  We will set ELF section type and flags for all linker
+     created sections.  If user specifies BFD section flags, we will
+     set ELF section type and flags based on BFD section flags in
+     elf_fake_sections.  */
+  if ((!sec->flags && abfd->direction != read_direction)
       || (sec->flags & SEC_LINKER_CREATED) != 0)
     {
       ssect = (*bed->get_sec_type_attr) (abfd, sec);
@@ -5906,9 +5909,11 @@ _bfd_elf_init_private_section_data (bfd 
       || obfd->xvec->flavour != bfd_target_elf_flavour)
     return TRUE;
 
-  /* FIXME: What if the output ELF section type has been set to
-     something different?  */
-  if (elf_section_type (osec) == SHT_NULL)
+  /* Don't copy the output ELF section type from input if the
+     output BFD section flags has been set to something different.
+     elf_fake_sections will set ELF section type based on BFD
+     section flags.  */
+  if (osec->flags == isec->flags || !osec->flags)
     elf_section_type (osec) = elf_section_type (isec);
 
   /* Set things up for objcopy and relocatable link.  The output

      parent reply	other threads:[~2006-04-25 21:24 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-04-21 18:08 Andrew STUBBS
2006-04-21 22:26 ` H. J. Lu
2006-04-21 23:32   ` Andreas Schwab
2006-04-23 22:13   ` H. J. Lu
2006-04-24 19:55     ` Andrew STUBBS
2006-04-25 16:37     ` Nick Clifton
2006-04-26  0:21       ` PATCH: binutils/2593: " H. J. Lu
2006-04-26 12:16         ` Nick Clifton
2006-04-26  0:39       ` H. J. Lu [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20060425202007.GA31738@lucon.org \
    --to=hjl@lucon.org \
    --cc=andrew.stubbs@st.com \
    --cc=binutils@sources.redhat.com \
    --cc=nickc@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).