[PATCH v4] MIPS/Gas: Disallow branch to absolute address for PIC

[PATCH v4] MIPS/Gas: Disallow branch to absolute address for PIC

[YunQiang Su]

The asm code like
	b	(0)
will generate binary like:

00000000 <.text>:
	0: 1000ffff b 0x0
           0: R_MIPS_PC16 *ABS*
	4: 00000000 nop
If this object is linked into an PIC/PIC dynamic executable,
this branch instruction will jump to a wrong address, with low
16bit unset.
Maybe we need a new dynamic relocation, which is not even defined
by MIPS psABI, and I don't think that it is worth at all.

See PR31343.
---
 gas/config/tc-mips.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/gas/config/tc-mips.c b/gas/config/tc-mips.c
index 43c12de2c8a..f3f5c1f469d 100644
--- a/gas/config/tc-mips.c
+++ b/gas/config/tc-mips.c
@@ -7560,6 +7560,9 @@ append_insn (struct mips_cl_insn *ip, expressionS *address_expr,
 		ip->insn_opcode |= ((address_expr->X_add_number >> shift)
 				    & 0xffff);
 	      }
+	    if (mips_pic != NO_PIC && address_expr->X_op == O_constant)
+	      as_bad (_("PIC code branch to absolute address (0x%lx)"),
+		      (unsigned long) address_expr->X_add_number);
 	  }
 	  break;
 
@@ -7577,6 +7580,9 @@ append_insn (struct mips_cl_insn *ip, expressionS *address_expr,
 		      (unsigned long) address_expr->X_add_number);
 	    ip->insn_opcode |= ((address_expr->X_add_number >> shift)
 				& 0x1fffff);
+	    if (mips_pic != NO_PIC && address_expr->X_op == O_constant)
+	      as_bad (_("PIC code branch to absolute address (0x%lx)"),
+		      (unsigned long) address_expr->X_add_number);
 	  }
 	  break;
 
@@ -7594,6 +7600,9 @@ append_insn (struct mips_cl_insn *ip, expressionS *address_expr,
 		      (unsigned long) address_expr->X_add_number);
 	    ip->insn_opcode |= ((address_expr->X_add_number >> shift)
 				& 0x3ffffff);
+	    if (mips_pic != NO_PIC && address_expr->X_op == O_constant)
+	      as_bad (_("PIC code branch to absolute address (0x%lx)"),
+		      (unsigned long) address_expr->X_add_number);
 	  }
 	  break;
 
-- 
2.39.2

Re: [PATCH v4] MIPS/Gas: Disallow branch to absolute address for PIC

[Maciej W. Rozycki]

On Wed, 7 Feb 2024, YunQiang Su wrote:

> The asm code like
> 	b	(0)
> will generate binary like:
> 
> 00000000 <.text>:
> 	0: 1000ffff b 0x0
>            0: R_MIPS_PC16 *ABS*
> 	4: 00000000 nop
> If this object is linked into an PIC/PIC dynamic executable,
> this branch instruction will jump to a wrong address, with low
> 16bit unset.

 NAK, `mips_pic' may be set to SVR4_PIC and still produce de facto non-PIC 
code, as per the original SVR4 MIPS ABI (with lazy binding stubs rather 
than PLT).  We continue supporting this mode.

 Originally NO_PIC was intended for bare metal code only, statically 
linked only, which was later extended to Linux, etc. with the addition of 
PLT and copy reloc support.

 PR31343 has to be sorted in the linker.

  Maciej

Re: [PATCH v4] MIPS/Gas: Disallow branch to absolute address for PIC

[YunQiang Su]

Maciej W. Rozycki <macro@orcam.me.uk> 于2024年2月7日周三 03:20写道：
>
> On Wed, 7 Feb 2024, YunQiang Su wrote:
>
> > The asm code like
> >       b       (0)
> > will generate binary like:
> >
> > 00000000 <.text>:
> >       0: 1000ffff b 0x0
> >            0: R_MIPS_PC16 *ABS*
> >       4: 00000000 nop
> > If this object is linked into an PIC/PIC dynamic executable,
> > this branch instruction will jump to a wrong address, with low
> > 16bit unset.
>
>  NAK, `mips_pic' may be set to SVR4_PIC and still produce de facto non-PIC
> code, as per the original SVR4 MIPS ABI (with lazy binding stubs rather
> than PLT).  We continue supporting this mode.
>
>  Originally NO_PIC was intended for bare metal code only, statically
> linked only, which was later extended to Linux, etc. with the addition of
> PLT and copy reloc support.
>
>  PR31343 has to be sorted in the linker.
>

So, what's the expected behavior?
Let's use the example:
        .set noreorder
        .set nomicromips
        .set nomips16
        b    (4)
        INSN0
        INSN1
        INSN2
        INSN3

At least we have 2 choice:
1. jump to INSN0, if we treat (4) same with (. + 4).
2. jump to INSN1, if we treat (4) as the offset in ISA document:
    "An 18-bit signed offset (the 16-bit offset field shifted left 2
bits) is added
     to the address of the instruction following the branch (not the
branch itself),
     in the branch delay slot, to form a PC-relative effective target address."

Currently, this syntax is not supported by ld even for static at all:

xx:
      b (8)
      ssnop
      ssnop
      ssnop
      ssnop

mipsel-linux-gnu-gcc -mno-shared -mno-abicalls -static -nostdlib 1.s
warning: cannot find entry symbol __start; defaulting to 00400110
/tmp/ccdHuq6V.o: in function `xx':
(.text+0x0): relocation truncated to fit: R_MIPS_PC16 against `*UND*'

I don't think that we should generate such confusion binary at all.
So, I prefer to disable this syntax for all cases, even for static objects:
only branch to label is allowed.

Re: [PATCH v4] MIPS/Gas: Disallow branch to absolute address for PIC

[Maciej W. Rozycki]

On Wed, 7 Feb 2024, YunQiang Su wrote:

> >  NAK, `mips_pic' may be set to SVR4_PIC and still produce de facto non-PIC
> > code, as per the original SVR4 MIPS ABI (with lazy binding stubs rather
> > than PLT).  We continue supporting this mode.
> >
> >  Originally NO_PIC was intended for bare metal code only, statically
> > linked only, which was later extended to Linux, etc. with the addition of
> > PLT and copy reloc support.
> >
> >  PR31343 has to be sorted in the linker.
> >
> 
> So, what's the expected behavior?

 The static linker is supposed to report, as a warning or an error 
(depending on what BFD's policy is), that it cannot resolve a relocation, 
either due to an overflow or due to the inability to emit a suitable 
dynamic relocation so as to defer the calculation to the dynamic loader.

> Let's use the example:
>         .set noreorder
>         .set nomicromips
>         .set nomips16
>         b    (4)
>         INSN0
>         INSN1
>         INSN2
>         INSN3
> 
> At least we have 2 choice:
> 1. jump to INSN0, if we treat (4) same with (. + 4).
> 2. jump to INSN1, if we treat (4) as the offset in ISA document:
>     "An 18-bit signed offset (the 16-bit offset field shifted left 2
> bits) is added
>      to the address of the instruction following the branch (not the
> branch itself),
>      in the branch delay slot, to form a PC-relative effective target address."
> 
> Currently, this syntax is not supported by ld even for static at all:
> 
> xx:
>       b (8)
>       ssnop
>       ssnop
>       ssnop
>       ssnop
> 
> mipsel-linux-gnu-gcc -mno-shared -mno-abicalls -static -nostdlib 1.s
> warning: cannot find entry symbol __start; defaulting to 00400110
> /tmp/ccdHuq6V.o: in function `xx':
> (.text+0x0): relocation truncated to fit: R_MIPS_PC16 against `*UND*'

 I gave valid use examples in PR31343 that link correctly.  I can image a 
tiny bare-metal app using branches to hardcoded locations for one reason 
or another.  Tools ought not to stand in the way (a general engineering 
principle and one of the GNU project in particular too).

  Maciej