public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* Re: PATCH: PR ld/12730: regression] crash when allocating in a static constructor
@ 2011-05-15 10:14 Craig Southeren
  2011-05-16  0:15 ` Alan Modra
  0 siblings, 1 reply; 16+ messages in thread
From: Craig Southeren @ 2011-05-15 10:14 UTC (permalink / raw)
  To: binutils

------------------------------------------------------------------------

>  On Mon, May 9, 2011 at 6:53 AM, Alan Modra<amodra@gmail.com>  wrote:
>  >  I'm starting to wonder whether ld/testsuite/ld-elf/pr12730.cc is
>  >  valid. ?Does gcc actually make any guarantee about order of static
>  >  constructors and __attribute ((constructor)) functions?
>
>  I have similar doubt.
>
>  >  Compiled with gcc-4.3 branch g++, the testcase segfaults at all
>  >  optimization levels. ?Compiled with gcc-4.4 branch g++, the testcase
>  >  runs at -O0 but segfaults at -O1 and above. ?It happens to run OK with
>  >  gcc mainline and 4.6. ?Given that behaviour, and the fact that some
>  >  popular distros ship gcc-4.4 based compilers, I'm thinking that the
>  >  testcase should be removed.
>  >
>
>  I will do that.
>

At the heart of the issue is the timing of initialising statics at the 
global/namespace level. Prior to the recent change, these statics were 
initialised the first time that any code from the enclosing translation 
unit was executed. Now, it appears that all such statics in all 
translation units are instantiated at start-up.

As the order of statics the global/namespace level is not strictly 
defined, the new implementation is probably compliant. However, this 
choice means that global/namespace statics do not have the same kind of 
behaviour as member statics.

Member statics are only initialised if the program control flow passes 
their declaration. If the control flow never executes the declaration, 
then the static is never instantiated.

Previously, global/namespace statics had similar behaviour - they were 
only initialised if code in the enclosing translation unit was executed.

This is no longer the case. Global/namespace statics now appear to be 
instantiated regardless of whether code in the enclosing translation 
unit is used.

This may increase the memory footprint for applications that have 
global/namespace statics in translation units containing code that may 
be conditionally executed. In some cases (such as PTLib) this may lead 
to different behaviour.

In the case of PTLib (disclaimer: I am the co-author and co-maintainer) 
we can work around this issue using the "initialise on first use" 
paradigm. But it may be that other application maintainers will not be 
so fortunate to have the zealous co-maintainers that tracked down this 
issue for us.

Of course, I expect that most application won't notice the difference, 
other than perhaps some slight increase in runtime memory usage,

    Craig

-- 

-----------------------------------------------------------------------
  Craig Southeren          Post Increment ñ VoIP Consulting and Software
  craigs@postincrement.com.au                    www.postincrement.com.au

  Phone:  +61 243654666      ICQ: #86852844
  Fax:    +61 243656905      MSN:craig_southeren@hotmail.com
  Mobile: +61 417231046      Jabber:craigs@jabber.org

  "Science is the poetry of reality." Richard Dawkins

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

* Re: PATCH: PR ld/12730: regression] crash when allocating in a static constructor
  2011-05-15 10:14 PATCH: PR ld/12730: regression] crash when allocating in a static constructor Craig Southeren
@ 2011-05-16  0:15 ` Alan Modra
  2011-05-16  0:37   ` Craig Southeren
  0 siblings, 1 reply; 16+ messages in thread
From: Alan Modra @ 2011-05-16  0:15 UTC (permalink / raw)
  To: Craig Southeren; +Cc: binutils

On Sun, May 15, 2011 at 08:14:19PM +1000, Craig Southeren wrote:
> At the heart of the issue is the timing of initialising statics at
> the global/namespace level.

You won't get much traction on this issue here on the binutils list.
We did have a ld bug that affected you but that has now been fixed.
Further discussion should go to one of the gcc lists.  If you can get
agreement that functions declared with __attribute__ ((constructor))
ought to be treated exactly as standard C++ namespace scope
constructors regarding initialisation order, then it would be good to
have your testcase added to the g++ testsuite.  That should ensure
both g++ and ld do not regress.

FWIW, I think your testcase is quite reasonable.  The main reason I
wanted the testcase removed from the ld testsuite because I found
the testcase failed using commonly available versions of g++, and
therefore a C++ testcase wasn't the best way to test ld behaviour.

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: PATCH: PR ld/12730: regression] crash when allocating in a static constructor
  2011-05-16  0:15 ` Alan Modra
@ 2011-05-16  0:37   ` Craig Southeren
  0 siblings, 0 replies; 16+ messages in thread
From: Craig Southeren @ 2011-05-16  0:37 UTC (permalink / raw)
  To: Alan Modra; +Cc: binutils

Hi Alan,

  Thanks for taking the time to reply.

  While there is a small part of me that would like to tilt against this particular windmill, the lack of any specific point of non-compliance means that I have no firm ground to stand-on. 

  In the end, it's would be about differences in interpretation of an ambiguous text based on historical precedents - which is an almost textbook recipe for a religious war. 

  I have no desire to cast the first stone, so I'm going to let this sleeping dog lie (and also stop mixing metaphors) :)

   Craig

Sent from my iPhone

On 16/05/2011, at 10:15 AM, Alan Modra <amodra@gmail.com> wrote:

> On Sun, May 15, 2011 at 08:14:19PM +1000, Craig Southeren wrote:
>> At the heart of the issue is the timing of initialising statics at
>> the global/namespace level.
> 
> You won't get much traction on this issue here on the binutils list.
> We did have a ld bug that affected you but that has now been fixed.
> Further discussion should go to one of the gcc lists.  If you can get
> agreement that functions declared with __attribute__ ((constructor))
> ought to be treated exactly as standard C++ namespace scope
> constructors regarding initialisation order, then it would be good to
> have your testcase added to the g++ testsuite.  That should ensure
> both g++ and ld do not regress.
> 
> FWIW, I think your testcase is quite reasonable.  The main reason I
> wanted the testcase removed from the ld testsuite because I found
> the testcase failed using commonly available versions of g++, and
> therefore a C++ testcase wasn't the best way to test ld behaviour.
> 
> -- 
> Alan Modra
> Australia Development Lab, IBM

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

* Re: PATCH: PR ld/12730: regression] crash when allocating in a static constructor
  2011-06-19 19:18               ` Thomas Schwinge
@ 2011-06-21 15:14                 ` Nick Clifton
  0 siblings, 0 replies; 16+ messages in thread
From: Nick Clifton @ 2011-06-21 15:14 UTC (permalink / raw)
  To: Thomas Schwinge; +Cc: H.J. Lu, Binutils

Hi Thomas,

> Why, oh why?  :-)

:)

> ld/testsuite/
>
> 	* ld-elf/elf.exp: Execute array_tests_pie tests on *-*-gnu*, too.

Approved - please apply.

Cheers
   Nick

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

* Re: PATCH: PR ld/12730: regression] crash when allocating in a static constructor
  2011-05-07 13:40             ` H.J. Lu
  2011-05-07 14:05               ` Alan Modra
@ 2011-06-19 19:18               ` Thomas Schwinge
  2011-06-21 15:14                 ` Nick Clifton
  1 sibling, 1 reply; 16+ messages in thread
From: Thomas Schwinge @ 2011-06-19 19:18 UTC (permalink / raw)
  To: H.J. Lu, Binutils

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

Hallo!

On Sat, 7 May 2011 06:39:42 -0700, "H.J. Lu" <hjl.tools@gmail.com> wrote:
> Here is the updated patch.  OK to install?

(Has been in the mean time.)

> ld/testsuite/
> 
> 2011-05-07  H.J. Lu  <hongjiu.lu@intel.com>
> 
> 	PR ld/12730
> 	* ld-elf/elf.exp (array_tests): Add "pr12730".
> 	(array_tests_pie): New.
> 	(array_tests_static): Add -static for ""static init array mixed".
> 	Add "static pr12730".  Run array_tests_pie for Linux.

> diff --git a/ld/testsuite/ld-elf/elf.exp b/ld/testsuite/ld-elf/elf.exp
> index 73a417c..6808d8a 100644
> --- a/ld/testsuite/ld-elf/elf.exp
> +++ b/ld/testsuite/ld-elf/elf.exp
> [...]
> +
> +# Run PIE tests only on Linux.
> +if { [istarget "*-*-linux*"] } {
> +    run_ld_link_exec_tests $xfails $array_tests_pie
> +}
> +
> [...]

Why, oh why?  :-)

ld/testsuite/

	* ld-elf/elf.exp: Execute array_tests_pie tests on *-*-gnu*, too.

---
 ld/testsuite/ld-elf/elf.exp |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ld/testsuite/ld-elf/elf.exp b/ld/testsuite/ld-elf/elf.exp
index ff0f03e..da24bed 100644
--- a/ld/testsuite/ld-elf/elf.exp
+++ b/ld/testsuite/ld-elf/elf.exp
@@ -150,8 +150,8 @@ set array_tests_static {
 set xfails [list "*-*-netbsdelf*"]
 run_ld_link_exec_tests $xfails $array_tests
 
-# Run PIE tests only on Linux.
-if { [istarget "*-*-linux*"] } {
+if { [istarget *-*-linux*]
+     || [istarget *-*-gnu*] } {
     run_ld_link_exec_tests $xfails $array_tests_pie
 }
 
-- 
tg: (38b56ed..) hurd/testsuite (depends on: master)

OK to commit?  (Or are such changes even obvious and don't need
approval?)


Grüße,
 Thomas

[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]

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

* Re: PATCH: PR ld/12730: regression] crash when allocating in a static constructor
  2011-05-09 13:53                 ` Alan Modra
@ 2011-05-09 14:09                   ` H.J. Lu
  0 siblings, 0 replies; 16+ messages in thread
From: H.J. Lu @ 2011-05-09 14:09 UTC (permalink / raw)
  To: Binutils

On Mon, May 9, 2011 at 6:53 AM, Alan Modra <amodra@gmail.com> wrote:
> I'm starting to wonder whether ld/testsuite/ld-elf/pr12730.cc is
> valid.  Does gcc actually make any guarantee about order of static
> constructors and __attribute ((constructor)) functions?

I have similar doubt.

> Compiled with gcc-4.3 branch g++, the testcase segfaults at all
> optimization levels.  Compiled with gcc-4.4 branch g++, the testcase
> runs at -O0 but segfaults at -O1 and above.  It happens to run OK with
> gcc mainline and 4.6.  Given that behaviour, and the fact that some
> popular distros ship gcc-4.4 based compilers, I'm thinking that the
> testcase should be removed.
>

I will do that.

-- 
H.J.

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

* Re: PATCH: PR ld/12730: regression] crash when allocating in a static constructor
  2011-05-07 14:05               ` Alan Modra
@ 2011-05-09 13:53                 ` Alan Modra
  2011-05-09 14:09                   ` H.J. Lu
  0 siblings, 1 reply; 16+ messages in thread
From: Alan Modra @ 2011-05-09 13:53 UTC (permalink / raw)
  To: H.J. Lu, Binutils

I'm starting to wonder whether ld/testsuite/ld-elf/pr12730.cc is
valid.  Does gcc actually make any guarantee about order of static
constructors and __attribute ((constructor)) functions?

Compiled with gcc-4.3 branch g++, the testcase segfaults at all
optimization levels.  Compiled with gcc-4.4 branch g++, the testcase
runs at -O0 but segfaults at -O1 and above.  It happens to run OK with
gcc mainline and 4.6.  Given that behaviour, and the fact that some
popular distros ship gcc-4.4 based compilers, I'm thinking that the
testcase should be removed.

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: PATCH: PR ld/12730: regression] crash when allocating in a static constructor
  2011-05-07 13:40             ` H.J. Lu
@ 2011-05-07 14:05               ` Alan Modra
  2011-05-09 13:53                 ` Alan Modra
  2011-06-19 19:18               ` Thomas Schwinge
  1 sibling, 1 reply; 16+ messages in thread
From: Alan Modra @ 2011-05-07 14:05 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

On Sat, May 07, 2011 at 06:39:42AM -0700, H.J. Lu wrote:
> +	  /* We need to reverse-copy input .ctors/.dtors sections if
> +	     they are placed in .init_array/.finit_array for output.  */
> +	  if (o->size > address_size
> +	      && (o->name[6] == 0 || o->name[6] == '.')

Possible segv on accessing name[6].  Test this after the strncmp
calls.  OK with that change.

> +	      && ((strncmp (o->name, ".ctors", 6) == 0
> +		   && strcmp (o->output_section->name,
> +			      ".init_array") == 0)
> +		  || (strncmp (o->name, ".dtors", 6) == 0
> +		      && strcmp (o->output_section->name,
> +				 ".fini_array") == 0)))

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: PATCH: PR ld/12730: regression] crash when allocating in a static constructor
  2011-05-07  8:11           ` Alan Modra
@ 2011-05-07 13:40             ` H.J. Lu
  2011-05-07 14:05               ` Alan Modra
  2011-06-19 19:18               ` Thomas Schwinge
  0 siblings, 2 replies; 16+ messages in thread
From: H.J. Lu @ 2011-05-07 13:40 UTC (permalink / raw)
  To: Binutils

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

On Sat, May 7, 2011 at 1:10 AM, Alan Modra <amodra@gmail.com> wrote:
> On Fri, May 06, 2011 at 09:16:20AM -0700, H.J. Lu wrote:
>> How about this patch?
>
> Much better, thanks.  But,
>
>>       * elf.c (_bfd_elf_section_reloc_offset): New.
>
> why not put your changes in _bfd_elf_section_offset?
>
>>       * elf64-x86-64.c (elf_x86_64_relocate_section): Call
>>       _bfd_elf_section_reloc_offset instead of
>>       _bfd_elf_section_offset.
>>       * elfxx-ia64.c (elfNN_ia64_install_dyn_reloc): Likewise.
>
> The world is more than just Intel..  Every other ELF target will need
> the same as you've done for x86_64 and ia64, and it would be better to
> reverse relocs even for REL targets just in case you have a reloc
> against a global sym.
>
>> +       /* We need to reverse-copy input .ctors/.dtors sections if
>> +          they are placed in .init_array/.finit_array for output.  */
>> +       if (o->size > address_size
>> +           && ((strcmp (o->name, ".ctors") == 0
>
> strncmp (o->name, ".ctors", 6) == 0
> && (o->name[6] == 0 || o->name[6] == '.')
>
> Ditto for dtors, or explain to me why not.  Seems to me that .ctors.nn
> section could possibly have more than one constructor.
>

Here is the updated patch.  OK to install?

Thanks.

-- 
H.J.
---
bfd/

2011-05-07  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/12730
	* elf.c (_bfd_elf_section_offset): Check SEC_ELF_REVERSE_COPY.

	* elflink.c (elf_link_input_bfd): Reverse copy .ctors/.dtors
	sections if needed.

	* section.c (SEC_ELF_REVERSE_COPY): New.
	* bfd-in2.h: Regenerated.

ld/testsuite/

2011-05-07  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/12730
	* ld-elf/elf.exp (array_tests): Add "pr12730".
	(array_tests_pie): New.
	(array_tests_static): Add -static for ""static init array mixed".
	Add "static pr12730".  Run array_tests_pie for Linux.

	* ld-elf/init-mixed.c (ctor1007): Renamed to ...
	(ctor1007a): This.
	(ctor1007b): New.
	(ctors1007): Remove ctor1007.  Add ctor1007b and ctor1007a.
	(dtor1007): Renamed to ...
	(dtor1007a): This.
	(dtor1007b): New.
	(dtors1007): Remove dtor1007.  Add dtor1007b and dtor1007a.
	(ctor65535): Renamed to ...
	(ctor65535a): This.
	(ctor65535b): New.
	(ctors65535): Remove ctor65535.  Add ctor65535b and ctor65535a.
	(dtor65535): Renamed to ...
	(dtor65535a): This.
	(dtor65535b): New.
	(dtors65535): Remove dtor65535.  Add dtor65535b and dtor65535a.

	* ld-elf/pr12730.cc: New.
	* ld-elf/pr12730.out: Likewise.

[-- Attachment #2: binutils-pr12730-4.patch --]
[-- Type: text/plain, Size: 10826 bytes --]

bfd/

2011-05-07  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/12730
	* elf.c (_bfd_elf_section_offset): Check SEC_ELF_REVERSE_COPY.

	* elflink.c (elf_link_input_bfd): Reverse copy .ctors/.dtors
	sections if needed. 

	* section.c (SEC_ELF_REVERSE_COPY): New.
	* bfd-in2.h: Regenerated.

ld/testsuite/

2011-05-07  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/12730
	* ld-elf/elf.exp (array_tests): Add pr12730".
	(array_tests_pie): New.
	(array_tests_static): Add -static for ""static init array mixed".
	Add "static pr12730".  Run array_tests_pie for Linux.

	* ld-elf/init-mixed.c (ctor1007): Renamed to ...
	(ctor1007a): This.
	(ctor1007b): New.
	(ctors1007): Remove ctor1007.  Add ctor1007b and ctor1007a.
	(dtor1007): Renamed to ...
	(dtor1007a): This.
	(dtor1007b): New.
	(dtors1007): Remove dtor1007.  Add dtor1007b and dtor1007a.
	(ctor65535): Renamed to ...
	(ctor65535a): This.
	(ctor65535b): New.
	(ctors65535): Remove ctor65535.  Add ctor65535b and ctor65535a.
	(dtor65535): Renamed to ...
	(dtor65535a): This.
	(dtor65535b): New.
	(dtors65535): Remove dtor65535.  Add dtor65535b and dtor65535a.

	* ld-elf/pr12730.cc: New.
	* ld-elf/pr12730.out: Likewise.

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 76836b1..7ef7f46 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -1320,6 +1320,11 @@ typedef struct bfd_section
      sections.  */
 #define SEC_COFF_SHARED_LIBRARY 0x4000000
 
+  /* This input section should be copied to output in reverse order
+     as an array of pointers.  This is for ELF linker internal use
+     only.  */
+#define SEC_ELF_REVERSE_COPY 0x4000000
+
   /* This section contains data which may be shared with other
      executables or shared objects. This is for COFF only.  */
 #define SEC_COFF_SHARED 0x8000000
diff --git a/bfd/elf.c b/bfd/elf.c
index b5a1952..6fccf42 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -9379,6 +9379,12 @@ _bfd_elf_section_offset (bfd *abfd,
     case ELF_INFO_TYPE_EH_FRAME:
       return _bfd_elf_eh_frame_section_offset (abfd, info, sec, offset);
     default:
+      if ((sec->flags & SEC_ELF_REVERSE_COPY) != 0)
+	{
+	  const struct elf_backend_data *bed = get_elf_backend_data (abfd);
+	  bfd_size_type address_size = bed->s->arch_size / 8;
+	  offset = sec->size - offset - address_size;
+	}
       return offset;
     }
 }
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 082355d..9a80697 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -9120,6 +9120,9 @@ elf_link_input_bfd (struct elf_final_link_info *finfo, bfd *input_bfd)
   asection *o;
   const struct elf_backend_data *bed;
   struct elf_link_hash_entry **sym_hashes;
+  bfd_size_type address_size;
+  bfd_vma r_type_mask;
+  int r_sym_shift;
 
   output_bfd = finfo->output_bfd;
   bed = get_elf_backend_data (output_bfd);
@@ -9290,6 +9293,19 @@ elf_link_input_bfd (struct elf_final_link_info *finfo, bfd *input_bfd)
 	*pindex = indx;
     }
 
+  if (bed->s->arch_size == 32)
+    {
+      r_type_mask = 0xff;
+      r_sym_shift = 8;
+      address_size = 4;
+    }
+  else
+    {
+      r_type_mask = 0xffffffff;
+      r_sym_shift = 32;
+      address_size = 8;
+    }
+
   /* Relocate the contents of each section.  */
   sym_hashes = elf_sym_hashes (input_bfd);
   for (o = input_bfd->sections; o != NULL; o = o->next)
@@ -9394,8 +9410,6 @@ elf_link_input_bfd (struct elf_final_link_info *finfo, bfd *input_bfd)
 	{
 	  Elf_Internal_Rela *internal_relocs;
 	  Elf_Internal_Rela *rel, *relend;
-	  bfd_vma r_type_mask;
-	  int r_sym_shift;
 	  int action_discarded;
 	  int ret;
 
@@ -9407,15 +9421,27 @@ elf_link_input_bfd (struct elf_final_link_info *finfo, bfd *input_bfd)
 	      && o->reloc_count > 0)
 	    return FALSE;
 
-	  if (bed->s->arch_size == 32)
+	  /* We need to reverse-copy input .ctors/.dtors sections if
+	     they are placed in .init_array/.finit_array for output.  */
+	  if (o->size > address_size
+	      && (o->name[6] == 0 || o->name[6] == '.')
+	      && ((strncmp (o->name, ".ctors", 6) == 0
+		   && strcmp (o->output_section->name,
+			      ".init_array") == 0)
+		  || (strncmp (o->name, ".dtors", 6) == 0
+		      && strcmp (o->output_section->name,
+				 ".fini_array") == 0)))
 	    {
-	      r_type_mask = 0xff;
-	      r_sym_shift = 8;
-	    }
-	  else
-	    {
-	      r_type_mask = 0xffffffff;
-	      r_sym_shift = 32;
+	      if (o->size != o->reloc_count * address_size)
+		{
+		  (*_bfd_error_handler)
+		    (_("error: %B: size of section %A is not "
+		       "multiple of address size"),
+		     input_bfd, o);
+		  bfd_set_error (bfd_error_on_input);
+		  return FALSE;
+		}
+	      o->flags |= SEC_ELF_REVERSE_COPY;
 	    }
 
 	  action_discarded = -1;
@@ -9876,12 +9902,34 @@ elf_link_input_bfd (struct elf_final_link_info *finfo, bfd *input_bfd)
 	default:
 	  {
 	    /* FIXME: octets_per_byte.  */
-	    if (! (o->flags & SEC_EXCLUDE)
-		&& ! bfd_set_section_contents (output_bfd, o->output_section,
-					       contents,
-					       (file_ptr) o->output_offset,
-					       o->size))
-	      return FALSE;
+	    if (! (o->flags & SEC_EXCLUDE))
+	      {
+		file_ptr offset = (file_ptr) o->output_offset;
+		bfd_size_type todo = o->size;
+		if ((o->flags & SEC_ELF_REVERSE_COPY))
+		  {
+		    /* Reverse-copy input section to output.  */
+		    do
+		      {
+			todo -= address_size;
+			if (! bfd_set_section_contents (output_bfd,
+							o->output_section,
+							contents + todo,
+							offset,
+							address_size))
+			  return FALSE;
+			if (todo == 0)
+			  break;
+			offset += address_size;
+		      }
+		    while (1);
+		  }
+		else if (! bfd_set_section_contents (output_bfd,
+						     o->output_section,
+						     contents,
+						     offset, todo))
+		  return FALSE;
+	      }
 	  }
 	  break;
 	}
diff --git a/bfd/section.c b/bfd/section.c
index 65ac5e6..3cd7e65 100644
--- a/bfd/section.c
+++ b/bfd/section.c
@@ -327,6 +327,11 @@ CODE_FRAGMENT
 .     sections.  *}
 .#define SEC_COFF_SHARED_LIBRARY 0x4000000
 .
+.  {* This input section should be copied to output in reverse order
+.     as an array of pointers.  This is for ELF linker internal use
+.     only.  *}
+.#define SEC_ELF_REVERSE_COPY 0x4000000
+.
 .  {* This section contains data which may be shared with other
 .     executables or shared objects. This is for COFF only.  *}
 .#define SEC_COFF_SHARED 0x8000000
diff --git a/ld/testsuite/ld-elf/elf.exp b/ld/testsuite/ld-elf/elf.exp
index 73a417c..6808d8a 100644
--- a/ld/testsuite/ld-elf/elf.exp
+++ b/ld/testsuite/ld-elf/elf.exp
@@ -81,17 +81,32 @@ set array_tests {
     {"init array" "" "" {init.c} "init" "init.out"}
     {"fini array" "" "" {fini.c} "fini" "fini.out"}
     {"init array mixed" "" "" {init-mixed.c} "init-mixed" "init-mixed.out" "-I."}
+    {"pr12730" "" "" {pr12730.cc} "pr12730" "pr12730.out" "" "c++"}
+}
+set array_tests_pie {
+    {"PIE preinit array" "-pie" "" {preinit.c} "preinit" "preinit.out" "-fPIE" }
+    {"PIE init array" "-pie" "" {init.c} "init" "init.out" "-fPIE"}
+    {"PIE fini array" "-pie" "" {fini.c} "fini" "fini.out" "-fPIE"}
+    {"PIE init array mixed" "-pie" "" {init-mixed.c} "init-mixed" "init-mixed.out" "-I. -fPIE"}
+    {"PIE pr12730" "-pie" "" {pr12730.cc} "pr12730" "pr12730.out" "-fPIE" "c++"}
 }
 set array_tests_static {
     {"static preinit array" "-static" "" {preinit.c} "preinit" "preinit.out"}
     {"static init array" "-static" "" {init.c} "init" "init.out"}
     {"static fini array" "-static" "" {fini.c} "fini" "fini.out"}
-    {"static init array mixed" "" "" {init-mixed.c} "init-mixed" "init-mixed.out" "-I."}
+    {"static init array mixed" "-static" "" {init-mixed.c} "init-mixed" "init-mixed.out" "-I."}
+    {"static pr12730" "-static" "" {pr12730.cc} "pr12730" "pr12730.out" "" "c++"}
 }
 
 # NetBSD ELF systems do not currently support the .*_array sections.
 set xfails [list "*-*-netbsdelf*"]
 run_ld_link_exec_tests $xfails $array_tests
+
+# Run PIE tests only on Linux.
+if { [istarget "*-*-linux*"] } {
+    run_ld_link_exec_tests $xfails $array_tests_pie
+}
+
 # Be cautious to not XFAIL for *-*-linux-gnu*, *-*-kfreebsd-gnu*, etc.
 switch -regexp $target_triplet {
     ^\[^-\]*-\[^-\]*-gnu.*$ {
diff --git a/ld/testsuite/ld-elf/init-mixed.c b/ld/testsuite/ld-elf/init-mixed.c
index 1d0c727..770a4b5 100644
--- a/ld/testsuite/ld-elf/init-mixed.c
+++ b/ld/testsuite/ld-elf/init-mixed.c
@@ -27,25 +27,39 @@ void (*const fini_array1005[]) ()
   = { fini1005 };
 
 static void
-ctor1007 ()
+ctor1007a ()
 {
   if (count != 1005)
     abort ();
+  count = 1006;
+}
+static void
+ctor1007b ()
+{
+  if (count != 1006)
+    abort ();
   count = 1007;
 }
 void (*const ctors1007[]) ()
   __attribute__ ((section (".ctors.64528"), aligned (sizeof (void *))))
-  = { ctor1007 };
+  = { ctor1007b, ctor1007a };
 static void
-dtor1007 ()
+dtor1007a ()
 {
-  if (count != 1007)
+  if (count != 1006)
     abort ();
   count = 1005;
 }
+static void
+dtor1007b ()
+{
+  if (count != 1007)
+    abort ();
+  count = 1006;
+}
 void (*const dtors1007[]) ()
   __attribute__ ((section (".dtors.64528"), aligned (sizeof (void *))))
-  = { dtor1007 };
+  = { dtor1007b, dtor1007a };
 
 static void
 init65530 ()
@@ -69,17 +83,31 @@ void (*const fini_array65530[]) ()
   = { fini65530 };
 
 static void
-ctor65535 ()
+ctor65535a ()
 {
   if (count != 65530)
     abort ();
   count = 65535;
 }
+static void
+ctor65535b ()
+{
+  if (count != 65535)
+    abort ();
+  count = 65536;
+}
 void (*const ctors65535[]) ()
   __attribute__ ((section (".ctors"), aligned (sizeof (void *))))
-  = { ctor65535 };
+  = { ctor65535b, ctor65535a };
+static void
+dtor65535b ()
+{
+  if (count != 65536)
+    abort ();
+  count = 65535;
+}
 static void
-dtor65535 ()
+dtor65535a ()
 {
   if (count != 65535)
     abort ();
@@ -87,7 +115,7 @@ dtor65535 ()
 }
 void (*const dtors65535[]) ()
   __attribute__ ((section (".dtors"), aligned (sizeof (void *))))
-  = { dtor65535 };
+  = { dtor65535b, dtor65535a };
 #endif
 
 int
diff --git a/ld/testsuite/ld-elf/pr12730.cc b/ld/testsuite/ld-elf/pr12730.cc
new file mode 100644
index 0000000..69f57f9
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr12730.cc
@@ -0,0 +1,38 @@
+#include <iostream>
+
+class Hello
+{
+public:
+   Hello ()
+    {}
+
+  ~Hello ()
+    {}
+
+  void act ()
+    { std::cout << "Hello, world!" << std::endl; }
+};
+
+
+template <class T>
+struct Foo
+{
+  T* _M_allocate_single_object ()
+    {
+      return new T;
+    }
+};
+
+static void __attribute__ (( constructor )) PWLIB_StaticLoader() {
+  Foo<Hello> allocator;
+  Hello* salut = allocator._M_allocate_single_object ();
+  salut->act ();
+}
+
+
+int
+main (int /*argc*/,
+      char* /*argv*/[])
+{
+  return 0;
+}
diff --git a/ld/testsuite/ld-elf/pr12730.out b/ld/testsuite/ld-elf/pr12730.out
new file mode 100644
index 0000000..af5626b
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr12730.out
@@ -0,0 +1 @@
+Hello, world!

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

* Re: PATCH: PR ld/12730: regression] crash when allocating in a static constructor
  2011-05-06 16:16         ` H.J. Lu
@ 2011-05-07  8:11           ` Alan Modra
  2011-05-07 13:40             ` H.J. Lu
  0 siblings, 1 reply; 16+ messages in thread
From: Alan Modra @ 2011-05-07  8:11 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Binutils

On Fri, May 06, 2011 at 09:16:20AM -0700, H.J. Lu wrote:
> How about this patch?

Much better, thanks.  But,

> 	* elf.c (_bfd_elf_section_reloc_offset): New.

why not put your changes in _bfd_elf_section_offset?

> 	* elf64-x86-64.c (elf_x86_64_relocate_section): Call
> 	_bfd_elf_section_reloc_offset instead of
> 	_bfd_elf_section_offset.
> 	* elfxx-ia64.c (elfNN_ia64_install_dyn_reloc): Likewise.

The world is more than just Intel..  Every other ELF target will need
the same as you've done for x86_64 and ia64, and it would be better to
reverse relocs even for REL targets just in case you have a reloc
against a global sym.

> +	  /* We need to reverse-copy input .ctors/.dtors sections if
> +	     they are placed in .init_array/.finit_array for output.  */
> +	  if (o->size > address_size
> +	      && ((strcmp (o->name, ".ctors") == 0

strncmp (o->name, ".ctors", 6) == 0
&& (o->name[6] == 0 || o->name[6] == '.')

Ditto for dtors, or explain to me why not.  Seems to me that .ctors.nn
section could possibly have more than one constructor.

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: PATCH: PR ld/12730: regression] crash when allocating in a static constructor
  2011-05-06 13:23       ` H.J. Lu
@ 2011-05-06 16:16         ` H.J. Lu
  2011-05-07  8:11           ` Alan Modra
  0 siblings, 1 reply; 16+ messages in thread
From: H.J. Lu @ 2011-05-06 16:16 UTC (permalink / raw)
  To: Binutils

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

On Fri, May 6, 2011 at 6:23 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, May 5, 2011 at 7:12 AM, Alan Modra <amodra@gmail.com> wrote:
>> On Thu, May 05, 2011 at 06:26:15AM -0700, H.J. Lu wrote:
>>> On Thu, May 5, 2011 at 1:27 AM, Alan Modra <amodra@gmail.com> wrote:
>>> > On Wed, May 04, 2011 at 10:18:32PM -0700, H.J. Lu wrote:
>>> >> When we put .ctors into .init_array, we have to reverse copy .ctors secton.
>>> >> Otherwise, constructor function may not work with C++ run-time library
>>> >> correctly.  OK for trunk?
>>> >
>>> > What about .dtors?  You have the same problem there.  I suspect, but
>>>
>>> You are right.  Here is the updated patch to handle .dtors sections
>>> with the updated testcase.  OK for trunk?
>>
>> No.
>>
>>> > haven't verified, that .ctors.* and .dtors.* also need reversing.  If
>>> > that is true then it would be better to do your reversing trick for
>>> > anything going to the .init_array output section that isn't named
>>> > .init_array* and similarly for .fini_array.
>>
>> Are you sure there is no need to reverse .ctors.* and .dtors.*?
>>
>> The reason I recommended testing the output section is that limits
>> section reversal to that particular output section.  I will not
>> approve a patch that ignores this recommendation.  You also should
>> remove reverse_copy_ctors.  That's plain wrong.  Consider people
>> linking using a custom (old) script that does not put .ctors into
>> .init_array.
>>
>>> > You also need to reverse any dynamic relocations applying to the
>>> > sections you are reversing.
>>>
>>> It isn't a problem since we apply relocations on the input sections
>>> first and copy relocated input sections to output where I reverse
>>> copy .ctors/.dtors sections if needed.
>>
>> Try compiling your testcase as a PIE or shared lib on a target that
>> uses RELA.  I haven't tried it, but I think the dynamic RELATIVE
>> relocs you'll get in .ctors will undo your section reversing.
>>
>
> I will fix it.
>

How about this patch?

Thanks.



-- 
H.J.
---
bfd/

2011-05-06  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/12730
	* elf-bfd.h (_bfd_elf_section_reloc_offset): New.

	* elf.c (_bfd_elf_section_reloc_offset): New.

	* elf64-x86-64.c (elf_x86_64_relocate_section): Call
	_bfd_elf_section_reloc_offset instead of
	_bfd_elf_section_offset.
	* elfxx-ia64.c (elfNN_ia64_install_dyn_reloc): Likewise.

	* elflink.c (elf_link_input_bfd): Reverse copy .ctors/.dtors
	sections if needed.  Call _bfd_elf_section_reloc_offset instead
	of _bfd_elf_section_offset.

	* section.c (SEC_ELF_REVERSE_COPY): New.
	* bfd-in2.h: Regenerated.

ld/testsuite/

2011-05-06  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/12730
	* ld-elf/elf.exp (array_tests): Add pr12730".
	(array_tests_pie): New.
	(array_tests_static): Add -static for ""static init array mixed".
	Add "static pr12730".  Run array_tests_pie for Linux.

	* ld-elf/init-mixed.c (ctor65535): Renamed to ...
	(ctor65535a): This.
	(ctor65535b): New.
	(ctors65535): Remove ctor65535.  Add ctor65535b and ctor65535a.
	(dtor65535): Renamed to ...
	(dtor65535a): This.
	(dtor65535b): New.
	(dtors65535): Remove dtor65535.  Add dtor65535b and dtor65535a.

	* ld-elf/pr12730.cc: New.
	* ld-elf/pr12730.out: Likewise.

[-- Attachment #2: binutils-pr12730-3.patch --]
[-- Type: text/plain, Size: 13308 bytes --]

bfd/

2011-05-06  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/12730
	* elf-bfd.h (_bfd_elf_section_reloc_offset): New.

	* elf.c (_bfd_elf_section_reloc_offset): New.

	* elf64-x86-64.c (elf_x86_64_relocate_section): Call
	_bfd_elf_section_reloc_offset instead of
	_bfd_elf_section_offset.
	* elfxx-ia64.c (elfNN_ia64_install_dyn_reloc): Likewise.

	* elflink.c (elf_link_input_bfd): Reverse copy .ctors/.dtors
	sections if needed.  Call _bfd_elf_section_reloc_offset instead
	of _bfd_elf_section_offset.

	* section.c (SEC_ELF_REVERSE_COPY): New.
	* bfd-in2.h: Regenerated.

ld/testsuite/

2011-05-06  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/12730
	* ld-elf/elf.exp (array_tests): Add pr12730".
	(array_tests_pie): New.
	(array_tests_static): Add -static for ""static init array mixed".
	Add "static pr12730".  Run array_tests_pie for Linux.

	* ld-elf/init-mixed.c (ctor65535): Renamed to ...
	(ctor65535a): This.
	(ctor65535b): New.
	(ctors65535): Remove ctor65535.  Add ctor65535b and ctor65535a.
	(dtor65535): Renamed to ...
	(dtor65535a): This.
	(dtor65535b): New.
	(dtors65535): Remove dtor65535.  Add dtor65535b and dtor65535a.

	* ld-elf/pr12730.cc: New.
	* ld-elf/pr12730.out: Likewise.

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 76836b1..7ef7f46 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -1320,6 +1320,11 @@ typedef struct bfd_section
      sections.  */
 #define SEC_COFF_SHARED_LIBRARY 0x4000000
 
+  /* This input section should be copied to output in reverse order
+     as an array of pointers.  This is for ELF linker internal use
+     only.  */
+#define SEC_ELF_REVERSE_COPY 0x4000000
+
   /* This section contains data which may be shared with other
      executables or shared objects. This is for COFF only.  */
 #define SEC_COFF_SHARED 0x8000000
diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index 735d2b5..39ccb90 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -1752,6 +1752,8 @@ extern bfd_vma _bfd_elf_rel_local_sym
   (bfd *, Elf_Internal_Sym *, asection **, bfd_vma);
 extern bfd_vma _bfd_elf_section_offset
   (bfd *, struct bfd_link_info *, asection *, bfd_vma);
+extern bfd_vma _bfd_elf_section_reloc_offset
+  (bfd *, struct bfd_link_info *, asection *, bfd_vma);
 
 extern unsigned long bfd_elf_hash
   (const char *);
diff --git a/bfd/elf.c b/bfd/elf.c
index b5a1952..f4addeb 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -9382,6 +9382,33 @@ _bfd_elf_section_offset (bfd *abfd,
       return offset;
     }
 }
+
+/* Similar to _bfd_elf_section_offset. But return the relocation offset
+   in reverse order if the SEC_ELF_REVERSE_COPY bit it set.  */
+
+bfd_vma
+_bfd_elf_section_reloc_offset (bfd *abfd,
+			       struct bfd_link_info *info,
+			       asection *sec,
+			       bfd_vma offset)
+{
+  switch (sec->sec_info_type)
+    {
+    case ELF_INFO_TYPE_STABS:
+      return _bfd_stab_section_offset (sec, elf_section_data (sec)->sec_info,
+				       offset);
+    case ELF_INFO_TYPE_EH_FRAME:
+      return _bfd_elf_eh_frame_section_offset (abfd, info, sec, offset);
+    default:
+      if ((sec->flags & SEC_ELF_REVERSE_COPY) != 0)
+	{
+	  const struct elf_backend_data *bed = get_elf_backend_data (abfd);
+	  bfd_size_type address_size = bed->s->arch_size / 8;
+	  offset = sec->size - offset - address_size;
+	}
+      return offset;
+    }
+}
 \f
 /* Create a new BFD as if by bfd_openr.  Rather than opening a file,
    reconstruct an ELF file by reading the segments out of remote memory
diff --git a/bfd/elf64-x86-64.c b/bfd/elf64-x86-64.c
index ae175e1..a7173d5 100644
--- a/bfd/elf64-x86-64.c
+++ b/bfd/elf64-x86-64.c
@@ -2948,10 +2948,8 @@ elf_x86_64_relocate_section (bfd *output_bfd,
 
 		  /* Need a dynamic relocation to get the real function
 		     address.  */
-		  outrel.r_offset = _bfd_elf_section_offset (output_bfd,
-							     info,
-							     input_section,
-							     rel->r_offset);
+		  outrel.r_offset = _bfd_elf_section_reloc_offset
+		    (output_bfd, info, input_section, rel->r_offset);
 		  if (outrel.r_offset == (bfd_vma) -1
 		      || outrel.r_offset == (bfd_vma) -2)
 		    abort ();
@@ -3357,9 +3355,10 @@ elf_x86_64_relocate_section (bfd *output_bfd,
 	      skip = FALSE;
 	      relocate = FALSE;
 
-	      outrel.r_offset =
-		_bfd_elf_section_offset (output_bfd, info, input_section,
-					 rel->r_offset);
+	      outrel.r_offset = 
+		_bfd_elf_section_reloc_offset (output_bfd, info,
+					       input_section,
+					       rel->r_offset);
 	      if (outrel.r_offset == (bfd_vma) -1)
 		skip = TRUE;
 	      else if (outrel.r_offset == (bfd_vma) -2)
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 082355d..b1c9e57 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -9120,6 +9120,9 @@ elf_link_input_bfd (struct elf_final_link_info *finfo, bfd *input_bfd)
   asection *o;
   const struct elf_backend_data *bed;
   struct elf_link_hash_entry **sym_hashes;
+  bfd_size_type address_size;
+  bfd_vma r_type_mask;
+  int r_sym_shift;
 
   output_bfd = finfo->output_bfd;
   bed = get_elf_backend_data (output_bfd);
@@ -9290,6 +9293,19 @@ elf_link_input_bfd (struct elf_final_link_info *finfo, bfd *input_bfd)
 	*pindex = indx;
     }
 
+  if (bed->s->arch_size == 32)
+    {
+      r_type_mask = 0xff;
+      r_sym_shift = 8;
+      address_size = 4;
+    }
+  else
+    {
+      r_type_mask = 0xffffffff;
+      r_sym_shift = 32;
+      address_size = 8;
+    }
+
   /* Relocate the contents of each section.  */
   sym_hashes = elf_sym_hashes (input_bfd);
   for (o = input_bfd->sections; o != NULL; o = o->next)
@@ -9394,8 +9410,6 @@ elf_link_input_bfd (struct elf_final_link_info *finfo, bfd *input_bfd)
 	{
 	  Elf_Internal_Rela *internal_relocs;
 	  Elf_Internal_Rela *rel, *relend;
-	  bfd_vma r_type_mask;
-	  int r_sym_shift;
 	  int action_discarded;
 	  int ret;
 
@@ -9407,15 +9421,26 @@ elf_link_input_bfd (struct elf_final_link_info *finfo, bfd *input_bfd)
 	      && o->reloc_count > 0)
 	    return FALSE;
 
-	  if (bed->s->arch_size == 32)
+	  /* We need to reverse-copy input .ctors/.dtors sections if
+	     they are placed in .init_array/.finit_array for output.  */
+	  if (o->size > address_size
+	      && ((strcmp (o->name, ".ctors") == 0
+		   && strcmp (o->output_section->name,
+			      ".init_array") == 0)
+		  || (strcmp (o->name, ".dtors") == 0
+		      && strcmp (o->output_section->name,
+				 ".fini_array") == 0)))
 	    {
-	      r_type_mask = 0xff;
-	      r_sym_shift = 8;
-	    }
-	  else
-	    {
-	      r_type_mask = 0xffffffff;
-	      r_sym_shift = 32;
+	      if (o->size != o->reloc_count * address_size)
+		{
+		  (*_bfd_error_handler)
+		    (_("error: %B: size of section %A is not "
+		       "multiple of address size"),
+		     input_bfd, o);
+		  bfd_set_error (bfd_error_on_input);
+		  return FALSE;
+		}
+	      o->flags |= SEC_ELF_REVERSE_COPY;
 	    }
 
 	  action_discarded = -1;
@@ -9629,9 +9654,8 @@ elf_link_input_bfd (struct elf_final_link_info *finfo, bfd *input_bfd)
 		      rela_normal = bed->rela_normal;
 		    }
 
-		  irela->r_offset = _bfd_elf_section_offset (output_bfd,
-							     finfo->info, o,
-							     irela->r_offset);
+		  irela->r_offset = _bfd_elf_section_reloc_offset
+		    (output_bfd, finfo->info, o, irela->r_offset);
 		  if (irela->r_offset >= (bfd_vma) -2)
 		    {
 		      /* This is a reloc for a deleted entry or somesuch.
@@ -9876,12 +9900,34 @@ elf_link_input_bfd (struct elf_final_link_info *finfo, bfd *input_bfd)
 	default:
 	  {
 	    /* FIXME: octets_per_byte.  */
-	    if (! (o->flags & SEC_EXCLUDE)
-		&& ! bfd_set_section_contents (output_bfd, o->output_section,
-					       contents,
-					       (file_ptr) o->output_offset,
-					       o->size))
-	      return FALSE;
+	    if (! (o->flags & SEC_EXCLUDE))
+	      {
+		file_ptr offset = (file_ptr) o->output_offset;
+		bfd_size_type todo = o->size;
+		if ((o->flags & SEC_ELF_REVERSE_COPY))
+		  {
+		    /* Reverse-copy input section to output.  */
+		    do
+		      {
+			todo -= address_size;
+			if (! bfd_set_section_contents (output_bfd,
+							o->output_section,
+							contents + todo,
+							offset,
+							address_size))
+			  return FALSE;
+			if (todo == 0)
+			  break;
+			offset += address_size;
+		      }
+		    while (1);
+		  }
+		else if (! bfd_set_section_contents (output_bfd,
+						     o->output_section,
+						     contents,
+						     offset, todo))
+		  return FALSE;
+	      }
 	  }
 	  break;
 	}
diff --git a/bfd/elfxx-ia64.c b/bfd/elfxx-ia64.c
index ca0a3bc..a0bb83c 100644
--- a/bfd/elfxx-ia64.c
+++ b/bfd/elfxx-ia64.c
@@ -3979,7 +3979,8 @@ elfNN_ia64_install_dyn_reloc (bfd *abfd, struct bfd_link_info *info,
   BFD_ASSERT (dynindx != -1);
   outrel.r_info = ELFNN_R_INFO (dynindx, type);
   outrel.r_addend = addend;
-  outrel.r_offset = _bfd_elf_section_offset (abfd, info, sec, offset);
+  outrel.r_offset = _bfd_elf_section_reloc_offset (abfd, info, sec,
+						   offset);
   if (outrel.r_offset >= (bfd_vma) -2)
     {
       /* Run for the hills.  We shouldn't be outputting a relocation
diff --git a/bfd/section.c b/bfd/section.c
index 65ac5e6..3cd7e65 100644
--- a/bfd/section.c
+++ b/bfd/section.c
@@ -327,6 +327,11 @@ CODE_FRAGMENT
 .     sections.  *}
 .#define SEC_COFF_SHARED_LIBRARY 0x4000000
 .
+.  {* This input section should be copied to output in reverse order
+.     as an array of pointers.  This is for ELF linker internal use
+.     only.  *}
+.#define SEC_ELF_REVERSE_COPY 0x4000000
+.
 .  {* This section contains data which may be shared with other
 .     executables or shared objects. This is for COFF only.  *}
 .#define SEC_COFF_SHARED 0x8000000
diff --git a/ld/testsuite/ld-elf/elf.exp b/ld/testsuite/ld-elf/elf.exp
index 73a417c..6808d8a 100644
--- a/ld/testsuite/ld-elf/elf.exp
+++ b/ld/testsuite/ld-elf/elf.exp
@@ -81,17 +81,32 @@ set array_tests {
     {"init array" "" "" {init.c} "init" "init.out"}
     {"fini array" "" "" {fini.c} "fini" "fini.out"}
     {"init array mixed" "" "" {init-mixed.c} "init-mixed" "init-mixed.out" "-I."}
+    {"pr12730" "" "" {pr12730.cc} "pr12730" "pr12730.out" "" "c++"}
+}
+set array_tests_pie {
+    {"PIE preinit array" "-pie" "" {preinit.c} "preinit" "preinit.out" "-fPIE" }
+    {"PIE init array" "-pie" "" {init.c} "init" "init.out" "-fPIE"}
+    {"PIE fini array" "-pie" "" {fini.c} "fini" "fini.out" "-fPIE"}
+    {"PIE init array mixed" "-pie" "" {init-mixed.c} "init-mixed" "init-mixed.out" "-I. -fPIE"}
+    {"PIE pr12730" "-pie" "" {pr12730.cc} "pr12730" "pr12730.out" "-fPIE" "c++"}
 }
 set array_tests_static {
     {"static preinit array" "-static" "" {preinit.c} "preinit" "preinit.out"}
     {"static init array" "-static" "" {init.c} "init" "init.out"}
     {"static fini array" "-static" "" {fini.c} "fini" "fini.out"}
-    {"static init array mixed" "" "" {init-mixed.c} "init-mixed" "init-mixed.out" "-I."}
+    {"static init array mixed" "-static" "" {init-mixed.c} "init-mixed" "init-mixed.out" "-I."}
+    {"static pr12730" "-static" "" {pr12730.cc} "pr12730" "pr12730.out" "" "c++"}
 }
 
 # NetBSD ELF systems do not currently support the .*_array sections.
 set xfails [list "*-*-netbsdelf*"]
 run_ld_link_exec_tests $xfails $array_tests
+
+# Run PIE tests only on Linux.
+if { [istarget "*-*-linux*"] } {
+    run_ld_link_exec_tests $xfails $array_tests_pie
+}
+
 # Be cautious to not XFAIL for *-*-linux-gnu*, *-*-kfreebsd-gnu*, etc.
 switch -regexp $target_triplet {
     ^\[^-\]*-\[^-\]*-gnu.*$ {
diff --git a/ld/testsuite/ld-elf/init-mixed.c b/ld/testsuite/ld-elf/init-mixed.c
index 1d0c727..2c9d434 100644
--- a/ld/testsuite/ld-elf/init-mixed.c
+++ b/ld/testsuite/ld-elf/init-mixed.c
@@ -69,17 +69,31 @@ void (*const fini_array65530[]) ()
   = { fini65530 };
 
 static void
-ctor65535 ()
+ctor65535a ()
 {
   if (count != 65530)
     abort ();
   count = 65535;
 }
+static void
+ctor65535b ()
+{
+  if (count != 65535)
+    abort ();
+  count = 65536;
+}
 void (*const ctors65535[]) ()
   __attribute__ ((section (".ctors"), aligned (sizeof (void *))))
-  = { ctor65535 };
+  = { ctor65535b, ctor65535a };
+static void
+dtor65535b ()
+{
+  if (count != 65536)
+    abort ();
+  count = 65535;
+}
 static void
-dtor65535 ()
+dtor65535a ()
 {
   if (count != 65535)
     abort ();
@@ -87,7 +101,7 @@ dtor65535 ()
 }
 void (*const dtors65535[]) ()
   __attribute__ ((section (".dtors"), aligned (sizeof (void *))))
-  = { dtor65535 };
+  = { dtor65535b, dtor65535a };
 #endif
 
 int
diff --git a/ld/testsuite/ld-elf/pr12730.cc b/ld/testsuite/ld-elf/pr12730.cc
new file mode 100644
index 0000000..69f57f9
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr12730.cc
@@ -0,0 +1,38 @@
+#include <iostream>
+
+class Hello
+{
+public:
+   Hello ()
+    {}
+
+  ~Hello ()
+    {}
+
+  void act ()
+    { std::cout << "Hello, world!" << std::endl; }
+};
+
+
+template <class T>
+struct Foo
+{
+  T* _M_allocate_single_object ()
+    {
+      return new T;
+    }
+};
+
+static void __attribute__ (( constructor )) PWLIB_StaticLoader() {
+  Foo<Hello> allocator;
+  Hello* salut = allocator._M_allocate_single_object ();
+  salut->act ();
+}
+
+
+int
+main (int /*argc*/,
+      char* /*argv*/[])
+{
+  return 0;
+}
diff --git a/ld/testsuite/ld-elf/pr12730.out b/ld/testsuite/ld-elf/pr12730.out
new file mode 100644
index 0000000..af5626b
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr12730.out
@@ -0,0 +1 @@
+Hello, world!

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

* Re: PATCH: PR ld/12730: regression] crash when allocating in a static constructor
  2011-05-05 14:13     ` Alan Modra
@ 2011-05-06 13:23       ` H.J. Lu
  2011-05-06 16:16         ` H.J. Lu
  0 siblings, 1 reply; 16+ messages in thread
From: H.J. Lu @ 2011-05-06 13:23 UTC (permalink / raw)
  To: Binutils

On Thu, May 5, 2011 at 7:12 AM, Alan Modra <amodra@gmail.com> wrote:
> On Thu, May 05, 2011 at 06:26:15AM -0700, H.J. Lu wrote:
>> On Thu, May 5, 2011 at 1:27 AM, Alan Modra <amodra@gmail.com> wrote:
>> > On Wed, May 04, 2011 at 10:18:32PM -0700, H.J. Lu wrote:
>> >> When we put .ctors into .init_array, we have to reverse copy .ctors secton.
>> >> Otherwise, constructor function may not work with C++ run-time library
>> >> correctly.  OK for trunk?
>> >
>> > What about .dtors?  You have the same problem there.  I suspect, but
>>
>> You are right.  Here is the updated patch to handle .dtors sections
>> with the updated testcase.  OK for trunk?
>
> No.
>
>> > haven't verified, that .ctors.* and .dtors.* also need reversing.  If
>> > that is true then it would be better to do your reversing trick for
>> > anything going to the .init_array output section that isn't named
>> > .init_array* and similarly for .fini_array.
>
> Are you sure there is no need to reverse .ctors.* and .dtors.*?
>
> The reason I recommended testing the output section is that limits
> section reversal to that particular output section.  I will not
> approve a patch that ignores this recommendation.  You also should
> remove reverse_copy_ctors.  That's plain wrong.  Consider people
> linking using a custom (old) script that does not put .ctors into
> .init_array.
>
>> > You also need to reverse any dynamic relocations applying to the
>> > sections you are reversing.
>>
>> It isn't a problem since we apply relocations on the input sections
>> first and copy relocated input sections to output where I reverse
>> copy .ctors/.dtors sections if needed.
>
> Try compiling your testcase as a PIE or shared lib on a target that
> uses RELA.  I haven't tried it, but I think the dynamic RELATIVE
> relocs you'll get in .ctors will undo your section reversing.
>

I will fix it.

-- 
H.J.

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

* Re: PATCH: PR ld/12730: regression] crash when allocating in a static constructor
  2011-05-05 13:27   ` H.J. Lu
@ 2011-05-05 14:13     ` Alan Modra
  2011-05-06 13:23       ` H.J. Lu
  0 siblings, 1 reply; 16+ messages in thread
From: Alan Modra @ 2011-05-05 14:13 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

On Thu, May 05, 2011 at 06:26:15AM -0700, H.J. Lu wrote:
> On Thu, May 5, 2011 at 1:27 AM, Alan Modra <amodra@gmail.com> wrote:
> > On Wed, May 04, 2011 at 10:18:32PM -0700, H.J. Lu wrote:
> >> When we put .ctors into .init_array, we have to reverse copy .ctors secton.
> >> Otherwise, constructor function may not work with C++ run-time library
> >> correctly.  OK for trunk?
> >
> > What about .dtors?  You have the same problem there.  I suspect, but
> 
> You are right.  Here is the updated patch to handle .dtors sections
> with the updated testcase.  OK for trunk?

No.

> > haven't verified, that .ctors.* and .dtors.* also need reversing.  If
> > that is true then it would be better to do your reversing trick for
> > anything going to the .init_array output section that isn't named
> > .init_array* and similarly for .fini_array.

Are you sure there is no need to reverse .ctors.* and .dtors.*?

The reason I recommended testing the output section is that limits
section reversal to that particular output section.  I will not
approve a patch that ignores this recommendation.  You also should
remove reverse_copy_ctors.  That's plain wrong.  Consider people
linking using a custom (old) script that does not put .ctors into
.init_array.

> > You also need to reverse any dynamic relocations applying to the
> > sections you are reversing.
> 
> It isn't a problem since we apply relocations on the input sections
> first and copy relocated input sections to output where I reverse
> copy .ctors/.dtors sections if needed.

Try compiling your testcase as a PIE or shared lib on a target that
uses RELA.  I haven't tried it, but I think the dynamic RELATIVE
relocs you'll get in .ctors will undo your section reversing.

-- 
Alan Modra
Australia Development Lab, IBM

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

* Re: PATCH: PR ld/12730: regression] crash when allocating in a static constructor
  2011-05-05  8:27 ` Alan Modra
@ 2011-05-05 13:27   ` H.J. Lu
  2011-05-05 14:13     ` Alan Modra
  0 siblings, 1 reply; 16+ messages in thread
From: H.J. Lu @ 2011-05-05 13:27 UTC (permalink / raw)
  To: binutils

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

On Thu, May 5, 2011 at 1:27 AM, Alan Modra <amodra@gmail.com> wrote:
> On Wed, May 04, 2011 at 10:18:32PM -0700, H.J. Lu wrote:
>> When we put .ctors into .init_array, we have to reverse copy .ctors secton.
>> Otherwise, constructor function may not work with C++ run-time library
>> correctly.  OK for trunk?
>
> What about .dtors?  You have the same problem there.  I suspect, but

You are right.  Here is the updated patch to handle .dtors sections
with the updated testcase.  OK for trunk?

> haven't verified, that .ctors.* and .dtors.* also need reversing.  If
> that is true then it would be better to do your reversing trick for
> anything going to the .init_array output section that isn't named
> .init_array* and similarly for .fini_array.
>
> You also need to reverse any dynamic relocations applying to the
> sections you are reversing.

It isn't a problem since we apply relocations on the input sections
first and copy relocated input sections to output where I reverse
copy .ctors/.dtors sections if needed.

Thanks.


-- 
H.J.
---
bfd/

2011-05-05  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/12730
	* elflink.c (elf_link_input_bfd): Reverse copy .ctors/.dtors
	sections if needed.

include/

2011-05-05  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/12730
	* bfdlink.h (bfd_link_info): Add reverse_copy_ctors_dtors.

ld/

2011-05-05  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/12730
	* emultempl/elf32.em (gld${EMULATION_NAME}_after_parse): New.
	(ld_${EMULATION_NAME}_emulation): Replace after_parse_default
	with gld${EMULATION_NAME}_after_parse.

ld/testsuite/

2011-05-05  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/12730
	* ld-elf/elf.exp (array_tests): Add pr12730".
	(array_tests_static): Add "static pr12730".

	* ld-elf/init-mixed.c (ctor65535): Renamed to ...
	(ctor65535a): This.
	(ctor65535b): New.
	(ctors65535): Remove ctor65535.  Add ctor65535b and ctor65535a.
	(dtor65535): Renamed to ...
	(dtor65535a): This.
	(dtor65535b): New.
	(dtors65535): Remove dtor65535.  Add dtor65535b and dtor65535a.

	* ld-elf/pr12730.cc: New.
	* ld-elf/pr12730.out: Likewise.

[-- Attachment #2: binutils-pr12730-2.patch --]
[-- Type: text/plain, Size: 7579 bytes --]

bfd/

2011-05-05  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/12730
	* elflink.c (elf_link_input_bfd): Reverse copy .ctors/.dtors
	sections if needed.

include/

2011-05-05  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/12730
	* bfdlink.h (bfd_link_info): Add reverse_copy_ctors_dtors.

ld/

2011-05-05  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/12730
	* emultempl/elf32.em (gld${EMULATION_NAME}_after_parse): New.
	(ld_${EMULATION_NAME}_emulation): Replace after_parse_default
	with gld${EMULATION_NAME}_after_parse.

ld/testsuite/

2011-05-05  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/12730
	* ld-elf/elf.exp (array_tests): Add pr12730".
	(array_tests_static): Add "static pr12730".

	* ld-elf/init-mixed.c (ctor65535): Renamed to ...
	(ctor65535a): This.
	(ctor65535b): New.
	(ctors65535): Remove ctor65535.  Add ctor65535b and ctor65535a.
	(dtor65535): Renamed to ...
	(dtor65535a): This.
	(dtor65535b): New.
	(dtors65535): Remove dtor65535.  Add dtor65535b and dtor65535a.

	* ld-elf/pr12730.cc: New.
	* ld-elf/pr12730.out: Likewise.

diff --git a/bfd/elflink.c b/bfd/elflink.c
index 082355d..c7027a2 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -9876,12 +9876,50 @@ elf_link_input_bfd (struct elf_final_link_info *finfo, bfd *input_bfd)
 	default:
 	  {
 	    /* FIXME: octets_per_byte.  */
-	    if (! (o->flags & SEC_EXCLUDE)
-		&& ! bfd_set_section_contents (output_bfd, o->output_section,
-					       contents,
-					       (file_ptr) o->output_offset,
-					       o->size))
-	      return FALSE;
+	    if (! (o->flags & SEC_EXCLUDE))
+	      {
+		bfd_size_type address_size = bed->s->arch_size / 8;
+		file_ptr offset = (file_ptr) o->output_offset;
+		bfd_size_type todo = o->size;
+		/* Check if we need to reverse-copy .ctors/.dtors
+		   sections.  */
+		if (finfo->info->reverse_copy_ctors_dtors
+		    && todo > address_size
+		    && o->name[0] == '.'
+		    && (o->name[1] == 'c' || o->name[1] == 'd')
+		    && strcmp (o->name + 2, "tors") == 0)
+		  {
+		    if ((o->size % address_size) != 0)
+		      {
+			(*_bfd_error_handler)
+			  (_("error: %B: size of section %A is not "
+			     "multiple of address size"),
+			   input_bfd, o);
+			bfd_set_error (bfd_error_on_input);
+			return FALSE;
+		      }
+
+		    do
+		      {
+			todo -= address_size;
+			if (! bfd_set_section_contents (output_bfd,
+							o->output_section,
+							contents + todo,
+							offset,
+							address_size))
+			  return FALSE;
+			if (todo == 0)
+			  break;
+			offset += address_size;
+		      }
+		    while (1);
+		  }
+		else if (! bfd_set_section_contents (output_bfd,
+						     o->output_section,
+						     contents,
+						     offset, todo))
+		  return FALSE;
+	      }
 	  }
 	  break;
 	}
diff --git a/include/bfdlink.h b/include/bfdlink.h
index 50a1423..3f230ad 100644
--- a/include/bfdlink.h
+++ b/include/bfdlink.h
@@ -354,6 +354,9 @@ struct bfd_link_info
      --dynamic-list command line options.  */
   unsigned int dynamic: 1;
 
+  /* TRUE if BFD should reverse-copy .ctors/.dtors sections.  */
+  unsigned int reverse_copy_ctors_dtors: 1;
+
   /* Non-NULL if .note.gnu.build-id section should be created.  */
   char *emit_note_gnu_build_id;
 
diff --git a/ld/emultempl/elf32.em b/ld/emultempl/elf32.em
index 17fb8bf..ce8c21e 100644
--- a/ld/emultempl/elf32.em
+++ b/ld/emultempl/elf32.em
@@ -63,6 +63,7 @@ fragment <<EOF
 
 /* Declare functions used by various EXTRA_EM_FILEs.  */
 static void gld${EMULATION_NAME}_before_parse (void);
+static void gld${EMULATION_NAME}_after_parse (void);
 static void gld${EMULATION_NAME}_after_open (void);
 static void gld${EMULATION_NAME}_before_allocation (void);
 static void gld${EMULATION_NAME}_after_allocation (void);
@@ -109,6 +110,19 @@ gld${EMULATION_NAME}_before_parse (void)
 EOF
 fi
 
+if test x"$LDEMUL_AFTER_PARSE" != xgld"$EMULATION_NAME"_after_parse; then
+fragment <<EOF
+
+static void
+gld${EMULATION_NAME}_after_parse (void)
+{
+  link_info.reverse_copy_ctors_dtors = `if test x"$ENABLE_INITFINI_ARRAY" = xyes ; then echo TRUE ; else echo FALSE ; fi`;
+  after_parse_default ();
+}
+
+EOF
+fi
+
 if test x"$LDEMUL_RECOGNIZED_FILE" != xgld"${EMULATION_NAME}"_load_symbols; then
 fragment <<EOF
 /* Handle the generation of DT_NEEDED tags.  */
@@ -2461,7 +2475,7 @@ struct ld_emulation_xfer_struct ld_${EMULATION_NAME}_emulation =
   ${LDEMUL_BEFORE_PARSE-gld${EMULATION_NAME}_before_parse},
   ${LDEMUL_SYSLIB-syslib_default},
   ${LDEMUL_HLL-hll_default},
-  ${LDEMUL_AFTER_PARSE-after_parse_default},
+  ${LDEMUL_AFTER_PARSE-gld${EMULATION_NAME}_after_parse},
   ${LDEMUL_AFTER_OPEN-gld${EMULATION_NAME}_after_open},
   ${LDEMUL_AFTER_ALLOCATION-gld${EMULATION_NAME}_after_allocation},
   ${LDEMUL_SET_OUTPUT_ARCH-set_output_arch_default},
diff --git a/ld/testsuite/ld-elf/elf.exp b/ld/testsuite/ld-elf/elf.exp
index 73a417c..6648a01 100644
--- a/ld/testsuite/ld-elf/elf.exp
+++ b/ld/testsuite/ld-elf/elf.exp
@@ -81,12 +81,14 @@ set array_tests {
     {"init array" "" "" {init.c} "init" "init.out"}
     {"fini array" "" "" {fini.c} "fini" "fini.out"}
     {"init array mixed" "" "" {init-mixed.c} "init-mixed" "init-mixed.out" "-I."}
+    {"pr12730" "" "" {pr12730.cc} "pr12730" "pr12730.out" "" "c++"}
 }
 set array_tests_static {
     {"static preinit array" "-static" "" {preinit.c} "preinit" "preinit.out"}
     {"static init array" "-static" "" {init.c} "init" "init.out"}
     {"static fini array" "-static" "" {fini.c} "fini" "fini.out"}
     {"static init array mixed" "" "" {init-mixed.c} "init-mixed" "init-mixed.out" "-I."}
+    {"static pr12730" "" "" {pr12730.cc} "pr12730" "pr12730.out" "" "c++"}
 }
 
 # NetBSD ELF systems do not currently support the .*_array sections.
diff --git a/ld/testsuite/ld-elf/init-mixed.c b/ld/testsuite/ld-elf/init-mixed.c
index 1d0c727..2c9d434 100644
--- a/ld/testsuite/ld-elf/init-mixed.c
+++ b/ld/testsuite/ld-elf/init-mixed.c
@@ -69,17 +69,31 @@ void (*const fini_array65530[]) ()
   = { fini65530 };
 
 static void
-ctor65535 ()
+ctor65535a ()
 {
   if (count != 65530)
     abort ();
   count = 65535;
 }
+static void
+ctor65535b ()
+{
+  if (count != 65535)
+    abort ();
+  count = 65536;
+}
 void (*const ctors65535[]) ()
   __attribute__ ((section (".ctors"), aligned (sizeof (void *))))
-  = { ctor65535 };
+  = { ctor65535b, ctor65535a };
+static void
+dtor65535b ()
+{
+  if (count != 65536)
+    abort ();
+  count = 65535;
+}
 static void
-dtor65535 ()
+dtor65535a ()
 {
   if (count != 65535)
     abort ();
@@ -87,7 +101,7 @@ dtor65535 ()
 }
 void (*const dtors65535[]) ()
   __attribute__ ((section (".dtors"), aligned (sizeof (void *))))
-  = { dtor65535 };
+  = { dtor65535b, dtor65535a };
 #endif
 
 int
diff --git a/ld/testsuite/ld-elf/pr12730.cc b/ld/testsuite/ld-elf/pr12730.cc
new file mode 100644
index 0000000..69f57f9
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr12730.cc
@@ -0,0 +1,38 @@
+#include <iostream>
+
+class Hello
+{
+public:
+   Hello ()
+    {}
+
+  ~Hello ()
+    {}
+
+  void act ()
+    { std::cout << "Hello, world!" << std::endl; }
+};
+
+
+template <class T>
+struct Foo
+{
+  T* _M_allocate_single_object ()
+    {
+      return new T;
+    }
+};
+
+static void __attribute__ (( constructor )) PWLIB_StaticLoader() {
+  Foo<Hello> allocator;
+  Hello* salut = allocator._M_allocate_single_object ();
+  salut->act ();
+}
+
+
+int
+main (int /*argc*/,
+      char* /*argv*/[])
+{
+  return 0;
+}
diff --git a/ld/testsuite/ld-elf/pr12730.out b/ld/testsuite/ld-elf/pr12730.out
new file mode 100644
index 0000000..af5626b
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr12730.out
@@ -0,0 +1 @@
+Hello, world!

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

* Re: PATCH: PR ld/12730: regression] crash when allocating in a static constructor
  2011-05-05  5:19 H.J. Lu
@ 2011-05-05  8:27 ` Alan Modra
  2011-05-05 13:27   ` H.J. Lu
  0 siblings, 1 reply; 16+ messages in thread
From: Alan Modra @ 2011-05-05  8:27 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

On Wed, May 04, 2011 at 10:18:32PM -0700, H.J. Lu wrote:
> When we put .ctors into .init_array, we have to reverse copy .ctors secton.
> Otherwise, constructor function may not work with C++ run-time library
> correctly.  OK for trunk?

What about .dtors?  You have the same problem there.  I suspect, but
haven't verified, that .ctors.* and .dtors.* also need reversing.  If
that is true then it would be better to do your reversing trick for
anything going to the .init_array output section that isn't named
.init_array* and similarly for .fini_array.

You also need to reverse any dynamic relocations applying to the
sections you are reversing.

-- 
Alan Modra
Australia Development Lab, IBM

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

* PATCH: PR ld/12730: regression] crash when allocating in a static constructor
@ 2011-05-05  5:19 H.J. Lu
  2011-05-05  8:27 ` Alan Modra
  0 siblings, 1 reply; 16+ messages in thread
From: H.J. Lu @ 2011-05-05  5:19 UTC (permalink / raw)
  To: binutils

Hi,

When we put .ctors into .init_array, we have to reverse copy .ctors secton.
Otherwise, constructor function may not work with C++ run-time library
correctly.  OK for trunk?

Thanks.


H.J.
---
bfd/

2011-05-04  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/12730
	* elflink.c (elf_link_input_bfd): Reverse copy .ctors section
	if needed.

include/

2011-05-04  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/12730
	* bfdlink.h (bfd_link_info): Add reverse_copy_ctors.

ld/

2011-05-04  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/12730
	* emultempl/elf32.em (gld${EMULATION_NAME}_after_parse): New.
	(ld_${EMULATION_NAME}_emulation): Replace after_parse_default
	with gld${EMULATION_NAME}_after_parse.

ld/testsuite/

2011-05-04  H.J. Lu  <hongjiu.lu@intel.com>

	PR ld/12730
	* ld-elf/elf.exp (array_tests): Add pr12730".
	(array_tests_static): Add "static pr12730".

	* ld-elf/pr12730.cc: New.
	* ld-elf/pr12730.out: Likewise.

diff --git a/bfd/elflink.c b/bfd/elflink.c
index 082355d..41aa16a 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -9876,12 +9876,46 @@ elf_link_input_bfd (struct elf_final_link_info *finfo, bfd *input_bfd)
 	default:
 	  {
 	    /* FIXME: octets_per_byte.  */
-	    if (! (o->flags & SEC_EXCLUDE)
-		&& ! bfd_set_section_contents (output_bfd, o->output_section,
-					       contents,
-					       (file_ptr) o->output_offset,
-					       o->size))
-	      return FALSE;
+	    if (! (o->flags & SEC_EXCLUDE))
+	      {
+		bfd_size_type address_size = bed->s->arch_size / 8;
+		file_ptr offset = (file_ptr) o->output_offset;
+		bfd_size_type todo = o->size;
+		if (finfo->info->reverse_copy_ctors
+		    && todo > address_size
+		    && strcmp (o->name, ".ctors") == 0)
+		  {
+		    if ((o->size % address_size) != 0)
+		      {
+			(*_bfd_error_handler)
+			  (_("error: %B: size of section %A is not "
+			     "multiple of address size"),
+			   input_bfd, o);
+			bfd_set_error (bfd_error_on_input);
+			return FALSE;
+		      }
+
+		    do
+		      {
+			todo -= address_size;
+			if (! bfd_set_section_contents (output_bfd,
+							o->output_section,
+							contents + todo,
+							offset,
+							address_size))
+			  return FALSE;
+			if (todo == 0)
+			  break;
+			offset += address_size;
+		      }
+		    while (1);
+		  }
+		else if (! bfd_set_section_contents (output_bfd,
+						     o->output_section,
+						     contents,
+						     offset, todo))
+		  return FALSE;
+	      }
 	  }
 	  break;
 	}
diff --git a/include/bfdlink.h b/include/bfdlink.h
index 50a1423..0139751 100644
--- a/include/bfdlink.h
+++ b/include/bfdlink.h
@@ -354,6 +354,9 @@ struct bfd_link_info
      --dynamic-list command line options.  */
   unsigned int dynamic: 1;
 
+  /* TRUE if BFD should reverse-copy .ctors section.  */
+  unsigned int reverse_copy_ctors: 1;
+
   /* Non-NULL if .note.gnu.build-id section should be created.  */
   char *emit_note_gnu_build_id;
 
diff --git a/ld/emultempl/elf32.em b/ld/emultempl/elf32.em
index 17fb8bf..562cda4 100644
--- a/ld/emultempl/elf32.em
+++ b/ld/emultempl/elf32.em
@@ -63,6 +63,7 @@ fragment <<EOF
 
 /* Declare functions used by various EXTRA_EM_FILEs.  */
 static void gld${EMULATION_NAME}_before_parse (void);
+static void gld${EMULATION_NAME}_after_parse (void);
 static void gld${EMULATION_NAME}_after_open (void);
 static void gld${EMULATION_NAME}_before_allocation (void);
 static void gld${EMULATION_NAME}_after_allocation (void);
@@ -109,6 +110,19 @@ gld${EMULATION_NAME}_before_parse (void)
 EOF
 fi
 
+if test x"$LDEMUL_AFTER_PARSE" != xgld"$EMULATION_NAME"_after_parse; then
+fragment <<EOF
+
+static void
+gld${EMULATION_NAME}_after_parse (void)
+{
+  link_info.reverse_copy_ctors = `if test x"$ENABLE_INITFINI_ARRAY" = xyes ; then echo TRUE ; else echo FALSE ; fi`;
+  after_parse_default ();
+}
+
+EOF
+fi
+
 if test x"$LDEMUL_RECOGNIZED_FILE" != xgld"${EMULATION_NAME}"_load_symbols; then
 fragment <<EOF
 /* Handle the generation of DT_NEEDED tags.  */
@@ -2461,7 +2475,7 @@ struct ld_emulation_xfer_struct ld_${EMULATION_NAME}_emulation =
   ${LDEMUL_BEFORE_PARSE-gld${EMULATION_NAME}_before_parse},
   ${LDEMUL_SYSLIB-syslib_default},
   ${LDEMUL_HLL-hll_default},
-  ${LDEMUL_AFTER_PARSE-after_parse_default},
+  ${LDEMUL_AFTER_PARSE-gld${EMULATION_NAME}_after_parse},
   ${LDEMUL_AFTER_OPEN-gld${EMULATION_NAME}_after_open},
   ${LDEMUL_AFTER_ALLOCATION-gld${EMULATION_NAME}_after_allocation},
   ${LDEMUL_SET_OUTPUT_ARCH-set_output_arch_default},
diff --git a/ld/testsuite/ld-elf/elf.exp b/ld/testsuite/ld-elf/elf.exp
index 73a417c..6648a01 100644
--- a/ld/testsuite/ld-elf/elf.exp
+++ b/ld/testsuite/ld-elf/elf.exp
@@ -81,12 +81,14 @@ set array_tests {
     {"init array" "" "" {init.c} "init" "init.out"}
     {"fini array" "" "" {fini.c} "fini" "fini.out"}
     {"init array mixed" "" "" {init-mixed.c} "init-mixed" "init-mixed.out" "-I."}
+    {"pr12730" "" "" {pr12730.cc} "pr12730" "pr12730.out" "" "c++"}
 }
 set array_tests_static {
     {"static preinit array" "-static" "" {preinit.c} "preinit" "preinit.out"}
     {"static init array" "-static" "" {init.c} "init" "init.out"}
     {"static fini array" "-static" "" {fini.c} "fini" "fini.out"}
     {"static init array mixed" "" "" {init-mixed.c} "init-mixed" "init-mixed.out" "-I."}
+    {"static pr12730" "" "" {pr12730.cc} "pr12730" "pr12730.out" "" "c++"}
 }
 
 # NetBSD ELF systems do not currently support the .*_array sections.
diff --git a/ld/testsuite/ld-elf/pr12730.cc b/ld/testsuite/ld-elf/pr12730.cc
new file mode 100644
index 0000000..69f57f9
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr12730.cc
@@ -0,0 +1,38 @@
+#include <iostream>
+
+class Hello
+{
+public:
+   Hello ()
+    {}
+
+  ~Hello ()
+    {}
+
+  void act ()
+    { std::cout << "Hello, world!" << std::endl; }
+};
+
+
+template <class T>
+struct Foo
+{
+  T* _M_allocate_single_object ()
+    {
+      return new T;
+    }
+};
+
+static void __attribute__ (( constructor )) PWLIB_StaticLoader() {
+  Foo<Hello> allocator;
+  Hello* salut = allocator._M_allocate_single_object ();
+  salut->act ();
+}
+
+
+int
+main (int /*argc*/,
+      char* /*argv*/[])
+{
+  return 0;
+}
diff --git a/ld/testsuite/ld-elf/pr12730.out b/ld/testsuite/ld-elf/pr12730.out
new file mode 100644
index 0000000..af5626b
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr12730.out
@@ -0,0 +1 @@
+Hello, world!

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

end of thread, other threads:[~2011-06-21 15:14 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-15 10:14 PATCH: PR ld/12730: regression] crash when allocating in a static constructor Craig Southeren
2011-05-16  0:15 ` Alan Modra
2011-05-16  0:37   ` Craig Southeren
  -- strict thread matches above, loose matches on Subject: below --
2011-05-05  5:19 H.J. Lu
2011-05-05  8:27 ` Alan Modra
2011-05-05 13:27   ` H.J. Lu
2011-05-05 14:13     ` Alan Modra
2011-05-06 13:23       ` H.J. Lu
2011-05-06 16:16         ` H.J. Lu
2011-05-07  8:11           ` Alan Modra
2011-05-07 13:40             ` H.J. Lu
2011-05-07 14:05               ` Alan Modra
2011-05-09 13:53                 ` Alan Modra
2011-05-09 14:09                   ` H.J. Lu
2011-06-19 19:18               ` Thomas Schwinge
2011-06-21 15:14                 ` Nick Clifton

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