From: "H.J. Lu" <hjl.tools@gmail.com>
To: Binutils <binutils@sourceware.org>
Subject: Re: PATCH: PR ld/12730: regression] crash when allocating in a static constructor
Date: Fri, 06 May 2011 16:16:00 -0000 [thread overview]
Message-ID: <BANLkTinaLJRc1Q5O=EnGH0eqRWMYBB5x6g@mail.gmail.com> (raw)
In-Reply-To: <BANLkTi=nhJSWTC2z+gxxOTBDODm_wM78gQ@mail.gmail.com>
[-- 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!
next prev parent reply other threads:[~2011-05-06 16:16 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='BANLkTinaLJRc1Q5O=EnGH0eqRWMYBB5x6g@mail.gmail.com' \
--to=hjl.tools@gmail.com \
--cc=binutils@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).