public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* PATCH: PR ld/10569: -z max-page-size may not work for linker  scripts
@ 2009-08-28  0:40 H.J. Lu
  2009-08-28  3:55 ` Alan Modra
  0 siblings, 1 reply; 15+ messages in thread
From: H.J. Lu @ 2009-08-28  0:40 UTC (permalink / raw)
  To: binutils

When a linker script change output target with

OUTPUT_FORMAT("elf32-i386")

and elf32-i386 isn't the default target, -z max-page-size won't work
correctly.

Here is a patch with 2 testcases.  OK to install?

Thanks.


H.J.
----
ld/

2009-08-27  H.J. Lu  <hongjiu.lu@intel.com>

	 PR ld/10569
	 * ldlang.c (lang_add_output_format): Call
	 bfd_emul_set_maxpagesize/bfd_emul_set_commonpagesize if
	 maximum/common page sizes are specified and output target
	 isn't the same as default target.

ld/testsuite

2009-08-27  H.J. Lu  <hongjiu.lu@intel.com>

	 PR ld/10569
	 * ld-elf/commonpage2.d: New.
	 * ld-elf/maxpage4.d: Likewise.
	 * ld-elf/maxpage4.t: Likewise.

Index: ld/testsuite/ld-elf/maxpage4.d
===================================================================
--- ld/testsuite/ld-elf/maxpage4.d	(revision 0)
+++ ld/testsuite/ld-elf/maxpage4.d	(revision 0)
@@ -0,0 +1,9 @@
+#source: maxpage1.s
+#as: --32
+#ld: -z max-page-size=0x200000 -T maxpage4.t
+#readelf: -l --wide
+#target: x86_64-*-linux*
+
+#...
+  LOAD+.*0x200000
+#pass
Index: ld/testsuite/ld-elf/maxpage4.t
===================================================================
--- ld/testsuite/ld-elf/maxpage4.t	(revision 0)
+++ ld/testsuite/ld-elf/maxpage4.t	(revision 0)
@@ -0,0 +1,9 @@
+OUTPUT_FORMAT("elf32-i386")
+OUTPUT_ARCH(i386)
+ENTRY(_start)
+SECTIONS
+{
+  .text : {*(.text)}
+  .data : {*(.data)}
+  /DISCARD/ : {*(*)}
+}
Index: ld/testsuite/ld-elf/commonpage2.d
===================================================================
--- ld/testsuite/ld-elf/commonpage2.d	(revision 0)
+++ ld/testsuite/ld-elf/commonpage2.d	(revision 0)
@@ -0,0 +1,9 @@
+#source: maxpage1.s
+#as: --32
+#ld: -z max-page-size=0x200000 -z common-page-size=0x100000 -T maxpage4.t
+#readelf: -l --wide
+#target: x86_64-*-linux*
+
+#...
+  LOAD+.*0x200000
+#pass
Index: ld/ldlang.c
===================================================================
--- ld/ldlang.c	(revision 6600)
+++ ld/ldlang.c	(working copy)
@@ -6708,6 +6708,20 @@ lang_add_output_format (const char *form
 	format = little;
 
       output_target = format;
+
+      /* If maximum/common page sizes are specified, we may need to
+         set them for OUTPUT_TARGET.  */
+      if ((config.maxpagesize != 0 || config.commonpagesize != 0)
+	  && strcmp (output_target, default_target) != 0)
+	{
+	  if (config.maxpagesize != 0)
+	    bfd_emul_set_maxpagesize (output_target,
+				      config.maxpagesize);
+
+	  if (config.commonpagesize != 0)
+	    bfd_emul_set_commonpagesize (output_target,
+					 config.commonpagesize);
+	}
     }
 }
 

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

* Re: PATCH: PR ld/10569: -z max-page-size may not work for linker  scripts
  2009-08-28  0:40 PATCH: PR ld/10569: -z max-page-size may not work for linker scripts H.J. Lu
@ 2009-08-28  3:55 ` Alan Modra
  2009-08-28  5:50   ` H.J. Lu
  0 siblings, 1 reply; 15+ messages in thread
From: Alan Modra @ 2009-08-28  3:55 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

On Thu, Aug 27, 2009 at 05:02:33PM -0700, H.J. Lu wrote:
> 	 PR ld/10569
> 	 * ldlang.c (lang_add_output_format): Call
> 	 bfd_emul_set_maxpagesize/bfd_emul_set_commonpagesize if
> 	 maximum/common page sizes are specified and output target
> 	 isn't the same as default target.

I think this is the wrong place to call bfd_emul_set_maxpagesize
(and the current call in elf32.em is wrong too).  You probably should
be calling bfd_emul_set_maxpagesize in open_output.  Also,
bfd_emul_get_maxpagesize in fold_name shouldn't be using
default_target.  Ditto for commonpagesize.

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: PATCH: PR ld/10569: -z max-page-size may not work for linker   scripts
  2009-08-28  3:55 ` Alan Modra
@ 2009-08-28  5:50   ` H.J. Lu
  2009-08-28  8:01     ` H.J. Lu
  0 siblings, 1 reply; 15+ messages in thread
From: H.J. Lu @ 2009-08-28  5:50 UTC (permalink / raw)
  To: H.J. Lu, binutils

On Thu, Aug 27, 2009 at 8:14 PM, Alan Modra<amodra@bigpond.net.au> wrote:
> On Thu, Aug 27, 2009 at 05:02:33PM -0700, H.J. Lu wrote:
>>        PR ld/10569
>>        * ldlang.c (lang_add_output_format): Call
>>        bfd_emul_set_maxpagesize/bfd_emul_set_commonpagesize if
>>        maximum/common page sizes are specified and output target
>>        isn't the same as default target.
>
> I think this is the wrong place to call bfd_emul_set_maxpagesize
> (and the current call in elf32.em is wrong too).  You probably should
> be calling bfd_emul_set_maxpagesize in open_output.  Also,

Will open_output be called before any linker scripts are processed?

> bfd_emul_get_maxpagesize in fold_name shouldn't be using
> default_target.  Ditto for commonpagesize.
>

That is true. I will fix it.

-- 
H.J.

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

* Re: PATCH: PR ld/10569: -z max-page-size may not work for linker   scripts
  2009-08-28  5:50   ` H.J. Lu
@ 2009-08-28  8:01     ` H.J. Lu
  2009-08-28  8:09       ` Alan Modra
  0 siblings, 1 reply; 15+ messages in thread
From: H.J. Lu @ 2009-08-28  8:01 UTC (permalink / raw)
  To: H.J. Lu, binutils

On Thu, Aug 27, 2009 at 8:32 PM, H.J. Lu<hjl.tools@gmail.com> wrote:
> On Thu, Aug 27, 2009 at 8:14 PM, Alan Modra<amodra@bigpond.net.au> wrote:
>> On Thu, Aug 27, 2009 at 05:02:33PM -0700, H.J. Lu wrote:
>>>        PR ld/10569
>>>        * ldlang.c (lang_add_output_format): Call
>>>        bfd_emul_set_maxpagesize/bfd_emul_set_commonpagesize if
>>>        maximum/common page sizes are specified and output target
>>>        isn't the same as default target.
>>
>> I think this is the wrong place to call bfd_emul_set_maxpagesize
>> (and the current call in elf32.em is wrong too).  You probably should
>> be calling bfd_emul_set_maxpagesize in open_output.  Also,
>
> Will open_output be called before any linker scripts are processed?
>
>> bfd_emul_get_maxpagesize in fold_name shouldn't be using
>> default_target.  Ditto for commonpagesize.
>>
>
> That is true. I will fix it.

There is no easy fix. Should we just set max/common page sizes for
all enabled ELF targets?


-- 
H.J.

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

* Re: PATCH: PR ld/10569: -z max-page-size may not work for linker  scripts
  2009-08-28  8:01     ` H.J. Lu
@ 2009-08-28  8:09       ` Alan Modra
  2009-08-28 14:31         ` H.J. Lu
  0 siblings, 1 reply; 15+ messages in thread
From: Alan Modra @ 2009-08-28  8:09 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

On Thu, Aug 27, 2009 at 08:55:46PM -0700, H.J. Lu wrote:
> On Thu, Aug 27, 2009 at 8:32 PM, H.J. Lu<hjl.tools@gmail.com> wrote:
> > On Thu, Aug 27, 2009 at 8:14 PM, Alan Modra<amodra@bigpond.net.au> wrote:
> >> I think this is the wrong place to call bfd_emul_set_maxpagesize
> >> (and the current call in elf32.em is wrong too).  You probably should
> >> be calling bfd_emul_set_maxpagesize in open_output.  Also,
> >
> > Will open_output be called before any linker scripts are processed?

Depends on what you mean by processed.  Yes, they are parsed and
converted to internal format before open_output, but why does it
matter?  Nothing much depends on maxpagesize until
lang_size_sections.

Perhaps even a nicer patch would be to put your
bfd_emul_set_maxpagesize calls in a LD_EMUL_SET_OUTPUT_ARCH hook,
which is called just after open_output.

> >> bfd_emul_get_maxpagesize in fold_name shouldn't be using
> >> default_target.  Ditto for commonpagesize.
> >>
> >
> > That is true. I will fix it.
> 
> There is no easy fix. Should we just set max/common page sizes for
> all enabled ELF targets?

Why can't you use output_target?

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: PATCH: PR ld/10569: -z max-page-size may not work for linker   scripts
  2009-08-28  8:09       ` Alan Modra
@ 2009-08-28 14:31         ` H.J. Lu
  2009-08-28 14:48           ` Alan Modra
  0 siblings, 1 reply; 15+ messages in thread
From: H.J. Lu @ 2009-08-28 14:31 UTC (permalink / raw)
  To: binutils

[-- Attachment #1: Type: text/plain, Size: 2599 bytes --]

On Thu, Aug 27, 2009 at 10:50 PM, Alan Modra<amodra@bigpond.net.au> wrote:
> On Thu, Aug 27, 2009 at 08:55:46PM -0700, H.J. Lu wrote:
>> On Thu, Aug 27, 2009 at 8:32 PM, H.J. Lu<hjl.tools@gmail.com> wrote:
>> > On Thu, Aug 27, 2009 at 8:14 PM, Alan Modra<amodra@bigpond.net.au> wrote:
>> >> I think this is the wrong place to call bfd_emul_set_maxpagesize
>> >> (and the current call in elf32.em is wrong too).  You probably should
>> >> be calling bfd_emul_set_maxpagesize in open_output.  Also,
>> >
>> > Will open_output be called before any linker scripts are processed?
>
> Depends on what you mean by processed.  Yes, they are parsed and
> converted to internal format before open_output, but why does it
> matter?  Nothing much depends on maxpagesize until
> lang_size_sections.

This linker script:

---
SECTIONS
{
  .text : {*(.text)}
  . = ALIGN(CONSTANT (MAXPAGESIZE));
  .data : {*(.data)}
  /DISCARD/ : {*(*)}
}
---

calls fold_name on MAXPAGESIZE before open_output is called.

>
> Perhaps even a nicer patch would be to put your
> bfd_emul_set_maxpagesize calls in a LD_EMUL_SET_OUTPUT_ARCH hook,
> which is called just after open_output.
>
>> >> bfd_emul_get_maxpagesize in fold_name shouldn't be using
>> >> default_target.  Ditto for commonpagesize.
>> >>
>> >
>> > That is true. I will fix it.
>>
>> There is no easy fix. Should we just set max/common page sizes for
>> all enabled ELF targets?
>
> Why can't you use output_target?
>

We may set and get page sizes in linker. In linker script, we should use
emulation target, which may be different from output target, to get
page size, We need to set page size for emulation target in
gld${EMULATION_NAME}_handle_option. We also need to set page
size for output target in ldemul_set_output_arch.

OK to install?

Thanks.


H.J.
---
ld/

2009-08-28  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/10569
	* ldemul.c (ldemul_set_output_arch): Set maximum/common page
	 sizes on output target.

	* ldexp.c: Include "ldfile.h" and "ldemul.h".
	(fold_name): Use ldemul_choose_target instead of default_target
	to get maximum/common page sizes.

	* ldmain.c (main): Initialize config.maxpagesize and
	config.commonpagesize to (bfd_vma) -1.

	* emultempl/elf32.em (gld${EMULATION_NAME}_handle_option): Use
	ldemul_choose_target instead of default_target to set
	maximum/common page sizes.


ld/testsuite

2009-08-28  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/10569
	* ld-elf/commonpage2.d: New.
	* ld-elf/maxpage4.d: Likewise.
	* ld-elf/maxpage4.t: Likewise.

[-- Attachment #2: ld-page-size-2.patch --]
[-- Type: application/octet-stream, Size: 5086 bytes --]

ld/

2009-08-28  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/10569
	* ldemul.c (ldemul_set_output_arch): Set maximum/common page
	 sizes on output target.

	* ldexp.c: Include "ldfile.h" and "ldemul.h".
	(fold_name): Use ldemul_choose_target instead of default_target
	to get maximum/common page sizes.

	* ldmain.c (main): Initialize config.maxpagesize and
	config.commonpagesize to (bfd_vma) -1.

	* emultempl/elf32.em: Use ldemul_choose_target instead of
	default_target to set maximum/common page sizes.


ld/testsuite

2009-08-28  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/10569
	* ld-elf/commonpage2.d: New.
	* ld-elf/maxpage4.d: Likewise.
	* ld-elf/maxpage4.t: Likewise.

Index: emultempl/elf32.em
===================================================================
--- emultempl/elf32.em	(revision 6643)
+++ emultempl/elf32.em	(working copy)
@@ -2178,8 +2178,8 @@ fragment <<EOF
 	  if (*end || (config.maxpagesize & (config.maxpagesize - 1)) != 0)
 	    einfo (_("%P%F: invalid maxium page size \`%s'\n"),
 		   optarg + 14);
-	  ASSERT (default_target != NULL);
-	  bfd_emul_set_maxpagesize (default_target, config.maxpagesize);
+	  bfd_emul_set_maxpagesize (ldemul_choose_target (0, NULL),
+				    config.maxpagesize);
 	}
       else if (CONST_STRNEQ (optarg, "common-page-size="))
 	{
@@ -2189,8 +2189,7 @@ fragment <<EOF
 	      || (config.commonpagesize & (config.commonpagesize - 1)) != 0)
 	    einfo (_("%P%F: invalid common page size \`%s'\n"),
 		   optarg + 17);
-	  ASSERT (default_target != NULL);
-	  bfd_emul_set_commonpagesize (default_target,
+	  bfd_emul_set_commonpagesize (ldemul_choose_target (0, NULL),
 				       config.commonpagesize);
 	}
       /* What about the other Solaris -z options? FIXME.  */
Index: testsuite/ld-elf/maxpage4.d
===================================================================
--- testsuite/ld-elf/maxpage4.d	(revision 0)
+++ testsuite/ld-elf/maxpage4.d	(revision 0)
@@ -0,0 +1,9 @@
+#source: maxpage1.s
+#as: --32
+#ld: -z max-page-size=0x200000 -T maxpage4.t
+#readelf: -l --wide
+#target: x86_64-*-linux*
+
+#...
+  LOAD+.*0x200000
+#pass
Index: testsuite/ld-elf/maxpage4.t
===================================================================
--- testsuite/ld-elf/maxpage4.t	(revision 0)
+++ testsuite/ld-elf/maxpage4.t	(revision 0)
@@ -0,0 +1,9 @@
+OUTPUT_FORMAT("elf32-i386")
+OUTPUT_ARCH(i386)
+ENTRY(_start)
+SECTIONS
+{
+  .text : {*(.text)}
+  .data : {*(.data)}
+  /DISCARD/ : {*(*)}
+}
Index: testsuite/ld-elf/commonpage2.d
===================================================================
--- testsuite/ld-elf/commonpage2.d	(revision 0)
+++ testsuite/ld-elf/commonpage2.d	(revision 0)
@@ -0,0 +1,9 @@
+#source: maxpage1.s
+#as: --32
+#ld: -z max-page-size=0x200000 -z common-page-size=0x100000 -T maxpage4.t
+#readelf: -l --wide
+#target: x86_64-*-linux*
+
+#...
+  LOAD+.*0x200000
+#pass
Index: ldmain.c
===================================================================
--- ldmain.c	(revision 6600)
+++ ldmain.c	(working copy)
@@ -248,6 +248,8 @@ main (int argc, char **argv)
   config.make_executable = TRUE;
   config.magic_demand_paged = TRUE;
   config.text_read_only = TRUE;
+  config.maxpagesize = (bfd_vma) -1;
+  config.commonpagesize = (bfd_vma) -1;
 
   command_line.warn_mismatch = TRUE;
   command_line.warn_search_mismatch = TRUE;
Index: ldemul.c
===================================================================
--- ldemul.c	(revision 6600)
+++ ldemul.c	(working copy)
@@ -82,6 +82,21 @@ void
 ldemul_set_output_arch (void)
 {
   ld_emulation->set_output_arch ();
+
+  if (config.maxpagesize != (bfd_vma) -1
+      || config.commonpagesize != (bfd_vma) -1)
+    {
+      const char *output_target = lang_get_output_target ();
+
+      if (config.maxpagesize != (bfd_vma) -1)
+	bfd_emul_set_maxpagesize (output_target,
+				  config.maxpagesize);
+
+      if (config.commonpagesize != (bfd_vma) -1)
+	bfd_emul_set_commonpagesize (output_target,
+				     config.commonpagesize);
+
+    }
 }
 
 void
Index: ldexp.c
===================================================================
--- ldexp.c	(revision 6600)
+++ ldexp.c	(working copy)
@@ -40,6 +40,8 @@
 #include "ldlex.h"
 #include <ldgram.h>
 #include "ldlang.h"
+#include "ldfile.h"
+#include "ldemul.h"
 #include "libiberty.h"
 #include "safe-ctype.h"
 
@@ -482,6 +484,8 @@ fold_trinary (etree_type *tree)
 static void
 fold_name (etree_type *tree)
 {
+  const char *target;
+
   memset (&expld.result, 0, sizeof (expld.result));
 
   switch (tree->type.node_code)
@@ -672,10 +676,11 @@ fold_name (etree_type *tree)
       break;
 
     case CONSTANT:
+      target = ldemul_choose_target (0, NULL);
       if (strcmp (tree->name.name, "MAXPAGESIZE") == 0)
-	new_abs (bfd_emul_get_maxpagesize (default_target));
+	new_abs (bfd_emul_get_maxpagesize (target));
       else if (strcmp (tree->name.name, "COMMONPAGESIZE") == 0)
-	new_abs (bfd_emul_get_commonpagesize (default_target));
+	new_abs (bfd_emul_get_commonpagesize (target));
       else
 	einfo (_("%F%S: unknown constant `%s' referenced in expression\n"),
 	       tree->name.name);

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

* Re: PATCH: PR ld/10569: -z max-page-size may not work for linker  scripts
  2009-08-28 14:31         ` H.J. Lu
@ 2009-08-28 14:48           ` Alan Modra
  2009-08-28 15:05             ` H.J. Lu
  0 siblings, 1 reply; 15+ messages in thread
From: Alan Modra @ 2009-08-28 14:48 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

On Fri, Aug 28, 2009 at 06:40:42AM -0700, H.J. Lu wrote:
> On Thu, Aug 27, 2009 at 10:50 PM, Alan Modra<amodra@bigpond.net.au> wrote:
> > On Thu, Aug 27, 2009 at 08:55:46PM -0700, H.J. Lu wrote:
> >> On Thu, Aug 27, 2009 at 8:32 PM, H.J. Lu<hjl.tools@gmail.com> wrote:
> >> > On Thu, Aug 27, 2009 at 8:14 PM, Alan Modra<amodra@bigpond.net.au> wrote:
> >> >> I think this is the wrong place to call bfd_emul_set_maxpagesize
> >> >> (and the current call in elf32.em is wrong too).  You probably should
> >> >> be calling bfd_emul_set_maxpagesize in open_output.  Also,
> >> >
> >> > Will open_output be called before any linker scripts are processed?
> >
> > Depends on what you mean by processed.  Yes, they are parsed and
> > converted to internal format before open_output, but why does it
> > matter?  Nothing much depends on maxpagesize until
> > lang_size_sections.
> 
> This linker script:
> 
> ---
> SECTIONS
> {
>   .text : {*(.text)}
>   . = ALIGN(CONSTANT (MAXPAGESIZE));
>   .data : {*(.data)}
>   /DISCARD/ : {*(*)}
> }
> ---
> 
> calls fold_name on MAXPAGESIZE before open_output is called.

You are allowed to fail in fold_name if output_target is not set.  See
many other examples where we return !result.valid_p if attempting to
evaluate early in the link.

> > Why can't you use output_target?
> >
> 
> We may set and get page sizes in linker. In linker script, we should use
> emulation target, which may be different from output target, to get
> page size, We need to set page size for emulation target in
> gld${EMULATION_NAME}_handle_option. We also need to set page
> size for output target in ldemul_set_output_arch.

If you accept a fail in fold_name, then I think your argument
collapses.  After output_target has been set, that's the right target
to use.

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: PATCH: PR ld/10569: -z max-page-size may not work for linker   scripts
  2009-08-28 14:48           ` Alan Modra
@ 2009-08-28 15:05             ` H.J. Lu
  2009-08-30 13:51               ` Alan Modra
  0 siblings, 1 reply; 15+ messages in thread
From: H.J. Lu @ 2009-08-28 15:05 UTC (permalink / raw)
  To: binutils

On Fri, Aug 28, 2009 at 7:31 AM, Alan Modra<amodra@bigpond.net.au> wrote:
> On Fri, Aug 28, 2009 at 06:40:42AM -0700, H.J. Lu wrote:
>> On Thu, Aug 27, 2009 at 10:50 PM, Alan Modra<amodra@bigpond.net.au> wrote:
>> > On Thu, Aug 27, 2009 at 08:55:46PM -0700, H.J. Lu wrote:
>> >> On Thu, Aug 27, 2009 at 8:32 PM, H.J. Lu<hjl.tools@gmail.com> wrote:
>> >> > On Thu, Aug 27, 2009 at 8:14 PM, Alan Modra<amodra@bigpond.net.au> wrote:
>> >> >> I think this is the wrong place to call bfd_emul_set_maxpagesize
>> >> >> (and the current call in elf32.em is wrong too).  You probably should
>> >> >> be calling bfd_emul_set_maxpagesize in open_output.  Also,
>> >> >
>> >> > Will open_output be called before any linker scripts are processed?
>> >
>> > Depends on what you mean by processed.  Yes, they are parsed and
>> > converted to internal format before open_output, but why does it
>> > matter?  Nothing much depends on maxpagesize until
>> > lang_size_sections.
>>
>> This linker script:
>>
>> ---
>> SECTIONS
>> {
>>   .text : {*(.text)}
>>   . = ALIGN(CONSTANT (MAXPAGESIZE));
>>   .data : {*(.data)}
>>   /DISCARD/ : {*(*)}
>> }
>> ---
>>
>> calls fold_name on MAXPAGESIZE before open_output is called.
>
> You are allowed to fail in fold_name if output_target is not set.  See
> many other examples where we return !result.valid_p if attempting to
> evaluate early in the link.

This is from one of linker tests. If I don't set page sizes in
gld${EMULATION_NAME}_handle_option, 2 linker tests will fail.

>> > Why can't you use output_target?
>> >
>>
>> We may set and get page sizes in linker. In linker script, we should use
>> emulation target, which may be different from output target, to get
>> page size, We need to set page size for emulation target in
>> gld${EMULATION_NAME}_handle_option. We also need to set page
>> size for output target in ldemul_set_output_arch.
>
> If you accept a fail in fold_name, then I think your argument
> collapses.  After output_target has been set, that's the right target
> to use.
>

Output target can be different from emulation target. Emulation target
may be ELF, which supports page sizes, while output target may not support
page sizes at all. We need to page sizes for both.


-- 
H.J.

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

* Re: PATCH: PR ld/10569: -z max-page-size may not work for linker  scripts
  2009-08-28 15:05             ` H.J. Lu
@ 2009-08-30 13:51               ` Alan Modra
  2009-08-31  2:02                 ` H.J. Lu
  0 siblings, 1 reply; 15+ messages in thread
From: Alan Modra @ 2009-08-30 13:51 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

On Fri, Aug 28, 2009 at 07:37:22AM -0700, H.J. Lu wrote:
> Output target can be different from emulation target. Emulation target
> may be ELF, which supports page sizes, while output target may not support
> page sizes at all. We need to page sizes for both.

Hmm, OK.  If we are doing something like an "--oformat srec" link,
then the bfd srec backend has no idea of page size, but the input
files might be ELF and we might be using an ELF ld script which
references MAXPAGESIZE.

So the linker needs to know maxpagesize independently of the output.
It just happens that the linker already has config.maxpagesize, so
let's use that in ldexp.c.  We will need to get config.maxpagesize
from the bfd emulation target early in the link process, before we've
parsed -z maxpagesize, and pass any changed value back before final
link time.

bfd/
	PR ld/10569
	* bfd.c (bfd_emul_get_maxpagesize): Don't abort.
	(bfd_emul_get_commonpagesize): Likewise.
ld/
	PR ld/10569
	* ldexp.c (fold_name <MAXPAGESIZE>): Return config.maxpagesize.
	(fold_name <COMMONPAGESIZE>): Similarly.
	* ldlang.c (output_target): Make global.
	* ldlang.h (output_target): Declare.
	* ldmain.c (main): Set config.maxpagesize from bfd_emul_get_maxpagesize.
	Similarly for config.commonpagesize.
	* ldemul.c (set_output_arch_default): Call bfd_emul_set_maxpagesize
	and bfd_emul_set_commonpagesize.
	* emultempl/elf32.em (gld${EMULATION_NAME}_handle_option): Don't call
	bfd_emul_set_maxpagesize or bfd_emul_set_commonpagesize here.

Index: bfd/bfd.c
===================================================================
RCS file: /cvs/src/src/bfd/bfd.c,v
retrieving revision 1.109
diff -u -p -r1.109 bfd.c
--- bfd/bfd.c	26 May 2009 14:12:02 -0000	1.109
+++ bfd/bfd.c	30 Aug 2009 05:17:57 -0000
@@ -1703,8 +1703,7 @@ DESCRIPTION
 	emulation.
 
 RETURNS
-	Returns the maximum page size in bytes for ELF, abort
-	otherwise.
+	Returns the maximum page size in bytes for ELF, 0 otherwise.
 */
 
 bfd_vma
@@ -1717,7 +1716,6 @@ bfd_emul_get_maxpagesize (const char *em
       && target->flavour == bfd_target_elf_flavour)
     return xvec_get_elf_backend_data (target)->maxpagesize;
 
-  abort ();
   return 0;
 }
 
@@ -1776,7 +1774,7 @@ DESCRIPTION
 	emulation.
 
 RETURNS
-	Returns the common page size in bytes for ELF, abort otherwise.
+	Returns the common page size in bytes for ELF, 0 otherwise.
 */
 
 bfd_vma
@@ -1789,7 +1787,6 @@ bfd_emul_get_commonpagesize (const char 
       && target->flavour == bfd_target_elf_flavour)
     return xvec_get_elf_backend_data (target)->commonpagesize;
 
-  abort ();
   return 0;
 }
 
Index: ld/ldexp.c
===================================================================
RCS file: /cvs/src/src/ld/ldexp.c,v
retrieving revision 1.77
diff -u -p -r1.77 ldexp.c
--- ld/ldexp.c	29 Aug 2009 22:11:01 -0000	1.77
+++ ld/ldexp.c	30 Aug 2009 05:19:24 -0000
@@ -673,9 +673,9 @@ fold_name (etree_type *tree)
 
     case CONSTANT:
       if (strcmp (tree->name.name, "MAXPAGESIZE") == 0)
-	new_abs (bfd_emul_get_maxpagesize (default_target));
+	new_abs (config.maxpagesize);
       else if (strcmp (tree->name.name, "COMMONPAGESIZE") == 0)
-	new_abs (bfd_emul_get_commonpagesize (default_target));
+	new_abs (config.commonpagesize);
       else
 	einfo (_("%F%S: unknown constant `%s' referenced in expression\n"),
 	       tree->name.name);
Index: ld/ldemul.c
===================================================================
RCS file: /cvs/src/src/ld/ldemul.c,v
retrieving revision 1.31
diff -u -p -r1.31 ldemul.c
--- ld/ldemul.c	10 Aug 2009 07:50:56 -0000	1.31
+++ ld/ldemul.c	30 Aug 2009 05:19:23 -0000
@@ -228,6 +228,9 @@ set_output_arch_default (void)
   /* Set the output architecture and machine if possible.  */
   bfd_set_arch_mach (link_info.output_bfd,
 		     ldfile_output_architecture, ldfile_output_machine);
+
+  bfd_emul_set_maxpagesize (output_target, config.maxpagesize);
+  bfd_emul_set_commonpagesize (output_target, config.commonpagesize);
 }
 
 void
Index: ld/ldlang.c
===================================================================
RCS file: /cvs/src/src/ld/ldlang.c,v
retrieving revision 1.316
diff -u -p -r1.316 ldlang.c
--- ld/ldlang.c	29 Aug 2009 22:11:01 -0000	1.316
+++ ld/ldlang.c	30 Aug 2009 05:19:27 -0000
@@ -52,6 +52,7 @@ static struct obstack map_obstack;
 #define obstack_chunk_alloc xmalloc
 #define obstack_chunk_free free
 static const char *startup_file;
+static const char *entry_symbol_default = "start";
 static bfd_boolean placed_commons = FALSE;
 static bfd_boolean stripped_excluded_sections = FALSE;
 static lang_output_section_statement_type *default_common_section;
@@ -59,7 +60,6 @@ static bfd_boolean map_option_f;
 static bfd_vma print_dot;
 static lang_input_statement_type *first_file;
 static const char *current_target;
-static const char *output_target;
 static lang_statement_list_type statement_list;
 static struct bfd_hash_table lang_definedness_table;
 static lang_statement_list_type *stat_save[10];
@@ -86,13 +86,13 @@ static void lang_finalize_version_expr_h
   (struct bfd_elf_version_expr_head *);
 
 /* Exported variables.  */
+const char *output_target;
 lang_output_section_statement_type *abs_output_section;
 lang_statement_list_type lang_output_section_statement;
 lang_statement_list_type *stat_ptr = &statement_list;
 lang_statement_list_type file_chain = { NULL, NULL };
 lang_statement_list_type input_file_chain;
 struct bfd_sym_chain entry_symbol = { NULL, NULL };
-static const char *entry_symbol_default = "start";
 const char *entry_section = ".text";
 bfd_boolean entry_from_cmdline;
 bfd_boolean lang_has_input_file = FALSE;
Index: ld/ldlang.h
===================================================================
RCS file: /cvs/src/src/ld/ldlang.h,v
retrieving revision 1.84
diff -u -p -r1.84 ldlang.h
--- ld/ldlang.h	10 Aug 2009 07:50:56 -0000	1.84
+++ ld/ldlang.h	30 Aug 2009 05:19:27 -0000
@@ -445,6 +445,7 @@ struct orphan_save
   lang_output_section_statement_type **os_tail;
 };
 
+extern const char *output_target;
 extern lang_output_section_statement_type *abs_output_section;
 extern lang_statement_list_type lang_output_section_statement;
 extern bfd_boolean lang_has_input_file;
Index: ld/ldmain.c
===================================================================
RCS file: /cvs/src/src/ld/ldmain.c,v
retrieving revision 1.136
diff -u -p -r1.136 ldmain.c
--- ld/ldmain.c	27 May 2009 13:31:24 -0000	1.136
+++ ld/ldmain.c	30 Aug 2009 05:19:29 -0000
@@ -280,6 +280,8 @@ main (int argc, char **argv)
   emulation = get_emulation (argc, argv);
   ldemul_choose_mode (emulation);
   default_target = ldemul_choose_target (argc, argv);
+  config.maxpagesize = bfd_emul_get_maxpagesize (default_target);
+  config.commonpagesize = bfd_emul_get_commonpagesize (default_target);
   lang_init ();
   ldemul_before_parse ();
   lang_has_input_file = FALSE;
Index: ld/emultempl/elf32.em
===================================================================
RCS file: /cvs/src/src/ld/emultempl/elf32.em,v
retrieving revision 1.199
diff -u -p -r1.199 elf32.em
--- ld/emultempl/elf32.em	26 Aug 2009 13:08:07 -0000	1.199
+++ ld/emultempl/elf32.em	30 Aug 2009 05:19:30 -0000
@@ -2178,8 +2178,6 @@ fragment <<EOF
 	  if (*end || (config.maxpagesize & (config.maxpagesize - 1)) != 0)
 	    einfo (_("%P%F: invalid maxium page size \`%s'\n"),
 		   optarg + 14);
-	  ASSERT (default_target != NULL);
-	  bfd_emul_set_maxpagesize (default_target, config.maxpagesize);
 	}
       else if (CONST_STRNEQ (optarg, "common-page-size="))
 	{
@@ -2189,9 +2187,6 @@ fragment <<EOF
 	      || (config.commonpagesize & (config.commonpagesize - 1)) != 0)
 	    einfo (_("%P%F: invalid common page size \`%s'\n"),
 		   optarg + 17);
-	  ASSERT (default_target != NULL);
-	  bfd_emul_set_commonpagesize (default_target,
-				       config.commonpagesize);
 	}
       /* What about the other Solaris -z options? FIXME.  */
       break;

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: PATCH: PR ld/10569: -z max-page-size may not work for linker   scripts
  2009-08-30 13:51               ` Alan Modra
@ 2009-08-31  2:02                 ` H.J. Lu
  2009-08-31  2:50                   ` Alan Modra
  0 siblings, 1 reply; 15+ messages in thread
From: H.J. Lu @ 2009-08-31  2:02 UTC (permalink / raw)
  To: H.J. Lu, binutils

On Sat, Aug 29, 2009 at 10:42 PM, Alan Modra<amodra@bigpond.net.au> wrote:
> On Fri, Aug 28, 2009 at 07:37:22AM -0700, H.J. Lu wrote:
>> Output target can be different from emulation target. Emulation target
>> may be ELF, which supports page sizes, while output target may not support
>> page sizes at all. We need to page sizes for both.
>
> Hmm, OK.  If we are doing something like an "--oformat srec" link,
> then the bfd srec backend has no idea of page size, but the input
> files might be ELF and we might be using an ELF ld script which
> references MAXPAGESIZE.
>
> So the linker needs to know maxpagesize independently of the output.
> It just happens that the linker already has config.maxpagesize, so
> let's use that in ldexp.c.  We will need to get config.maxpagesize
> from the bfd emulation target early in the link process, before we've
> parsed -z maxpagesize, and pass any changed value back before final
> link time.
>

What should happen case where

1. No -z page size command line option is given.
2. Output target is ELF and whose page size is different from default target.

Your checkin changes the output page size to the page size of the default
target.

-- 
H.J.

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

* Re: PATCH: PR ld/10569: -z max-page-size may not work for linker  scripts
  2009-08-31  2:02                 ` H.J. Lu
@ 2009-08-31  2:50                   ` Alan Modra
  2009-08-31  6:19                     ` H.J. Lu
  0 siblings, 1 reply; 15+ messages in thread
From: Alan Modra @ 2009-08-31  2:50 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

On Sun, Aug 30, 2009 at 11:42:20AM -0700, H.J. Lu wrote:
> What should happen case where
> 
> 1. No -z page size command line option is given.
> 2. Output target is ELF and whose page size is different from default target.
> 
> Your checkin changes the output page size to the page size of the default
> target.

Yes, I treat "--oformat other_elf" just the same as "--oformat srec".
I don't really have a strong opinion as to what should be done in
this case.

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: PATCH: PR ld/10569: -z max-page-size may not work for linker   scripts
  2009-08-31  2:50                   ` Alan Modra
@ 2009-08-31  6:19                     ` H.J. Lu
  2009-08-31  9:09                       ` Alan Modra
  2009-08-31 18:16                       ` H.J. Lu
  0 siblings, 2 replies; 15+ messages in thread
From: H.J. Lu @ 2009-08-31  6:19 UTC (permalink / raw)
  To: H.J. Lu, binutils

On Sun, Aug 30, 2009 at 6:47 PM, Alan Modra<amodra@bigpond.net.au> wrote:
> On Sun, Aug 30, 2009 at 11:42:20AM -0700, H.J. Lu wrote:
>> What should happen case where
>>
>> 1. No -z page size command line option is given.
>> 2. Output target is ELF and whose page size is different from default target.
>>
>> Your checkin changes the output page size to the page size of the default
>> target.
>
> Yes, I treat "--oformat other_elf" just the same as "--oformat srec".
> I don't really have a strong opinion as to what should be done in
> this case.
>

It can also happen with

OUTPUT_FORMAT("elf32-i386")
OUTPUT_ARCH(i386)

in linker script and all input files are elf32-i386. I won't expect
the page size
won't be 4KB. This is a regression.

-- 
H.J.

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

* Re: PATCH: PR ld/10569: -z max-page-size may not work for linker  scripts
  2009-08-31  6:19                     ` H.J. Lu
@ 2009-08-31  9:09                       ` Alan Modra
  2009-08-31 18:16                       ` H.J. Lu
  1 sibling, 0 replies; 15+ messages in thread
From: Alan Modra @ 2009-08-31  9:09 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

On Sun, Aug 30, 2009 at 07:02:47PM -0700, H.J. Lu wrote:
> On Sun, Aug 30, 2009 at 6:47 PM, Alan Modra<amodra@bigpond.net.au> wrote:
> > On Sun, Aug 30, 2009 at 11:42:20AM -0700, H.J. Lu wrote:
> >> What should happen case where
> >>
> >> 1. No -z page size command line option is given.
> >> 2. Output target is ELF and whose page size is different from default target.
> >>
> >> Your checkin changes the output page size to the page size of the default
> >> target.
> >
> > Yes, I treat "--oformat other_elf" just the same as "--oformat srec".
> > I don't really have a strong opinion as to what should be done in
> > this case.
> >
> 
> It can also happen with
> 
> OUTPUT_FORMAT("elf32-i386")
> OUTPUT_ARCH(i386)
> 
> in linker script and all input files are elf32-i386. I won't expect
> the page size
> won't be 4KB. This is a regression.

You're kidding me.  A regression?  A change in behaviour for a
contrived test case, you mean.  We had this case wrong before my patch
anyway, in an even worse way.  "CONSTANT (MAXPAGESIZE)" in a linker
script would use the default emulation maxpagesize, while the backend
bfd linker would use the output target maxpagesize.

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: PATCH: PR ld/10569: -z max-page-size may not work for linker   scripts
  2009-08-31  6:19                     ` H.J. Lu
  2009-08-31  9:09                       ` Alan Modra
@ 2009-08-31 18:16                       ` H.J. Lu
  2009-09-01  0:00                         ` Alan Modra
  1 sibling, 1 reply; 15+ messages in thread
From: H.J. Lu @ 2009-08-31 18:16 UTC (permalink / raw)
  To: H.J. Lu, binutils

[-- Attachment #1: Type: text/plain, Size: 1533 bytes --]

On Sun, Aug 30, 2009 at 7:02 PM, H.J. Lu<hjl.tools@gmail.com> wrote:
> On Sun, Aug 30, 2009 at 6:47 PM, Alan Modra<amodra@bigpond.net.au> wrote:
>> On Sun, Aug 30, 2009 at 11:42:20AM -0700, H.J. Lu wrote:
>>> What should happen case where
>>>
>>> 1. No -z page size command line option is given.
>>> 2. Output target is ELF and whose page size is different from default target.
>>>
>>> Your checkin changes the output page size to the page size of the default
>>> target.
>>
>> Yes, I treat "--oformat other_elf" just the same as "--oformat srec".
>> I don't really have a strong opinion as to what should be done in
>> this case.
>>
>
> It can also happen with
>
> OUTPUT_FORMAT("elf32-i386")
> OUTPUT_ARCH(i386)
>
> in linker script and all input files are elf32-i386. I won't expect
> the page size
> won't be 4KB. This is a regression.
>

Here is a patch to call bfd_emul_set_maxpagesize and
bfd_emul_set_commonpagesize only if they. OK to
install?

Thanks.


-- 
H.J.
---
2009-08-31  H.J. Lu  <hongjiu.lu@intel.com>

	* ld.h (ld_config_type): Add maxpagesize_set and
	commonpagesize_set.

	* ldemul.c (set_output_arch_default): Call
	bfd_emul_set_maxpagesize/bfd_emul_set_commonpagesize only
	if config.maxpagesize_set/config.commonpagesize_set is TRUE.

	* ldmain.c (main): Initialize config.maxpagesize_set and
	config.commonpagesize_set to FALSE.

	* emultempl/elf32.em (gld${EMULATION_NAME}_handle_option): Set
	config.maxpagesize_set/config.commonpagesize_set to TRUE when
	config.maxpagesize/config.commonpagesize is set.

[-- Attachment #2: ld-page-size-3.patch --]
[-- Type: text/plain, Size: 3098 bytes --]

2009-08-31  H.J. Lu  <hongjiu.lu@intel.com>

	* ld.h (ld_config_type): Add maxpagesize_set and
	commonpagesize_set.

	* ldemul.c (set_output_arch_default): Call
	bfd_emul_set_commonpagesize/bfd_emul_set_commonpagesize only
	if config.maxpagesize_set/config.commonpagesize_set is TRUE.

	* ldmain.c (main): Initialize config.maxpagesize_set and
	config.commonpagesize_set to FALSE.

	* emultempl/elf32.em (gld${EMULATION_NAME}_handle_option): Set
	config.maxpagesize_set/config.commonpagesize_set to TRUE when
	config.maxpagesize/config.commonpagesize is set.

Index: ld/emultempl/elf32.em
===================================================================
--- ld/emultempl/elf32.em	(revision 6667)
+++ ld/emultempl/elf32.em	(working copy)
@@ -2178,6 +2178,7 @@ fragment <<EOF
 	  if (*end || (config.maxpagesize & (config.maxpagesize - 1)) != 0)
 	    einfo (_("%P%F: invalid maxium page size \`%s'\n"),
 		   optarg + 14);
+	  config.maxpagesize_set = TRUE;
 	}
       else if (CONST_STRNEQ (optarg, "common-page-size="))
 	{
@@ -2187,6 +2188,7 @@ fragment <<EOF
 	      || (config.commonpagesize & (config.commonpagesize - 1)) != 0)
 	    einfo (_("%P%F: invalid common page size \`%s'\n"),
 		   optarg + 17);
+	  config.commonpagesize_set = TRUE;
 	}
       /* What about the other Solaris -z options? FIXME.  */
       break;
Index: ld/ld.h
===================================================================
--- ld/ld.h	(revision 6665)
+++ ld/ld.h	(working copy)
@@ -280,8 +280,14 @@ typedef struct {
   /* The maximum page size for ELF.  */
   bfd_vma maxpagesize;
 
+  /* The maximum page size for ELF is set.  */
+  bfd_boolean maxpagesize_set;
+
   /* The common page size for ELF.  */
   bfd_vma commonpagesize;
+
+  /* The common page size for ELF is set.  */
+  bfd_boolean commonpagesize_set;
 } ld_config_type;
 
 extern ld_config_type config;
Index: ld/ldmain.c
===================================================================
--- ld/ldmain.c	(revision 6667)
+++ ld/ldmain.c	(working copy)
@@ -281,7 +281,9 @@ main (int argc, char **argv)
   ldemul_choose_mode (emulation);
   default_target = ldemul_choose_target (argc, argv);
   config.maxpagesize = bfd_emul_get_maxpagesize (default_target);
+  config.maxpagesize_set = FALSE;
   config.commonpagesize = bfd_emul_get_commonpagesize (default_target);
+  config.commonpagesize_set = FALSE;
   lang_init ();
   ldemul_before_parse ();
   lang_has_input_file = FALSE;
Index: ld/ldemul.c
===================================================================
--- ld/ldemul.c	(revision 6667)
+++ ld/ldemul.c	(working copy)
@@ -229,8 +229,10 @@ set_output_arch_default (void)
   bfd_set_arch_mach (link_info.output_bfd,
 		     ldfile_output_architecture, ldfile_output_machine);
 
-  bfd_emul_set_maxpagesize (output_target, config.maxpagesize);
-  bfd_emul_set_commonpagesize (output_target, config.commonpagesize);
+  if (config.maxpagesize_set)
+    bfd_emul_set_maxpagesize (output_target, config.maxpagesize);
+  if (config.commonpagesize_set)
+    bfd_emul_set_commonpagesize (output_target, config.commonpagesize);
 }
 
 void

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

* Re: PATCH: PR ld/10569: -z max-page-size may not work for linker  scripts
  2009-08-31 18:16                       ` H.J. Lu
@ 2009-09-01  0:00                         ` Alan Modra
  0 siblings, 0 replies; 15+ messages in thread
From: Alan Modra @ 2009-09-01  0:00 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

On Mon, Aug 31, 2009 at 10:42:56AM -0700, H.J. Lu wrote:
> Here is a patch to call bfd_emul_set_maxpagesize and
> bfd_emul_set_commonpagesize only if they. OK to
> install?

No.  For the reason given in my last email.

-- 
Alan Modra
Australia Development Lab, IBM

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

end of thread, other threads:[~2009-09-01  0:00 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-08-28  0:40 PATCH: PR ld/10569: -z max-page-size may not work for linker scripts H.J. Lu
2009-08-28  3:55 ` Alan Modra
2009-08-28  5:50   ` H.J. Lu
2009-08-28  8:01     ` H.J. Lu
2009-08-28  8:09       ` Alan Modra
2009-08-28 14:31         ` H.J. Lu
2009-08-28 14:48           ` Alan Modra
2009-08-28 15:05             ` H.J. Lu
2009-08-30 13:51               ` Alan Modra
2009-08-31  2:02                 ` H.J. Lu
2009-08-31  2:50                   ` Alan Modra
2009-08-31  6:19                     ` H.J. Lu
2009-08-31  9:09                       ` Alan Modra
2009-08-31 18:16                       ` H.J. Lu
2009-09-01  0:00                         ` 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).