* [PATCH v2] BFD: Fix the bug of R_LARCH_AGLIN caused by discard section
@ 2024-03-22 8:29 mengqinggang
2024-04-19 6:04 ` Fangrui Song
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: mengqinggang @ 2024-03-22 8:29 UTC (permalink / raw)
To: binutils
Cc: xuchenghua, chenglulu, liuzhensong, cailulu, xry111, i.swmail,
maskray, luweining, wanglei, hejinyang, mengqinggang
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] BFD: Fix the bug of R_LARCH_AGLIN caused by discard section
2024-03-22 8:29 [PATCH v2] BFD: Fix the bug of R_LARCH_AGLIN caused by discard section mengqinggang
@ 2024-04-19 6:04 ` Fangrui Song
[not found] ` <CAN30aBHJen+LBPvSGhbgpFZsHB5CgDdp1TcGB1DgiRa6ZyG7hQ@mail.gmail.com>
[not found] ` <DS7PR12MB5765652E1B142D51C3BC88B1CB0D2@DS7PR12MB5765.namprd12.prod.outlook.com>
2 siblings, 0 replies; 8+ messages in thread
From: Fangrui Song @ 2024-04-19 6:04 UTC (permalink / raw)
To: mengqinggang, Alan Modra
Cc: binutils, xuchenghua, chenglulu, liuzhensong, cailulu, xry111,
i.swmail, maskray, luweining, wanglei, hejinyang
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 ?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] BFD: Fix the bug of R_LARCH_AGLIN caused by discard section
[not found] ` <CAN30aBHJen+LBPvSGhbgpFZsHB5CgDdp1TcGB1DgiRa6ZyG7hQ@mail.gmail.com>
@ 2024-04-19 6:16 ` Fangrui Song
2024-04-19 8:05 ` Jinyang He
0 siblings, 1 reply; 8+ messages in thread
From: Fangrui Song @ 2024-04-19 6:16 UTC (permalink / raw)
To: mengqinggang, Alan Modra
Cc: binutils, xuchenghua, chenglulu, liuzhensong, cailulu, xry111,
i.swmail, maskray, luweining, wanglei, hejinyang
On Thu, Apr 18, 2024 at 11:04 PM Fangrui Song <i@maskray.me> wrote:
>
> 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 ?
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?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] BFD: Fix the bug of R_LARCH_AGLIN caused by discard section
2024-04-19 6:16 ` Fangrui Song
@ 2024-04-19 8:05 ` Jinyang He
0 siblings, 0 replies; 8+ messages in thread
From: Jinyang He @ 2024-04-19 8:05 UTC (permalink / raw)
To: Fangrui Song, mengqinggang, Alan Modra
Cc: binutils, xuchenghua, chenglulu, liuzhensong, cailulu, xry111,
i.swmail, maskray, luweining, wanglei
On 2024-04-19 14:16, Fangrui Song wrote:
> On Thu, Apr 18, 2024 at 11:04 PM Fangrui Song <i@maskray.me> wrote:
>> 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 ?
> 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.).
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] BFD: Fix the bug of R_LARCH_AGLIN caused by discard section
[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>
0 siblings, 2 replies; 8+ messages in thread
From: mengqinggang @ 2024-04-19 14:02 UTC (permalink / raw)
To: Fangrui Song, Alan Modra
Cc: binutils, xuchenghua, chenglulu, liuzhensong, cailulu, xry111,
i.swmail, maskray, luweining, wanglei, hejinyang
在 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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] BFD: Fix the bug of R_LARCH_AGLIN caused by discard section
2024-04-19 14:02 ` mengqinggang
@ 2024-04-19 17:52 ` Fangrui Song
[not found] ` <DS7PR12MB57658BF4F8A3BB985411FE51CB0D2@DS7PR12MB5765.namprd12.prod.outlook.com>
1 sibling, 0 replies; 8+ messages in thread
From: Fangrui Song @ 2024-04-19 17:52 UTC (permalink / raw)
To: mengqinggang, hejinyang
Cc: Alan Modra, binutils, xuchenghua, chenglulu, liuzhensong,
cailulu, xry111, i.swmail, maskray, luweining, wanglei
> > On Thu, Apr 18, 2024 at 11:04 PM Fangrui Song <i@maskray.me> wrote:
> > 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.).
Elf64_Sym is relatively smaller with just 24 bytes (unlike other
control structures in ELF).
To bypass a specific oddity in relocatable linking, consider using a
SHN_ABS symbol.
You can define a SHN_ABS STB_GLOBAL STV_HIDDEN symbol to avoid
redundant copies within the link unit.
Duplicate SHN_ABS symbols of the same st_value do not cause duplicate
symbol definitions (except mold).
Alternatively, use STB_WEAK to make the deduplication intention clearer.
> > On Fri, Mar 22, 2024 at 1:29 AM mengqinggang <mengqinggang@loongson.cn> wrote:
> > 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.
This is exactly my concern. Using a STT_SECTION symbol requires a special case.
https://inbox.sourceware.org/binutils/20020506132720.GT3698@bubble.sa.bigpond.net.au/
specifies
> mips is the fly in the ointment here, and the reason for elf_backend_rela_normal.
!rela_normal is weird, and we should not introduce more weird stuff
(R_LARCH_ALIGN referencing STT_SECTION).
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] BFD: Fix the bug of R_LARCH_AGLIN caused by discard section
[not found] ` <DS7PR12MB57658BF4F8A3BB985411FE51CB0D2@DS7PR12MB5765.namprd12.prod.outlook.com>
@ 2024-04-30 11:30 ` mengqinggang
2024-05-01 4:06 ` Fangrui Song
0 siblings, 1 reply; 8+ messages in thread
From: mengqinggang @ 2024-04-30 11:30 UTC (permalink / raw)
To: Fangrui Song, hejinyang
Cc: Alan Modra, binutils, xuchenghua, chenglulu, liuzhensong,
cailulu, xry111, i.swmail, maskray, luweining, wanglei
在 2024/4/20 上午1:52, Fangrui Song 写道:
>>> On Thu, Apr 18, 2024 at 11:04 PM Fangrui Song <i@maskray.me> wrote:
>>> 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.).
> Elf64_Sym is relatively smaller with just 24 bytes (unlike other
> control structures in ELF).
> To bypass a specific oddity in relocatable linking, consider using a
> SHN_ABS symbol.
>
> You can define a SHN_ABS STB_GLOBAL STV_HIDDEN symbol to avoid
> redundant copies within the link unit.
> Duplicate SHN_ABS symbols of the same st_value do not cause duplicate
> symbol definitions (except mold).
> Alternatively, use STB_WEAK to make the deduplication intention clearer.
A SHN_ABS STB_GLOBAL/STB_GLOBAL STV_HIDDEN and non-zero symbol can be
referenced by a relocation.
A zero or local SHN_ABS symbol will directly add it's value to the
addend of the relocation.
rela_normal can prevent common code from handling MIPS relocations in a
relocatable link.
R_LARCH_ALIGN does not need to handle in a relocatable link. I prefer to
use section symbol
instead of adding an SHN_ABS symbol.
>
>>> On Fri, Mar 22, 2024 at 1:29 AM mengqinggang <mengqinggang@loongson.cn> wrote:
>>> 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.
> This is exactly my concern. Using a STT_SECTION symbol requires a special case.
>
> https://inbox.sourceware.org/binutils/20020506132720.GT3698@bubble.sa.bigpond.net.au/
> specifies
>
>> mips is the fly in the ointment here, and the reason for elf_backend_rela_normal.
> !rela_normal is weird, and we should not introduce more weird stuff
> (R_LARCH_ALIGN referencing STT_SECTION).
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] BFD: Fix the bug of R_LARCH_AGLIN caused by discard section
2024-04-30 11:30 ` mengqinggang
@ 2024-05-01 4:06 ` Fangrui Song
0 siblings, 0 replies; 8+ messages in thread
From: Fangrui Song @ 2024-05-01 4:06 UTC (permalink / raw)
To: mengqinggang
Cc: hejinyang, Alan Modra, binutils, xuchenghua, chenglulu,
liuzhensong, cailulu, xry111, i.swmail, maskray, luweining,
wanglei
On Tue, Apr 30, 2024 at 4:30 AM mengqinggang <mengqinggang@loongson.cn> wrote:
>
>
> 在 2024/4/20 上午1:52, Fangrui Song 写道:
> >>> On Thu, Apr 18, 2024 at 11:04 PM Fangrui Song <i@maskray.me> wrote:
> >>> 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.).
> > Elf64_Sym is relatively smaller with just 24 bytes (unlike other
> > control structures in ELF).
> > To bypass a specific oddity in relocatable linking, consider using a
> > SHN_ABS symbol.
> >
> > You can define a SHN_ABS STB_GLOBAL STV_HIDDEN symbol to avoid
> > redundant copies within the link unit.
> > Duplicate SHN_ABS symbols of the same st_value do not cause duplicate
> > symbol definitions (except mold).
> > Alternatively, use STB_WEAK to make the deduplication intention clearer.
>
>
> A SHN_ABS STB_GLOBAL/STB_GLOBAL STV_HIDDEN and non-zero symbol can be
> referenced by a relocation.
> A zero or local SHN_ABS symbol will directly add it's value to the
> addend of the relocation.
The SHN_ABS STB_GLOBAL STV_HIDDEN symbol can be undefined.
The linker can define it like other special symbols
(_GLOBAL_OFFSET_TABLE, .TOC., _TLS_MODULE_OFFSET_, _SDA_BASE_,
__global_pointer$).
> rela_normal can prevent common code from handling MIPS relocations in a
> relocatable link.
> R_LARCH_ALIGN does not need to handle in a relocatable link. I prefer to
> use section symbol
> instead of adding an SHN_ABS symbol.
Then we can remove the special case for R_LARCH_ALIGN when copying
relocations for -r / --emit-relocs.
>
> >
> >>> On Fri, Mar 22, 2024 at 1:29 AM mengqinggang <mengqinggang@loongson.cn> wrote:
> >>> 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.
> > This is exactly my concern. Using a STT_SECTION symbol requires a special case.
> >
> > https://inbox.sourceware.org/binutils/20020506132720.GT3698@bubble.sa.bigpond.net.au/
> > specifies
> >
> >> mips is the fly in the ointment here, and the reason for elf_backend_rela_normal.
> > !rela_normal is weird, and we should not introduce more weird stuff
> > (R_LARCH_ALIGN referencing STT_SECTION).
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-05-01 4:13 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-22 8:29 [PATCH v2] BFD: Fix the bug of R_LARCH_AGLIN caused by discard section mengqinggang
2024-04-19 6:04 ` Fangrui Song
[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
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).