[v6] MIPS: Reject branch absolute relocs for PIC for linking

  The asm code like:
	b	8
will emit absolute relocs like:
	R_MIPS_PC16	*ABS*

If they are included into PIC shared objects or PIE executables,
the branch target will be like 0x12340000, which will make the
programs crash.
---
 bfd/elfxx-mips.c                                | 9 +++++++++
 ld/testsuite/ld-mips-elf/mips-elf.exp           | 1 +
 ld/testsuite/ld-mips-elf/pic-reject-abs-reloc.d | 5 +++++
 ld/testsuite/ld-mips-elf/pic-reject-abs-reloc.s | 2 ++
 4 files changed, 17 insertions(+)
 create mode 100644 ld/testsuite/ld-mips-elf/pic-reject-abs-reloc.d
 create mode 100644 ld/testsuite/ld-mips-elf/pic-reject-abs-reloc.s
  

A random comment:

On Wed, 21 Feb 2024, YunQiang Su wrote:

> The asm code like:
> 	b	8
> will emit absolute relocs like:
> 	R_MIPS_PC16	*ABS*
> 
> If they are included into PIC shared objects or PIE executables,
> the branch target will be like 0x12340000, which will make the
> programs crash.
> ---
>  bfd/elfxx-mips.c                                | 9 +++++++++
>  ld/testsuite/ld-mips-elf/mips-elf.exp           | 1 +
>  ld/testsuite/ld-mips-elf/pic-reject-abs-reloc.d | 5 +++++
>  ld/testsuite/ld-mips-elf/pic-reject-abs-reloc.s | 2 ++
>  4 files changed, 17 insertions(+)
>  create mode 100644 ld/testsuite/ld-mips-elf/pic-reject-abs-reloc.d
>  create mode 100644 ld/testsuite/ld-mips-elf/pic-reject-abs-reloc.s
> 
> diff --git a/bfd/elfxx-mips.c b/bfd/elfxx-mips.c
> index 69dd71419ff..9542250dec4 100644
> --- a/bfd/elfxx-mips.c
> +++ b/bfd/elfxx-mips.c
> @@ -9258,6 +9258,15 @@ _bfd_mips_elf_check_relocs (bfd *abfd, struct bfd_link_info *info,
>  		   (h) ? h->root.root.string : "a local symbol");
>  	      break;
>  	    default:
> +	      if (branch_reloc_p (r_type) && r_symndx == STN_UNDEF)
> +		{
> +		  howto = MIPS_ELF_RTYPE_TO_HOWTO (abfd, r_type, NEWABI_P (abfd));
> +		  info->callbacks->einfo
> +		    /* xgettext:c-format */
> +		    (_("%X%H: relocation %s against `*ABS*' cannot be used"

There's no "*ABS*" in the source and IMHO that'd look confusing 
to innocent users.  How about "...against an absolute value"?  
Or "...against an absolute value or absolute symbol"?  Perhaps 
the latter is a bit too wordy, but also more complete.

> +		       " when making a PIC/PIE object\n"),
> +		     abfd, sec, rel->r_offset, howto->name);
> +		}
>  	      break;

brgds, H-P
  

> > +               howto = MIPS_ELF_RTYPE_TO_HOWTO (abfd, r_type, NEWABI_P (abfd));
> > +               info->callbacks->einfo
> > +                 /* xgettext:c-format */
> > +                 (_("%X%H: relocation %s against `*ABS*' cannot be used"
>
> There's no "*ABS*" in the source and IMHO that'd look confusing
> to innocent users.  How about "...against an absolute value"?
> Or "...against an absolute value or absolute symbol"?  Perhaps
> the latter is a bit too wordy, but also more complete.
>

Good idea. I think that we should use "...against an absolute value".
"absolute symbol"  is not included in this code, as it's r_symndx is
not STN_UNDEF.
An example is gas/testsuite/gas/mips/branch-absolute.s.

Thank you,
  

On Thu, 22 Feb 2024, YunQiang Su wrote:

> > > +               howto = MIPS_ELF_RTYPE_TO_HOWTO (abfd, r_type, NEWABI_P (abfd));
> > > +               info->callbacks->einfo
> > > +                 /* xgettext:c-format */
> > > +                 (_("%X%H: relocation %s against `*ABS*' cannot be used"
> >
> > There's no "*ABS*" in the source and IMHO that'd look confusing
> > to innocent users.  How about "...against an absolute value"?
> > Or "...against an absolute value or absolute symbol"?  Perhaps
> > the latter is a bit too wordy, but also more complete.
> >
> 
> Good idea. I think that we should use "...against an absolute value".
> "absolute symbol"  is not included in this code, as it's r_symndx is
> not STN_UNDEF.

 Well code says otherwise:

+	      if (branch_reloc_p (r_type) && r_symndx == STN_UNDEF)

did you mean "the relocation's r_symndx is STN_UNDEF"?

 But it can be a relocation against an absolute symbol, for example if the 
symbol referred to by a relocation is supplied by another module in the 
link or a linker script and said symbol is absolute.  This case has to be 
covered in testing as well, as I infer from your code it won't be handled 
correctly.

 In fact this situation is not unique to branch relocations, because no 
PC-relative relocation against an absolute symbol or value can be resolved 
at link time for PIC/PIE links.  So you need to make your code generic for 
any relocations that have the `pc_relative' flag set in their howto unless 
we have a way to make a dynamic relocation out of such a relocation (which 
is none right now, as we still only have R_MIPS_REL32 as the sole dynamic 
relocation, except for TLS stuff).

 Extra test cases will be appreciated if you can come up with ones.

  Maciej
  

Maciej W. Rozycki <macro@orcam.me.uk> 于2024年2月22日周四 21:13写道：
>
> On Thu, 22 Feb 2024, YunQiang Su wrote:
>
> > > > +               howto = MIPS_ELF_RTYPE_TO_HOWTO (abfd, r_type, NEWABI_P (abfd));
> > > > +               info->callbacks->einfo
> > > > +                 /* xgettext:c-format */
> > > > +                 (_("%X%H: relocation %s against `*ABS*' cannot be used"
> > >
> > > There's no "*ABS*" in the source and IMHO that'd look confusing
> > > to innocent users.  How about "...against an absolute value"?
> > > Or "...against an absolute value or absolute symbol"?  Perhaps
> > > the latter is a bit too wordy, but also more complete.
> > >
> >
> > Good idea. I think that we should use "...against an absolute value".
> > "absolute symbol"  is not included in this code, as it's r_symndx is
> > not STN_UNDEF.
>
>  Well code says otherwise:
>
> +             if (branch_reloc_p (r_type) && r_symndx == STN_UNDEF)
>
> did you mean "the relocation's r_symndx is STN_UNDEF"?
>
>  But it can be a relocation against an absolute symbol, for example if the
> symbol referred to by a relocation is supplied by another module in the
> link or a linker script and said symbol is absolute.  This case has to be

I have no idea whether we should emit this error for this case:
   If the symbol is set by a linker script, and is near enough with
our instruction,
   the object should be ok for PIC?
If not near enough, a "relocation truncated to fit" error will emit.

> covered in testing as well, as I infer from your code it won't be handled
> correctly.
>
>  In fact this situation is not unique to branch relocations, because no
> PC-relative relocation against an absolute symbol or value can be resolved
> at link time for PIC/PIE links.  So you need to make your code generic for
> any relocations that have the `pc_relative' flag set in their howto unless
> we have a way to make a dynamic relocation out of such a relocation (which
> is none right now, as we still only have R_MIPS_REL32 as the sole dynamic
> relocation, except for TLS stuff).
>
>  Extra test cases will be appreciated if you can come up with ones.
>
>   Maciej
  

On Sun, 25 Feb 2024, YunQiang Su wrote:

> > > Good idea. I think that we should use "...against an absolute value".
> > > "absolute symbol"  is not included in this code, as it's r_symndx is
> > > not STN_UNDEF.
> >
> >  Well code says otherwise:
> >
> > +             if (branch_reloc_p (r_type) && r_symndx == STN_UNDEF)
> >
> > did you mean "the relocation's r_symndx is STN_UNDEF"?
> >
> >  But it can be a relocation against an absolute symbol, for example if the
> > symbol referred to by a relocation is supplied by another module in the
> > link or a linker script and said symbol is absolute.  This case has to be
> 
> I have no idea whether we should emit this error for this case:
>    If the symbol is set by a linker script, and is near enough with
> our instruction,
>    the object should be ok for PIC?
> If not near enough, a "relocation truncated to fit" error will emit.

 If the symbol referred is absolute, then the case is no different from an 
absolute relocation.  You just can't calculate a PC-relative reference to 
an absolute value at static link time, because the distance from PC to 
that value will depend on the base address at load time.  It is different 
from a reference to a regular (non-absolute) symbol that just turns out 
too distant for a PC-relative relocation to reach (where the "relocation 
truncated to fit" case applies).

  Maciej