public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* ld big trouble.
@ 2009-09-02 13:48 Dave Korn
  2009-09-02 14:02 ` Dave Korn
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Dave Korn @ 2009-09-02 13:48 UTC (permalink / raw)
  To: binutils


    Hi gang,

  Here's ld's toplevel from ldlang.c:

> void
> lang_process (void)
> {
>   [ ... ]
>   if (! link_info.relocatable)
>     {
>       asection *found;
>       [ ... ]
>       /* Look for a text section and set the readonly attribute in it.  */
>       found = bfd_get_section_by_name (link_info.output_bfd, ".text");
> 
>       if (found != NULL)
> 	{
> 	  if (config.text_read_only)
> 	    found->flags |= SEC_READONLY;
> 	  else
> 	    found->flags &= ~SEC_READONLY;
> 	}
>     }
>   [ ... ] 

  Why only ".text" here, and not a more general match for (e.g.) ".text.*"?
GCC's partitioning options generate sections with names like ".text.unlikely",
or ".text.cold" and ".text.hot".  Shouldn't these text sections be governed by
the same text_read_only flag?  It breaks auto-import on PE platforms, because
that requires that all text sections (well, all that have fixup imports
anyway) be writable.

  The answer to these questions tells me whether to use a more general string
matcher across all sections here, or whether to do special processing in the
ldemul_before_allocation() hooks in the PE/PE+ emulations.  However the
"text_read_only" member appears to have been added without being mentioned in
a ChangeLog, and nor is it commented where declared, so I'm reluctant to
attempt to divine its intended semantics.  I'm off to go and spend some time
digging through the mailing lists and cvs history, but I thought I'd leave
this here while I get on with that in case anyone has a definitive word to say
about it.

    cheers,
      DaveK

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ld big trouble.
  2009-09-02 13:48 ld big trouble Dave Korn
@ 2009-09-02 14:02 ` Dave Korn
  2009-09-02 14:09 ` Kai Tietz
  2009-09-02 14:46 ` Alan Modra
  2 siblings, 0 replies; 8+ messages in thread
From: Dave Korn @ 2009-09-02 14:02 UTC (permalink / raw)
  To: Dave Korn; +Cc: binutils

Dave Korn wrote:
> However the
> "text_read_only" member appears to have been added without being mentioned in
> a ChangeLog, 

  And it was there since before the beginning of recorded time (i.e. CVS
revision 1.1) so the original motivation may well be lost in the mists of time
now.  Hmm.

  Maybe I should add some kind of hook to let targets specify if they want a
broader (or even narrower) range of text sections to be matched?

    cheers,
      DaveK

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ld big trouble.
  2009-09-02 13:48 ld big trouble Dave Korn
  2009-09-02 14:02 ` Dave Korn
@ 2009-09-02 14:09 ` Kai Tietz
  2009-09-02 14:39   ` Dave Korn
  2009-09-02 14:46 ` Alan Modra
  2 siblings, 1 reply; 8+ messages in thread
From: Kai Tietz @ 2009-09-02 14:09 UTC (permalink / raw)
  To: Dave Korn; +Cc: binutils

2009/9/2 Dave Korn <dave.korn.cygwin@googlemail.com>:
>
>    Hi gang,
>
>  Here's ld's toplevel from ldlang.c:
>
>> void
>> lang_process (void)
>> {
>>   [ ... ]
>>   if (! link_info.relocatable)
>>     {
>>       asection *found;
>>       [ ... ]
>>       /* Look for a text section and set the readonly attribute in it.  */
>>       found = bfd_get_section_by_name (link_info.output_bfd, ".text");
>>
>>       if (found != NULL)
>>       {
>>         if (config.text_read_only)
>>           found->flags |= SEC_READONLY;
>>         else
>>           found->flags &= ~SEC_READONLY;
>>       }
>>     }
>>   [ ... ]
>
>  Why only ".text" here, and not a more general match for (e.g.) ".text.*"?
> GCC's partitioning options generate sections with names like ".text.unlikely",
> or ".text.cold" and ".text.hot".  Shouldn't these text sections be governed by
> the same text_read_only flag?  It breaks auto-import on PE platforms, because
> that requires that all text sections (well, all that have fixup imports
> anyway) be writable.
>
>  The answer to these questions tells me whether to use a more general string
> matcher across all sections here, or whether to do special processing in the
> ldemul_before_allocation() hooks in the PE/PE+ emulations.  However the
> "text_read_only" member appears to have been added without being mentioned in
> a ChangeLog, and nor is it commented where declared, so I'm reluctant to
> attempt to divine its intended semantics.  I'm off to go and spend some time
> digging through the mailing lists and cvs history, but I thought I'd leave
> this here while I get on with that in case anyone has a definitive word to say
> about it.
>
>    cheers,
>      DaveK
>

Well, I don't know who implemented it, but I know why it is present.
For x64, new implementation of pseudo-relocation v1, and for v2 this
isn't necessary anymore
This patch itself was necessary as old relocation code doesn't changed
memory access for modification.
In general it is a rude hack IMHO, as it allows code modifications in
a general way, which can be a security issue.

Cheers,
Kai
-- 
|  (\_/) This is Bunny. Copy and paste
| (='.'=) Bunny into your signature to help
| (")_(") him gain world domination

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ld big trouble.
  2009-09-02 14:09 ` Kai Tietz
@ 2009-09-02 14:39   ` Dave Korn
  0 siblings, 0 replies; 8+ messages in thread
From: Dave Korn @ 2009-09-02 14:39 UTC (permalink / raw)
  To: Kai Tietz; +Cc: Dave Korn, binutils

Kai Tietz wrote:

> Well, I don't know who implemented it, but I know why it is present.

  It's used by ELF platforms too, at least according to a quick grep, so I
suspect it isn't there /solely/ to support auto-import; which is the essence
of my question: should I change this for everyone, or just PE?

    cheers,
      DaveK

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ld big trouble.
  2009-09-02 13:48 ld big trouble Dave Korn
  2009-09-02 14:02 ` Dave Korn
  2009-09-02 14:09 ` Kai Tietz
@ 2009-09-02 14:46 ` Alan Modra
  2009-09-02 15:37   ` Dave Korn
  2 siblings, 1 reply; 8+ messages in thread
From: Alan Modra @ 2009-09-02 14:46 UTC (permalink / raw)
  To: Dave Korn; +Cc: binutils

On Wed, Sep 02, 2009 at 03:01:58PM +0100, Dave Korn wrote:
> GCC's partitioning options generate sections with names like ".text.unlikely",
> or ".text.cold" and ".text.hot".  Shouldn't these text sections be governed by
> the same text_read_only flag?  It breaks auto-import on PE platforms, because
> that requires that all text sections (well, all that have fixup imports
> anyway) be writable.

Input .text.* sections are usually merged in to the .text output
section on a final link.

> "text_read_only" member appears to have been added without being mentioned in

It's main use was to make .text writable for omagic a.out binaries.
I suspect that on !link_info.relocatable && !config.text_read_only you
could loop over all output sections removing SEC_READONLY, and do
nothing otherwise.  .text ought to be SEC_READONLY by default.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ld big trouble.
  2009-09-02 14:46 ` Alan Modra
@ 2009-09-02 15:37   ` Dave Korn
  2009-09-02 16:06     ` Andreas Schwab
  2009-09-02 23:22     ` Alan Modra
  0 siblings, 2 replies; 8+ messages in thread
From: Dave Korn @ 2009-09-02 15:37 UTC (permalink / raw)
  To: Dave Korn, binutils

Alan Modra wrote:
> On Wed, Sep 02, 2009 at 03:01:58PM +0100, Dave Korn wrote:
>> GCC's partitioning options generate sections with names like ".text.unlikely",
>> or ".text.cold" and ".text.hot".  Shouldn't these text sections be governed by
>> the same text_read_only flag?  It breaks auto-import on PE platforms, because
>> that requires that all text sections (well, all that have fixup imports
>> anyway) be writable.
> 
> Input .text.* sections are usually merged in to the .text output
> section on a final link.

  Right.  The PE scripts only include *(.text), and my initial plan was to
just add *(.text.*).  But then it occurred to me: the reasoning behind this
partitioning is to gain benefits from caching of frequently used stuff, and on
that line of reasoning it might make sense to keep them as separate sections,
so they can be allocated in separate memory pages that most OS's VMMs will
swap out of the working set when they don't get frequently accessed.

>> "text_read_only" member appears to have been added without being mentioned in
> 
> It's main use was to make .text writable for omagic a.out binaries.

  IIRC a.out only supports a single section each for text, data and bss, so
making the code alter the flags for other text sections wouldn't make any
difference there.  But I notice that both generic.em and elf32.em at least
respect it and use it to select alternate default linker scripts (haven't yet
checked to see if there are *actually* any differences in the generated
scripts), so I wouldn't want to do anything that might surprise anyone there.

> I suspect that on !link_info.relocatable && !config.text_read_only you
> could loop over all output sections removing SEC_READONLY, and do
> nothing otherwise.  .text ought to be SEC_READONLY by default.

  All output sections?  Surely we wouldn't want to e.g. remove SEC_READONLY
from .rdata and such?

  I'm leaning toward leaving the PE linker script alone and adding a second
pass over all sections in the PE emulation to remove SEC_READONLY from any of
the other non-default .text secitons.  But maybe the gain from having the cold
text in entirely separate memory pages is minimal enough that I should just
change the scripts; I don't really know.

    cheers,
      DaveK

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ld big trouble.
  2009-09-02 15:37   ` Dave Korn
@ 2009-09-02 16:06     ` Andreas Schwab
  2009-09-02 23:22     ` Alan Modra
  1 sibling, 0 replies; 8+ messages in thread
From: Andreas Schwab @ 2009-09-02 16:06 UTC (permalink / raw)
  To: Dave Korn; +Cc: binutils

Dave Korn <dave.korn.cygwin@googlemail.com> writes:

> Alan Modra wrote:
>> I suspect that on !link_info.relocatable && !config.text_read_only you
>> could loop over all output sections removing SEC_READONLY, and do
>> nothing otherwise.  .text ought to be SEC_READONLY by default.
>
>   All output sections?  Surely we wouldn't want to e.g. remove SEC_READONLY
> from .rdata and such?

I think the "text" in text_read_only refers to the text segment in ELF
speak.  In a.out there is no difference between sections and segments.

Andreas.

-- 
Andreas Schwab, schwab@redhat.com
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84  5EC7 45C6 250E 6F00 984E
"And now for something completely different."

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: ld big trouble.
  2009-09-02 15:37   ` Dave Korn
  2009-09-02 16:06     ` Andreas Schwab
@ 2009-09-02 23:22     ` Alan Modra
  1 sibling, 0 replies; 8+ messages in thread
From: Alan Modra @ 2009-09-02 23:22 UTC (permalink / raw)
  To: Dave Korn; +Cc: binutils

On Wed, Sep 02, 2009 at 04:51:16PM +0100, Dave Korn wrote:
> Alan Modra wrote:
> > On Wed, Sep 02, 2009 at 03:01:58PM +0100, Dave Korn wrote:
> >> GCC's partitioning options generate sections with names like ".text.unlikely",
> >> or ".text.cold" and ".text.hot".  Shouldn't these text sections be governed by
> >> the same text_read_only flag?  It breaks auto-import on PE platforms, because
> >> that requires that all text sections (well, all that have fixup imports
> >> anyway) be writable.
> > 
> > Input .text.* sections are usually merged in to the .text output
> > section on a final link.
> 
>   Right.  The PE scripts only include *(.text), and my initial plan was to
> just add *(.text.*).  But then it occurred to me: the reasoning behind this
> partitioning is to gain benefits from caching of frequently used stuff, and on
> that line of reasoning it might make sense to keep them as separate sections,
> so they can be allocated in separate memory pages that most OS's VMMs will
> swap out of the working set when they don't get frequently accessed.

You get cache locality by using a link script with
  .text { *(<hot_name_pattern>) *(<cold_name_pattern>) }

This causes the linker to place all hot sections together, followed by
cold sections.

Hmm, it looks like "gcc -freorder-blocks-and-partition" isn't properly
supported in ld at the moment.  I'll look at fixing that.

> >> "text_read_only" member appears to have been added without being mentioned in
> > 
> > It's main use was to make .text writable for omagic a.out binaries.
> 
>   IIRC a.out only supports a single section each for text, data and bss, so
> making the code alter the flags for other text sections wouldn't make any
> difference there.  But I notice that both generic.em and elf32.em at least
> respect it and use it to select alternate default linker scripts (haven't yet
> checked to see if there are *actually* any differences in the generated
> scripts), so I wouldn't want to do anything that might surprise anyone there.

We support omagic style ELF too.

> > I suspect that on !link_info.relocatable && !config.text_read_only you
> > could loop over all output sections removing SEC_READONLY, and do
> > nothing otherwise.  .text ought to be SEC_READONLY by default.
> 
>   All output sections?  Surely we wouldn't want to e.g. remove SEC_READONLY
> from .rdata and such?

Yes, I believe so.  ld -N (omagic) mashes all sections together (no
page alignment of data segment) which gives us just one read/write
segment for the whole program.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2009-09-02 23:22 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-09-02 13:48 ld big trouble Dave Korn
2009-09-02 14:02 ` Dave Korn
2009-09-02 14:09 ` Kai Tietz
2009-09-02 14:39   ` Dave Korn
2009-09-02 14:46 ` Alan Modra
2009-09-02 15:37   ` Dave Korn
2009-09-02 16:06     ` Andreas Schwab
2009-09-02 23:22     ` Alan Modra

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).