Bug 10858 - ld generate broken PIE binaries on MIPS
Summary: ld generate broken PIE binaries on MIPS
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: 2.20
: P2 normal
Target Milestone: ---
Assignee: unassigned
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-10-28 13:45 UTC by Aurelien Jarno
Modified: 2014-05-28 19:43 UTC (History)
3 users (show)

See Also:
Host: mipsel-unknown-linux-gnu
Target: mipsel-unknown-linux-gnu
Build: mipsel-unknown-linux-gnu
Last reconfirmed:


Attachments
likely fix (459 bytes, patch)
2009-11-04 02:06 UTC, Alan Modra
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aurelien Jarno 2009-10-28 13:45:05 UTC
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.
Comment 1 Alan Modra 2009-10-29 03:47:18 UTC
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.
Comment 2 Aurelien Jarno 2009-10-29 08:33:56 UTC
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.
Comment 3 Aurelien Jarno 2009-11-01 10:44:41 UTC
The problem has been introduced by this (huge) changeset:

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

	* 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@googlemail.com>

	* 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@googlemail.com>

	* 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@googlemail.com>

	* 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@googlemail.com>

	* 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@googlemail.com>

	* 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@googlemail.com>

	* 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@googlemail.com>

	* elfxx-mips.c (allocate_dynrelocs): Ignore indirect and warning
	symbols.
Comment 4 Aurelien Jarno 2009-11-03 13:03:29 UTC
I have been able to bisect the bug further, here is the changeset that causes 
the problem:

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

       * 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.
Comment 5 Alan Modra 2009-11-04 02:06:50 UTC
Created attachment 4356 [details]
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.
Comment 6 Aurelien Jarno 2009-11-04 09:46:33 UTC
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.
Comment 7 Aurelien Jarno 2009-11-04 10:47:35 UTC
The alternative patch also fixes the problem, and also doesn't introduce any 
regressions in the testsuite.
Comment 8 Jay Foad 2010-02-21 18:23:00 UTC
I've recently hit this problem myself. Is there any chance of one of the fixes
being committed? Thanks.
Comment 9 Sourceware Commits 2010-02-24 14:22:04 UTC
Subject: Bug 10858

CVSROOT:	/cvs/src
Module name:	src
Changes by:	nickc@sourceware.org	2010-02-24 14:21:51

Modified files:
	bfd            : ChangeLog elfxx-mips.c 

Log message:
	PR binutils/10858
	* elfxx-mips.c (mips_elf_create_dynamic_relocation): Ise
	SYMBOL_REFERENCES_LOCAL to exclude entries from the dynamic symbol
	table.

Patches:
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/bfd/ChangeLog.diff?cvsroot=src&r1=1.4940&r2=1.4941
http://sources.redhat.com/cgi-bin/cvsweb.cgi/src/bfd/elfxx-mips.c.diff?cvsroot=src&r1=1.266&r2=1.267

Comment 10 Nick Clifton 2010-02-24 14:22:36 UTC
Hi Jay, Hi Aurelian,

  I have checked in the modified version of Alan's patch.

Cheers
  Nick
Comment 11 Jay Foad 2010-02-24 14:24:51 UTC
Thanks Nick!
Comment 12 Jackie Rosen 2014-02-16 17:46:49 UTC Comment hidden (spam)