From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x133.google.com (mail-lf1-x133.google.com [IPv6:2a00:1450:4864:20::133]) by sourceware.org (Postfix) with ESMTPS id 9B3E3384AB5A for ; Thu, 16 May 2024 08:29:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9B3E3384AB5A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=rivosinc.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=rivosinc.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 9B3E3384AB5A Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::133 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1715848150; cv=none; b=okEO6aXcOzO2skUo0tWvVy6C1DNMjTP/jUdK2GKMRthGfEMSlsJ+xQagrYOcQMOlSnVkBQl+3VU7/7qxdxjJUBsVMi0vV8jiQBRN4Tr2agO+Hl5HXh/U0VNQcOF5i4ucrGs0UaW35b1imXRjTbgs8YDFRJTRlCmpflDn6w+9ze4= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1715848150; c=relaxed/simple; bh=wrU6Kxzb4qwusKLsJax9WVTwk3SV9K3HyQcgtroq8yQ=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=BhtMbnX1ZGnoyMBb90vxmTxKqlFuMYDCjLYhtLiPfWBT2uO4Amgg7yBagxQiK1vVUnUMZienjAFPWX3NgI57EWaIxFGXu0z+G1EJlR5CE8rhLNFtPlsiU/ekGIS5Qwwg5Guj1ONLRWruUVL6jB8uOCtMY8E482x1xH3LmCV2/dk= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-lf1-x133.google.com with SMTP id 2adb3069b0e04-51f57713684so709208e87.1 for ; Thu, 16 May 2024 01:29:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20230601.gappssmtp.com; s=20230601; t=1715848144; x=1716452944; darn=sourceware.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=TL2+mw9PDg7bEybEJZqGEzgiA1ll9y7og20DcMoggkI=; b=ALnRfUE9zcto95LO2xMPYrVAidctlupJHjCu/LIMgEyk1DIa1RhY0A7krAFXSjg1ik PhUj4MdJYqHRFTx8B6bbRS0fIJYqhzKsgtlGF1G3bW51iyM4vJvpRhCTd1clVGOruDF8 MzGJKjwkGH72gB/KD3rMOKgwcqIS/ahxUGKiBg5/EMPdGKlGbJVkCTMKKHVSwx1SvZlK Xes+kGDYS2MfNePIL5+nChuVKJTWW0ueKL/1ASjNkLDygyAg6UPkySpS4p9bE4Pg6VbS DHxFqSKkscykAjX7XTgmDab3pw54BzKxzadeTrdN/TXl65HrpLgHt8XU9TGaUDkTk2o6 ILzQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1715848144; x=1716452944; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=TL2+mw9PDg7bEybEJZqGEzgiA1ll9y7og20DcMoggkI=; b=KJRfbcbe+ptkMCCJTbeDJgb1smOx+6EOSy8cdNGH8ronSNa5x2xq9low9ATgp80qph 0TKgJWCyghjTeMHNLaczd7B9hyr1goLPOky4TOC4Vh8lm3LYzZjOA9ClhwDCZrI1rSue qVKyTAmu9PeeFhrxREyXoG2Ay9XwP2q+KgSoTDYDjXpQIXZfTwKpoDCwIUlYGVtLNnpo U0l6oPAFdVB1jBzc4HPZcJF+Cdz1yspoBw2z+IxVrgVEXNQSaa/XUiXBKElGoczc2aip iKjUcitEdllaQn8Gvaqw9z8UEpwSc393pwd1ujMIl8vKUkKowp75wJb3lgXAp3FVLMmJ aweg== X-Gm-Message-State: AOJu0Yy3s1YHdOTsV8L9lMWCr14nXrwTCZ1PVdpp74dJSxv7B+hBBrCp SZdQqsy1iipLlf0d50SnNhsSXpwCqn9JQDxQpQH9QSvCs28axl80Hwgsym/QH6AgyJYKATy47Sv KlgTnIqnlfMQKmP9YTriZFVmkBbKoBeCcFJi6Yw== X-Google-Smtp-Source: AGHT+IFZllSjoKD0oCz86mO6srvSXNHXqkiV+B27vryOjedKtZEM8juVEPycl6c7Z2mKSYOnzsgE2tuD8XFaP47kbj4= X-Received: by 2002:a05:6512:b07:b0:51f:51a6:eea8 with SMTP id 2adb3069b0e04-5220fa71dc5mr14939006e87.7.1715848143559; Thu, 16 May 2024 01:29:03 -0700 (PDT) MIME-Version: 1.0 References: <20240506044520.2780464-1-hau.hsu@sifive.com> <20240506044520.2780464-2-hau.hsu@sifive.com> <89C7C17E-5E9D-4E00-A3AE-FF1F014C0939@sifive.com> <6B8C7B37-CAB2-40E0-9C3F-CA8F8B37D367@sifive.com> In-Reply-To: <6B8C7B37-CAB2-40E0-9C3F-CA8F8B37D367@sifive.com> From: Nelson Chu Date: Thu, 16 May 2024 16:28:52 +0800 Message-ID: Subject: Re: [PATCH 2/2] riscv: Fix R_RISCV_IRELATIVE overwrite and order issues To: Hau Hsu Cc: Binutils , Kito Cheng Content-Type: multipart/alternative; boundary="0000000000001608c706188e0779" X-Spam-Status: No, score=-9.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,GIT_PATCH_0,HTML_MESSAGE,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,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 List-Id: --0000000000001608c706188e0779 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable I think the quite easy solution is - don't use riscv_elf_append_rela to emit dynamic IRELATIVE for R_RISCV_32/64 into iplt section. Seems like commit 51a8a7c2e3cc only handles the finish_dynamic_symbol, but forgot to apply a similar fix in the relocate_section. https://github.com/bminor/binutils-gdb/blob/master/bfd/elfnn-riscv.c#L2408 diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c index 604f6de4511..dca446c0495 100644 --- a/bfd/elfnn-riscv.c +++ b/bfd/elfnn-riscv.c @@ -2399,13 +2399,16 @@ riscv_elf_relocate_section (bfd *output_bfd, 2. .rela.got section in dynamic executable. 3. .rela.iplt section in static executable. */ if (bfd_link_pic (info)) - sreloc =3D htab->elf.irelifunc; + riscv_elf_append_rela (output_bfd, htab->elf.irelifunc, &outrel); else if (htab->elf.splt !=3D NULL) - sreloc =3D htab->elf.srelgot; + riscv_elf_append_rela (output_bfd, htab->elf.srelgot, &outrel); else - sreloc =3D htab->elf.irelplt; - - riscv_elf_append_rela (output_bfd, sreloc, &outrel); + { + const struct elf_backend_data *bed =3D get_elf_backend_data (output_bfd); + bfd_vma iplt_idx =3D htab->last_iplt_index--; + bfd_byte *loc =3D htab->elf.irelplt->contents + iplt_idx * sizeof (ElfNN_External_Rela); + bed->s->swap_reloca_out (output_bfd, &outrel, loc); + } /* If this reloc is against an external symbol, we do not want to fiddle with the addend. Otherwise, The above changes seem to fix the testcase you provided, but without testing fully riscv-gnu-toolchain regressions. Or we should find a way to handle reloc_index for iplt, and all use riscv_elf_append_rela to emit the dynamic relocation. Nelson On Thu, May 16, 2024 at 3:30=E2=80=AFPM Hau Hsu wrote: > > On May 16, 2024, at 3:07=E2=80=AFPM, Nelson Chu wro= te: > > On Thu, May 16, 2024 at 2:16=E2=80=AFPM Hau Hsu wrot= e: > >> Hi Nelson, >> >> Sorry for the late reply. >> >> On May 8, 2024, at 9:00=E2=80=AFAM, Nelson Chu wro= te: >> >> Can you provide the testcase to show the case which I mentioned in the >> pr13302? Which is that an ifunc with a dynamic jump slot calls another >> ifunc, and are not in the rel.dyn. >> >> I am not quite sure what's the issue you mentioned in PR13302. >> You mean the order issue of cause by one ifunc (generates >> JUMP_SLOT[32|64]) calls another JUMP_SLOT[32|64] ifunc? >> >> Since according to the testcase below, it seems no requirement to apply >> this fix though. >> >> The new test case uses two methods to call ifuncs: >> 1. Through a normal function call: PLT + GOT >> 2. Through a global function pointer: GOT only >> >> Without the fix, the relocation of the first method overwrites the >> second's. >> The relocation section of my test case would be: >> >> Relocation section '.rela.plt' at offset 0x94 contains 2 entries: >> Offset Info Type Sym. Value Symbol's Name + Adde= nd >> 000110dc 0000003a R_RISCV_IRELATIVE 100c0 >> 00000000 00000000 R_RISCV_NONE 0 >> >> >> With the fix, it becomes >> >> Relocation section '.rela.plt' at offset 0x94 contains 2 entries: >> Offset Info Type Sym. Value Symbol's Name + Adde= nd >> 000110e0 0000003a R_RISCV_IRELATIVE 100c0 >> 000110dc 0000003a R_RISCV_IRELATIVE 100c0 >> >> > So it seems like the overwrite problem, not the order problem we were > discussing in the pr13302... > > > Yes. Sorry that I didn't explain the whole story well. > > This PR is originally to fix the overwrite problem, as my commit message > says: > > This commit resolved two issues: > > 1. When an ifunc is referenced by a pointer, the relocation of > > the pointer in .rela.plt would be overwritten by normal ifunc call. > We found the issue when building glibc testbench statically. > > To fix this issue, I sent a PR ( > https://sourceware.org/pipermail/binutils/2023-July/128485.html) about a > year ago. > The PR use the method smilier to your previous commit, i.e. > > https://sourceware.org/git/?p=3Dbinutils-gdb.git;a=3Dcommit;h=3D51a8a7c2e= 3cc0730831963651a55d23d1fae624d > Then you suggested to check whether the relocation order is correct. > After that I checked the X86 implementation, I sent this PR. > > > >> --- /dev/null >>> +++ b/ld/testsuite/ld-riscv-elf/ifunc-macro.s >>> @@ -0,0 +1,19 @@ >>> +/* Define macros to handle similar behaviors for rv32/rv64. >>> + Assumes macro "__64_bit__" defined for rv64. >>> + The macro is specifically defined for ifunc tests in >>> ld-riscv-elf.exp. */ >>> + >>> +.macro PTR_DATA name >>> +.ifdef __64_bit__ >>> + .quad \name >>> +.else >>> + .long \name >>> +.endif >>> +.endm >>> + >>> +.macro LOAD rd, rs, offset >>> +.ifdef __64_bit__ >>> + ld \rd, \offset (\rs) >>> +.else >>> + lw \rd, \offset (\rs) >>> +.endif >>> +.endm >> >> > Btw, can we not use these macroes? > > > No problem. I just want to avoid similar codes in the ifunc tests. > > > Nelson > > > --0000000000001608c706188e0779--