public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* 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
* 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
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-05  5:19 PATCH: PR ld/12730: regression] crash when allocating in a static constructor 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
2011-05-15 10:14 Craig Southeren
2011-05-16  0:15 ` Alan Modra
2011-05-16  0:37   ` Craig Southeren

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