From patchwork Fri Mar 4 16:07:48 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 335 Return-Path: Delivered-To: unknown Received: from imap.gmail.com (74.125.159.109) by localhost6.localdomain6 with IMAP4-SSL; 08 Jun 2011 14:41:48 -0000 Delivered-To: patches@linaro.org Received: by 10.224.60.68 with SMTP id o4cs19859qah; Fri, 4 Mar 2011 08:07:52 -0800 (PST) Received: by 10.216.60.193 with SMTP id u43mr704925wec.103.1299254871802; Fri, 04 Mar 2011 08:07:51 -0800 (PST) Received: from mail-ww0-f50.google.com (mail-ww0-f50.google.com [74.125.82.50]) by mx.google.com with ESMTPS id z43si4434687weq.101.2011.03.04.08.07.51 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 04 Mar 2011 08:07:51 -0800 (PST) Received-SPF: neutral (google.com: 74.125.82.50 is neither permitted nor denied by best guess record for domain of richard.sandiford@linaro.org) client-ip=74.125.82.50; Authentication-Results: mx.google.com; spf=neutral (google.com: 74.125.82.50 is neither permitted nor denied by best guess record for domain of richard.sandiford@linaro.org) smtp.mail=richard.sandiford@linaro.org Received: by wwb31 with SMTP id 31so3012612wwb.31 for ; Fri, 04 Mar 2011 08:07:51 -0800 (PST) Received: by 10.227.179.69 with SMTP id bp5mr680922wbb.192.1299254871139; Fri, 04 Mar 2011 08:07:51 -0800 (PST) Received: from richards-thinkpad (gbibp9ph1--blueice2n1.emea.ibm.com [195.212.29.75]) by mx.google.com with ESMTPS id u9sm1898742wbg.0.2011.03.04.08.07.49 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 04 Mar 2011 08:07:50 -0800 (PST) From: Richard Sandiford To: binutils@sourceware.org Mail-Followup-To: binutils@sourceware.org, patches@linaro.org, richard.sandiford@linaro.org Cc: patches@linaro.org Subject: [3/6] Fix overcounting of ARM PLT references Date: Fri, 04 Mar 2011 16:07:48 +0000 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 If a shared object has only non-call references to a function that is defined in the same shared object, we will nevertheless generate an unused PLT entry for it. This is due to some overeager counting in elf32_arm_check_relocs. The patch below restructures check_relocs and its gc_sweep_hook counterpart to make things a little more obvious (I hope). This also makes it easier to slip in the ifunc handling. I've kept a fair bit of the existing commentary, but moved it around. Note that the R_ARM_ABS12 case was misplaced: it fell through into the PLT relocs instead of the absolute relocs. Tested on arm-linux-gnueabi. OK to install? Richard bfd/ * elf32-arm.c (elf32_arm_check_relocs): Use call_reloc_p, may_need_local_target_p and may_become_dynamic_p to classify the relocation type. Don't check info->symbolic or h->def_regular when deciding whether to record a potential dynamic reloc. Don't treat potential dynamic relocs as PLT references. (elf32_arm_gc_sweep_hook): Update to match. Assert that we don't try to make the PLT reference count go negative. ld/testsuite/ * ld-arm/arm-lib-plt-2a.s, ld-arm/arm-lib-plt-2b.s, ld-arm/arm-lib-plt-2.dd, ld-arm/arm-lib-plt-2.rd: New tests. * ld-arm/arm-elf.exp: Run them. Index: bfd/elf32-arm.c =================================================================== --- bfd/elf32-arm.c 2011-03-04 10:21:47.000000000 +0000 +++ bfd/elf32-arm.c 2011-03-04 10:38:37.000000000 +0000 @@ -11243,7 +11243,10 @@ elf32_arm_gc_sweep_hook (bfd * { unsigned long r_symndx; struct elf_link_hash_entry *h = NULL; + struct elf32_arm_link_hash_entry *eh; int r_type; + bfd_boolean may_become_dynamic_p; + bfd_boolean may_need_local_target_p; r_symndx = ELF32_R_SYM (rel->r_info); if (r_symndx >= symtab_hdr->sh_info) @@ -11253,6 +11256,10 @@ elf32_arm_gc_sweep_hook (bfd * || h->root.type == bfd_link_hash_warning) h = (struct elf_link_hash_entry *) h->root.u.i.link; } + eh = (struct elf32_arm_link_hash_entry *) h; + + may_become_dynamic_p = FALSE; + may_need_local_target_p = FALSE; r_type = ELF32_R_TYPE (rel->r_info); r_type = arm_real_reloc_type (globals, r_type); @@ -11278,10 +11285,6 @@ elf32_arm_gc_sweep_hook (bfd * globals->tls_ldm_got.refcount -= 1; break; - case R_ARM_ABS32: - case R_ARM_ABS32_NOI: - case R_ARM_REL32: - case R_ARM_REL32_NOI: case R_ARM_PC24: case R_ARM_PLT32: case R_ARM_CALL: @@ -11290,6 +11293,20 @@ elf32_arm_gc_sweep_hook (bfd * case R_ARM_THM_CALL: case R_ARM_THM_JUMP24: case R_ARM_THM_JUMP19: + may_need_local_target_p = TRUE; + break; + + case R_ARM_ABS12: + if (!globals->vxworks_p) + { + may_need_local_target_p = TRUE; + break; + } + /* Fall through. */ + case R_ARM_ABS32: + case R_ARM_ABS32_NOI: + case R_ARM_REL32: + case R_ARM_REL32_NOI: case R_ARM_MOVW_ABS_NC: case R_ARM_MOVT_ABS: case R_ARM_MOVW_PREL_NC: @@ -11299,39 +11316,45 @@ elf32_arm_gc_sweep_hook (bfd * case R_ARM_THM_MOVW_PREL_NC: case R_ARM_THM_MOVT_PREL: /* Should the interworking branches be here also? */ - - if (h != NULL) - { - struct elf32_arm_link_hash_entry *eh; - struct elf_dyn_relocs **pp; - struct elf_dyn_relocs *p; - - eh = (struct elf32_arm_link_hash_entry *) h; - - if (h->plt.refcount > 0) - { - h->plt.refcount -= 1; - if (r_type == R_ARM_THM_CALL) - eh->plt_maybe_thumb_refcount--; - - if (r_type == R_ARM_THM_JUMP24 - || r_type == R_ARM_THM_JUMP19) - eh->plt_thumb_refcount--; - } - - for (pp = &eh->dyn_relocs; (p = *pp) != NULL; pp = &p->next) - if (p->sec == sec) - { - /* Everything must go for SEC. */ - *pp = p->next; - break; - } - } + if ((info->shared || globals->root.is_relocatable_executable) + && (sec->flags & SEC_ALLOC) != 0 + && (h != NULL + || (r_type != R_ARM_REL32 && r_type != R_ARM_REL32_NOI))) + may_become_dynamic_p = TRUE; + else + may_need_local_target_p = TRUE; break; default: break; } + + if (may_need_local_target_p && h != NULL) + { + BFD_ASSERT (h->plt.refcount > 0); + h->plt.refcount -= 1; + + if (r_type == R_ARM_THM_CALL) + eh->plt_maybe_thumb_refcount--; + + if (r_type == R_ARM_THM_JUMP24 + || r_type == R_ARM_THM_JUMP19) + eh->plt_thumb_refcount--; + } + + if (may_become_dynamic_p && h != NULL) + { + struct elf_dyn_relocs **pp; + struct elf_dyn_relocs *p; + + for (pp = &eh->dyn_relocs; (p = *pp) != NULL; pp = &p->next) + if (p->sec == sec) + { + /* Everything must go for SEC. */ + *pp = p->next; + break; + } + } } return TRUE; @@ -11350,7 +11373,9 @@ elf32_arm_check_relocs (bfd *abfd, struc bfd *dynobj; asection *sreloc; struct elf32_arm_link_hash_table *htab; - bfd_boolean needs_plt; + bfd_boolean call_reloc_p; + bfd_boolean may_become_dynamic_p; + bfd_boolean may_need_local_target_p; unsigned long nsyms; if (info->relocatable) @@ -11413,6 +11438,10 @@ elf32_arm_check_relocs (bfd *abfd, struc eh = (struct elf32_arm_link_hash_entry *) h; + call_reloc_p = FALSE; + may_become_dynamic_p = FALSE; + may_need_local_target_p = FALSE; + /* Could be done earlier, if h were already available. */ r_type = elf32_arm_tls_transition (info, r_type, h); switch (r_type) @@ -11524,13 +11553,6 @@ elf32_arm_check_relocs (bfd *abfd, struc } break; - case R_ARM_ABS12: - /* VxWorks uses dynamic R_ARM_ABS12 relocations for - ldr __GOTT_INDEX__ offsets. */ - if (!htab->vxworks_p) - break; - /* Fall through. */ - case R_ARM_PC24: case R_ARM_PLT32: case R_ARM_CALL: @@ -11539,8 +11561,19 @@ elf32_arm_check_relocs (bfd *abfd, struc case R_ARM_THM_CALL: case R_ARM_THM_JUMP24: case R_ARM_THM_JUMP19: - needs_plt = 1; - goto normal_reloc; + call_reloc_p = TRUE; + may_need_local_target_p = TRUE; + break; + + case R_ARM_ABS12: + /* VxWorks uses dynamic R_ARM_ABS12 relocations for + ldr __GOTT_INDEX__ offsets. */ + if (!htab->vxworks_p) + { + may_need_local_target_p = TRUE; + break; + } + /* Fall through. */ case R_ARM_MOVW_ABS_NC: case R_ARM_MOVT_ABS: @@ -11565,134 +11598,19 @@ elf32_arm_check_relocs (bfd *abfd, struc case R_ARM_MOVT_PREL: case R_ARM_THM_MOVW_PREL_NC: case R_ARM_THM_MOVT_PREL: - needs_plt = 0; - normal_reloc: /* Should the interworking branches be listed here? */ - if (h != NULL) - { - /* If this reloc is in a read-only section, we might - need a copy reloc. We can't check reliably at this - stage whether the section is read-only, as input - sections have not yet been mapped to output sections. - Tentatively set the flag for now, and correct in - adjust_dynamic_symbol. */ - if (!info->shared) - h->non_got_ref = 1; - - /* We may need a .plt entry if the function this reloc - refers to is in a different object. We can't tell for - sure yet, because something later might force the - symbol local. */ - if (needs_plt) - h->needs_plt = 1; - - /* If we create a PLT entry, this relocation will reference - it, even if it's an ABS32 relocation. */ - h->plt.refcount += 1; - - /* It's too early to use htab->use_blx here, so we have to - record possible blx references separately from - relocs that definitely need a thumb stub. */ - - if (r_type == R_ARM_THM_CALL) - eh->plt_maybe_thumb_refcount += 1; - - if (r_type == R_ARM_THM_JUMP24 - || r_type == R_ARM_THM_JUMP19) - eh->plt_thumb_refcount += 1; - } - - /* If we are creating a shared library or relocatable executable, - and this is a reloc against a global symbol, or a non PC - relative reloc against a local symbol, then we need to copy - the reloc into the shared library. However, if we are linking - with -Bsymbolic, we do not need to copy a reloc against a - global symbol which is defined in an object we are - including in the link (i.e., DEF_REGULAR is set). At - this point we have not seen all the input files, so it is - possible that DEF_REGULAR is not set now but will be set - later (it is never cleared). We account for that - possibility below by storing information in the - dyn_relocs field of the hash table entry. */ + /* If we are creating a shared library or relocatable + executable, and this is a reloc against a global symbol, + or a non-PC-relative reloc against a local symbol, + then we may need to copy the reloc into the output. */ if ((info->shared || htab->root.is_relocatable_executable) && (sec->flags & SEC_ALLOC) != 0 - && ((r_type == R_ARM_ABS32 || r_type == R_ARM_ABS32_NOI) - || (h != NULL && ! needs_plt - && (! info->symbolic || ! h->def_regular)))) - { - struct elf_dyn_relocs *p, **head; - - /* When creating a shared object, we must copy these - reloc types into the output file. We create a reloc - section in dynobj and make room for this reloc. */ - if (sreloc == NULL) - { - sreloc = _bfd_elf_make_dynamic_reloc_section - (sec, dynobj, 2, abfd, ! htab->use_rel); - - if (sreloc == NULL) - return FALSE; - - /* BPABI objects never have dynamic relocations mapped. */ - if (htab->symbian_p) - { - flagword flags; - - flags = bfd_get_section_flags (dynobj, sreloc); - flags &= ~(SEC_LOAD | SEC_ALLOC); - bfd_set_section_flags (dynobj, sreloc, flags); - } - } - - /* If this is a global symbol, we count the number of - relocations we need for this symbol. */ - if (h != NULL) - { - head = &((struct elf32_arm_link_hash_entry *) h)->dyn_relocs; - } - else - { - /* Track dynamic relocs needed for local syms too. - We really need local syms available to do this - easily. Oh well. */ - asection *s; - void *vpp; - Elf_Internal_Sym *isym; - - isym = bfd_sym_from_r_symndx (&htab->sym_cache, - abfd, r_symndx); - if (isym == NULL) - return FALSE; - - s = bfd_section_from_elf_index (abfd, isym->st_shndx); - if (s == NULL) - s = sec; - - vpp = &elf_section_data (s)->local_dynrel; - head = (struct elf_dyn_relocs **) vpp; - } - - p = *head; - if (p == NULL || p->sec != sec) - { - bfd_size_type amt = sizeof *p; - - p = (struct elf_dyn_relocs *) - bfd_alloc (htab->root.dynobj, amt); - if (p == NULL) - return FALSE; - p->next = *head; - *head = p; - p->sec = sec; - p->count = 0; - p->pc_count = 0; - } - - if (r_type == R_ARM_REL32 || r_type == R_ARM_REL32_NOI) - p->pc_count += 1; - p->count += 1; - } + && (h != NULL + || (r_type != R_ARM_REL32 && r_type != R_ARM_REL32_NOI))) + may_become_dynamic_p = TRUE; + else + may_need_local_target_p = TRUE; break; /* This relocation describes the C++ object vtable hierarchy. @@ -11711,6 +11629,112 @@ elf32_arm_check_relocs (bfd *abfd, struc return FALSE; break; } + + if (h != NULL) + { + if (call_reloc_p) + /* We may need a .plt entry if the function this reloc + refers to is in a different object, regardless of the + symbol's type. We can't tell for sure yet, because + something later might force the symbol local. */ + h->needs_plt = 1; + else if (may_need_local_target_p) + /* If this reloc is in a read-only section, we might + need a copy reloc. We can't check reliably at this + stage whether the section is read-only, as input + sections have not yet been mapped to output sections. + Tentatively set the flag for now, and correct in + adjust_dynamic_symbol. */ + h->non_got_ref = 1; + } + + if (may_need_local_target_p && h != NULL) + { + /* If the symbol is a function that doesn't bind locally, + this relocation will need a PLT entry. */ + h->plt.refcount += 1; + + /* It's too early to use htab->use_blx here, so we have to + record possible blx references separately from + relocs that definitely need a thumb stub. */ + + if (r_type == R_ARM_THM_CALL) + eh->plt_maybe_thumb_refcount += 1; + + if (r_type == R_ARM_THM_JUMP24 + || r_type == R_ARM_THM_JUMP19) + eh->plt_thumb_refcount += 1; + } + + if (may_become_dynamic_p) + { + struct elf_dyn_relocs *p, **head; + + /* Create a reloc section in dynobj. */ + if (sreloc == NULL) + { + sreloc = _bfd_elf_make_dynamic_reloc_section + (sec, dynobj, 2, abfd, ! htab->use_rel); + + if (sreloc == NULL) + return FALSE; + + /* BPABI objects never have dynamic relocations mapped. */ + if (htab->symbian_p) + { + flagword flags; + + flags = bfd_get_section_flags (dynobj, sreloc); + flags &= ~(SEC_LOAD | SEC_ALLOC); + bfd_set_section_flags (dynobj, sreloc, flags); + } + } + + /* If this is a global symbol, count the number of + relocations we need for this symbol. */ + if (h != NULL) + head = &((struct elf32_arm_link_hash_entry *) h)->dyn_relocs; + else + { + /* Track dynamic relocs needed for local syms too. + We really need local syms available to do this + easily. Oh well. */ + asection *s; + void *vpp; + Elf_Internal_Sym *isym; + + isym = bfd_sym_from_r_symndx (&htab->sym_cache, + abfd, r_symndx); + if (isym == NULL) + return FALSE; + + s = bfd_section_from_elf_index (abfd, isym->st_shndx); + if (s == NULL) + s = sec; + + vpp = &elf_section_data (s)->local_dynrel; + head = (struct elf_dyn_relocs **) vpp; + } + + p = *head; + if (p == NULL || p->sec != sec) + { + bfd_size_type amt = sizeof *p; + + p = (struct elf_dyn_relocs *) bfd_alloc (htab->root.dynobj, amt); + if (p == NULL) + return FALSE; + p->next = *head; + *head = p; + p->sec = sec; + p->count = 0; + p->pc_count = 0; + } + + if (r_type == R_ARM_REL32 || r_type == R_ARM_REL32_NOI) + p->pc_count += 1; + p->count += 1; + } } return TRUE; Index: ld/testsuite/ld-arm/arm-lib-plt-2a.s =================================================================== --- /dev/null 2011-02-21 12:47:04.267827113 +0000 +++ ld/testsuite/ld-arm/arm-lib-plt-2a.s 2011-03-04 10:38:06.000000000 +0000 @@ -0,0 +1,5 @@ + .globl foo + .type foo,%function +foo: + mov pc,lr + .size foo,.-foo Index: ld/testsuite/ld-arm/arm-lib-plt-2b.s =================================================================== --- /dev/null 2011-02-21 12:47:04.267827113 +0000 +++ ld/testsuite/ld-arm/arm-lib-plt-2b.s 2011-03-04 10:38:06.000000000 +0000 @@ -0,0 +1,2 @@ + .data + .word foo Index: ld/testsuite/ld-arm/arm-lib-plt-2.dd =================================================================== --- /dev/null 2011-02-21 12:47:04.267827113 +0000 +++ ld/testsuite/ld-arm/arm-lib-plt-2.dd 2011-03-04 10:38:06.000000000 +0000 @@ -0,0 +1,4 @@ + +.*: file format .* + +# There shouldn't be any code at all. Index: ld/testsuite/ld-arm/arm-lib-plt-2.rd =================================================================== --- /dev/null 2011-02-21 12:47:04.267827113 +0000 +++ ld/testsuite/ld-arm/arm-lib-plt-2.rd 2011-03-04 10:38:06.000000000 +0000 @@ -0,0 +1,6 @@ + +Relocation section '.rel.dyn' .*: + Offset .* +.* R_ARM_ABS32 00000000 foo + +# There shouldn't be any .rel.plt relocations. Index: ld/testsuite/ld-arm/arm-elf.exp =================================================================== --- ld/testsuite/ld-arm/arm-elf.exp 2011-03-04 10:20:52.000000000 +0000 +++ ld/testsuite/ld-arm/arm-elf.exp 2011-03-04 10:38:06.000000000 +0000 @@ -80,6 +80,14 @@ set armelftests { {"Simple PIC shared library" "-shared" "" {arm-lib-plt32.s} {{objdump -fdw arm-lib-plt32.d} {objdump -Rw arm-lib-plt32.r}} "arm-lib-plt32.so"} + {"Indirect cross-library function reference (set-up)" + "-shared" "" {arm-lib-plt-2a.s} + {} + "arm-lib-plt-2a.so"} + {"Indirect cross-library function reference" + "-shared tmpdir/arm-lib-plt-2a.so" "" {arm-lib-plt-2b.s} + {{objdump -dr arm-lib-plt-2.dd} {readelf --relocs arm-lib-plt-2.rd}} + "arm-lib-plt-2b.so"} {"Simple dynamic application" "tmpdir/arm-lib.so" "" {arm-app.s} {{objdump -fdw arm-app.d} {objdump -Rw arm-app.r}} "arm-app"}