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 6DCA9384AB48 for ; Fri, 19 Apr 2024 08:06:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 6DCA9384AB48 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 6DCA9384AB48 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=1713513972; cv=none; b=qGpiQZr3Ov1bs98IvXKtK8PovEzCpyBO/zfttCxwnUleAx5XFcM0QTdvi1vCKW0ObPDAeZkCFPeyIu2r3flWbi9x8KmIz2k+Yi8U3O8MpKizMIBe/cBuUW/h5sp2cFzL//lceNPgKVgXoRkyW8LhsbU6ZwgrM5/AMc6hr++Rx8w= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713513972; c=relaxed/simple; bh=NNC43/7m6O6dFmoU1nsoux6sBrSspg5pbPs1/fE0Y+M=; h=Subject:To:From:Message-ID:Date:MIME-Version; b=vY8t1qNVsbp6ZZyLBTzmRO+hbf5nCbPmcICwx24FUG5ukZhJDoo8z07OMouVULNbuqDbIK7a3biON5CWWjECmAabLTzcQlPPh51BeCTW9sTmtoeAHmF1mR8IsLc3dhpu4h/g9VFtNI+o0sjcM8VgB32PaDAbZ7vieCK+jxMOil0= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from loongson.cn (unknown [111.9.175.10]) by gateway (Coremail) with SMTP id _____8DxebrlJSJm4bIpAA--.13273S3; Fri, 19 Apr 2024 16:05:57 +0800 (CST) Received: from [10.136.12.26] (unknown [111.9.175.10]) by localhost.localdomain (Coremail) with SMTP id AQAAf8CxrhPhJSJm9AGAAA--.56933S3; Fri, 19 Apr 2024 16:05:55 +0800 (CST) Subject: Re: [PATCH v2] BFD: Fix the bug of R_LARCH_AGLIN caused by discard section To: Fangrui Song , mengqinggang , 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 References: <20240322082915.3900449-1-mengqinggang@loongson.cn> From: Jinyang He Message-ID: <7452b3b1-034c-a68f-c449-4d888628c04b@loongson.cn> Date: Fri, 19 Apr 2024 16:05:53 +0800 User-Agent: Mozilla/5.0 (X11; Linux loongarch64; 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:AQAAf8CxrhPhJSJm9AGAAA--.56933S3 X-CM-SenderInfo: pkhmx0p1dqwqxorr0wxvrqhubq/ X-Coremail-Antispam: 1Uk129KBj93XoW3uw43Jry3Jw4xJr4rtw18WFX_yoWktFy7pr yUAF45CFW8AF1UWw1qv345JF47Zws7ur4jga47tw40yrZ8trySgF4vkFy3GFWDGr97Cryr XryjvayUZF9YyagCm3ZEXasCq-sJn29KB7ZKAUJUUUU8529EdanIXcx71UUUUU7KY7ZEXa sCq-sGcSsGvfJ3Ic02F40EFcxC0VAKzVAqx4xG6I80ebIjqfuFe4nvWSU5nxnvy29KBjDU 0xBIdaVrnRJUUUv2b4IE77IF4wAFF20E14v26r1j6r4UM7CY07I20VC2zVCF04k26cxKx2 IYs7xG6rWj6s0DM7CIcVAFz4kK6r1Y6r17M28lY4IEw2IIxxk0rwA2F7IY1VAKz4vEj48v e4kI8wA2z4x0Y4vE2Ix0cI8IcVAFwI0_Jr0_JF4l84ACjcxK6xIIjxv20xvEc7CjxVAFwI 0_Jr0_Gr1l84ACjcxK6I8E87Iv67AKxVW8Jr0_Cr1UM28EF7xvwVC2z280aVCY1x0267AK xVW8Jr0_Cr1UM2AIxVAIcxkEcVAq07x20xvEncxIr21l57IF6xkI12xvs2x26I8E6xACxx 1l5I8CrVACY4xI64kE6c02F40Ex7xfMcIj6xIIjxv20xvE14v26r106r15McIj6I8E87Iv 67AKxVWUJVW8JwAm72CE4IkC6x0Yz7v_Jr0_Gr1lF7xvr2IY64vIr41lc7I2V7IY0VAS07 AlzVAYIcxG8wCF04k20xvY0x0EwIxGrwCFx2IqxVCFs4IE7xkEbVWUJVW8JwC20s026c02 F40E14v26r1j6r18MI8I3I0E7480Y4vE14v26r106r1rMI8E67AF67kF1VAFwI0_Jw0_GF ylIxkGc2Ij64vIr41lIxAIcVC0I7IYx2IY67AKxVWUJVWUCwCI42IY6xIIjxv20xvEc7Cj xVAFwI0_Jr0_Gr1lIxAIcVCF04k26cxKx2IYs7xG6r1j6r1xMIIF0xvEx4A2jsIE14v26r 1j6r4UMIIF0xvEx4A2jsIEc7CjxVAFwI0_Jr0_GrUvcSsGvfC2KfnxnUUI43ZEXa7IU8j- e5UUUUU== X-Spam-Status: No, score=-13.5 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: On 2024-04-19 14:16, Fangrui Song wrote: > On Thu, Apr 18, 2024 at 11:04 PM Fangrui Song wrote: >> 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. >> >>> 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 ? > I should make it clear that I think this R_LARCH_ALIGN referencing > STT_SECTION with addend align+256*align_limit representation is > questionable. > Why do you break the regular semantics of STT_SECTION relocatable linking? > > Can an absolute symbol be used instead? Here just some my thoughts about ABS symbol. It cause more symbol cost in the "*.o" files. For ABS symbols, if several "*.o" files are linked together, there will be several extra symbols. Llvm works OK by creating ABS symbol (I tried, but forgot the details), but GNU AS is not. Because it applies its ABS value to addend (, Qinggang has investigated before.).