[2.43 PATCH] LoongArch: Fix broken DESC => IE transition for 2.43 branch

[Xi Ruoyao]

On Fri, 2024-12-27 at 14:12 +0800, Lulu Cai wrote:
> When I apply this patch on the 2.43 branch and compile with gcc 15, the 
> following warning is triggered.
> 
> binutils-gdb/bfd/elfnn-loongarch.c: In function 
> ‘loongarch_elf_relax_section’:
> binutils-gdb/bfd/elfnn-loongarch.c:5471:16: error: ‘symval’ may be used 
> uninitialized [-Werror=maybe-uninitialized]
> 5471 | symval += rel->r_addend;
> > ~~~~~~~^~~~~~~~~~~~~~~~~
> binutils-gdb/bfd/elfnn-loongarch.c:5328:15: note: ‘symval’ was declared here
> 5328 | bfd_vma symval;
> > ^~~~~~
> cc1: all warnings being treated as errors

Hmm it seems a compiler bug...  Whenever we set sym_sec to non-NULL,
symval is also set.  And line 5471 should be skipped with the "continue"
at line 5446 then if sym_sec is NULL.

I'll initialize the variable in v2 but also send a GCC bug report.

> On 12/19/24 1:23 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
> > Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> > ---
> >   bfd/elfnn-loongarch.c | 43 ++++++++++++++++++++++---------------------
> >   1 file changed, 22 insertions(+), 21 deletions(-)
> > 
> > diff --git a/bfd/elfnn-loongarch.c b/bfd/elfnn-loongarch.c
> > index 312707bb00b..1c34797c2e1 100644
> > --- a/bfd/elfnn-loongarch.c
> > +++ b/bfd/elfnn-loongarch.c
> > @@ -5326,7 +5326,7 @@ loongarch_elf_relax_section (bfd *abfd, asection *sec,
> >       {
> >         char symtype;
> >         bfd_vma symval;
> > -      asection *sym_sec;
> > +      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: 834b779ad28c3cecb5ad91a1ab2af3c8453c3b3a
> 
> 

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University