[Bug ld/10858] New: ld generate broken PIE binaries on MIPS

Most PIE binaries generated on MIPS are broken. Here is a small testcase to
demonstrate the problem:

/* compile with cc -pie -fPIE -o fpie fpie.c */

#include <stdio.h>
#include <string.h>

int val0 = 3;
int val1 = 4;

int *vals[2] = {
    &val0,
    &val1
};

int main(void)
{
    printf("val0 is %i\n", *vals[0]);
    printf("val1 is %i\n", *vals[1]);

    return 0;/* compile with cc -pie -fPIE -o fpie fpie.c */

#include <stdio.h>
#include <string.h>

int val0 = 3;
int val1 = 4;

int *vals[2] = {
    &val0,
    &val1
};

int main(void)
{
    printf("val0 is %i\n", *vals[0]);
    printf("val1 is %i\n", *vals[1]);

    return 0;
}

Note that for an unknown reason, gcc 4.3 does not generate a pie binary
with -pie (not specific to MIPS), so you may want to try it with gcc 4.1.

Such a code crashes on startup during the relocation phase. This is due to a
wrong ELF header. Compared to a working binutils, val0 and val1 do not appear
in '.dynsym'. As a consequence '.rel.dyn' is filled with bad entries, causing
the crash:

Relocation section '.rel.dyn' at offset 0x594 contains 5 entries:
 Offset     Info    Type            Sym.Value  Sym. Name
00000000  00000000 R_MIPS_NONE
00010ab0  00000003 R_MIPS_REL32
00010ab4  00000003 R_MIPS_REL32
00010ad0  ffffff03 R_MIPS_REL32      bad symbol index: 00ffffff
00010ad4  ffffff03 R_MIPS_REL32      bad symbol index: 00ffffff

Manually editing '.rel.dyn' with an hex editor to point the entries to another
symbol cause the crash to disappear. Of course the value is then wrong.

--
           Summary: ld generate broken PIE binaries on MIPS
           Product: binutils
           Version: 2.20
            Status: NEW
          Severity: normal
          Priority: P2
         Component: ld
        AssignedTo: unassigned at sources dot redhat dot com
        ReportedBy: aurelien at aurel32 dot net
                CC: bug-binutils at gnu dot org
 GCC build triplet: mipsel-unknown-linux-gnu
  GCC host triplet: mipsel-unknown-linux-gnu
GCC target triplet: mipsel-unknown-linux-gnu


http://sourceware.org/bugzilla/show_bug.cgi?id=10858

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From amodra at bigpond dot net dot au  2009-10-29 03:47 -------
Please identify the version of your working binutils.  It would be really useful
to know which patch introduced the failure, or failing that, as narrow a date
range as you can manage easily.

--


http://sourceware.org/bugzilla/show_bug.cgi?id=10858

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From aurelien at aurel32 dot net  2009-10-29 08:33 -------
Unfortunately, bisecting takes time, especially on my slow machine. What I can
tell for now is that the CVS from 2008/07/03 works the CVS from 2008/08/14 does
not work.

About the gcc 4.3 issue not activating pie with the -pie flag, it only happens
if the output filename contains fpie.

--


http://sourceware.org/bugzilla/show_bug.cgi?id=10858

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From aurelien at aurel32 dot net  2009-11-01 10:44 -------
The problem has been introduced by this (huge) changeset:

2008-08-07  Richard Sandiford  <rdsandiford@...>

        * elf-bfd.h (elf_backend_data): Add a "rela_plts_and_copies_p" field.
        * elfxx-target.h (elf_backend_rela_plts_and_copies_p): New macro.
        (elfNN_bed): Use it.
        * elf.c (_bfd_elf_get_synthetic_symtab): Use rela_plts_and_copies_p
        instead of default_use_rela_p to choose between ".rel.plt" and
        ".rela.plt".
        * elflink.c (_bfd_elf_create_dynamic_sections): Use
        rela_plts_and_copies_p instead of default_use_rela_p to choose
        between ".rel.plt" and ".rela.plt", and between ".rel.bss" and
        ".rela.bss".

2008-08-07  Richard Sandiford  <rdsandiford@...>

        * elf-bfd.h (MIPS_ELF_TDATA): New elf_object_id.
        * elf32-mips.c (bfd_elf32_mkobject): Define.
        * elf64-mips.c (bfd_elf64_mkobject): Likewise.
        * elfn32-mips.c (bfd_elf32_mkobject): Likewise.
        * elfxx-mips.h (_bfd_mips_elf_mkobject): Declare.
        * elfxx-mips.c (is_mips_elf): New macro.
        (_bfd_mips_elf_mkobject): New function.
        (_bfd_mips_elf_final_link): Use is_mips_elf.
        (_bfd_mips_elf_merge_private_bfd_data): Likewise.

2008-08-07  Richard Sandiford  <rdsandiford@...>

        * elfxx-mips.c (mips_elf_record_relocs): Defer allocation of a
        global GOT entry when deferring allocation of dynamic relocations.
        (allocate_dynrelocs): When allocating deferred dynamic relocations,
        also do the deferred allocation of a GOT entry.

2008-08-07  Richard Sandiford  <rdsandiford@...>

        * elfxx-mips.c (mips_got_info): Add a "reloc_only_gotno" field.
        (mips_elf_got_section): Delete.
        (mips_elf_sort_hash_table): Use g->reloc_only_gotno to decide
        how many reloc-only entries there are.
        (mips_elf_count_got_symbols): Adjust g->reloc_only_gotno as
        well as g->global_gotno.
        (mips_elf_make_got_per_bfd): Initialize reloc_only_gotno.
        (mips_elf_multi_got): Likewise.  Use gg->reloc_only_gotno
        rather than gg->assigned_gotno to store the number of
        reloc-only GOT entries.
        (mips_elf_create_got_section): Remove the MAYBE_EXCLUDE parameter.
        Initialize reloc_only_gotno.
        (mips_elf_calculate_relocation): Check htab->got_info instead of
        dynobj when deciding whether to call mips_elf_adjust_gp,
        (_bfd_mips_elf_create_dynamic_sections): Adjust the call
        to mips_elf_create_got_section.
        (mips_elf_record_relocs): Likewise.  Remove redundant
        "dynobj == NULL" code.  Do not use mips_elf_create_got_section
        or mips_elf_record_global_got_symbol for R_MIPS_32, R_MIPS_REL32
        and R_MIPS_64; limit global_got_area to GGA_RELOC_ONLY instead.
        (_bfd_mips_elf_finish_dynamic_symbol): Use htab->sgot instead
        of mips_elf_got_section.
        (_bfd_mips_vxworks_finish_dynamic_symbol): Likewise.
        (_bfd_mips_elf_finish_dynamic_sections): Likewise.
        Move the initial assignment of G to the block that uses it;
        it is used for an unrelated purpose later.

2008-08-07  Richard Sandiford  <rdsandiford@...>

        * elfxx-mips.c (count_section_dynsyms): Move before the new first use.
        (mips_elf_sort_hash_table): Take the output bfd as a parameter.
        Remove the MAX_LOCAL parameter.  Exit early if there are no
        dynamic symbols, if there is no dynobj, or if there is no
        GOT section.  Use count_section_dynsyms instead of MAX_LOCAL.
        Assert == rather than <= when checking hsd.max_unref_got_dynindx.
        Also assert that g->global_gotno is right.
        (mips_elf_count_forced_local_got_symbols): Rename to...
        (mips_elf_count_got_symbols): ...and count global GOT entries too.
        Set the global_got_area of a forced-local GGA_RELOC_ONLY symbol
        to GGA_NONE.
        (mips_elf_multi_got): Don't sort the symbol table.
        (mips_elf_lay_out_got): Likewise.  Use mips_elf_count_got_symbols
        to count the number of global GOT entries.
        (_bfd_mips_elf_final_link): Unconditionally call
        mips_elf_sort_hash_table.

2008-08-07  Richard Sandiford  <rdsandiford@...>

        * elfxx-mips.c (GGA_NORMAL, GGA_RELOC_ONLY, GGA_NONE): New macros.
        (mips_elf_link_hash_entry): Add a "global_got_area" field.
        (mips_elf_link_hash_newfunc): Initialize it.
        (mips_elf_sort_hash_table_f): Use h->global_got_area instead of
        h->root.got.offset.  Do not handle forced_local symbols specially.
        (mips_elf_record_global_got_symbol): Set h->global_got_area
        instead of h->root.got.offset.
        (mips_elf_recreate_got): Assert that h->global_got_area == GGA_NONE
        for indirect and warning symbols.
        (mips_elf_count_forced_local_got_symbols): Change the argument
        from a "elf_link_hash_entry" to "mips_elf_link_hash_entry".
        Use and set h->global_got_area instead of h->root.got.offset.
        Set it to GGA_NONE for all forced-local symbols.
        (mips_elf_set_global_got_offset): Set h->global_got_area
        instead of h->root.got.offset.  Use g->global_got_area instead
        of a combination of dynindx, forced_local and tls_type.
        (mips_elf_multi_got): Remove disabled code.  Pass GGA_* values to
        mips_elf_set_global_got_offset.
        (mips_elf_lay_out_got): Use mips_elf_link_hash_traverse instead
        of elf_link_hash_traverse.
        (_bfd_mips_elf_copy_indirect_symbol): Copy the indirect symbol's
        global_got_area to the direct symbol if the latter's value is higher.
        Set the indirect symbol's area to GGA_NONE.

2008-08-07  Richard Sandiford  <rdsandiford@...>

        * elf32-mips.c (elf_backend_hide_symbol): Delete.
        * elfn32-mips.c (elf_backend_hide_symbol): Likewise.
        * elf64-mips.c (elf_backend_hide_symbol): Likewise.
        * elfxx-mips.h (elf_backend_hide_symbol): Likewise.
        * elfxx-mips.c (mips_elf_link_hash_entry): Remove "forced_local"
        and add "needs_lazy_stub".
        (mips_elf_link_hash_newfunc): Update accordingly.
        (mips_elf_link_hash_table): Remove "computed_got_sizes" and
        add "lazy_stub_count".
        (_bfd_mips_elf_link_hash_table_create): Update accordingly.
        (mips_elf_output_extsym): Use hd->needs_lazy_stub to detect
        cases where a lazy stub is being used.
        (mips_elf_sort_hash_table_f): Use h->root.forced_local instead
        of h->forced_local.
        (mips_elf_record_global_got_symbol): Use _bfd_elf_link_hash_hide_symbol
        instead of _bfd_mips_elf_hide_symbol.  Do not increment local_gotno
        here.
        (mips_elf_allocate_dynamic_relocations): Move before new first use.
        (mips_elf_check_recreate_got, mips_elf_recreate_got): New functions.
        (mips_elf_resolve_final_got_entries): Move earlier in file.  Make at
        most two passes over the hash table.  Use mips_elf_check_recreate_got
        to see if there are any indirect or warning entries and
        mips_elf_recreate_got to create a new GOT without them.
        Return a boolean success value.
        (mips_elf_count_forced_local_got_entries): New function.
        (mips_elf_make_got_per_bfd): Check h->root.forced_local instead of
        h->forced_local.
        (mips_elf_set_global_got_offset): Likewise.
        (mips_elf_set_no_stub): Replace with...
        (mips_elf_forbid_lazy_stubs): ...this new function.
        (mips_elf_resolve_final_got_entry): Delete.
        (mips_elf_multi_got): Fix formatting.  Use mips_elf_forbid_lazy_stubs
        instead of mips_elf_set_no_stub.  Move the code that sets
        global offsets and allocates dynamic relocations from the main
        _bfd_mips_elf_size_dynamic_sections loop to here.
        (_bfd_mips_elf_adjust_dynamic_symbol): Do not allocate room in
        .MIPS.stubs here; just set hmips->needs_lazy_stub and increment
        htab->lazy_stub_count.
        (_bfd_mips_elf_always_size_sections): Move the stub-estimation
        code to mips_elf_estimate_stub_size and the GOT-sizing code to
        mips_elf_lay_out_got.  Do not call these functions here.
        (mips_elf_estimate_stub_size): New function, split
        out from _bfd_mips_elf_always_size_sections.  Call
        mips_elf_resolve_final_got_entries earlier.  Count the number
        of forced-local entries.  Do not add stub sizes to loadable_size;
        after this patch, the stub sizes are already included in the main
        estimate.  Allocate dynamic relocations here rather than in the
        main _bfd_mips_elf_size_dynamic_sections loop.
        (mips_elf_estimate_stub_size): New function, split out from
        _bfd_mips_elf_always_size_sections.
        (mips_elf_allocate_lazy_stub): New function.
        (mips_elf_lay_out_lazy_stubs): Likewise.
        (_bfd_mips_elf_size_dynamic_sections): Call
mips_elf_estimate_stub_size,
        mips_elf_lay_out_got and mips_elf_lay_out_lazy_stubs.  Do not handle
        the allocation of sreldyn specially.
        (_bfd_mips_elf_hide_symbol): Delete.

2008-08-07  Richard Sandiford  <rdsandiford@...>

        * elfxx-mips.c (allocate_dynrelocs): Ignore indirect and warning
        symbols.


--


http://sourceware.org/bugzilla/show_bug.cgi?id=10858

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From aurelien at aurel32 dot net  2009-11-03 13:03 -------
I have been able to bisect the bug further, here is the changeset that causes
the problem:

2008-08-07  Richard Sandiford  <rdsandiford@...>

       * elfxx-mips.c (mips_got_info): Add a "reloc_only_gotno" field.
       (mips_elf_got_section): Delete.
       (mips_elf_sort_hash_table): Use g->reloc_only_gotno to decide
       how many reloc-only entries there are.
       (mips_elf_count_got_symbols): Adjust g->reloc_only_gotno as
       well as g->global_gotno.
       (mips_elf_make_got_per_bfd): Initialize reloc_only_gotno.
       (mips_elf_multi_got): Likewise.  Use gg->reloc_only_gotno
       rather than gg->assigned_gotno to store the number of
       reloc-only GOT entries.
       (mips_elf_create_got_section): Remove the MAYBE_EXCLUDE parameter.
       Initialize reloc_only_gotno.
       (mips_elf_calculate_relocation): Check htab->got_info instead of
       dynobj when deciding whether to call mips_elf_adjust_gp,
       (_bfd_mips_elf_create_dynamic_sections): Adjust the call
       to mips_elf_create_got_section.
       (mips_elf_record_relocs): Likewise.  Remove redundant
       "dynobj == NULL" code.  Do not use mips_elf_create_got_section
       or mips_elf_record_global_got_symbol for R_MIPS_32, R_MIPS_REL32
       and R_MIPS_64; limit global_got_area to GGA_RELOC_ONLY instead.
       (_bfd_mips_elf_finish_dynamic_symbol): Use htab->sgot instead
       of mips_elf_got_section.
       (_bfd_mips_vxworks_finish_dynamic_symbol): Likewise.
       (_bfd_mips_elf_finish_dynamic_sections): Likewise.
       Move the initial assignment of G to the block that uses it;
       it is used for an unrelated purpose later.


--


http://sourceware.org/bugzilla/show_bug.cgi?id=10858

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From amodra at bigpond dot net dot au  2009-11-04 02:06 -------
Created an attachment (id=4356)
 --> (http://sourceware.org/bugzilla/attachment.cgi?id=4356&action=view)
likely fix

I believe this patch will fix your problem, but I don't have mips machines and
toolchains to properly test.  Hmm, an alternate (and better!) fix would be to
use " if (!SYMBOL_REFERENCES_LOCAL (info, &h->root))" here.

--


http://sourceware.org/bugzilla/show_bug.cgi?id=10858

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From aurelien at aurel32 dot net  2009-11-04 09:46 -------
Thanks a lot for the patch, I confirm it works for both the versions just after
the problem has been introduced and trunk. I also checked it introduces no
regressions on the trunk version.

I haven't tested your alternative patch yet, I will do that soon.

--


http://sourceware.org/bugzilla/show_bug.cgi?id=10858

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils

------- Additional Comments From aurelien at aurel32 dot net  2009-11-04 10:47 -------
The alternative patch also fixes the problem, and also doesn't introduce any
regressions in the testsuite.

--


http://sourceware.org/bugzilla/show_bug.cgi?id=10858

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.


_______________________________________________
bug-binutils mailing list
bug-binutils@...
http://lists.gnu.org/mailman/listinfo/bug-binutils