* [PATCH 0/5] Fixes / improvements to ARC BFD target support. @ 2016-08-16 15:51 Cupertino Miranda 2016-08-16 15:51 ` [PATCH 1/5] Fixes to legacy relocations Cupertino Miranda ` (5 more replies) 0 siblings, 6 replies; 19+ messages in thread From: Cupertino Miranda @ 2016-08-16 15:51 UTC (permalink / raw) To: binutils; +Cc: Francois.Bedard, Claudiu.Zissulescu Hello everyone, Here are a new set of patches for ARC bfd improvements. The individual patch emails explain the respective improvement/fix. Looking forward for your review and comments. Best regards, Cupertino ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/5] Fixes to legacy relocations. 2016-08-16 15:51 [PATCH 0/5] Fixes / improvements to ARC BFD target support Cupertino Miranda @ 2016-08-16 15:51 ` Cupertino Miranda 2016-08-19 7:41 ` Nick Clifton 2016-08-16 15:51 ` [PATCH 2/5] Content for TLS_IE_GOT not written to .got Cupertino Miranda ` (4 subsequent siblings) 5 siblings, 1 reply; 19+ messages in thread From: Cupertino Miranda @ 2016-08-16 15:51 UTC (permalink / raw) To: binutils; +Cc: Francois.Bedard, Claudiu.Zissulescu, Cupertino Miranda Added support for ARC_SDA_12 reloc. Fixed ARC_N32_ME. Added ME (middle-endian) to ARC_SDA_12 reloc. bfd/ChangeLog: Cupertino Miranda <cmiranda@synopsys.com> reloc.c: Fixed type in ARC_SECTOFF relocations. Added ARC_SDA_12 relocation. bfd-in2.h: Regenerated from the previous changes. libbfd.h: Regenerated from the previous changes. include/ChangeLog: Cupertino Miranda <cmiranda@synopsys.com> elf/arc-reloc.def: Fixed relocation formula for N*, SDA, SDA_12, SDA_16_LD*, S13_PCREL, N32_ME, SECTOFF_* relocations. opcode/arc-func.h (replace_disp12s): Added. Used for SDA_12 relocation. --- bfd/bfd-in2.h | 7 ++++--- bfd/libbfd.h | 7 ++++--- bfd/reloc.c | 8 +++++--- include/elf/arc-reloc.def | 45 ++++++++++++++++++++++++++------------------- include/opcode/arc-func.h | 15 +++++++++++++++ 5 files changed, 54 insertions(+), 28 deletions(-) diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h index ca2e5ee..c7995f0 100644 --- a/bfd/bfd-in2.h +++ b/bfd/bfd-in2.h @@ -3713,13 +3713,14 @@ pc-relative or some form of GOT-indirect relocation. */ BFD_RELOC_AC_SECTOFF_U8, BFD_RELOC_AC_SECTOFF_U8_1, BFD_RELOC_AC_SECTOFF_U8_2, - BFD_RELOC_AC_SECTFOFF_S9, - BFD_RELOC_AC_SECTFOFF_S9_1, - BFD_RELOC_AC_SECTFOFF_S9_2, + BFD_RELOC_AC_SECTOFF_S9, + BFD_RELOC_AC_SECTOFF_S9_1, + BFD_RELOC_AC_SECTOFF_S9_2, BFD_RELOC_ARC_SECTOFF_ME_1, BFD_RELOC_ARC_SECTOFF_ME_2, BFD_RELOC_ARC_SECTOFF_1, BFD_RELOC_ARC_SECTOFF_2, + BFD_RELOC_ARC_SDA_12, BFD_RELOC_ARC_SDA16_ST2, BFD_RELOC_ARC_32_PCREL, BFD_RELOC_ARC_PC32, diff --git a/bfd/libbfd.h b/bfd/libbfd.h index 9d751ee..1eb988f 100644 --- a/bfd/libbfd.h +++ b/bfd/libbfd.h @@ -1722,13 +1722,14 @@ static const char *const bfd_reloc_code_real_names[] = { "@@uninitialized@@", "BFD_RELOC_AC_SECTOFF_U8", "BFD_RELOC_AC_SECTOFF_U8_1", "BFD_RELOC_AC_SECTOFF_U8_2", - "BFD_RELOC_AC_SECTFOFF_S9", - "BFD_RELOC_AC_SECTFOFF_S9_1", - "BFD_RELOC_AC_SECTFOFF_S9_2", + "BFD_RELOC_AC_SECTOFF_S9", + "BFD_RELOC_AC_SECTOFF_S9_1", + "BFD_RELOC_AC_SECTOFF_S9_2", "BFD_RELOC_ARC_SECTOFF_ME_1", "BFD_RELOC_ARC_SECTOFF_ME_2", "BFD_RELOC_ARC_SECTOFF_1", "BFD_RELOC_ARC_SECTOFF_2", + "BFD_RELOC_ARC_SDA_12", "BFD_RELOC_ARC_SDA16_ST2", "BFD_RELOC_ARC_32_PCREL", "BFD_RELOC_ARC_PC32", diff --git a/bfd/reloc.c b/bfd/reloc.c index 0e5fde2..a2bfe2b 100644 --- a/bfd/reloc.c +++ b/bfd/reloc.c @@ -3613,11 +3613,11 @@ ENUMX ENUMX BFD_RELOC_AC_SECTOFF_U8_2 ENUMX - BFD_RELOC_AC_SECTFOFF_S9 + BFD_RELOC_AC_SECTOFF_S9 ENUMX - BFD_RELOC_AC_SECTFOFF_S9_1 + BFD_RELOC_AC_SECTOFF_S9_1 ENUMX - BFD_RELOC_AC_SECTFOFF_S9_2 + BFD_RELOC_AC_SECTOFF_S9_2 ENUMX BFD_RELOC_ARC_SECTOFF_ME_1 ENUMX @@ -3627,6 +3627,8 @@ ENUMX ENUMX BFD_RELOC_ARC_SECTOFF_2 ENUMX + BFD_RELOC_ARC_SDA_12 +ENUMX BFD_RELOC_ARC_SDA16_ST2 ENUMX BFD_RELOC_ARC_32_PCREL diff --git a/include/elf/arc-reloc.def b/include/elf/arc-reloc.def index 17c2429..2d11b3f 100644 --- a/include/elf/arc-reloc.def +++ b/include/elf/arc-reloc.def @@ -69,35 +69,35 @@ ARC_RELOC_HOWTO(ARC_N8, 8, \ 8, \ replace_bits8, \ bitfield, \ - ( S - A )) + ( A - S )) ARC_RELOC_HOWTO(ARC_N16, 9, \ 1, \ 16, \ replace_bits16, \ bitfield, \ - ( S - A )) + ( A - S )) ARC_RELOC_HOWTO(ARC_N24, 10, \ 2, \ 24, \ replace_bits24, \ bitfield, \ - ( S - A )) + ( A - S )) ARC_RELOC_HOWTO(ARC_N32, 11, \ 2, \ 32, \ replace_word32, \ bitfield, \ - ( S - A )) + ( A - S )) ARC_RELOC_HOWTO(ARC_SDA, 12, \ 2, \ 9, \ replace_disp9, \ bitfield, \ - ( S + A )) + ( ME ( ( ( S + A ) - _SDA_BASE_ ) ) )) ARC_RELOC_HOWTO(ARC_SECTOFF, 13, \ 2, \ @@ -167,28 +167,28 @@ ARC_RELOC_HOWTO(ARC_SDA16_LD, 22, \ 9, \ replace_disp9s, \ signed, \ - ( ( ( S + A ) - _SDA_BASE_ ) )) + ( ( S + A ) - _SDA_BASE_ )) ARC_RELOC_HOWTO(ARC_SDA16_LD1, 23, \ 1, \ 9, \ replace_disp9s, \ signed, \ - ( ( ( ( S + A ) - _SDA_BASE_ ) >> 1 ) )) + ( ( ( S + A ) - _SDA_BASE_ ) >> 1 )) ARC_RELOC_HOWTO(ARC_SDA16_LD2, 24, \ 1, \ 9, \ replace_disp9s, \ signed, \ - ( ( ( ( S + A ) - _SDA_BASE_ ) >> 2 ) )) + ( ( ( S + A ) - _SDA_BASE_ ) >> 2 )) ARC_RELOC_HOWTO(ARC_S13_PCREL, 25, \ 1, \ 11, \ replace_disp13s, \ signed, \ - ( ( ( ( S + A ) - P ) >> 2 ) )) + ( ( ( S + A ) - P ) >> 2 )) ARC_RELOC_HOWTO(ARC_W, 26, \ 2, \ @@ -216,7 +216,7 @@ ARC_RELOC_HOWTO(ARC_N32_ME, 28, \ 32, \ replace_word32, \ bitfield, \ - ( ME ( ( S - A ) ) )) + ( ME ( ( A - S ) ) )) ARC_RELOC_HOWTO(ARC_SECTOFF_ME, 29, \ 2, \ @@ -244,42 +244,42 @@ ARC_RELOC_HOWTO(AC_SECTOFF_U8, 35, \ 9, \ replace_disp9ls, \ bitfield, \ - ( ( S + A ) - SECTSTART )) + ( ME ( ( ( S + A ) - SECTSTART ) ) )) ARC_RELOC_HOWTO(AC_SECTOFF_U8_1, 36, \ 2, \ 9, \ replace_disp9ls, \ bitfield, \ - ( ( ( S + A ) - SECTSTART ) >> 1 )) + ( ME ( ( ( ( S + A ) - SECTSTART ) >> 1 ) ) )) ARC_RELOC_HOWTO(AC_SECTOFF_U8_2, 37, \ 2, \ 9, \ replace_disp9ls, \ bitfield, \ - ( ( ( S + A ) - SECTSTART ) >> 2 )) + ( ME ( ( ( ( S + A ) - SECTSTART ) >> 2 ) ) )) -ARC_RELOC_HOWTO(AC_SECTFOFF_S9, 38, \ +ARC_RELOC_HOWTO(AC_SECTOFF_S9, 38, \ 2, \ 9, \ replace_disp9ls, \ bitfield, \ - ( ( S + A ) - SECTSTART )) + ( ME ( ( ( ( S + A ) - SECTSTART ) - 256 ) ) )) -ARC_RELOC_HOWTO(AC_SECTFOFF_S9_1, 39, \ +ARC_RELOC_HOWTO(AC_SECTOFF_S9_1, 39, \ 2, \ 9, \ replace_disp9ls, \ bitfield, \ - ( ( ( S + A ) - SECTSTART ) >> 1 )) + ( ME ( ( ( ( ( S + A ) - SECTSTART ) - 256 ) >> 1 ) ) )) -ARC_RELOC_HOWTO(AC_SECTFOFF_S9_2, 40, \ +ARC_RELOC_HOWTO(AC_SECTOFF_S9_2, 40, \ 2, \ 9, \ replace_disp9ls, \ bitfield, \ - ( ( ( S + A ) - SECTSTART ) >> 2 )) + ( ME ( ( ( ( ( S + A ) - SECTSTART ) - 256 ) >> 2 ) ) )) ARC_RELOC_HOWTO(ARC_SECTOFF_ME_1, 41, \ 2, \ @@ -309,6 +309,13 @@ ARC_RELOC_HOWTO(ARC_SECTOFF_2, 44, \ bitfield, \ ( ( ( S - SECTSTART ) + A ) >> 2 )) +ARC_RELOC_HOWTO(ARC_SDA_12, 45, \ + 2, \ + 12, \ + replace_disp12s, \ + signed, \ + ( ME ( ( ( S + A ) - _SDA_BASE_ ) ) )) + ARC_RELOC_HOWTO(ARC_SDA16_ST2, 48, \ 1, \ 9, \ diff --git a/include/opcode/arc-func.h b/include/opcode/arc-func.h index cafb92f..c92382b 100644 --- a/include/opcode/arc-func.h +++ b/include/opcode/arc-func.h @@ -264,3 +264,18 @@ replace_disp9s1 (unsigned insn, int value ATTRIBUTE_UNUSED) } #endif /* REPLACE_disp9s1 */ + +/* mask = 00000000000000000000111111222222. */ +#ifndef REPLACE_disp12s +#define REPLACE_disp12s +ATTRIBUTE_UNUSED static unsigned +replace_disp12s (unsigned insn, int value ATTRIBUTE_UNUSED) +{ + insn = insn & ~0xfff; + insn |= ((value >> 0) & 0x003f) << 6; + insn |= ((value >> 6) & 0x003f) << 0; + + return insn; +} + +#endif /* REPLACE_disp12s */ -- 2.9.0 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/5] Fixes to legacy relocations. 2016-08-16 15:51 ` [PATCH 1/5] Fixes to legacy relocations Cupertino Miranda @ 2016-08-19 7:41 ` Nick Clifton 0 siblings, 0 replies; 19+ messages in thread From: Nick Clifton @ 2016-08-19 7:41 UTC (permalink / raw) To: Cupertino Miranda, binutils; +Cc: Francois.Bedard, Claudiu.Zissulescu Hi Cupertino, > bfd/ChangeLog: > > Cupertino Miranda <cmiranda@synopsys.com> > reloc.c: Fixed type in ARC_SECTOFF relocations. Added ARC_SDA_12 > relocation. > bfd-in2.h: Regenerated from the previous changes. > libbfd.h: Regenerated from the previous changes. > > include/ChangeLog: > > Cupertino Miranda <cmiranda@synopsys.com> > elf/arc-reloc.def: Fixed relocation formula for N*, SDA, SDA_12, > SDA_16_LD*, S13_PCREL, N32_ME, SECTOFF_* relocations. > opcode/arc-func.h (replace_disp12s): Added. Used for SDA_12 relocation. Approved - please apply. Cheers Nick ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 2/5] Content for TLS_IE_GOT not written to .got. 2016-08-16 15:51 [PATCH 0/5] Fixes / improvements to ARC BFD target support Cupertino Miranda 2016-08-16 15:51 ` [PATCH 1/5] Fixes to legacy relocations Cupertino Miranda @ 2016-08-16 15:51 ` Cupertino Miranda 2016-08-19 7:43 ` Nick Clifton 2016-08-16 15:51 ` [PATCH 4/5] Fixed -init, -fini linker options Cupertino Miranda ` (3 subsequent siblings) 5 siblings, 1 reply; 19+ messages in thread From: Cupertino Miranda @ 2016-08-16 15:51 UTC (permalink / raw) To: binutils; +Cc: Francois.Bedard, Claudiu.Zissulescu, Cupertino Miranda When no dynamic relocation was generated the .got content would not be updated for the TLS_IE_GOT relocation addresses. bfd/ChangeLog: Cupertino Miranda <cmiranda@synopsys.com> arc-got.h (relocate_fix_got_relocs_for_got_info): Fixed addresses in debug comments. Fixed address in .got related to TLS_IE_GOT dynamic relocation. --- bfd/arc-got.h | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/bfd/arc-got.h b/bfd/arc-got.h index 7c3cfd9..9ac0295 100644 --- a/bfd/arc-got.h +++ b/bfd/arc-got.h @@ -334,13 +334,15 @@ relocate_fix_got_relocs_for_got_info (struct got_entry ** list_p, ? 4 : 0)); ARC_DEBUG ("arc_info: FIXED -> %s value = %#lx " - "@ %p, for symbol %s\n", + "@ %lx, for symbol %s\n", (entry->type == GOT_TLS_GD ? "GOT_TLS_GD" : "GOT_TLS_IE"), (long) (sym_value - sec_vma), - htab->sgot->contents + entry->offset - + (entry->existing_entries == TLS_GOT_MOD_AND_OFF - ? 4 : 0), + (long) (htab->sgot->output_section->vma + + htab->sgot->output_offset->vma + + entry->offset + + (entry->existing_entries == TLS_GOT_MOD_AND_OFF + ? 4 : 0)), symbol_name); } break; @@ -351,14 +353,22 @@ relocate_fix_got_relocs_for_got_info (struct got_entry ** list_p, bfd_vma ATTRIBUTE_UNUSED sec_vma = tls_sec->output_section->vma; + bfd_put_32 (output_bfd, + sym_value - sec_vma, + htab->sgot->contents + entry->offset + + (entry->existing_entries == TLS_GOT_MOD_AND_OFF + ? 4 : 0)); + ARC_DEBUG ("arc_info: FIXED -> %s value = %#lx " "@ %p, for symbol %s\n", (entry->type == GOT_TLS_GD ? "GOT_TLS_GD" : "GOT_TLS_IE"), (long) (sym_value - sec_vma), - htab->sgot->contents + entry->offset - + (entry->existing_entries == TLS_GOT_MOD_AND_OFF - ? 4 : 0), + (long) (htab->sgot->output_section->vma + + htab->sgot->output_offset->vma + + entry->offset + + (entry->existing_entries == TLS_GOT_MOD_AND_OFF + ? 4 : 0)), symbol_name); } break; -- 2.9.0 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/5] Content for TLS_IE_GOT not written to .got. 2016-08-16 15:51 ` [PATCH 2/5] Content for TLS_IE_GOT not written to .got Cupertino Miranda @ 2016-08-19 7:43 ` Nick Clifton 2016-08-24 14:55 ` Cupertino Miranda 0 siblings, 1 reply; 19+ messages in thread From: Nick Clifton @ 2016-08-19 7:43 UTC (permalink / raw) To: Cupertino Miranda, binutils; +Cc: Francois.Bedard, Claudiu.Zissulescu Hi Cupertino, > When no dynamic relocation was generated the .got content would not be > updated for the TLS_IE_GOT relocation addresses. I think that this particular patch ought to be accompanied by a testcase to make sure that it continues to work. Could you see if you rustle one up please ? Cheers Nick ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/5] Content for TLS_IE_GOT not written to .got. 2016-08-19 7:43 ` Nick Clifton @ 2016-08-24 14:55 ` Cupertino Miranda 2016-08-25 10:59 ` Nick Clifton 0 siblings, 1 reply; 19+ messages in thread From: Cupertino Miranda @ 2016-08-24 14:55 UTC (permalink / raw) To: Nick Clifton, Cupertino Miranda, binutils Cc: Francois.Bedard, Claudiu.Zissulescu [-- Attachment #1: Type: text/plain, Size: 500 bytes --] Hi Nick, Indeed, a test case makes sense. Please find a new patch attached. Cheers, Cupertino On 08/19/2016 09:43 AM, Nick Clifton wrote: > Hi Cupertino, > >> When no dynamic relocation was generated the .got content would not be >> updated for the TLS_IE_GOT relocation addresses. > I think that this particular patch ought to be accompanied by a testcase > to make sure that it continues to work. Could you see if you rustle one > up please ? > > > Cheers > Nick > [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0002-Content-for-TLS_IE_GOT-not-written-to-.got.patch --] [-- Type: text/x-patch; name="0002-Content-for-TLS_IE_GOT-not-written-to-.got.patch", Size: 3498 bytes --] From 7af0a7dffd4d396214b1ffa28a5923dafd7e4154 Mon Sep 17 00:00:00 2001 From: Cupertino Miranda <cmiranda@synopsys.com> Date: Tue, 12 Jul 2016 16:31:40 +0200 Subject: [PATCH 2/5] Content for TLS_IE_GOT not written to .got. When no dynamic relocation was generated the .got content would not be updated for the TLS_IE_GOT relocation addresses. bfd/ChangeLog: Cupertino Miranda <cmiranda@synopsys.com> arc-got.h (relocate_fix_got_relocs_for_got_info): Fixed addresses in debug comments. Fixed address in .got related to TLS_IE_GOT dynamic relocation. ld/ChangeLog: Cupertino Miranda <cmiranda@synopsys.com> testsuite/ld-arc/tls_ie-01.s: Added to verify associated fix. testsuite/ld-arc/tls_ie-01.d: Likewise --- bfd/arc-got.h | 24 +++++++++++++++++------- ld/testsuite/ld-arc/tls_ie-01.d | 9 +++++++++ ld/testsuite/ld-arc/tls_ie-01.s | 10 ++++++++++ 3 files changed, 36 insertions(+), 7 deletions(-) create mode 100644 ld/testsuite/ld-arc/tls_ie-01.d create mode 100644 ld/testsuite/ld-arc/tls_ie-01.s diff --git a/bfd/arc-got.h b/bfd/arc-got.h index 7c3cfd9..9ac0295 100644 --- a/bfd/arc-got.h +++ b/bfd/arc-got.h @@ -334,13 +334,15 @@ relocate_fix_got_relocs_for_got_info (struct got_entry ** list_p, ? 4 : 0)); ARC_DEBUG ("arc_info: FIXED -> %s value = %#lx " - "@ %p, for symbol %s\n", + "@ %lx, for symbol %s\n", (entry->type == GOT_TLS_GD ? "GOT_TLS_GD" : "GOT_TLS_IE"), (long) (sym_value - sec_vma), - htab->sgot->contents + entry->offset - + (entry->existing_entries == TLS_GOT_MOD_AND_OFF - ? 4 : 0), + (long) (htab->sgot->output_section->vma + + htab->sgot->output_offset->vma + + entry->offset + + (entry->existing_entries == TLS_GOT_MOD_AND_OFF + ? 4 : 0)), symbol_name); } break; @@ -351,14 +353,22 @@ relocate_fix_got_relocs_for_got_info (struct got_entry ** list_p, bfd_vma ATTRIBUTE_UNUSED sec_vma = tls_sec->output_section->vma; + bfd_put_32 (output_bfd, + sym_value - sec_vma, + htab->sgot->contents + entry->offset + + (entry->existing_entries == TLS_GOT_MOD_AND_OFF + ? 4 : 0)); + ARC_DEBUG ("arc_info: FIXED -> %s value = %#lx " "@ %p, for symbol %s\n", (entry->type == GOT_TLS_GD ? "GOT_TLS_GD" : "GOT_TLS_IE"), (long) (sym_value - sec_vma), - htab->sgot->contents + entry->offset - + (entry->existing_entries == TLS_GOT_MOD_AND_OFF - ? 4 : 0), + (long) (htab->sgot->output_section->vma + + htab->sgot->output_offset->vma + + entry->offset + + (entry->existing_entries == TLS_GOT_MOD_AND_OFF + ? 4 : 0)), symbol_name); } break; diff --git a/ld/testsuite/ld-arc/tls_ie-01.d b/ld/testsuite/ld-arc/tls_ie-01.d new file mode 100644 index 0000000..8d53ef5 --- /dev/null +++ b/ld/testsuite/ld-arc/tls_ie-01.d @@ -0,0 +1,9 @@ +#source: tls_ie-01.s +#as: -mcpu=arc700 +#ld: +#objdump: -s -j .got + +[^:]+: file format elf32-littlearc + +Contents of section \.got: + [0-9a-f]+ 00000000 04000000 .+ diff --git a/ld/testsuite/ld-arc/tls_ie-01.s b/ld/testsuite/ld-arc/tls_ie-01.s new file mode 100644 index 0000000..74f40ed --- /dev/null +++ b/ld/testsuite/ld-arc/tls_ie-01.s @@ -0,0 +1,10 @@ + .tls_common foo,4,4 + .tls_common bar,4,4 + + .text + .align 4 + + .global __start +__start: + ld r14, [pcl, @foo@tlsie] + ld r15, [pcl, @bar@tlsie] -- 2.9.0 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/5] Content for TLS_IE_GOT not written to .got. 2016-08-24 14:55 ` Cupertino Miranda @ 2016-08-25 10:59 ` Nick Clifton 0 siblings, 0 replies; 19+ messages in thread From: Nick Clifton @ 2016-08-25 10:59 UTC (permalink / raw) To: Cupertino Miranda, binutils; +Cc: Francois.Bedard, Claudiu.Zissulescu Hi Cupertino, > Indeed, a test case makes sense. > > Please find a new patch attached. Approved with this change. Cheers Nick ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 4/5] Fixed -init, -fini linker options. 2016-08-16 15:51 [PATCH 0/5] Fixes / improvements to ARC BFD target support Cupertino Miranda 2016-08-16 15:51 ` [PATCH 1/5] Fixes to legacy relocations Cupertino Miranda 2016-08-16 15:51 ` [PATCH 2/5] Content for TLS_IE_GOT not written to .got Cupertino Miranda @ 2016-08-16 15:51 ` Cupertino Miranda 2016-08-19 7:54 ` Nick Clifton 2016-08-16 15:51 ` [PATCH 5/5] Dynamic TLS GOT entries would not be relocated Cupertino Miranda ` (2 subsequent siblings) 5 siblings, 1 reply; 19+ messages in thread From: Cupertino Miranda @ 2016-08-16 15:51 UTC (permalink / raw) To: binutils; +Cc: Francois.Bedard, Claudiu.Zissulescu, Cupertino Miranda ARC was overloading this options by forcing DT_INIT AND DT_FINI to always point to _init and _fini, respectively. bfd/ChangeLog: Cupertino Miranda <cmiranda@synospsys.com> elf32-arc.c (elf_arc_finish_dynamic_sections): Changed. --- bfd/elf32-arc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bfd/elf32-arc.c b/bfd/elf32-arc.c index 684f013..857b7b6 100644 --- a/bfd/elf32-arc.c +++ b/bfd/elf32-arc.c @@ -2227,8 +2227,8 @@ elf_arc_finish_dynamic_sections (bfd * output_bfd, switch (internal_dyn.d_tag) { - GET_SYMBOL_OR_SECTION (DT_INIT, "_init", NULL) - GET_SYMBOL_OR_SECTION (DT_FINI, "_fini", NULL) + GET_SYMBOL_OR_SECTION (DT_INIT, info->init_function, NULL) + GET_SYMBOL_OR_SECTION (DT_FINI, info->fini_function, NULL) GET_SYMBOL_OR_SECTION (DT_PLTGOT, NULL, ".plt") GET_SYMBOL_OR_SECTION (DT_JMPREL, NULL, ".rela.plt") GET_SYMBOL_OR_SECTION (DT_PLTRELSZ, NULL, ".rela.plt") @@ -2376,8 +2376,8 @@ elf_arc_size_dynamic_sections (bfd * output_bfd, section. Checking if the .init section is present. We also create DT_INIT and DT_FINI entries if the init_str has been changed by the user. */ - ADD_DYNAMIC_SYMBOL ("init", DT_INIT); - ADD_DYNAMIC_SYMBOL ("fini", DT_FINI); + ADD_DYNAMIC_SYMBOL (info->init_function, DT_INIT); + ADD_DYNAMIC_SYMBOL (info->fini_function, DT_FINI); } else { -- 2.9.0 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 4/5] Fixed -init, -fini linker options. 2016-08-16 15:51 ` [PATCH 4/5] Fixed -init, -fini linker options Cupertino Miranda @ 2016-08-19 7:54 ` Nick Clifton 0 siblings, 0 replies; 19+ messages in thread From: Nick Clifton @ 2016-08-19 7:54 UTC (permalink / raw) To: Cupertino Miranda, binutils; +Cc: Francois.Bedard, Claudiu.Zissulescu Hi Cupertino, > bfd/ChangeLog: > > Cupertino Miranda <cmiranda@synospsys.com> > > elf32-arc.c (elf_arc_finish_dynamic_sections): Changed. Approved - please apply. Cheers Nick ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 5/5] Dynamic TLS GOT entries would not be relocated. 2016-08-16 15:51 [PATCH 0/5] Fixes / improvements to ARC BFD target support Cupertino Miranda ` (2 preceding siblings ...) 2016-08-16 15:51 ` [PATCH 4/5] Fixed -init, -fini linker options Cupertino Miranda @ 2016-08-16 15:51 ` Cupertino Miranda 2016-08-19 7:54 ` Nick Clifton 2016-08-16 15:51 ` [PATCH 3/5] Several fixes related to ARC PIE support Cupertino Miranda 2016-08-19 7:40 ` [PATCH 0/5] Fixes / improvements to ARC BFD target support Nick Clifton 5 siblings, 1 reply; 19+ messages in thread From: Cupertino Miranda @ 2016-08-16 15:51 UTC (permalink / raw) To: binutils; +Cc: Francois.Bedard, Claudiu.Zissulescu, Cupertino Miranda bfd/ChangeLog: Cupertino Miranda <cmiranda@synopsys.com> elf32-arc.c (elf_arc_relocate_section): Changed. --- bfd/elf32-arc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bfd/elf32-arc.c b/bfd/elf32-arc.c index 857b7b6..dae46be 100644 --- a/bfd/elf32-arc.c +++ b/bfd/elf32-arc.c @@ -1374,6 +1374,8 @@ elf_arc_relocate_section (bfd * output_bfd, if ((is_reloc_for_GOT (howto) || is_reloc_for_TLS (howto))) { + reloc_data.should_relocate = TRUE; + struct got_entry **list = get_got_entry_list_for_symbol (output_bfd, r_symndx, h); -- 2.9.0 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 5/5] Dynamic TLS GOT entries would not be relocated. 2016-08-16 15:51 ` [PATCH 5/5] Dynamic TLS GOT entries would not be relocated Cupertino Miranda @ 2016-08-19 7:54 ` Nick Clifton 2016-08-24 14:56 ` Cupertino Miranda 0 siblings, 1 reply; 19+ messages in thread From: Nick Clifton @ 2016-08-19 7:54 UTC (permalink / raw) To: Cupertino Miranda, binutils; +Cc: Francois.Bedard, Claudiu.Zissulescu Hi Cupertino, > bfd/ChangeLog: > > Cupertino Miranda <cmiranda@synopsys.com> > > elf32-arc.c (elf_arc_relocate_section): Changed. Why ? Cheers Nick ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 5/5] Dynamic TLS GOT entries would not be relocated. 2016-08-19 7:54 ` Nick Clifton @ 2016-08-24 14:56 ` Cupertino Miranda 2016-08-25 11:03 ` Nick Clifton 0 siblings, 1 reply; 19+ messages in thread From: Cupertino Miranda @ 2016-08-24 14:56 UTC (permalink / raw) To: Nick Clifton, Cupertino Miranda, binutils Cc: Francois.Bedard, Claudiu.Zissulescu [-- Attachment #1: Type: text/plain, Size: 387 bytes --] Hi, Indeed description should have been more verbose/existent. Please find a new patch with new description and testcase. Cheers, Cupertino On 08/19/2016 09:54 AM, Nick Clifton wrote: > Hi Cupertino, > >> bfd/ChangeLog: >> >> Cupertino Miranda <cmiranda@synopsys.com> >> >> elf32-arc.c (elf_arc_relocate_section): Changed. > Why ? > > > Cheers > Nick > > [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0005-Dynamic-TLS-GOT-entries-would-not-be-relocated.patch --] [-- Type: text/x-patch; name="0005-Dynamic-TLS-GOT-entries-would-not-be-relocated.patch", Size: 2351 bytes --] From 1e84cd9dedf83b5771ca38257740226add64b011 Mon Sep 17 00:00:00 2001 From: Cupertino Miranda <cmiranda@synopsys.com> Date: Thu, 21 Jul 2016 15:32:35 +0200 Subject: [PATCH 5/5] Dynamic TLS GOT entries would not be relocated. Forgot to set should_relocate to TRUE in case of GOT and TLS relocations of undefined symbols for shared libraries. In dynamic libraries if symbol is not known the instruction relocation would not be resolved to point to the respective .got entry. A test was created to detect similar future mistakes. bfd/ChangeLog: Cupertino Miranda <cmiranda@synopsys.com> elf32-arc.c (elf_arc_relocate_section): Changed. Set should_relocate to TRUE for GOT and TLS relocs. ld/ChangeLog: Cupertino Miranda <cmiranda@synopsys.com> ld/testsuite/ld-arc/tls_gd-01.s: Added a testcase for this patch. ld/testsuite/ld-arc/tls_gd-01.d: Likewise. --- bfd/elf32-arc.c | 2 ++ ld/testsuite/ld-arc/tls_gd-01.d | 13 +++++++++++++ ld/testsuite/ld-arc/tls_gd-01.s | 7 +++++++ 3 files changed, 22 insertions(+) create mode 100644 ld/testsuite/ld-arc/tls_gd-01.d create mode 100644 ld/testsuite/ld-arc/tls_gd-01.s diff --git a/bfd/elf32-arc.c b/bfd/elf32-arc.c index e4b2d09..6ebe5bf 100644 --- a/bfd/elf32-arc.c +++ b/bfd/elf32-arc.c @@ -1374,6 +1374,8 @@ elf_arc_relocate_section (bfd * output_bfd, if ((is_reloc_for_GOT (howto) || is_reloc_for_TLS (howto))) { + reloc_data.should_relocate = TRUE; + struct got_entry **list = get_got_entry_list_for_symbol (output_bfd, r_symndx, h); diff --git a/ld/testsuite/ld-arc/tls_gd-01.d b/ld/testsuite/ld-arc/tls_gd-01.d new file mode 100644 index 0000000..f7027af --- /dev/null +++ b/ld/testsuite/ld-arc/tls_gd-01.d @@ -0,0 +1,13 @@ +#source: tls_gd-01.s +#as: -mcpu=arc700 +#ld: -shared +#objdump: -d + +[^:]+: file format elf32-littlearc + + +Disassembly of section \.text: + +[0-9a-f]+ <__start>: + [0-9a-f]+: 2700 7f80 0000 2080 add r0,pcl,0x2080 + [0-9a-f]+: 2700 7f80 0000 2080 add r0,pcl,0x2080 diff --git a/ld/testsuite/ld-arc/tls_gd-01.s b/ld/testsuite/ld-arc/tls_gd-01.s new file mode 100644 index 0000000..a217a13 --- /dev/null +++ b/ld/testsuite/ld-arc/tls_gd-01.s @@ -0,0 +1,7 @@ + .text + .align 4 + + .global __start +__start: + add r0, pcl, @baz@tlsgd + add r0, pcl, @bar@tlsgd -- 2.9.0 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 5/5] Dynamic TLS GOT entries would not be relocated. 2016-08-24 14:56 ` Cupertino Miranda @ 2016-08-25 11:03 ` Nick Clifton 0 siblings, 0 replies; 19+ messages in thread From: Nick Clifton @ 2016-08-25 11:03 UTC (permalink / raw) To: Cupertino Miranda, binutils; +Cc: Francois.Bedard, Claudiu.Zissulescu Hi Cupertino, > Indeed description should have been more verbose/existent. > Please find a new patch with new description and testcase. Approved - please apply. Cheers Nick ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/5] Several fixes related to ARC PIE support. 2016-08-16 15:51 [PATCH 0/5] Fixes / improvements to ARC BFD target support Cupertino Miranda ` (3 preceding siblings ...) 2016-08-16 15:51 ` [PATCH 5/5] Dynamic TLS GOT entries would not be relocated Cupertino Miranda @ 2016-08-16 15:51 ` Cupertino Miranda 2016-08-19 7:53 ` Nick Clifton 2016-08-19 7:40 ` [PATCH 0/5] Fixes / improvements to ARC BFD target support Nick Clifton 5 siblings, 1 reply; 19+ messages in thread From: Cupertino Miranda @ 2016-08-16 15:51 UTC (permalink / raw) To: binutils; +Cc: Francois.Bedard, Claudiu.Zissulescu, Cupertino Miranda Fixed conditions related to dynamic relocs relative offset patching. Added arc_link_hash_table to be able to always generate and track .rela.bss section. bfd/ChangeLog: Cupertino Miranda <cmiranda@synopsys.com> elf-bfd.h: Added ARC_ELF_DATA to enum elf_target_id. elf32-arc.c (struct elf_arc_link_hash_entry): Added. (struct elf_arc_link_hash_table): Likewise. (elf_arc_link_hash_newfunc): Likewise. (elf_arc_link_hash_table_free): Likewise. (arc_elf_link_hash_table_create): Likewise. (elf_arc_relocate_section): Fixed conditions related to dynamic (elf_arc_check_relocs): Likewise. (arc_elf_create_dynamic_sections): Added (elf_arc_adjust_dynamic_symbol): Changed access to .rela.bss to be done through the hash table. --- bfd/elf-bfd.h | 1 + bfd/elf32-arc.c | 190 ++++++++++++++++++++++++++++++++++++++++++++------------ 2 files changed, 150 insertions(+), 41 deletions(-) diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h index 424ea30..d18df17 100644 --- a/bfd/elf-bfd.h +++ b/bfd/elf-bfd.h @@ -444,6 +444,7 @@ enum elf_target_id { AARCH64_ELF_DATA = 1, ALPHA_ELF_DATA, + ARC_ELF_DATA, ARM_ELF_DATA, AVR_ELF_DATA, BFIN_ELF_DATA, diff --git a/bfd/elf32-arc.c b/bfd/elf32-arc.c index 00828be..684f013 100644 --- a/bfd/elf32-arc.c +++ b/bfd/elf32-arc.c @@ -301,6 +301,92 @@ struct arc_reloc_map unsigned char elf_reloc_val; }; +/* ARC ELF linker hash entry. */ +struct elf_arc_link_hash_entry +{ + struct elf_link_hash_entry root; + + /* Track dynamic relocs copied for this symbol. */ + struct elf_dyn_relocs *dyn_relocs; +}; + +/* ARC ELF linker hash table. */ +struct elf_arc_link_hash_table +{ + struct elf_link_hash_table elf; + + /* Short-cuts to get to dynamic linker sections. */ + asection *srelbss; +}; + +static struct bfd_hash_entry * +elf_arc_link_hash_newfunc (struct bfd_hash_entry *entry, + struct bfd_hash_table *table, + const char *string) +{ + /* Allocate the structure if it has not already been allocated by a + subclass. */ + if (entry == NULL) + { + entry = (struct bfd_hash_entry *) + bfd_hash_allocate (table, + sizeof (struct elf_arc_link_hash_entry)); + if (entry == NULL) + return entry; + } + + /* Call the allocation method of the superclass. */ + entry = _bfd_elf_link_hash_newfunc (entry, table, string); + if (entry != NULL) + { + struct elf_arc_link_hash_entry *eh; + + eh = (struct elf_arc_link_hash_entry *) entry; + eh->dyn_relocs = NULL; + } + + return entry; +} + +/* Destroy an ARC ELF linker hash table. */ +/* +static void +elf_arc_link_hash_table_free (bfd *obfd) +{ + _bfd_elf_link_hash_table_free (obfd); +} +*/ + +/* Create an X86-64 ELF linker hash table. */ + +static struct bfd_link_hash_table * +arc_elf_link_hash_table_create (bfd *abfd) +{ + struct elf_arc_link_hash_table *ret; + + ret = (struct elf_arc_link_hash_table *) bfd_zmalloc (sizeof (*ret)); + if (ret == NULL) + return NULL; + + if (!_bfd_elf_link_hash_table_init (&ret->elf, abfd, + elf_arc_link_hash_newfunc, + sizeof (struct elf_arc_link_hash_entry), + ARC_ELF_DATA)) + { + free (ret); + return NULL; + } + + ret->srelbss = NULL; + + ret->elf.init_got_refcount.refcount = 0; + ret->elf.init_got_refcount.glist = NULL; + ret->elf.init_got_offset.offset = 0; + ret->elf.init_got_offset.glist = NULL; + + return &ret->elf.root; +} + #define ARC_RELOC_HOWTO(TYPE, VALUE, SIZE, BITSIZE, RELOC_FUNCTION, OVERFLOW, FORMULA) \ { BFD_RELOC_##TYPE, R_##TYPE }, static const struct arc_reloc_map arc_reloc_map[] = @@ -1273,7 +1359,7 @@ elf_arc_relocate_section (bfd * output_bfd, reloc_data.should_relocate = TRUE; } - else if (!bfd_link_pic (info)) + else if (!bfd_link_pic (info) || bfd_link_executable (info)) (*info->callbacks->undefined_symbol) (info, h->root.root.string, input_bfd, input_section, rel->r_offset, TRUE); @@ -1317,7 +1403,7 @@ elf_arc_relocate_section (bfd * output_bfd, case R_ARC_32_ME: case R_ARC_PC32: case R_ARC_32_PCREL: - if ((bfd_link_pic (info))// || bfd_link_pie (info)) + if ((bfd_link_pic (info)) && ((r_type != R_ARC_PC32 && r_type != R_ARC_32_PCREL) || (h != NULL && h->dynindx != -1 @@ -1470,6 +1556,49 @@ elf_arc_relocate_section (bfd * output_bfd, return TRUE; } +#define elf_arc_hash_table(p) \ + (elf_hash_table_id ((struct elf_link_hash_table *) ((p)->hash)) \ + == ARC_ELF_DATA ? ((struct elf_arc_link_hash_table *) ((p)->hash)) : NULL) + +/* Create .plt, .rela.plt, .got, .got.plt, .rela.got, .dynbss, and + .rela.bss sections in DYNOBJ, and set up shortcuts to them in our + hash table. */ + +static bfd_boolean +arc_elf_create_dynamic_sections (bfd *dynobj, + struct bfd_link_info *info) +{ + struct elf_arc_link_hash_table *htab; + + if (!_bfd_elf_create_dynamic_sections (dynobj, info)) + return FALSE; + + htab = elf_arc_hash_table (info); + if (htab == NULL) + return FALSE; + + if (bfd_link_executable (info)) + { + /* Always allow copy relocs for building executables. */ + asection *s = bfd_get_linker_section (dynobj, ".rela.bss"); + if (s == NULL) + { + const struct elf_backend_data *bed = get_elf_backend_data (dynobj); + s = bfd_make_section_anyway_with_flags (dynobj, + ".rela.bss", + (bed->dynamic_sec_flags + | SEC_READONLY)); + if (s == NULL + || ! bfd_set_section_alignment (dynobj, s, + bed->s->log_file_align)) + return FALSE; + } + htab->srelbss = s; + } + + return TRUE; +} + static struct dynamic_sections arc_create_dynamic_sections (bfd * abfd, struct bfd_link_info *info) { @@ -1615,10 +1744,9 @@ elf_arc_check_relocs (bfd * abfd, /* FALLTHROUGH */ case R_ARC_PC32: case R_ARC_32_PCREL: - if ((bfd_link_pic (info))// || bfd_link_pie (info)) + if ((bfd_link_pic (info)) && ((r_type != R_ARC_PC32 && r_type != R_ARC_32_PCREL) || (h != NULL - && h->dynindx != -1 && (!info->symbolic || !h->def_regular)))) { if (sreloc == NULL) @@ -1967,14 +2095,14 @@ elf_arc_adjust_dynamic_symbol (struct bfd_link_info *info, .rela.bss section we are going to use. */ if ((h->root.u.def.section->flags & SEC_ALLOC) != 0) { - asection *srel; + struct elf_arc_link_hash_table *arc_htab = elf_arc_hash_table (info); - srel = bfd_get_section_by_name (dynobj, ".rela.bss"); - BFD_ASSERT (srel != NULL); - srel->size += sizeof (Elf32_External_Rela); + BFD_ASSERT (arc_htab->srelbss != NULL); + arc_htab->srelbss->size += sizeof (Elf32_External_Rela); h->needs_copy = 1; } + /* TODO: Move this also to arc_hash_table. */ s = bfd_get_section_by_name (dynobj, ".dynbss"); BFD_ASSERT (s != NULL); @@ -2020,17 +2148,21 @@ elf_arc_finish_dynamic_symbol (bfd * output_bfd, if (h->needs_copy) { + struct elf_arc_link_hash_table *arc_htab = elf_arc_hash_table (info); + + if (h->dynindx == -1 + || (h->root.type != bfd_link_hash_defined + && h->root.type != bfd_link_hash_defweak) + || arc_htab->srelbss == NULL) + abort (); + bfd_vma rel_offset = (h->root.u.def.value + h->root.u.def.section->output_section->vma + h->root.u.def.section->output_offset); - asection *srelbss - = bfd_get_section_by_name (h->root.u.def.section->owner, - ".rela.bss"); - - bfd_byte * loc = srelbss->contents - + (srelbss->reloc_count * sizeof (Elf32_External_Rela)); - srelbss->reloc_count++; + bfd_byte * loc = arc_htab->srelbss->contents + + (arc_htab->srelbss->reloc_count * sizeof (Elf32_External_Rela)); + arc_htab->srelbss->reloc_count++; Elf_Internal_Rela rel; rel.r_addend = 0; @@ -2394,31 +2526,6 @@ const struct elf_size_info arc_elf32_size_info = #define elf_backend_size_info arc_elf32_size_info -static struct bfd_link_hash_table * -arc_elf_link_hash_table_create (bfd *abfd) -{ - struct elf_link_hash_table *htab; - - htab = bfd_zmalloc (sizeof (*htab)); - if (htab == NULL) - return NULL; - - if (!_bfd_elf_link_hash_table_init (htab, abfd, - _bfd_elf_link_hash_newfunc, - sizeof (struct elf_link_hash_entry), - GENERIC_ELF_DATA)) - { - free (htab); - return NULL; - } - - htab->init_got_refcount.refcount = 0; - htab->init_got_refcount.glist = NULL; - htab->init_got_offset.offset = 0; - htab->init_got_offset.glist = NULL; - return (struct bfd_link_hash_table *) htab; -} - /* Hook called by the linker routine which adds symbols from an object file. */ @@ -2444,6 +2551,7 @@ elf_arc_add_symbol_hook (bfd * abfd, #define TARGET_BIG_SYM arc_elf32_be_vec #define TARGET_BIG_NAME "elf32-bigarc" #define ELF_ARCH bfd_arch_arc +#define ELF_TARGET_ID ARC_ELF_DATA #define ELF_MACHINE_CODE EM_ARC_COMPACT #define ELF_MACHINE_ALT1 EM_ARC_COMPACT2 #define ELF_MAXPAGESIZE 0x2000 @@ -2462,7 +2570,7 @@ elf_arc_add_symbol_hook (bfd * abfd, #define elf_backend_relocate_section elf_arc_relocate_section #define elf_backend_check_relocs elf_arc_check_relocs -#define elf_backend_create_dynamic_sections _bfd_elf_create_dynamic_sections +#define elf_backend_create_dynamic_sections arc_elf_create_dynamic_sections #define elf_backend_reloc_type_class elf32_arc_reloc_type_class -- 2.9.0 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/5] Several fixes related to ARC PIE support. 2016-08-16 15:51 ` [PATCH 3/5] Several fixes related to ARC PIE support Cupertino Miranda @ 2016-08-19 7:53 ` Nick Clifton 2016-08-24 14:55 ` Cupertino Miranda 0 siblings, 1 reply; 19+ messages in thread From: Nick Clifton @ 2016-08-19 7:53 UTC (permalink / raw) To: Cupertino Miranda, binutils; +Cc: Francois.Bedard, Claudiu.Zissulescu Hi Cupertino, > bfd/ChangeLog: > Cupertino Miranda <cmiranda@synopsys.com> > > elf-bfd.h: Added ARC_ELF_DATA to enum elf_target_id. > elf32-arc.c (struct elf_arc_link_hash_entry): Added. > (struct elf_arc_link_hash_table): Likewise. > (elf_arc_link_hash_newfunc): Likewise. > (elf_arc_link_hash_table_free): Likewise. > (arc_elf_link_hash_table_create): Likewise. > (elf_arc_relocate_section): Fixed conditions related to dynamic > (elf_arc_check_relocs): Likewise. > (arc_elf_create_dynamic_sections): Added > (elf_arc_adjust_dynamic_symbol): Changed access to .rela.bss to be done > through the hash table. One minor point and one question: > +/* Create an X86-64 ELF linker hash table. */ Cut and paste typo - I assume that you meant 'Create an ARC ELF linker hash table'. > @@ -2020,17 +2148,21 @@ elf_arc_finish_dynamic_symbol (bfd * output_bfd, > > if (h->needs_copy) > { > + struct elf_arc_link_hash_table *arc_htab = elf_arc_hash_table (info); > + > + if (h->dynindx == -1 > + || (h->root.type != bfd_link_hash_defined > + && h->root.type != bfd_link_hash_defweak) > + || arc_htab->srelbss == NULL) > + abort (); I am not sure if an abort is the correct function to call here. Abort should only be called if there is an internal error in the library. If it is possible for bad user input to trigger the situation then an error message should be generated instead. I have not followed through all the logic, so maybe the conditions being tested should all be satisfied by earlier parts in the linking process, (in which case a BFD_ASSERT would be better than a direct call to abort), but it seems to me that it might be possible for a dynamic symbol to become undefined somehow, and in that case an error message would be more appropriate. Cheers Nick ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/5] Several fixes related to ARC PIE support. 2016-08-19 7:53 ` Nick Clifton @ 2016-08-24 14:55 ` Cupertino Miranda 2016-08-25 11:06 ` Nick Clifton 0 siblings, 1 reply; 19+ messages in thread From: Cupertino Miranda @ 2016-08-24 14:55 UTC (permalink / raw) To: Nick Clifton, Cupertino Miranda, binutils Cc: Francois.Bedard, Claudiu.Zissulescu [-- Attachment #1: Type: text/plain, Size: 1890 bytes --] Hi Nick, New patch attached. I noticed that in the patch I also did not include the hash table free function. Now it is correct. On 08/19/2016 09:53 AM, Nick Clifton wrote: > One minor point and one question: > >> +/* Create an X86-64 ELF linker hash table. */ > Cut and paste typo - I assume that you meant 'Create an ARC ELF linker hash table'. Indeed, cut and paste typo. >> @@ -2020,17 +2148,21 @@ elf_arc_finish_dynamic_symbol (bfd * output_bfd, >> >> if (h->needs_copy) >> { >> + struct elf_arc_link_hash_table *arc_htab = elf_arc_hash_table (info); >> + >> + if (h->dynindx == -1 >> + || (h->root.type != bfd_link_hash_defined >> + && h->root.type != bfd_link_hash_defweak) >> + || arc_htab->srelbss == NULL) >> + abort (); > I am not sure if an abort is the correct function to call here. Abort should > only be called if there is an internal error in the library. If it is possible > for bad user input to trigger the situation then an error message should be > generated instead. I have not followed through all the logic, so maybe the > conditions being tested should all be satisfied by earlier parts in the linking > process, (in which case a BFD_ASSERT would be better than a direct call to abort), > but it seems to me that it might be possible for a dynamic symbol to become > undefined somehow, and in that case an error message would be more appropriate. IMHO, the abort is really just a failsafe detection to validate that we can indeed do the copying, if we do not abort we will segm fault or generate bad code. To be honest, I would not bet on the correctness of this needs_copy conditioning, but a priori it should verify that all the requirements for the copy are met. If not it should be verified and reported before this point. Looking forward for your comments. Cheers, Cupertino [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0003-Several-fixes-related-to-ARC-PIE-support.patch --] [-- Type: text/x-patch; name="0003-Several-fixes-related-to-ARC-PIE-support.patch", Size: 9748 bytes --] From 1b736e93146545d3db3922be2a9ee127bbd38eb2 Mon Sep 17 00:00:00 2001 From: Cupertino Miranda <cmiranda@synopsys.com> Date: Wed, 13 Jul 2016 18:04:20 +0200 Subject: [PATCH 3/5] Several fixes related to ARC PIE support. Fixed conditions related to dynamic relocs relative offset patching. Added arc_link_hash_table to be able to always generate and track .rela.bss section. bfd/ChangeLog: Cupertino Miranda <cmiranda@synopsys.com> elf-bfd.h: Added ARC_ELF_DATA to enum elf_target_id. elf32-arc.c (struct elf_arc_link_hash_entry): Added. (struct elf_arc_link_hash_table): Likewise. (elf_arc_link_hash_newfunc): Likewise. (elf_arc_link_hash_table_free): Likewise. (arc_elf_link_hash_table_create): Likewise. (elf_arc_relocate_section): Fixed conditions related to dynamic (elf_arc_check_relocs): Likewise. (arc_elf_create_dynamic_sections): Added (elf_arc_adjust_dynamic_symbol): Changed access to .rela.bss to be done through the hash table. --- bfd/elf-bfd.h | 1 + bfd/elf32-arc.c | 190 ++++++++++++++++++++++++++++++++++++++++++++------------ 2 files changed, 150 insertions(+), 41 deletions(-) diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h index 424ea30..d18df17 100644 --- a/bfd/elf-bfd.h +++ b/bfd/elf-bfd.h @@ -444,6 +444,7 @@ enum elf_target_id { AARCH64_ELF_DATA = 1, ALPHA_ELF_DATA, + ARC_ELF_DATA, ARM_ELF_DATA, AVR_ELF_DATA, BFIN_ELF_DATA, diff --git a/bfd/elf32-arc.c b/bfd/elf32-arc.c index 00828be..08dd867 100644 --- a/bfd/elf32-arc.c +++ b/bfd/elf32-arc.c @@ -301,6 +301,92 @@ struct arc_reloc_map unsigned char elf_reloc_val; }; +/* ARC ELF linker hash entry. */ +struct elf_arc_link_hash_entry +{ + struct elf_link_hash_entry root; + + /* Track dynamic relocs copied for this symbol. */ + struct elf_dyn_relocs *dyn_relocs; +}; + +/* ARC ELF linker hash table. */ +struct elf_arc_link_hash_table +{ + struct elf_link_hash_table elf; + + /* Short-cuts to get to dynamic linker sections. */ + asection *srelbss; +}; + +static struct bfd_hash_entry * +elf_arc_link_hash_newfunc (struct bfd_hash_entry *entry, + struct bfd_hash_table *table, + const char *string) +{ + /* Allocate the structure if it has not already been allocated by a + subclass. */ + if (entry == NULL) + { + entry = (struct bfd_hash_entry *) + bfd_hash_allocate (table, + sizeof (struct elf_arc_link_hash_entry)); + if (entry == NULL) + return entry; + } + + /* Call the allocation method of the superclass. */ + entry = _bfd_elf_link_hash_newfunc (entry, table, string); + if (entry != NULL) + { + struct elf_arc_link_hash_entry *eh; + + eh = (struct elf_arc_link_hash_entry *) entry; + eh->dyn_relocs = NULL; + } + + return entry; +} + +/* Destroy an ARC ELF linker hash table. */ +static void +elf_arc_link_hash_table_free (bfd *obfd) +{ + _bfd_elf_link_hash_table_free (obfd); +} + +/* Create an ARC ELF linker hash table. */ + +static struct bfd_link_hash_table * +arc_elf_link_hash_table_create (bfd *abfd) +{ + struct elf_arc_link_hash_table *ret; + + ret = (struct elf_arc_link_hash_table *) bfd_zmalloc (sizeof (*ret)); + if (ret == NULL) + return NULL; + + if (!_bfd_elf_link_hash_table_init (&ret->elf, abfd, + elf_arc_link_hash_newfunc, + sizeof (struct elf_arc_link_hash_entry), + ARC_ELF_DATA)) + { + free (ret); + return NULL; + } + + ret->srelbss = NULL; + + ret->elf.init_got_refcount.refcount = 0; + ret->elf.init_got_refcount.glist = NULL; + ret->elf.init_got_offset.offset = 0; + ret->elf.init_got_offset.glist = NULL; + + ret->elf.root.hash_table_free = elf_arc_link_hash_table_free; + + return &ret->elf.root; +} + #define ARC_RELOC_HOWTO(TYPE, VALUE, SIZE, BITSIZE, RELOC_FUNCTION, OVERFLOW, FORMULA) \ { BFD_RELOC_##TYPE, R_##TYPE }, static const struct arc_reloc_map arc_reloc_map[] = @@ -1273,7 +1359,7 @@ elf_arc_relocate_section (bfd * output_bfd, reloc_data.should_relocate = TRUE; } - else if (!bfd_link_pic (info)) + else if (!bfd_link_pic (info) || bfd_link_executable (info)) (*info->callbacks->undefined_symbol) (info, h->root.root.string, input_bfd, input_section, rel->r_offset, TRUE); @@ -1317,7 +1403,7 @@ elf_arc_relocate_section (bfd * output_bfd, case R_ARC_32_ME: case R_ARC_PC32: case R_ARC_32_PCREL: - if ((bfd_link_pic (info))// || bfd_link_pie (info)) + if ((bfd_link_pic (info)) && ((r_type != R_ARC_PC32 && r_type != R_ARC_32_PCREL) || (h != NULL && h->dynindx != -1 @@ -1470,6 +1556,49 @@ elf_arc_relocate_section (bfd * output_bfd, return TRUE; } +#define elf_arc_hash_table(p) \ + (elf_hash_table_id ((struct elf_link_hash_table *) ((p)->hash)) \ + == ARC_ELF_DATA ? ((struct elf_arc_link_hash_table *) ((p)->hash)) : NULL) + +/* Create .plt, .rela.plt, .got, .got.plt, .rela.got, .dynbss, and + .rela.bss sections in DYNOBJ, and set up shortcuts to them in our + hash table. */ + +static bfd_boolean +arc_elf_create_dynamic_sections (bfd *dynobj, + struct bfd_link_info *info) +{ + struct elf_arc_link_hash_table *htab; + + if (!_bfd_elf_create_dynamic_sections (dynobj, info)) + return FALSE; + + htab = elf_arc_hash_table (info); + if (htab == NULL) + return FALSE; + + if (bfd_link_executable (info)) + { + /* Always allow copy relocs for building executables. */ + asection *s = bfd_get_linker_section (dynobj, ".rela.bss"); + if (s == NULL) + { + const struct elf_backend_data *bed = get_elf_backend_data (dynobj); + s = bfd_make_section_anyway_with_flags (dynobj, + ".rela.bss", + (bed->dynamic_sec_flags + | SEC_READONLY)); + if (s == NULL + || ! bfd_set_section_alignment (dynobj, s, + bed->s->log_file_align)) + return FALSE; + } + htab->srelbss = s; + } + + return TRUE; +} + static struct dynamic_sections arc_create_dynamic_sections (bfd * abfd, struct bfd_link_info *info) { @@ -1615,10 +1744,9 @@ elf_arc_check_relocs (bfd * abfd, /* FALLTHROUGH */ case R_ARC_PC32: case R_ARC_32_PCREL: - if ((bfd_link_pic (info))// || bfd_link_pie (info)) + if ((bfd_link_pic (info)) && ((r_type != R_ARC_PC32 && r_type != R_ARC_32_PCREL) || (h != NULL - && h->dynindx != -1 && (!info->symbolic || !h->def_regular)))) { if (sreloc == NULL) @@ -1967,14 +2095,14 @@ elf_arc_adjust_dynamic_symbol (struct bfd_link_info *info, .rela.bss section we are going to use. */ if ((h->root.u.def.section->flags & SEC_ALLOC) != 0) { - asection *srel; + struct elf_arc_link_hash_table *arc_htab = elf_arc_hash_table (info); - srel = bfd_get_section_by_name (dynobj, ".rela.bss"); - BFD_ASSERT (srel != NULL); - srel->size += sizeof (Elf32_External_Rela); + BFD_ASSERT (arc_htab->srelbss != NULL); + arc_htab->srelbss->size += sizeof (Elf32_External_Rela); h->needs_copy = 1; } + /* TODO: Move this also to arc_hash_table. */ s = bfd_get_section_by_name (dynobj, ".dynbss"); BFD_ASSERT (s != NULL); @@ -2020,17 +2148,21 @@ elf_arc_finish_dynamic_symbol (bfd * output_bfd, if (h->needs_copy) { + struct elf_arc_link_hash_table *arc_htab = elf_arc_hash_table (info); + + if (h->dynindx == -1 + || (h->root.type != bfd_link_hash_defined + && h->root.type != bfd_link_hash_defweak) + || arc_htab->srelbss == NULL) + abort (); + bfd_vma rel_offset = (h->root.u.def.value + h->root.u.def.section->output_section->vma + h->root.u.def.section->output_offset); - asection *srelbss - = bfd_get_section_by_name (h->root.u.def.section->owner, - ".rela.bss"); - - bfd_byte * loc = srelbss->contents - + (srelbss->reloc_count * sizeof (Elf32_External_Rela)); - srelbss->reloc_count++; + bfd_byte * loc = arc_htab->srelbss->contents + + (arc_htab->srelbss->reloc_count * sizeof (Elf32_External_Rela)); + arc_htab->srelbss->reloc_count++; Elf_Internal_Rela rel; rel.r_addend = 0; @@ -2394,31 +2526,6 @@ const struct elf_size_info arc_elf32_size_info = #define elf_backend_size_info arc_elf32_size_info -static struct bfd_link_hash_table * -arc_elf_link_hash_table_create (bfd *abfd) -{ - struct elf_link_hash_table *htab; - - htab = bfd_zmalloc (sizeof (*htab)); - if (htab == NULL) - return NULL; - - if (!_bfd_elf_link_hash_table_init (htab, abfd, - _bfd_elf_link_hash_newfunc, - sizeof (struct elf_link_hash_entry), - GENERIC_ELF_DATA)) - { - free (htab); - return NULL; - } - - htab->init_got_refcount.refcount = 0; - htab->init_got_refcount.glist = NULL; - htab->init_got_offset.offset = 0; - htab->init_got_offset.glist = NULL; - return (struct bfd_link_hash_table *) htab; -} - /* Hook called by the linker routine which adds symbols from an object file. */ @@ -2444,6 +2551,7 @@ elf_arc_add_symbol_hook (bfd * abfd, #define TARGET_BIG_SYM arc_elf32_be_vec #define TARGET_BIG_NAME "elf32-bigarc" #define ELF_ARCH bfd_arch_arc +#define ELF_TARGET_ID ARC_ELF_DATA #define ELF_MACHINE_CODE EM_ARC_COMPACT #define ELF_MACHINE_ALT1 EM_ARC_COMPACT2 #define ELF_MAXPAGESIZE 0x2000 @@ -2462,7 +2570,7 @@ elf_arc_add_symbol_hook (bfd * abfd, #define elf_backend_relocate_section elf_arc_relocate_section #define elf_backend_check_relocs elf_arc_check_relocs -#define elf_backend_create_dynamic_sections _bfd_elf_create_dynamic_sections +#define elf_backend_create_dynamic_sections arc_elf_create_dynamic_sections #define elf_backend_reloc_type_class elf32_arc_reloc_type_class -- 2.9.0 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/5] Several fixes related to ARC PIE support. 2016-08-24 14:55 ` Cupertino Miranda @ 2016-08-25 11:06 ` Nick Clifton 0 siblings, 0 replies; 19+ messages in thread From: Nick Clifton @ 2016-08-25 11:06 UTC (permalink / raw) To: Cupertino Miranda, binutils; +Cc: Francois.Bedard, Claudiu.Zissulescu Hi Cupertino, > New patch attached. > I noticed that in the patch I also did not include the hash table free > function. Now it is correct. Well, I still do not like the abort, but OK, patch approved. Cheers Nick ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/5] Fixes / improvements to ARC BFD target support. 2016-08-16 15:51 [PATCH 0/5] Fixes / improvements to ARC BFD target support Cupertino Miranda ` (4 preceding siblings ...) 2016-08-16 15:51 ` [PATCH 3/5] Several fixes related to ARC PIE support Cupertino Miranda @ 2016-08-19 7:40 ` Nick Clifton 2016-08-24 14:53 ` Cupertino Miranda 5 siblings, 1 reply; 19+ messages in thread From: Nick Clifton @ 2016-08-19 7:40 UTC (permalink / raw) To: Cupertino Miranda, binutils; +Cc: Francois.Bedard, Claudiu.Zissulescu Hi Cupertino, > Here are a new set of patches for ARC bfd improvements. > The individual patch emails explain the respective improvement/fix. For future reference, please could you also mention how you tested these changes, and if there were any regressions. Given that there are several different possible configurations for ARC toolchains, it would be good to know that a variety were tested. Cheers Nick ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 0/5] Fixes / improvements to ARC BFD target support. 2016-08-19 7:40 ` [PATCH 0/5] Fixes / improvements to ARC BFD target support Nick Clifton @ 2016-08-24 14:53 ` Cupertino Miranda 0 siblings, 0 replies; 19+ messages in thread From: Cupertino Miranda @ 2016-08-24 14:53 UTC (permalink / raw) To: Nick Clifton, Cupertino Miranda, binutils Cc: Francois.Bedard, Claudiu.Zissulescu Hi Nick, Thank you very much for your fast review and approvals. Unfortunately, I took some holidays in the period and could not reply immediately. ;-) Regarding testing we have nightly builds that run tens of configurations of ARC. It includes all GNU Dejagnu (Binutils and GCC) tests both in baremetal and Linux, Uclibc tests on Linux and Newlib on baremetal. All running in both board and simulator. All of our submitted patches have at least proven to be passing those tests. Nevertheless, it does not mean that mistakes are not made and we could sometimes submit broken patches. ;-) Cheers, Cupertino On 08/19/2016 09:40 AM, Nick Clifton wrote: > Hi Cupertino, > >> Here are a new set of patches for ARC bfd improvements. >> The individual patch emails explain the respective improvement/fix. > For future reference, please could you also mention how you tested > these changes, and if there were any regressions. Given that there > are several different possible configurations for ARC toolchains, it > would be good to know that a variety were tested. > > Cheers > Nick > > ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2016-08-25 11:06 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-08-16 15:51 [PATCH 0/5] Fixes / improvements to ARC BFD target support Cupertino Miranda 2016-08-16 15:51 ` [PATCH 1/5] Fixes to legacy relocations Cupertino Miranda 2016-08-19 7:41 ` Nick Clifton 2016-08-16 15:51 ` [PATCH 2/5] Content for TLS_IE_GOT not written to .got Cupertino Miranda 2016-08-19 7:43 ` Nick Clifton 2016-08-24 14:55 ` Cupertino Miranda 2016-08-25 10:59 ` Nick Clifton 2016-08-16 15:51 ` [PATCH 4/5] Fixed -init, -fini linker options Cupertino Miranda 2016-08-19 7:54 ` Nick Clifton 2016-08-16 15:51 ` [PATCH 5/5] Dynamic TLS GOT entries would not be relocated Cupertino Miranda 2016-08-19 7:54 ` Nick Clifton 2016-08-24 14:56 ` Cupertino Miranda 2016-08-25 11:03 ` Nick Clifton 2016-08-16 15:51 ` [PATCH 3/5] Several fixes related to ARC PIE support Cupertino Miranda 2016-08-19 7:53 ` Nick Clifton 2016-08-24 14:55 ` Cupertino Miranda 2016-08-25 11:06 ` Nick Clifton 2016-08-19 7:40 ` [PATCH 0/5] Fixes / improvements to ARC BFD target support Nick Clifton 2016-08-24 14:53 ` Cupertino Miranda
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).