[PATCH v2] BFD: Fix the bug of R_LARCH_AGLIN caused by discard section
mengqinggang
mengqinggang@loongson.cn
Fri Apr 19 14:02:30 GMT 2024
在 2024/4/19 下午2:04, Fangrui Song 写道:
> 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.
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.
More information about the Binutils
mailing list