public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Nick Clifton <nickc@redhat.com>
To: Catherine Moore <clm@codesourcery.com>
Cc: binutils@sourceware.org, Tristan Gingold <gingold@adacore.com>
Subject: Re: PING Re: [RFA] Linker script extension SECTION_FLAGS
Date: Tue, 28 Jun 2011 11:37:00 -0000	[thread overview]
Message-ID: <4E09BCFF.2030208@redhat.com> (raw)
In-Reply-To: <4E025F3A.6020003@codesourcery.com>

Hi Catherine,

> Thanks for the patch review. I've developed a new patch that addresses
> the comments that both you and Tristan made regarding the original
> patch. I've now associated the INPUT_SECTION_FLAGS with the input
> section specifications instead of the output sectionas you and others
> suggested. I've tested arm-coff, mips-elf and ppc-elf. What do you
> think? Is this okay to commit?

I like this patch, but I am afraid that I cannot approve it yet.  There 
are still several problems that need to be addressed:

   * You did not test building a toolchain configured with 
"--enable-targets=all --enable-64-bit-bfd".  There were several targets 
that did not build because they did not define an appropriate 
xxx_bfd_lookup_section_flags macro.


   * The new linker tests fail for targets that do not support the 
INPUT_SECTION_FLAGS feature.  (Ie the ones that use 
bfd_generic_lookup_section_flags).


   * The ld.patch file contained the new linker tests, so the 
ld-test.patch file could not be applied afterwards.


   * The code did not check for contradictory input section flags, eg:

      INPUT_SECTION_FLAGS (SHF_TLS & !SHF_TLS)


   * There was no mention of this new feature in the ld/NEWS file.


   * You have not provided a patch for GOLD.  I would like to keep the 
two linkers in sync, at least as far as features go.


I hope that you will be willing to resolve these issues and submit a 
revised patch.

Cheers
   Nick

  reply	other threads:[~2011-06-28 11:37 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-18 19:32 Catherine Moore
2011-05-18 19:33 ` Catherine Moore
2011-05-19  0:07 ` Ian Lance Taylor
2011-05-24 22:57   ` Catherine Moore
2011-05-25 12:28     ` Tristan Gingold
2011-06-06 22:02     ` PING " Catherine Moore
2011-06-07 13:11       ` Nick Clifton
2011-06-22 21:32         ` Catherine Moore
2011-06-28 11:37           ` Nick Clifton [this message]
2011-06-28 11:56             ` Tristan Gingold
2011-06-28 12:22               ` Nick Clifton
2011-06-28 13:30               ` Ian Lance Taylor
2011-06-30 21:11             ` Catherine Moore
2011-07-11 13:55               ` Nick Clifton
2012-02-09  5:27               ` Alan Modra
2011-05-19  7:44 ` Tristan Gingold

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=4E09BCFF.2030208@redhat.com \
    --to=nickc@redhat.com \
    --cc=binutils@sourceware.org \
    --cc=clm@codesourcery.com \
    --cc=gingold@adacore.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).