[2.43,RESEND,v2] LoongArch: Fix broken DESC => IE transition for 2.43 branch

Message ID 20241227082743.160489-1-xry111@xry111.site
State New
Headers
Series [2.43,RESEND,v2] LoongArch: Fix broken DESC => IE transition for 2.43 branch |

Commit Message

Xi Ruoyao Dec. 27, 2024, 8:26 a.m. UTC
  If code compiled with -fPIC -mtls-dialect=desc is linked into a PDE or
PIE, and the code refers to external DSO symbols, we can produce broken
link unit as check_relocs expects DESC => IE transition to happen and
emits a TLS IE entry in the GOT, but a too early "continue" in
relax_section actually jumps over the DESC => IE transition so the code
sequence is unchanged and still expecting a TLS descriptor (instead of
an IE entry) in the GOT.

The bug is already fixed in master branch by commit 5c3d09c1855b
("LoongArch: Optimize the relaxation process") so this fix is only
needed for the 2.43 branch.

Reported-by: Icenowy Zheng <uwu@icenowy.me>
Closes: https://gcc.gnu.org/PR118114
Tested-by: Icenowy Zheng <uwu@icenowy.me>
Signed-off-by: Xi Ruoyao <xry111@xry111.site>
---

v1 => v2: "Fix" GCC false warning https://gcc.gnu.org/PR118216.

Resending because I missed "v2" in the subject when I posted this the
first time :(.

 bfd/elfnn-loongarch.c | 45 ++++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 22 deletions(-)


base-commit: 0cdfcb9b8c4b726e755367c30cd54131e3f017eb
  

Comments

Lulu Cai Dec. 28, 2024, 8:33 a.m. UTC | #1
Thanks, the patch has been pushed to binutils-2_43-branch.

On 12/27/24 4:26 PM, Xi Ruoyao wrote:
> If code compiled with -fPIC -mtls-dialect=desc is linked into a PDE or
> PIE, and the code refers to external DSO symbols, we can produce broken
> link unit as check_relocs expects DESC => IE transition to happen and
> emits a TLS IE entry in the GOT, but a too early "continue" in
> relax_section actually jumps over the DESC => IE transition so the code
> sequence is unchanged and still expecting a TLS descriptor (instead of
> an IE entry) in the GOT.
>
> The bug is already fixed in master branch by commit 5c3d09c1855b
> ("LoongArch: Optimize the relaxation process") so this fix is only
> needed for the 2.43 branch.
>
> Reported-by: Icenowy Zheng <uwu@icenowy.me>
> Closes: https://gcc.gnu.org/PR118114
> Tested-by: Icenowy Zheng <uwu@icenowy.me>
> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> ---
>
> v1 => v2: "Fix" GCC false warning https://gcc.gnu.org/PR118216.
>
> Resending because I missed "v2" in the subject when I posted this the
> first time :(.
>
>   bfd/elfnn-loongarch.c | 45 ++++++++++++++++++++++---------------------
>   1 file changed, 23 insertions(+), 22 deletions(-)
>
> diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> index 312707bb00b..7760b5279f8 100644
> --- a/bfd/elfnn-loongarch.c
> +++ b/bfd/elfnn-loongarch.c
> @@ -5325,8 +5325,8 @@ loongarch_elf_relax_section (bfd *abfd, asection *sec,
>     for (unsigned int i = 0; i < sec->reloc_count; i++)
>       {
>         char symtype;
> -      bfd_vma symval;
> -      asection *sym_sec;
> +      bfd_vma symval = 0; /* "= 0" for https://gcc.gnu.org/PR118216 */
> +      asection *sym_sec = NULL;
>         bool local_got = false;
>         Elf_Internal_Rela *rel = relocs + i;
>         struct elf_link_hash_entry *h = NULL;
> @@ -5418,14 +5418,33 @@ loongarch_elf_relax_section (bfd *abfd, asection *sec,
>   	      symval = h->root.u.def.value;
>   	      sym_sec = h->root.u.def.section;
>   	    }
> -	  else
> -	    continue;
>   
>   	  if (h && LARCH_REF_LOCAL (info, h))
>   	    local_got = true;
>   	  symtype = h->type;
>   	}
>   
> +      /* If the conditions for tls type transition are met, type
> +	 transition is performed instead of relax.
> +	 During the transition from DESC->IE/LE, there are 2 situations
> +	 depending on the different configurations of the relax/norelax
> +	 option.
> +	 If the -relax option is used, the extra nops will be removed,
> +	 and this transition is performed in pass 0.
> +	 If the --no-relax option is used, nop will be retained, and
> +	 this transition is performed in pass 1.  */
> +      if (IS_LOONGARCH_TLS_TRANS_RELOC (r_type)
> +	  && (i + 1 != sec->reloc_count)
> +	  && ELFNN_R_TYPE (rel[1].r_info) == R_LARCH_RELAX
> +	  && loongarch_can_trans_tls (abfd, info, h, r_symndx, r_type))
> +	{
> +	  loongarch_tls_perform_trans (abfd, sec, rel, h, info);
> +	  r_type = ELFNN_R_TYPE (rel->r_info);
> +	}
> +
> +      if (!sym_sec)
> +	continue;
> +
>         if (sym_sec->sec_info_type == SEC_INFO_TYPE_MERGE
>   	   && (sym_sec->flags & SEC_MERGE))
>   	{
> @@ -5453,24 +5472,6 @@ loongarch_elf_relax_section (bfd *abfd, asection *sec,
>   
>         symval += sec_addr (sym_sec);
>   
> -      /* If the conditions for tls type transition are met, type
> -	 transition is performed instead of relax.
> -	 During the transition from DESC->IE/LE, there are 2 situations
> -	 depending on the different configurations of the relax/norelax
> -	 option.
> -	 If the -relax option is used, the extra nops will be removed,
> -	 and this transition is performed in pass 0.
> -	 If the --no-relax option is used, nop will be retained, and
> -	 this transition is performed in pass 1.  */
> -      if (IS_LOONGARCH_TLS_TRANS_RELOC (r_type)
> -	  && (i + 1 != sec->reloc_count)
> -	  && ELFNN_R_TYPE (rel[1].r_info) == R_LARCH_RELAX
> -	  && loongarch_can_trans_tls (abfd, info, h, r_symndx, r_type))
> -	{
> -	  loongarch_tls_perform_trans (abfd, sec, rel, h, info);
> -	  r_type = ELFNN_R_TYPE (rel->r_info);
> -	}
> -
>         switch (r_type)
>   	{
>   	case R_LARCH_ALIGN:
>
> base-commit: 0cdfcb9b8c4b726e755367c30cd54131e3f017eb
  

Patch

diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
index 312707bb00b..7760b5279f8 100644
--- a/bfd/elfnn-loongarch.c
+++ b/bfd/elfnn-loongarch.c
@@ -5325,8 +5325,8 @@  loongarch_elf_relax_section (bfd *abfd, asection *sec,
   for (unsigned int i = 0; i < sec->reloc_count; i++)
     {
       char symtype;
-      bfd_vma symval;
-      asection *sym_sec;
+      bfd_vma symval = 0; /* "= 0" for https://gcc.gnu.org/PR118216 */
+      asection *sym_sec = NULL;
       bool local_got = false;
       Elf_Internal_Rela *rel = relocs + i;
       struct elf_link_hash_entry *h = NULL;
@@ -5418,14 +5418,33 @@  loongarch_elf_relax_section (bfd *abfd, asection *sec,
 	      symval = h->root.u.def.value;
 	      sym_sec = h->root.u.def.section;
 	    }
-	  else
-	    continue;
 
 	  if (h && LARCH_REF_LOCAL (info, h))
 	    local_got = true;
 	  symtype = h->type;
 	}
 
+      /* If the conditions for tls type transition are met, type
+	 transition is performed instead of relax.
+	 During the transition from DESC->IE/LE, there are 2 situations
+	 depending on the different configurations of the relax/norelax
+	 option.
+	 If the -relax option is used, the extra nops will be removed,
+	 and this transition is performed in pass 0.
+	 If the --no-relax option is used, nop will be retained, and
+	 this transition is performed in pass 1.  */
+      if (IS_LOONGARCH_TLS_TRANS_RELOC (r_type)
+	  && (i + 1 != sec->reloc_count)
+	  && ELFNN_R_TYPE (rel[1].r_info) == R_LARCH_RELAX
+	  && loongarch_can_trans_tls (abfd, info, h, r_symndx, r_type))
+	{
+	  loongarch_tls_perform_trans (abfd, sec, rel, h, info);
+	  r_type = ELFNN_R_TYPE (rel->r_info);
+	}
+
+      if (!sym_sec)
+	continue;
+
       if (sym_sec->sec_info_type == SEC_INFO_TYPE_MERGE
 	   && (sym_sec->flags & SEC_MERGE))
 	{
@@ -5453,24 +5472,6 @@  loongarch_elf_relax_section (bfd *abfd, asection *sec,
 
       symval += sec_addr (sym_sec);
 
-      /* If the conditions for tls type transition are met, type
-	 transition is performed instead of relax.
-	 During the transition from DESC->IE/LE, there are 2 situations
-	 depending on the different configurations of the relax/norelax
-	 option.
-	 If the -relax option is used, the extra nops will be removed,
-	 and this transition is performed in pass 0.
-	 If the --no-relax option is used, nop will be retained, and
-	 this transition is performed in pass 1.  */
-      if (IS_LOONGARCH_TLS_TRANS_RELOC (r_type)
-	  && (i + 1 != sec->reloc_count)
-	  && ELFNN_R_TYPE (rel[1].r_info) == R_LARCH_RELAX
-	  && loongarch_can_trans_tls (abfd, info, h, r_symndx, r_type))
-	{
-	  loongarch_tls_perform_trans (abfd, sec, rel, h, info);
-	  r_type = ELFNN_R_TYPE (rel->r_info);
-	}
-
       switch (r_type)
 	{
 	case R_LARCH_ALIGN: