Palmer Dabbelt palmer@dabbelt.com
Sat Sep 19 20:53:39 GMT 2020

• Previous message (by thread): PR26569, R_RISCV_RVC_JUMP results in buffer overflow
• Next message (by thread): PR26569, R_RISCV_RVC_JUMP results in buffer overflow
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

---

On Thu, 17 Sep 2020 01:20:18 PDT (-0700), amodra@gmail.com wrote:
> This patch corrects "size" and "bitsize" in R_RISCV_RVC_* reloc howtos
> so that elfnn-riscv.c:perform_relocation doesn't access past the end
> of a section.  I've also corrected "size" in the R_RISCV_CALL* reloc
> howtos since these relocs apply to two consecutive instructions.  That
> caused fallout in the assembler with complaints about "fixup not
> contained within frag" due to tc-riscv.c:append_insn finishing off a
> frag after the auipc insn making up a "call" macro.  Which is a little
> rude since the CALL reloc also relocates the following jalr.  I wasn't
> game to change the frag handling, so worked around the error using
> TC_FIX_SIZE_SLACK, and adjusted fx_size for the RELAX reloc following
> a CALL.
>
> I've also changed R_RISCV_ALIGN and R_RISCV_TPREL_ADD marker reloc
> howtos to look like R_RISCV_NONE, and corrected dst_mask for numerous
> relocs, not that it matters very much.
>
> OK to apply?

LGTM.  There's some comments, but they seem amenable to a follow-on.

Thanks!

>
> bfd/
> 	PR 26569
> 	* elfxx-riscv.c (howto_table): Correct size and bitsize of
> 	R_RISCV_RVC_BRANCH, R_RISCV_RVC_JUMP, and R_RISCV_RVC_LUI.
> 	Correct size for R_RISCV_TLS_DTPMOD32, R_RISCV_TLS_DTPREL32,
> 	R_RISCV_CALL, and R_RISCV_CALL_PLT.  Make R_RISCV_TPREL_ADD and
> 	R_RISCV_ALIGN like R_RISCV_NONE.  Correct dst_mask many relocs.
> gas/
> 	* config/tc-riscv.c (md_apply_fix): Zero fx_size of RELAX fixup.
> 	* config/tc-riscv.g (TC_FX_SIZE_SLACK): Define.
>
> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> index cfdd867e0d..181969521a 100644
> --- a/bfd/elfxx-riscv.c
> +++ b/bfd/elfxx-riscv.c
> @@ -69,7 +69,7 @@ static reloc_howto_type howto_table[] =
>  	 "R_RISCV_32",			/* name */
>  	 FALSE,				/* partial_inplace */
>  	 0,				/* src_mask */
> -	 MINUS_ONE,			/* dst_mask */
> +	 0xffffffff,			/* dst_mask */
>  	 FALSE),			/* pcrel_offset */
>
>    /* 64 bit relocation.  */
> @@ -99,7 +99,7 @@ static reloc_howto_type howto_table[] =
>  	 "R_RISCV_RELATIVE",		/* name */
>  	 FALSE,				/* partial_inplace */
>  	 0,				/* src_mask */
> -	 MINUS_ONE,			/* dst_mask */
> +	 0xffffffff,			/* dst_mask */
>  	 FALSE),			/* pcrel_offset */
>
>    HOWTO (R_RISCV_COPY,			/* type */
> @@ -133,7 +133,7 @@ static reloc_howto_type howto_table[] =
>    /* Dynamic TLS relocations.  */
>    HOWTO (R_RISCV_TLS_DTPMOD32,		/* type */
>  	 0,				/* rightshift */
> -	 4,				/* size */
> +	 2,				/* size */
>  	 32,				/* bitsize */
>  	 FALSE,				/* pc_relative */
>  	 0,				/* bitpos */
> @@ -142,7 +142,7 @@ static reloc_howto_type howto_table[] =
>  	 "R_RISCV_TLS_DTPMOD32",	/* name */
>  	 FALSE,				/* partial_inplace */
>  	 0,				/* src_mask */
> -	 MINUS_ONE,			/* dst_mask */
> +	 0xffffffff,			/* dst_mask */
>  	 FALSE),			/* pcrel_offset */
>
>    HOWTO (R_RISCV_TLS_DTPMOD64,		/* type */
> @@ -161,7 +161,7 @@ static reloc_howto_type howto_table[] =
>
>    HOWTO (R_RISCV_TLS_DTPREL32,		/* type */
>  	 0,				/* rightshift */
> -	 4,				/* size */
> +	 2,				/* size */
>  	 32,				/* bitsize */
>  	 FALSE,				/* pc_relative */
>  	 0,				/* bitpos */
> @@ -170,7 +170,7 @@ static reloc_howto_type howto_table[] =
>  	 "R_RISCV_TLS_DTPREL32",	/* name */
>  	 TRUE,				/* partial_inplace */
>  	 0,				/* src_mask */
> -	 MINUS_ONE,			/* dst_mask */
> +	 0xffffffff,			/* dst_mask */
>  	 FALSE),			/* pcrel_offset */
>
>    HOWTO (R_RISCV_TLS_DTPREL64,		/* type */
> @@ -198,7 +198,7 @@ static reloc_howto_type howto_table[] =
>  	 "R_RISCV_TLS_TPREL32",		/* name */
>  	 FALSE,				/* partial_inplace */
>  	 0,				/* src_mask */
> -	 MINUS_ONE,			/* dst_mask */
> +	 0xffffffff,			/* dst_mask */
>  	 FALSE),			/* pcrel_offset */
>
>    HOWTO (R_RISCV_TLS_TPREL64,		/* type */
> @@ -254,7 +254,7 @@ static reloc_howto_type howto_table[] =
>    /* 32-bit PC-relative function call (AUIPC/JALR).  */
>    HOWTO (R_RISCV_CALL,			/* type */
>  	 0,				/* rightshift */
> -	 2,				/* size */
> +	 4,				/* size */
>  	 64,				/* bitsize */
>  	 TRUE,				/* pc_relative */
>  	 0,				/* bitpos */
> @@ -270,7 +270,7 @@ static reloc_howto_type howto_table[] =
>    /* Like R_RISCV_CALL, but not locally binding.  */
>    HOWTO (R_RISCV_CALL_PLT,		/* type */
>  	 0,				/* rightshift */
> -	 2,				/* size */
> +	 4,				/* size */
>  	 64,				/* bitsize */
>  	 TRUE,				/* pc_relative */
>  	 0,				/* bitpos */
> @@ -466,14 +466,14 @@ static reloc_howto_type howto_table[] =
>    /* TLS LE thread pointer usage.  May be relaxed.  */
>    HOWTO (R_RISCV_TPREL_ADD,		/* type */
>  	 0,				/* rightshift */
> -	 2,				/* size */
> -	 32,				/* bitsize */
> +	 3,				/* size */
> +	 0,				/* bitsize */
>  	 FALSE,				/* pc_relative */
>  	 0,				/* bitpos */
>  	 complain_overflow_dont,	/* complain_on_overflow */
>  	 bfd_elf_generic_reloc,		/* special_function */
>  	 "R_RISCV_TPREL_ADD",		/* name */
> -	 TRUE,				/* partial_inplace */
> +	 FALSE,				/* partial_inplace */
>  	 0,				/* src_mask */
>  	 0,				/* dst_mask */
>  	 FALSE),			/* pcrel_offset */
> @@ -490,7 +490,7 @@ static reloc_howto_type howto_table[] =
>  	 "R_RISCV_ADD8",		/* name */
>  	 FALSE,				/* partial_inplace */
>  	 0,				/* src_mask */
> -	 MINUS_ONE,			/* dst_mask */
> +	 0xff,				/* dst_mask */
>  	 FALSE),			/* pcrel_offset */
>
>    /* 16-bit in-place addition, for local label subtraction.  */
> @@ -505,7 +505,7 @@ static reloc_howto_type howto_table[] =
>  	 "R_RISCV_ADD16",		/* name */
>  	 FALSE,				/* partial_inplace */
>  	 0,				/* src_mask */
> -	 MINUS_ONE,			/* dst_mask */
> +	 0xffff,			/* dst_mask */
>  	 FALSE),			/* pcrel_offset */
>
>    /* 32-bit in-place addition, for local label subtraction.  */
> @@ -520,7 +520,7 @@ static reloc_howto_type howto_table[] =
>  	 "R_RISCV_ADD32",		/* name */
>  	 FALSE,				/* partial_inplace */
>  	 0,				/* src_mask */
> -	 MINUS_ONE,			/* dst_mask */
> +	 0xffffffff,			/* dst_mask */
>  	 FALSE),			/* pcrel_offset */
>
>    /* 64-bit in-place addition, for local label subtraction.  */
> @@ -550,7 +550,7 @@ static reloc_howto_type howto_table[] =
>  	 "R_RISCV_SUB8",		/* name */
>  	 FALSE,				/* partial_inplace */
>  	 0,				/* src_mask */
> -	 MINUS_ONE,			/* dst_mask */
> +	 0xff,				/* dst_mask */
>  	 FALSE),			/* pcrel_offset */
>
>    /* 16-bit in-place addition, for local label subtraction.  */
> @@ -565,7 +565,7 @@ static reloc_howto_type howto_table[] =
>  	 "R_RISCV_SUB16",		/* name */
>  	 FALSE,				/* partial_inplace */
>  	 0,				/* src_mask */
> -	 MINUS_ONE,			/* dst_mask */
> +	 0xffff,			/* dst_mask */
>  	 FALSE),			/* pcrel_offset */
>
>    /* 32-bit in-place addition, for local label subtraction.  */
> @@ -580,7 +580,7 @@ static reloc_howto_type howto_table[] =
>  	 "R_RISCV_SUB32",		/* name */
>  	 FALSE,				/* partial_inplace */
>  	 0,				/* src_mask */
> -	 MINUS_ONE,			/* dst_mask */
> +	 0xffffffff,			/* dst_mask */
>  	 FALSE),			/* pcrel_offset */
>
>    /* 64-bit in-place addition, for local label subtraction.  */
> @@ -633,7 +633,7 @@ static reloc_howto_type howto_table[] =
>       addend rounded up to the next power of two.  */
>    HOWTO (R_RISCV_ALIGN,			/* type */
>  	 0,				/* rightshift */
> -	 2,				/* size */
> +	 3,				/* size */

Can't these be up to a page in size?

>  	 0,				/* bitsize */
>  	 FALSE,				/* pc_relative */
>  	 0,				/* bitpos */
> @@ -648,8 +648,8 @@ static reloc_howto_type howto_table[] =
>    /* 8-bit PC-relative branch offset.  */
>    HOWTO (R_RISCV_RVC_BRANCH,		/* type */
>  	 0,				/* rightshift */
> -	 2,				/* size */
> -	 32,				/* bitsize */
> +	 1,				/* size */
> +	 16,				/* bitsize */
>  	 TRUE,				/* pc_relative */
>  	 0,				/* bitpos */
>  	 complain_overflow_signed,	/* complain_on_overflow */
> @@ -663,8 +663,8 @@ static reloc_howto_type howto_table[] =
>    /* 11-bit PC-relative jump offset.  */
>    HOWTO (R_RISCV_RVC_JUMP,		/* type */
>  	 0,				/* rightshift */
> -	 2,				/* size */
> -	 32,				/* bitsize */
> +	 1,				/* size */
> +	 16,				/* bitsize */
>  	 TRUE,				/* pc_relative */
>  	 0,				/* bitpos */
>  	 complain_overflow_dont,	/* complain_on_overflow */
> @@ -678,8 +678,8 @@ static reloc_howto_type howto_table[] =
>    /* High 6 bits of 18-bit absolute address.  */
>    HOWTO (R_RISCV_RVC_LUI,		/* type */
>  	 0,				/* rightshift */
> -	 2,				/* size */
> -	 32,				/* bitsize */
> +	 1,				/* size */
> +	 16,				/* bitsize */
>  	 FALSE,				/* pc_relative */
>  	 0,				/* bitpos */
>  	 complain_overflow_dont,	/* complain_on_overflow */
> @@ -807,7 +807,7 @@ static reloc_howto_type howto_table[] =
>  	 "R_RISCV_SET8",		/* name */
>  	 FALSE,				/* partial_inplace */
>  	 0,				/* src_mask */
> -	 MINUS_ONE,			/* dst_mask */
> +	 0xff,				/* dst_mask */
>  	 FALSE),			/* pcrel_offset */
>
>    /* 16-bit in-place setting, for local label subtraction.  */
> @@ -822,7 +822,7 @@ static reloc_howto_type howto_table[] =
>  	 "R_RISCV_SET16",		/* name */
>  	 FALSE,				/* partial_inplace */
>  	 0,				/* src_mask */
> -	 MINUS_ONE,			/* dst_mask */
> +	 0xffff,			/* dst_mask */
>  	 FALSE),			/* pcrel_offset */
>
>    /* 32-bit in-place setting, for local label subtraction.  */
> @@ -837,7 +837,7 @@ static reloc_howto_type howto_table[] =
>  	 "R_RISCV_SET32",		/* name */
>  	 FALSE,				/* partial_inplace */
>  	 0,				/* src_mask */
> -	 MINUS_ONE,			/* dst_mask */
> +	 0xffffffff,			/* dst_mask */
>  	 FALSE),			/* pcrel_offset */
>
>    /* 32-bit PC relative.  */
> @@ -852,7 +852,7 @@ static reloc_howto_type howto_table[] =
>  	 "R_RISCV_32_PCREL",		/* name */
>  	 FALSE,				/* partial_inplace */
>  	 0,				/* src_mask */
> -	 MINUS_ONE,			/* dst_mask */
> +	 0xffffffff,			/* dst_mask */
>  	 FALSE),			/* pcrel_offset */
>  };
>
> diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
> index 9df6d3f415..e1f7673eb5 100644
> --- a/gas/config/tc-riscv.c
> +++ b/gas/config/tc-riscv.c
> @@ -3031,6 +3031,7 @@ md_apply_fix (fixS *fixP, valueT *valP, segT seg ATTRIBUTE_UNUSED)
>        fixP->fx_next = xmemdup (fixP, sizeof (*fixP), sizeof (*fixP));
>        fixP->fx_next->fx_addsy = fixP->fx_next->fx_subsy = NULL;
>        fixP->fx_next->fx_r_type = BFD_RELOC_RISCV_RELAX;
> +      fixP->fx_next->fx_size = 0;
>      }
>  }
>
> diff --git a/gas/config/tc-riscv.h b/gas/config/tc-riscv.h
> index 43d751ad0d..83cff8f2df 100644
> --- a/gas/config/tc-riscv.h
> +++ b/gas/config/tc-riscv.h
> @@ -77,6 +77,14 @@ extern int riscv_parse_long_option (const char *);
>  #define md_pre_output_hook riscv_pre_output_hook()
>  extern void riscv_pre_output_hook (void);
>
> +/* Call macro frags are tied off wrongly at the auipc which has an
> +   eight byte R_RISCV_CALL reloc covering both the auipc and
> +   immediately following jalr.  For now, work around this by allowing
> +   CALL relocs to extend past the end of a frag.  */
> +#define TC_FX_SIZE_SLACK(FIX) \
> +  (((FIX)->fx_r_type == BFD_RELOC_RISCV_CALL			\
> +    || (FIX)->fx_r_type == BFD_RELOC_RISCV_CALL_PLT) ? 4 : 0)
> +

I haven't tried it yet, but I think something like this would eliminate the
need for this slack?

diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
index 9df6d3f415..cec235b986 100644
--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -1133,9 +1133,7 @@ append_insn (struct riscv_cl_insn *ip, expressionS *address_expr,
      optimized away or compressed by the linker during relaxation, to prevent
      the assembler from computing static offsets across such an instruction.
      This is necessary to get correct EH info.  */
-  if (reloc_type == BFD_RELOC_RISCV_CALL
-      || reloc_type == BFD_RELOC_RISCV_CALL_PLT
-      || reloc_type == BFD_RELOC_RISCV_HI20
+  if (reloc_type == BFD_RELOC_RISCV_HI20
       || reloc_type == BFD_RELOC_RISCV_PCREL_HI20
       || reloc_type == BFD_RELOC_RISCV_TPREL_HI20
       || reloc_type == BFD_RELOC_RISCV_TPREL_ADD)
@@ -1313,6 +1311,8 @@ riscv_call (int destreg, int tempreg, expressionS *ep,
 {
   macro_build (ep, "auipc", "d,u", tempreg, reloc);
   macro_build (NULL, "jalr", "d,s", destreg, tempreg);
+  frag_wane(frag_now);
+  frag_new(0);
 }
 
 /* Load an integer constant into a register.  */


>  /* Let the linker resolve all the relocs due to relaxation.  */
>  #define tc_fix_adjustable(fixp) 0
>  #define md_allow_local_subtract(l,r,s) 0

---

• Previous message (by thread): PR26569, R_RISCV_RVC_JUMP results in buffer overflow
• Next message (by thread): PR26569, R_RISCV_RVC_JUMP results in buffer overflow
• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

---

More information about the Binutils mailing list