From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.loongson.cn (mail.loongson.cn [114.242.206.163]) by sourceware.org (Postfix) with ESMTP id 9A67E384AB7E for ; Fri, 19 Apr 2024 14:02:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 9A67E384AB7E Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=loongson.cn Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=loongson.cn ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 9A67E384AB7E Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=114.242.206.163 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713535369; cv=none; b=C0TIZskSuQJnc03WzTucz+08fOu2MXCTavu//u9Z8w/5BEYeQiDaDQex4DgqJadlsLk4AUjZXpFMD8BtJyGf7t/erxTyYCCOCuk1GUPjLlnIMOfUEgRDwHj6JdJdXE6zQm8i3E72xoAP6qNxj5ODDyn3Juu0GtEnQa8YrH2kqV0= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713535369; c=relaxed/simple; bh=Hwm5Y7h+NsheKx2u06NJ7XuO2kwrfalFPAFjCah2gcY=; h=Subject:To:From:Message-ID:Date:MIME-Version; b=Yl5hafs75lI9XfhdGnw+2r32O73a8RFBteY5RDUiUCkvxfRyi3y+czcgexwACG+KiKLvpbxBVykZh0nZqUmyRwbwkYOoOAw3VNJywFgzlWK0k/zP51f9WdPGyaQ05j4Yac4LtaamkmR1al8H++I6fzsgNTXDAWUM4pDcQ3brOAc= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from loongson.cn (unknown [10.20.4.171]) by gateway (Coremail) with SMTP id _____8Bx+vF6eSJmbggAAA--.334S3; Fri, 19 Apr 2024 22:02:34 +0800 (CST) Received: from [10.20.4.171] (unknown [10.20.4.171]) by localhost.localdomain (Coremail) with SMTP id AQAAf8CxMeB3eSJmGxcAAA--.597S3; Fri, 19 Apr 2024 22:02:32 +0800 (CST) Subject: Re: [PATCH v2] BFD: Fix the bug of R_LARCH_AGLIN caused by discard section To: Fangrui Song , Alan Modra Cc: binutils@sourceware.org, xuchenghua@loongson.cn, chenglulu@loongson.cn, liuzhensong@loongson.cn, cailulu@loongson.cn, xry111@xry111.site, i.swmail@xen0n.name, maskray@google.com, luweining@loongson.cn, wanglei@loongson.cn, hejinyang@loongson.cn References: <20240322082915.3900449-1-mengqinggang@loongson.cn> From: mengqinggang Message-ID: <3f357195-d95d-4215-cbfb-d9259f660d45@loongson.cn> Date: Fri, 19 Apr 2024 22:02:30 +0800 User-Agent: Mozilla/5.0 (X11; Linux mips64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-CM-TRANSID:AQAAf8CxMeB3eSJmGxcAAA--.597S3 X-CM-SenderInfo: 5phqw15lqjwttqj6z05rqj20fqof0/ X-Coremail-Antispam: 1Uk129KBj93XoWfXr18WF17GrW7XF47Aw4rXrc_yoWkWryDpr yUAF45CFW8AF1UWw1qv345AFsxZws7uF4j9a47tw40yrZ8tryFgF4vkrW3GFWDGrykCryr Xr10vayUZFyvyagCm3ZEXasCq-sJn29KB7ZKAUJUUUUU529EdanIXcx71UUUUU7KY7ZEXa sCq-sGcSsGvfJ3Ic02F40EFcxC0VAKzVAqx4xG6I80ebIjqfuFe4nvWSU5nxnvy29KBjDU 0xBIdaVrnRJUUUvjb4IE77IF4wAFF20E14v26r1j6r4UM7CY07I20VC2zVCF04k26cxKx2 IYs7xG6rWj6s0DM7CIcVAFz4kK6r106r15M28lY4IEw2IIxxk0rwA2F7IY1VAKz4vEj48v e4kI8wA2z4x0Y4vE2Ix0cI8IcVAFwI0_Jr0_JF4l84ACjcxK6xIIjxv20xvEc7CjxVAFwI 0_Jr0_Gr1l84ACjcxK6I8E87Iv67AKxVWUJVW8JwA2z4x0Y4vEx4A2jsIEc7CjxVAFwI0_ Jr0_Gr1le2I262IYc4CY6c8Ij28IcVAaY2xG8wAqjxCEc2xF0cIa020Ex4CE44I27wAqx4 xG64xvF2IEw4CE5I8CrVC2j2WlYx0E2Ix0cI8IcVAFwI0_Jr0_Jr4lYx0Ex4A2jsIE14v2 6r1j6r4UMcvjeVCFs4IE7xkEbVWUJVW8JwACjcxG0xvEwIxGrwCYjI0SjxkI62AI1cAE67 vIY487MxAIw28IcxkI7VAKI48JMxC20s026xCaFVCjc4AY6r1j6r4UMI8I3I0E5I8CrVAF wI0_Jr0_Jr4lx2IqxVCjr7xvwVAFwI0_JrI_JrWlx4CE17CEb7AF67AKxVWUtVW8ZwCIc4 0Y0x0EwIxGrwCI42IY6xIIjxv20xvE14v26r1j6r1xMIIF0xvE2Ix0cI8IcVCY1x0267AK xVWUJVW8JwCI42IY6xAIw20EY4v20xvaj40_Jr0_JF4lIxAIcVC2z280aVAFwI0_Jr0_Gr 1lIxAIcVC2z280aVCY1x0267AKxVWUJVW8JbIYCTnIWIevJa73UjIFyTuYvjxU25EfUUUU U X-Spam-Status: No, score=-13.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,KAM_DMARC_STATUS,NICE_REPLY_A,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: 在 2024/4/19 下午2:04, Fangrui Song 写道: > On Fri, Mar 22, 2024 at 1:29 AM mengqinggang wrote: >> To represent the first and third expression of .align, R_LARCH_ALIGN need to >> associate with a symbol. We defind a local symbol for R_LARCH_AGLIN. >> But if the section of the local symbo is discarded, it may result in >> a undefined symbol error. >> >> Instead, we use the section name symbols, and this does not need to >> add extra symbols. >> >> During partial linking (ld -r), if the symbol associated with a relocation is >> STT_SECTION type, the addend of relocation needs to add the section output >> offset. We prevent it for R_LARCH_ALIGN. >> >> The elf_backend_data.rela_normal only can set all relocations of a target to >> rela_normal. Add a new function is_rela_normal to elf_backend_data, it can >> set part of relocations to rela_normal. >> --- >> bfd/elf-bfd.h | 4 ++ >> bfd/elflink.c | 5 +- >> bfd/elfnn-loongarch.c | 16 ++++++ >> bfd/elfxx-target.h | 5 ++ >> gas/config/tc-loongarch.c | 5 +- >> gas/testsuite/gas/loongarch/relax_align.d | 56 ++++++++----------- >> .../ld-loongarch-elf/relax-align-discard.lds | 4 ++ >> .../ld-loongarch-elf/relax-align-discard.s | 17 ++++++ >> ld/testsuite/ld-loongarch-elf/relax.exp | 12 ++++ >> 9 files changed, 86 insertions(+), 38 deletions(-) >> create mode 100644 ld/testsuite/ld-loongarch-elf/relax-align-discard.lds >> create mode 100644 ld/testsuite/ld-loongarch-elf/relax-align-discard.s >> >> diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h >> index c5d325435b6..af507b93df5 100644 >> --- a/bfd/elf-bfd.h >> +++ b/bfd/elf-bfd.h >> @@ -1721,6 +1721,10 @@ struct elf_backend_data >> backend relocate_section routine for relocatable linking. */ >> unsigned rela_normal : 1; >> >> + /* Whether a relocation is rela_normal. Compared with rela_normal, >> + is_rela_normal can set part of relocations to rela_normal. */ >> + bool (*is_rela_normal) (Elf_Internal_Rela *); >> + >> /* Set if DT_REL/DT_RELA/DT_RELSZ/DT_RELASZ should not include PLT >> relocations. */ >> unsigned dtrel_excludes_plt : 1; >> diff --git a/bfd/elflink.c b/bfd/elflink.c >> index 5a6cb07b2ce..8223db98186 100644 >> --- a/bfd/elflink.c >> +++ b/bfd/elflink.c >> @@ -11692,7 +11692,10 @@ elf_link_input_bfd (struct elf_final_link_info *flinfo, bfd *input_bfd) >> { >> rel_hash = PTR_ADD (esdo->rela.hashes, esdo->rela.count); >> rela_hash_list = rel_hash; >> - rela_normal = bed->rela_normal; >> + if (bed->is_rela_normal != NULL) >> + rela_normal = bed->is_rela_normal (irela); >> + else >> + rela_normal = bed->rela_normal; >> } >> >> irela->r_offset = _bfd_elf_section_offset (output_bfd, >> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c >> index c42052f9321..1679aa5da7d 100644 >> --- a/bfd/elfnn-loongarch.c >> +++ b/bfd/elfnn-loongarch.c >> @@ -5454,6 +5454,21 @@ elf_loongarch64_hash_symbol (struct elf_link_hash_entry *h) >> return _bfd_elf_hash_symbol (h); >> } >> >> +/* If a relocation is rela_normal and the symbol associated with the >> + relocation is STT_SECTION type, the addend of the relocation would add >> + sec->output_offset when partial linking (ld -r). >> + See elf_backend_data.rela_normal and elf_link_input_bfd(). >> + The addend of R_LARCH_ALIGN is used to represent the first and third >> + expression of .align, it should be a constant when linking. */ >> + >> +static bool >> +loongarch_elf_is_rela_normal (Elf_Internal_Rela *rel) >> +{ >> + if (R_LARCH_ALIGN == ELFNN_R_TYPE (rel->r_info)) >> + return false; >> + return true; >> +} >> + >> #define TARGET_LITTLE_SYM loongarch_elfNN_vec >> #define TARGET_LITTLE_NAME "elfNN-loongarch" >> #define ELF_ARCH bfd_arch_loongarch >> @@ -5489,6 +5504,7 @@ elf_loongarch64_hash_symbol (struct elf_link_hash_entry *h) >> #define elf_backend_grok_psinfo loongarch_elf_grok_psinfo >> #define elf_backend_hash_symbol elf_loongarch64_hash_symbol >> #define bfd_elfNN_bfd_relax_section loongarch_elf_relax_section >> +#define elf_backend_is_rela_normal loongarch_elf_is_rela_normal >> >> #define elf_backend_dtrel_excludes_plt 1 >> >> diff --git a/bfd/elfxx-target.h b/bfd/elfxx-target.h >> index 1e6992b5793..6e2d948b69b 100644 >> --- a/bfd/elfxx-target.h >> +++ b/bfd/elfxx-target.h >> @@ -709,6 +709,10 @@ >> #define elf_backend_rela_normal 0 >> #endif >> >> +#ifndef elf_backend_is_rela_normal >> +#define elf_backend_is_rela_normal NULL >> +#endif >> + >> #ifndef elf_backend_dtrel_excludes_plt >> #define elf_backend_dtrel_excludes_plt 0 >> #endif >> @@ -955,6 +959,7 @@ static const struct elf_backend_data elfNN_bed = >> elf_backend_default_use_rela_p, >> elf_backend_rela_plts_and_copies_p, >> elf_backend_rela_normal, >> + elf_backend_is_rela_normal, >> elf_backend_dtrel_excludes_plt, >> elf_backend_sign_extend_vma, >> elf_backend_want_got_plt, >> diff --git a/gas/config/tc-loongarch.c b/gas/config/tc-loongarch.c >> index 30aefce36fd..6b1a89738ef 100644 >> --- a/gas/config/tc-loongarch.c >> +++ b/gas/config/tc-loongarch.c >> @@ -1791,10 +1791,7 @@ loongarch_frag_align_code (int n, int max) >> if (fragP->fr_subtype != 0 && offset > fragP->fr_subtype). */ >> if (max > 0 && (bfd_vma) max < worst_case_bytes) >> { >> - s = symbol_find (".Lla-relax-align"); >> - if (s == NULL) >> - s = (symbolS *)local_symbol_make (".Lla-relax-align", now_seg, >> - &zero_address_frag, 0); >> + s = symbol_find (now_seg->name); >> ex.X_add_symbol = s; >> ex.X_op = O_symbol; >> ex.X_add_number = (max << 8) | n; >> diff --git a/gas/testsuite/gas/loongarch/relax_align.d b/gas/testsuite/gas/loongarch/relax_align.d >> index fc1fd032611..6710927be1b 100644 >> --- a/gas/testsuite/gas/loongarch/relax_align.d >> +++ b/gas/testsuite/gas/loongarch/relax_align.d >> @@ -7,40 +7,30 @@ >> >> Disassembly of section .text: >> >> -[ ]*0000000000000000 <.Lla-relax-align>: >> -[ ]+0:[ ]+4c000020[ ]+ret >> -[ ]+4:[ ]+03400000[ ]+nop >> -[ ]+4: R_LARCH_ALIGN[ ]+\*ABS\*\+0xc >> +[ ]*0000000000000000 <.text>: >> +[ ]+0:[ ]+1a000004[ ]+pcalau12i[ ]+\$a0, 0 >> +[ ]+0: R_LARCH_PCALA_HI20[ ]+L1 >> +[ ]+0: R_LARCH_RELAX[ ]+\*ABS\* >> +[ ]+4:[ ]+02c00084[ ]+addi.d[ ]+\$a0, \$a0, 0 >> +[ ]+4: R_LARCH_PCALA_LO12[ ]+L1 >> +[ ]+4: R_LARCH_RELAX[ ]+\*ABS\* >> [ ]+8:[ ]+03400000[ ]+nop >> +[ ]+8: R_LARCH_ALIGN[ ]+.text\+0x4 >> [ ]+c:[ ]+03400000[ ]+nop >> -[ ]+10:[ ]+4c000020[ ]+ret >> -[ ]+14:[ ]+03400000[ ]+nop >> -[ ]+14: R_LARCH_ALIGN[ ]+\*ABS\*\+0xc >> -[ ]+18:[ ]+03400000[ ]+nop >> +[ ]+10:[ ]+03400000[ ]+nop >> +[ ]+14:[ ]+1a000004[ ]+pcalau12i[ ]+\$a0, 0 >> +[ ]+14: R_LARCH_PCALA_HI20[ ]+L1 >> +[ ]+14: R_LARCH_RELAX[ ]+\*ABS\* >> +[ ]+18:[ ]+02c00084[ ]+addi.d[ ]+\$a0, \$a0, 0 >> +[ ]+18: R_LARCH_PCALA_LO12[ ]+L1 >> +[ ]+18: R_LARCH_RELAX[ ]+\*ABS\* >> [ ]+1c:[ ]+03400000[ ]+nop >> -[ ]+20:[ ]+4c000020[ ]+ret >> +[ ]+1c: R_LARCH_ALIGN[ ]+.text\+0x404 >> +[ ]+20:[ ]+03400000[ ]+nop >> [ ]+24:[ ]+03400000[ ]+nop >> -[ ]+24: R_LARCH_ALIGN[ ]+.Lla-relax-align\+0x104 >> -[ ]+28:[ ]+03400000[ ]+nop >> -[ ]+2c:[ ]+03400000[ ]+nop >> -[ ]+30:[ ]+4c000020[ ]+ret >> -[ ]+34:[ ]+03400000[ ]+nop >> -[ ]+34: R_LARCH_ALIGN[ ]+.Lla-relax-align\+0xb04 >> -[ ]+38:[ ]+03400000[ ]+nop >> -[ ]+3c:[ ]+03400000[ ]+nop >> -[ ]+40:[ ]+4c000020[ ]+ret >> -[ ]+44:[ ]+03400000[ ]+nop >> -[ ]+44: R_LARCH_ALIGN[ ]+\*ABS\*\+0xc >> -[ ]+48:[ ]+03400000[ ]+nop >> -[ ]+4c:[ ]+03400000[ ]+nop >> -[ ]+50:[ ]+4c000020[ ]+ret >> -[ ]+54:[ ]+03400000[ ]+nop >> -[ ]+54: R_LARCH_ALIGN[ ]+\*ABS\*\+0xc >> -[ ]+58:[ ]+03400000[ ]+nop >> -[ ]+5c:[ ]+03400000[ ]+nop >> -[ ]+60:[ ]+4c000020[ ]+ret >> -[ ]+64:[ ]+03400000[ ]+nop >> -[ ]+64: R_LARCH_ALIGN[ ]+\*ABS\*\+0xc >> -[ ]+68:[ ]+03400000[ ]+nop >> -[ ]+6c:[ ]+03400000[ ]+nop >> -[ ]+70:[ ]+4c000020[ ]+ret >> +[ ]+28:[ ]+1a000004[ ]+pcalau12i[ ]+\$a0, 0 >> +[ ]+28: R_LARCH_PCALA_HI20[ ]+L1 >> +[ ]+28: R_LARCH_RELAX[ ]+\*ABS\* >> +[ ]+2c:[ ]+02c00084[ ]+addi.d[ ]+\$a0, \$a0, 0 >> +[ ]+2c: R_LARCH_PCALA_LO12[ ]+L1 >> +[ ]+2c: R_LARCH_RELAX[ ]+\*ABS\* >> diff --git a/ld/testsuite/ld-loongarch-elf/relax-align-discard.lds b/ld/testsuite/ld-loongarch-elf/relax-align-discard.lds >> new file mode 100644 >> index 00000000000..4a81323d926 >> --- /dev/null >> +++ b/ld/testsuite/ld-loongarch-elf/relax-align-discard.lds >> @@ -0,0 +1,4 @@ >> +SECTIONS >> +{ >> + /DISCARD/ : { *(.another.*) } >> +} >> diff --git a/ld/testsuite/ld-loongarch-elf/relax-align-discard.s b/ld/testsuite/ld-loongarch-elf/relax-align-discard.s >> new file mode 100644 >> index 00000000000..b65d63f370f >> --- /dev/null >> +++ b/ld/testsuite/ld-loongarch-elf/relax-align-discard.s >> @@ -0,0 +1,17 @@ >> +# Use the section name symbol for R_LARCH_ALIGN to avoid discard section problem >> +.section ".another.text", "ax" >> +.cfi_startproc >> +break 0 >> +.cfi_def_cfa_offset 16 >> +.p2align 5 >> +break 1 >> +.cfi_endproc >> + >> +.text >> +.cfi_startproc >> +break 0 >> +.cfi_def_cfa_offset 16 >> +.p2align 5 >> +break 1 >> +.cfi_endproc >> + >> diff --git a/ld/testsuite/ld-loongarch-elf/relax.exp b/ld/testsuite/ld-loongarch-elf/relax.exp >> index 7d95a9ca41d..ed71fb45b46 100644 >> --- a/ld/testsuite/ld-loongarch-elf/relax.exp >> +++ b/ld/testsuite/ld-loongarch-elf/relax.exp >> @@ -295,6 +295,18 @@ if [istarget loongarch64-*-*] { >> "relax-align" \ >> ] \ >> ] >> + >> + run_ld_link_tests \ >> + [list \ >> + [list \ >> + "loongarch relax align discard" \ >> + "-e 0x0 -T relax-align-discard.lds -r" "" \ >> + "" \ >> + {relax-align-discard.s} \ >> + {} \ >> + "relax-align-discard" \ >> + ] \ >> + ] >> } >> >> set objdump_flags "-s -j .data" >> -- >> 2.36.0 > > I just saw this was pushed as commit daeda14191c1710ce967259a47ef4e0a3fb6eebf. > > The addition of the generic elf_backend_is_rela_normal flag seems like > something a global maintainer should take a closer look at. > In particular, I'm curious if Alan, the author of the "rela_normal" > commit (b491616acb5462a3694160ffef6413c160fed10a), has any thoughts on > this. > > The idea appears to be > (https://github.com/loongson/la-abi-specs/blob/release/laelf.adoc#:~:text=R_LARCH_ALIGN) > > .text > break 1 > .p2align 4, , 8 // R_LARCH_ALIGN .text+0x0804 > break 8 > > In a relocatable link, the addend associated with the STT_SECTION > symbol is kept unchanged. Relocatable link merge input file text sections into one text section. If a relocation reference a section symbol, the addend would add the section offset in the final one text section. > >> But if the section of the local symbo is discarded, it may result in a undefined symbol error. > How does this happen when the R_LARCH_ALIGN relocation references > another local symbol instead of .text ? It needs to add an additional local symbol. And to avoid discarding section question (https://sourceware.org/pipermail/binutils/2024-January/131615.html), each section needs to add a local symbol.