From patchwork Wed Jun 26 10:04:51 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Xi Ruoyao X-Patchwork-Id: 92877 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id C4D1A387089C for ; Wed, 26 Jun 2024 10:06:14 +0000 (GMT) X-Original-To: binutils@sourceware.org Delivered-To: binutils@sourceware.org Received: from xry111.site (xry111.site [89.208.246.23]) by sourceware.org (Postfix) with ESMTPS id 2E746384AB57 for ; Wed, 26 Jun 2024 10:05:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2E746384AB57 Authentication-Results: sourceware.org; dmarc=pass (p=reject dis=none) header.from=xry111.site Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=xry111.site ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 2E746384AB57 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=89.208.246.23 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1719396322; cv=none; b=U6b2hm+d3M6k7nKwo42nrv+Ga9AtIYL+dSte2CN2JMYpDcMoVJ3AWOQFpf76SyAiXZtXxG1WizpJ/c2Rzc/OWidAF0r+T7PNChLtEnxWlgNGPSjCTLb31BNFM+QusbHjDW0peHR/srucLBeSfOBaP2Mym1Z6QB8BWx5BI4MKUpk= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1719396322; c=relaxed/simple; bh=UvRej48J6Y+aGWapzfSL1KBUY9UOaw8n1znEm0mpN1s=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=mVHc1kra9PlWkKdg711JfDVBT3jYvKap5XGJVDmolk+/+VzJXN+4MSQ3SlVSjDWfEdoEbp6XOqOsn5Dw3ayoO7kusxcrpZsL22scs5GdV0KeHulNw1Yw0KqEwM43SoKwYmtaQ1HHMQEkzpcjj23wmxyoURGQXKTtWa6WNpP5O9M= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=xry111.site; s=default; t=1719396314; bh=UvRej48J6Y+aGWapzfSL1KBUY9UOaw8n1znEm0mpN1s=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=lv4QQJyEapocpHuGSZ93EaYGRooQ7ob5gYNvHNF3s048g5CJ4IsTNLS9og8iCiHQq ktYrVpP0SabHydUhuHZ6krrHyPe6ajJgBlmq+eUoAjTKD0ZI+shzu+gFYVxzMB1d8E WDxFOxZ9umUt9zGDZzgXPLL5LYmO7ELrOa7xouqM= Received: from stargazer.. (unknown [113.200.174.20]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (Client did not present a certificate) (Authenticated sender: xry111@xry111.site) by xry111.site (Postfix) with ESMTPSA id 8598B66D69; Wed, 26 Jun 2024 06:05:12 -0400 (EDT) From: Xi Ruoyao To: binutils@sourceware.org Cc: mengqinggang@loongson.cn, Zhensong Liu , i.swmail@xen0n.name, maskray@google.com, Szabolcs Nagy , Xi Ruoyao Subject: [PATCH v2 2/5] LoongArch: Fix bad reloc with mixed visibility ifunc symbols in shared libraries Date: Wed, 26 Jun 2024 18:04:51 +0800 Message-ID: <20240626100453.98946-4-xry111@xry111.site> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20240626100453.98946-2-xry111@xry111.site> References: <20240626100453.98946-2-xry111@xry111.site> MIME-Version: 1.0 X-Spam-Status: No, score=-8.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_STOCKGEN, LIKELY_SPAM_FROM, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: binutils-bounces+patchwork=sourceware.org@sourceware.org With a simple test case: .globl ifunc .globl ifunc_hidden .hidden ifunc_hidden .type ifunc, %gnu_indirect_function .type ifunc_hidden, %gnu_indirect_function .text .align 2 ifunc: ret ifunc_hidden: ret test: bl ifunc bl ifunc_hidden "ld -shared" produces a shared object with one R_LARCH_NONE (instead of R_LARCH_JUMP_SLOT as we expect) to relocate the GOT entry of "ifunc". It's because the indices in .plt and .rela.plt mismatches for STV_DEFAULT STT_IFUNC symbols when another PLT entry exists for a STV_HIDDEN STT_IFUNC symbol, and such a mismatch breaks the logic of loongarch_elf_finish_dynamic_symbol. Fix the issue by reordering .plt so the indices no longer mismatch. Signed-off-by: Xi Ruoyao --- bfd/elfnn-loongarch.c | 77 ++++++++++++++++--- ld/testsuite/ld-loongarch-elf/ifunc-reloc.d | 19 +++++ ld/testsuite/ld-loongarch-elf/ifunc-reloc.s | 55 +++++++++++++ .../ld-loongarch-elf/ld-loongarch-elf.exp | 1 + 4 files changed, 140 insertions(+), 12 deletions(-) create mode 100644 ld/testsuite/ld-loongarch-elf/ifunc-reloc.d create mode 100644 ld/testsuite/ld-loongarch-elf/ifunc-reloc.s diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c index 3a55ac93e20..c02e3f4bd9c 100644 --- a/bfd/elfnn-loongarch.c +++ b/bfd/elfnn-loongarch.c @@ -1691,9 +1691,10 @@ local_allocate_ifunc_dyn_relocs (struct bfd_link_info *info, ifunc dynamic relocs. */ static bool -elfNN_allocate_ifunc_dynrelocs (struct elf_link_hash_entry *h, void *inf) +elfNN_allocate_ifunc_dynrelocs (struct elf_link_hash_entry *h, + struct bfd_link_info *info, + bool ref_local) { - struct bfd_link_info *info; /* An example of a bfd_link_hash_indirect symbol is versioned symbol. For example: __gxx_personality_v0(bfd_link_hash_indirect) -> __gxx_personality_v0(bfd_link_hash_defined) @@ -1709,20 +1710,18 @@ elfNN_allocate_ifunc_dynrelocs (struct elf_link_hash_entry *h, void *inf) if (h->root.type == bfd_link_hash_warning) h = (struct elf_link_hash_entry *) h->root.u.i.link; - info = (struct bfd_link_info *) inf; - /* Since STT_GNU_IFUNC symbol must go through PLT, we handle it here if it is defined and referenced in a non-shared object. */ if (h->type == STT_GNU_IFUNC && h->def_regular) { - if (SYMBOL_REFERENCES_LOCAL (info, h)) + if (ref_local && SYMBOL_REFERENCES_LOCAL (info, h)) return local_allocate_ifunc_dyn_relocs (info, h, &h->dyn_relocs, PLT_ENTRY_SIZE, PLT_HEADER_SIZE, GOT_ENTRY_SIZE, false); - else + else if (!ref_local && !SYMBOL_REFERENCES_LOCAL (info, h)) return _bfd_elf_allocate_ifunc_dyn_relocs (info, h, &h->dyn_relocs, PLT_ENTRY_SIZE, @@ -1734,6 +1733,23 @@ elfNN_allocate_ifunc_dynrelocs (struct elf_link_hash_entry *h, void *inf) return true; } +static bool +elfNN_allocate_ifunc_dynrelocs_ref_local (struct elf_link_hash_entry *h, + void *info) +{ + return elfNN_allocate_ifunc_dynrelocs (h, (struct bfd_link_info *) info, + true); +} + +static bool +elfNN_allocate_ifunc_dynrelocs_ref_global (struct elf_link_hash_entry *h, + void *info) +{ + return elfNN_allocate_ifunc_dynrelocs (h, (struct bfd_link_info *) info, + false); +} + + /* Allocate space in .plt, .got and associated reloc sections for ifunc dynamic relocs. */ @@ -1749,7 +1765,7 @@ elfNN_allocate_local_ifunc_dynrelocs (void **slot, void *inf) || h->root.type != bfd_link_hash_defined) abort (); - return elfNN_allocate_ifunc_dynrelocs (h, inf); + return elfNN_allocate_ifunc_dynrelocs_ref_local (h, inf); } /* Set DF_TEXTREL if we find any dynamic relocs that apply to @@ -1909,11 +1925,48 @@ loongarch_elf_late_size_sections (bfd *output_bfd, sym dynamic relocs. */ elf_link_hash_traverse (&htab->elf, allocate_dynrelocs, info); - /* Allocate global ifunc sym .plt and .got entries, and space for global - ifunc sym dynamic relocs. */ - elf_link_hash_traverse (&htab->elf, elfNN_allocate_ifunc_dynrelocs, info); - - /* Allocate .plt and .got entries, and space for local ifunc symbols. */ + /* Allocate global ifunc sym .plt and .got entries, and space for + *preemptible* ifunc sym dynamic relocs. Note that we must do it + for *all* preemptible ifunc (including local ifuncs and STV_HIDDEN + ifuncs) before doing it for any non-preemptible ifunc symbol: + assuming we are not so careful, when we link a shared library the + correlation of .plt and .rela.plt might look like: + + idx in .plt idx in .rela.plt + ext_func1@plt 0 0 + ext_func2@plt 1 1 + ext_func3@plt 2 2 + hidden_ifunc1@plt 3 None: it's in .rela.got + hidden_ifunc2@plt 4 None: it's in .rela.got + normal_ifunc1@plt 5 != 3 + normal_ifunc2@plt 6 != 4 + local_ifunc@plt 7 None: it's in .rela.got + + Now oops the indices for normal_ifunc{1,2} in .rela.plt were different + from the indices in .plt :(. This would break finish_dynamic_symbol + which assumes the index in .rela.plt matches the index in .plt. + + So let's be careful and make it correct: + + idx in .plt idx in .rela.plt + ext_func1@plt 0 0 + ext_func2@plt 1 1 + ext_func3@plt 2 2 + normal_ifunc1@plt 3 3 + normal_ifunc2@plt 4 4 + hidden_ifunc1@plt 5 None: it's in .rela.got + hidden_ifunc2@plt 6 None: it's in .rela.got + local_ifunc@plt 7 None: it's in .rela.got + + Now normal_ifuncs first. */ + elf_link_hash_traverse (&htab->elf, + elfNN_allocate_ifunc_dynrelocs_ref_global, info); + + /* Now hidden_ifuncs ifuncs. */ + elf_link_hash_traverse (&htab->elf, + elfNN_allocate_ifunc_dynrelocs_ref_local, info); + + /* Finally local ifuncs. */ htab_traverse (htab->loc_hash_table, elfNN_allocate_local_ifunc_dynrelocs, info); diff --git a/ld/testsuite/ld-loongarch-elf/ifunc-reloc.d b/ld/testsuite/ld-loongarch-elf/ifunc-reloc.d new file mode 100644 index 00000000000..cb592874b1e --- /dev/null +++ b/ld/testsuite/ld-loongarch-elf/ifunc-reloc.d @@ -0,0 +1,19 @@ +#ld: -shared +#readelf: -Wr + +#... +.*'\.rela\.dyn'.* +#... +.* R_LARCH_RELATIVE .* +.* R_LARCH_IRELATIVE .* +.* R_LARCH_IRELATIVE .* +.* R_LARCH_IRELATIVE .* +#... +.*'\.rela\.plt'.* +#... +.* R_LARCH_JUMP_SLOT .* +.* R_LARCH_JUMP_SLOT .* +.* R_LARCH_JUMP_SLOT .* +.* R_LARCH_JUMP_SLOT .* +.* R_LARCH_JUMP_SLOT .* +.* R_LARCH_JUMP_SLOT .* diff --git a/ld/testsuite/ld-loongarch-elf/ifunc-reloc.s b/ld/testsuite/ld-loongarch-elf/ifunc-reloc.s new file mode 100644 index 00000000000..e59f2b20e36 --- /dev/null +++ b/ld/testsuite/ld-loongarch-elf/ifunc-reloc.s @@ -0,0 +1,55 @@ +.globl foo +.globl foo_hidden1 +.globl foo_hidden2 +.globl foo_protected + +.type foo, %gnu_indirect_function +.type foo_hidden1, %gnu_indirect_function +.type foo_hidden2, %gnu_indirect_function +.type foo_protected, %gnu_indirect_function +.type foo_internal, %gnu_indirect_function + +.hidden foo_hidden1 +.hidden foo_hidden2 + +.protected foo_protected + +.globl ext_ifunc1 +.globl ext_ifunc2 +.type ext_ifunc1, %gnu_indirect_function +.type ext_ifunc2, %gnu_indirect_function + +.text +.align 2 +foo: + ret + +foo_hidden1: + ret + +foo_hidden2: + ret + +foo_protected: + ret + +foo_internal: + ret + +test: + la.got $a0, num + # The order is deliberately shuffled. + bl ext_ifunc1 + bl foo + bl foo_hidden1 + bl ext_func1 + bl foo_protected + bl foo_internal + bl foo_hidden2 + bl ext_func2 + bl ext_ifunc2 + +.data +.align 3 +num: + .quad 114514 diff --git a/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp b/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp index 4d9f4aebaff..6cff117e29b 100644 --- a/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp +++ b/ld/testsuite/ld-loongarch-elf/ld-loongarch-elf.exp @@ -133,6 +133,7 @@ if [istarget "loongarch64-*-*"] { run_dump_test "reloc_ler_with_shared" run_dump_test "reloc_abs_with_shared" run_dump_test "r_larch_32_elf64" + run_dump_test "ifunc-reloc" } if [check_pie_support] {