From: Fangrui Song <i@maskray.me>
To: mengqinggang <mengqinggang@loongson.cn>, Alan Modra <amodra@gmail.com>
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
Subject: Re: [PATCH v2] BFD: Fix the bug of R_LARCH_AGLIN caused by discard section
Date: Thu, 18 Apr 2024 23:04:39 -0700 [thread overview]
Message-ID: <DS7PR12MB5765E5E87A84497C99BE9022CB0D2@DS7PR12MB5765.namprd12.prod.outlook.com> (raw)
In-Reply-To: <20240322082915.3900449-1-mengqinggang@loongson.cn>
On Fri, Mar 22, 2024 at 1:29 AM mengqinggang <mengqinggang@loongson.cn> 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 ?
next prev parent reply other threads:[~2024-04-19 6:13 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-22 8:29 mengqinggang
2024-04-19 6:04 ` Fangrui Song [this message]
[not found] ` <CAN30aBHJen+LBPvSGhbgpFZsHB5CgDdp1TcGB1DgiRa6ZyG7hQ@mail.gmail.com>
2024-04-19 6:16 ` Fangrui Song
2024-04-19 8:05 ` Jinyang He
[not found] ` <DS7PR12MB5765652E1B142D51C3BC88B1CB0D2@DS7PR12MB5765.namprd12.prod.outlook.com>
2024-04-19 14:02 ` mengqinggang
2024-04-19 17:52 ` Fangrui Song
[not found] ` <DS7PR12MB57658BF4F8A3BB985411FE51CB0D2@DS7PR12MB5765.namprd12.prod.outlook.com>
2024-04-30 11:30 ` mengqinggang
2024-05-01 4:06 ` Fangrui Song
[not found] ` <CAN30aBGUsRtSo-mY7ZeVvMLe06E6rBZjWgypg+3thmvi2uOa5g@mail.gmail.com>
2024-05-16 0:19 ` Fangrui Song
[not found] ` <DS7PR12MB5765981A7461D751519DF440CBED2@DS7PR12MB5765.namprd12.prod.outlook.com>
2024-05-16 7:09 ` mengqinggang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DS7PR12MB5765E5E87A84497C99BE9022CB0D2@DS7PR12MB5765.namprd12.prod.outlook.com \
--to=i@maskray.me \
--cc=amodra@gmail.com \
--cc=binutils@sourceware.org \
--cc=cailulu@loongson.cn \
--cc=chenglulu@loongson.cn \
--cc=hejinyang@loongson.cn \
--cc=i.swmail@xen0n.name \
--cc=liuzhensong@loongson.cn \
--cc=luweining@loongson.cn \
--cc=maskray@google.com \
--cc=mengqinggang@loongson.cn \
--cc=wanglei@loongson.cn \
--cc=xry111@xry111.site \
--cc=xuchenghua@loongson.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).