From patchwork Fri Mar 4 16:05:54 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Sandiford X-Patchwork-Id: 333 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:47 -0000 Delivered-To: patches@linaro.org Received: by 10.224.60.68 with SMTP id o4cs19792qah; Fri, 4 Mar 2011 08:05:58 -0800 (PST) Received: by 10.227.169.77 with SMTP id x13mr691036wby.151.1299254757683; Fri, 04 Mar 2011 08:05:57 -0800 (PST) Received: from mail-wy0-f178.google.com (mail-wy0-f178.google.com [74.125.82.178]) by mx.google.com with ESMTPS id s1si4459140wbs.33.2011.03.04.08.05.57 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 04 Mar 2011 08:05:57 -0800 (PST) Received-SPF: neutral (google.com: 74.125.82.178 is neither permitted nor denied by best guess record for domain of richard.sandiford@linaro.org) client-ip=74.125.82.178; Authentication-Results: mx.google.com; spf=neutral (google.com: 74.125.82.178 is neither permitted nor denied by best guess record for domain of richard.sandiford@linaro.org) smtp.mail=richard.sandiford@linaro.org Received: by wyf28 with SMTP id 28so2655725wyf.37 for ; Fri, 04 Mar 2011 08:05:57 -0800 (PST) Received: by 10.216.56.65 with SMTP id l43mr690321wec.113.1299254757098; Fri, 04 Mar 2011 08:05:57 -0800 (PST) Received: from richards-thinkpad (gbibp9ph1--blueice2n1.emea.ibm.com [195.212.29.75]) by mx.google.com with ESMTPS id n1sm1243786weq.31.2011.03.04.08.05.55 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 04 Mar 2011 08:05:56 -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: [2/6] Always initialise ARM got entries in final_link_relocate Date: Fri, 04 Mar 2011 16:05:54 +0000 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 Current linkers generate dynamic GOT relocations for global-but-locally- binding symbols in executables. E.g.: .globl foo .type fo,%function foo: ldr r1,1f 1: .word foo(GOT) .size foo,.-foo generates a dynamic R_ARM_GLOB_DAT entry for foo (even though, of course, all other references to foo, such as through absolute accesses or calls, would not have dynamic reloctions). This behaviour isn't specific to ARM, but it seems like unnecessary bloat. The reason I looked at this is that we currently have a rather awkward split in the way we initialise GOT entries. We initialise GOT entries for dynamic symbols in elf32_arm_finish_dynamic_symbol, but we initalise all other GOT entries in elf32_arm_final_link_relocate. We then have to be careful which case we're dealing with: if (! WILL_CALL_FINISH_DYNAMIC_SYMBOL (dyn, info->shared, h) || (info->shared && SYMBOL_REFERENCES_LOCAL (info, h)) || (ELF_ST_VISIBILITY (h->other) && h->root.type == bfd_link_hash_undefweak)) { /* This is actually a static link, or it is a -Bsymbolic link and the symbol is defined locally. We must initialize this entry in the global offset table. Since the offset must always be a multiple of 4, we use the least significant bit to record whether we have initialized it already. When doing a dynamic link, we create a .rel(a).got relocation entry to initialize the value. This is done in the finish_dynamic_symbol routine. */ This gets even more complicated with ifuncs, where we may need irelative relocations. It seemed easier to handle everything in one place, and to fix the unnecessary relocs described above while there. The patch also includes a testcase (unresolved-1) to show why the dynamic_sections_created condition is important. We already pass the test, but I thought it was worth including to make sure that we don't regress in future. Tested on arm-linux-gnueabi. OK to install? Richard bfd/ * elf32-arm.c (elf32_arm_final_link_relocate): Always fill in the GOT entry here, rather than leaving it to finish_dynamic_symbol. Only create a dynamic relocation for local references if info->shared. (allocate_dynrelocs_for_symbol): Update dynamic relocation allocation accordingly. (elf32_arm_finish_dynamic_symbol): Don't initialise the GOT entry here. ld/testsuite/ * ld-arm/exec-got-1a.s, ld-arm/exec-got-1b.s, ld-arm/exec-got-1.d, ld-arm/unresolved-1.s, ld-arm/unresolved-1.d, ld-arm/unresolved-1-dyn.d: New tests. * ld-arm/arm-elf.exp: Run them. Index: bfd/elf32-arm.c =================================================================== --- bfd/elf32-arm.c 2011-03-04 10:41:18.000000000 +0000 +++ bfd/elf32-arm.c 2011-03-04 15:57:59.000000000 +0000 @@ -8278,45 +8278,67 @@ elf32_arm_final_link_relocate (reloc_how if (h != NULL) { bfd_vma off; - bfd_boolean dyn; off = h->got.offset; BFD_ASSERT (off != (bfd_vma) -1); - dyn = globals->root.dynamic_sections_created; + if ((off & 1) != 0) + { + /* We have already processsed one GOT relocation against + this symbol. */ + off &= ~1; + if (globals->root.dynamic_sections_created + && !SYMBOL_REFERENCES_LOCAL (info, h)) + *unresolved_reloc_p = FALSE; + } + else + { + Elf_Internal_Rela outrel; - if (! WILL_CALL_FINISH_DYNAMIC_SYMBOL (dyn, info->shared, h) - || (info->shared - && SYMBOL_REFERENCES_LOCAL (info, h)) - || (ELF_ST_VISIBILITY (h->other) - && h->root.type == bfd_link_hash_undefweak)) - { - /* This is actually a static link, or it is a -Bsymbolic link - and the symbol is defined locally. We must initialize this - entry in the global offset table. Since the offset must - always be a multiple of 4, we use the least significant bit - to record whether we have initialized it already. - - When doing a dynamic link, we create a .rel(a).got relocation - entry to initialize the value. This is done in the - finish_dynamic_symbol routine. */ - if ((off & 1) != 0) - off &= ~1; + if (!SYMBOL_REFERENCES_LOCAL (info, h)) + { + /* If the symbol doesn't resolve locally in a static + object, we have an undefined reference. If the + symbol doesn't resolve locally in a dynamic object, + it should be resolved by the dynamic linker. */ + if (globals->root.dynamic_sections_created) + { + outrel.r_info = ELF32_R_INFO (h->dynindx, R_ARM_GLOB_DAT); + *unresolved_reloc_p = FALSE; + } + else + outrel.r_info = 0; + outrel.r_addend = 0; + } else { - /* If we are addressing a Thumb function, we need to - adjust the address by one, so that attempts to - call the function pointer will correctly - interpret it as Thumb code. */ + if (info->shared) + outrel.r_info = ELF32_R_INFO (0, R_ARM_RELATIVE); + else + outrel.r_info = 0; + outrel.r_addend = value; if (sym_flags == STT_ARM_TFUNC) - value |= 1; + outrel.r_addend |= 1; + } - bfd_put_32 (output_bfd, value, sgot->contents + off); - h->got.offset |= 1; + /* The GOT entry is initialized to zero by default. + See if we should install a different value. */ + if (outrel.r_addend != 0 + && (outrel.r_info == 0 || globals->use_rel)) + { + bfd_put_32 (output_bfd, outrel.r_addend, + sgot->contents + off); + outrel.r_addend = 0; } - } - else - *unresolved_reloc_p = FALSE; + if (outrel.r_info != 0) + { + outrel.r_offset = (sgot->output_section->vma + + sgot->output_offset + + off); + elf32_arm_add_dynreloc (output_bfd, info, srelgot, &outrel); + } + h->got.offset |= 1; + } value = sgot->output_offset + off; } else @@ -12222,10 +12244,14 @@ allocate_dynrelocs (struct elf_link_hash if ((tls_type & GOT_TLS_GD) && indx != 0) elf32_arm_allocate_dynrelocs (info, htab->root.srelgot, 1); } - else if ((ELF_ST_VISIBILITY (h->other) == STV_DEFAULT - || h->root.type != bfd_link_hash_undefweak) - && (info->shared - || WILL_CALL_FINISH_DYNAMIC_SYMBOL (dyn, 0, h))) + else if (!SYMBOL_REFERENCES_LOCAL (info, h)) + { + if (htab->root.dynamic_sections_created) + /* Reserve room for the GOT entry's R_ARM_GLOB_DAT relocation. */ + elf32_arm_allocate_dynrelocs (info, htab->root.srelgot, 1); + } + else if (info->shared) + /* Reserve room for the GOT entry's R_ARM_RELATIVE relocation. */ elf32_arm_allocate_dynrelocs (info, htab->root.srelgot, 1); } } @@ -13016,53 +13042,6 @@ elf32_arm_finish_dynamic_symbol (bfd * o } } - if (h->got.offset != (bfd_vma) -1 - && (! GOT_TLS_GD_ANY_P (elf32_arm_hash_entry (h)->tls_type)) - && (elf32_arm_hash_entry (h)->tls_type & GOT_TLS_IE) == 0) - { - asection * sgot; - asection * srel; - Elf_Internal_Rela rel; - bfd_vma offset; - - /* This symbol has an entry in the global offset table. Set it - up. */ - sgot = htab->root.sgot; - srel = htab->root.srelgot; - BFD_ASSERT (sgot != NULL && srel != NULL); - - offset = (h->got.offset & ~(bfd_vma) 1); - rel.r_addend = 0; - rel.r_offset = (sgot->output_section->vma - + sgot->output_offset - + offset); - - /* If this is a static link, or it is a -Bsymbolic link and the - symbol is defined locally or was forced to be local because - of a version file, we just want to emit a RELATIVE reloc. - The entry in the global offset table will already have been - initialized in the relocate_section function. */ - if (info->shared - && SYMBOL_REFERENCES_LOCAL (info, h)) - { - BFD_ASSERT ((h->got.offset & 1) != 0); - rel.r_info = ELF32_R_INFO (0, R_ARM_RELATIVE); - if (!htab->use_rel) - { - rel.r_addend = bfd_get_32 (output_bfd, sgot->contents + offset); - bfd_put_32 (output_bfd, (bfd_vma) 0, sgot->contents + offset); - } - } - else - { - BFD_ASSERT ((h->got.offset & 1) == 0); - bfd_put_32 (output_bfd, (bfd_vma) 0, sgot->contents + offset); - rel.r_info = ELF32_R_INFO (h->dynindx, R_ARM_GLOB_DAT); - } - - elf32_arm_add_dynreloc (output_bfd, info, srel, &rel); - } - if (h->needs_copy) { asection * s; Index: ld/testsuite/ld-arm/exec-got-1a.s =================================================================== --- /dev/null 2011-02-21 12:47:04.267827113 +0000 +++ ld/testsuite/ld-arm/exec-got-1a.s 2011-03-04 12:57:49.000000000 +0000 @@ -0,0 +1,5 @@ + .globl foo + .type foo,%object + .size foo,4 + .data +foo: .word 1 Index: ld/testsuite/ld-arm/exec-got-1b.s =================================================================== --- /dev/null 2011-02-21 12:47:04.267827113 +0000 +++ ld/testsuite/ld-arm/exec-got-1b.s 2011-03-04 12:57:49.000000000 +0000 @@ -0,0 +1,10 @@ + .globl _start + .type _start,%function +_start: + ldr r1,1f + ldr r1,2f +1: + .word foo(GOT) +2: + .word _start(GOT) + .size _start,.-_start Index: ld/testsuite/ld-arm/exec-got-1.d =================================================================== --- /dev/null 2011-02-21 12:47:04.267827113 +0000 +++ ld/testsuite/ld-arm/exec-got-1.d 2011-03-04 12:57:49.000000000 +0000 @@ -0,0 +1,4 @@ + +Relocation section '\.rel\.dyn' .* + Offset .* +.* R_ARM_GLOB_DAT * 00000000 * foo Index: ld/testsuite/ld-arm/unresolved-1.s =================================================================== --- /dev/null 2011-02-21 12:47:04.267827113 +0000 +++ ld/testsuite/ld-arm/unresolved-1.s 2011-03-04 12:57:49.000000000 +0000 @@ -0,0 +1,6 @@ + .globl _start +_start: + ldr r4,1f + mov pc,lr +1: + .word foo(GOT) Index: ld/testsuite/ld-arm/unresolved-1.d =================================================================== --- /dev/null 2011-02-21 12:47:04.267827113 +0000 +++ ld/testsuite/ld-arm/unresolved-1.d 2011-03-04 12:57:49.000000000 +0000 @@ -0,0 +1,8 @@ +#ld: --warn-unresolved +#warning: \(\.text\+0x8\): warning: undefined reference to `foo' +#objdump: -sj.rel.dyn -sj.got + +.* + +Contents of section \.got: + *[^ ]* 00000000 00000000 00000000 00000000 .* Index: ld/testsuite/ld-arm/unresolved-1-dyn.d =================================================================== --- /dev/null 2011-02-21 12:47:04.267827113 +0000 +++ ld/testsuite/ld-arm/unresolved-1-dyn.d 2011-03-04 12:57:49.000000000 +0000 @@ -0,0 +1,8 @@ +#source: unresolved-1.s +#ld: --warn-unresolved tmpdir/mixed-lib.so +#warning: \(\.text\+0x8\): warning: undefined reference to `foo' +#readelf: -r + +Relocation section '\.rel\.dyn' .* + Offset .* +.* R_ARM_GLOB_DAT +00000000 +foo Index: ld/testsuite/ld-arm/arm-elf.exp =================================================================== --- ld/testsuite/ld-arm/arm-elf.exp 2011-03-04 10:40:28.000000000 +0000 +++ ld/testsuite/ld-arm/arm-elf.exp 2011-03-04 15:57:05.000000000 +0000 @@ -295,6 +295,14 @@ set armelftests { {"Data only mapping symbols" "-T data-only-map.ld -Map map" "" {data-only-map.s} {{objdump -dr data-only-map.d}} "data-only-map"} + {"GOT relocations in executables (setup)" "-shared" + "" {exec-got-1a.s} + {} + "exec-got-1.so"} + {"GOT relocations in executables" "tmpdir/exec-got-1.so" + "" {exec-got-1b.s} + {{readelf --relocs exec-got-1.d}} + "exec-got-1"} } run_ld_link_tests $armelftests @@ -582,3 +590,5 @@ run_dump_test "attr-merge-vfp-5r" run_dump_test "attr-merge-vfp-6" run_dump_test "attr-merge-vfp-6r" run_dump_test "attr-merge-incompatible" +run_dump_test "unresolved-1" +run_dump_test "unresolved-1-dyn"