public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* PR26569, R_RISCV_RVC_JUMP results in buffer overflow
@ 2020-09-17  8:20 Alan Modra
  2020-09-17 10:10 ` Nelson Chu
  2020-09-19 20:53 ` Palmer Dabbelt
  0 siblings, 2 replies; 5+ messages in thread
From: Alan Modra @ 2020-09-17  8:20 UTC (permalink / raw)
  To: binutils

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?

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 */
 	 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)
+
 /* 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

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: PR26569, R_RISCV_RVC_JUMP results in buffer overflow
  2020-09-17  8:20 PR26569, R_RISCV_RVC_JUMP results in buffer overflow Alan Modra
@ 2020-09-17 10:10 ` Nelson Chu
  2020-09-19 20:53 ` Palmer Dabbelt
  1 sibling, 0 replies; 5+ messages in thread
From: Nelson Chu @ 2020-09-17 10:10 UTC (permalink / raw)
  To: Alan Modra; +Cc: Binutils

Hi Alan,

LGTM, thanks for fixing them.  I have quickly tested my commonly used
toolchain (rv32i/rv64gc), and everything looks good for now.  But I'm
not a riscv FSF binutils maintainer, so I can not approve anything.
Just to mention that we may also encounter the similar size problems
in the future, and thank you for fixing it now.

Nelson

On Thu, Sep 17, 2020 at 4:20 PM Alan Modra via Binutils
<binutils@sourceware.org> 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?
>
> 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 */
>          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)
> +
>  /* 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
>
> --
> Alan Modra
> Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: PR26569, R_RISCV_RVC_JUMP results in buffer overflow
  2020-09-17  8:20 PR26569, R_RISCV_RVC_JUMP results in buffer overflow Alan Modra
  2020-09-17 10:10 ` Nelson Chu
@ 2020-09-19 20:53 ` Palmer Dabbelt
  2020-09-21  0:15   ` Alan Modra
  1 sibling, 1 reply; 5+ messages in thread
From: Palmer Dabbelt @ 2020-09-19 20:53 UTC (permalink / raw)
  To: amodra; +Cc: binutils, Andrew Waterman, Jim Wilson

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: PR26569, R_RISCV_RVC_JUMP results in buffer overflow
  2020-09-19 20:53 ` Palmer Dabbelt
@ 2020-09-21  0:15   ` Alan Modra
  2020-09-26 16:30     ` Palmer Dabbelt
  0 siblings, 1 reply; 5+ messages in thread
From: Alan Modra @ 2020-09-21  0:15 UTC (permalink / raw)
  To: Palmer Dabbelt; +Cc: binutils, Andrew Waterman, Jim Wilson

On Sat, Sep 19, 2020 at 01:53:39PM -0700, Palmer Dabbelt wrote:
> On Thu, 17 Sep 2020 01:20:18 PDT (-0700), amodra@gmail.com wrote:
> >    HOWTO (R_RISCV_ALIGN,			/* type */
> >  	 0,				/* rightshift */
> > -	 2,				/* size */
> > +	 3,				/* size */
> 
> Can't these be up to a page in size?

These relocs are special, and can't be handled by the howto struct and
generic code without implementing a special_function.  So what is in
the howto doesn't matter very much but this "size" agrees with
"bitsize", "3" being the value to say the number of bytes is zero.

> >  	 0,				/* bitsize */
> >  	 FALSE,				/* pc_relative */
> >  	 0,				/* bitpos */
> > --- 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?

Almost.  There needs to be a frag_grow(8) before auipc is output.
Otherwise if you're unlucky the frag runs out of room after the auipc
and you hit the same "fixup not contained within frag".

> 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.  */

I've committed the following.
----

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.  Fixed by
changing the frag handling a little.

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.

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 (append_insn): Don't tie off frags at CALL
	relocs.
	(riscv_call): Tie them off after the jalr.
	(md_apply_fix): Zero fx_size of RELAX fixup.

diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
index cfdd867e0d..e5adea57b1 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 */
 	 0,				/* bitsize */
 	 FALSE,				/* pc_relative */
 	 0,				/* bitpos */
@@ -643,13 +643,13 @@ static reloc_howto_type howto_table[] =
 	 FALSE,				/* partial_inplace */
 	 0,				/* src_mask */
 	 0,				/* dst_mask */
-	 TRUE),				/* pcrel_offset */
+	 FALSE),			/* pcrel_offset */
 
   /* 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..eb31e42a2e 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)
@@ -1311,8 +1309,13 @@ static void
 riscv_call (int destreg, int tempreg, expressionS *ep,
 	    bfd_reloc_code_real_type reloc)
 {
+  /* Ensure the jalr is emitted to the same frag as the auipc.  */
+  frag_grow (8);
   macro_build (ep, "auipc", "d,u", tempreg, reloc);
   macro_build (NULL, "jalr", "d,s", destreg, tempreg);
+  /* See comment at end of append_insn.  */
+  frag_wane (frag_now);
+  frag_new (0);
 }
 
 /* Load an integer constant into a register.  */
@@ -3031,6 +3034,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;
     }
 }
 

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: PR26569, R_RISCV_RVC_JUMP results in buffer overflow
  2020-09-21  0:15   ` Alan Modra
@ 2020-09-26 16:30     ` Palmer Dabbelt
  0 siblings, 0 replies; 5+ messages in thread
From: Palmer Dabbelt @ 2020-09-26 16:30 UTC (permalink / raw)
  To: amodra; +Cc: binutils, Andrew Waterman, Jim Wilson

On Sun, 20 Sep 2020 17:15:26 PDT (-0700), amodra@gmail.com wrote:
> On Sat, Sep 19, 2020 at 01:53:39PM -0700, Palmer Dabbelt wrote:
>> On Thu, 17 Sep 2020 01:20:18 PDT (-0700), amodra@gmail.com wrote:
>> >    HOWTO (R_RISCV_ALIGN,			/* type */
>> >  	 0,				/* rightshift */
>> > -	 2,				/* size */
>> > +	 3,				/* size */
>>
>> Can't these be up to a page in size?
>
> These relocs are special, and can't be handled by the howto struct and
> generic code without implementing a special_function.  So what is in
> the howto doesn't matter very much but this "size" agrees with
> "bitsize", "3" being the value to say the number of bytes is zero.
>
>> >  	 0,				/* bitsize */
>> >  	 FALSE,				/* pc_relative */
>> >  	 0,				/* bitpos */
>> > --- 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?
>
> Almost.  There needs to be a frag_grow(8) before auipc is output.
> Otherwise if you're unlucky the frag runs out of room after the auipc
> and you hit the same "fixup not contained within frag".
>
>> 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.  */
>
> I've committed the following.

Thanks!

> ----
>
> 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.  Fixed by
> changing the frag handling a little.
>
> 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.
>
> 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 (append_insn): Don't tie off frags at CALL
> 	relocs.
> 	(riscv_call): Tie them off after the jalr.
> 	(md_apply_fix): Zero fx_size of RELAX fixup.
>
> diff --git a/bfd/elfxx-riscv.c b/bfd/elfxx-riscv.c
> index cfdd867e0d..e5adea57b1 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 */
>  	 0,				/* bitsize */
>  	 FALSE,				/* pc_relative */
>  	 0,				/* bitpos */
> @@ -643,13 +643,13 @@ static reloc_howto_type howto_table[] =
>  	 FALSE,				/* partial_inplace */
>  	 0,				/* src_mask */
>  	 0,				/* dst_mask */
> -	 TRUE),				/* pcrel_offset */
> +	 FALSE),			/* pcrel_offset */
>
>    /* 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..eb31e42a2e 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)
> @@ -1311,8 +1309,13 @@ static void
>  riscv_call (int destreg, int tempreg, expressionS *ep,
>  	    bfd_reloc_code_real_type reloc)
>  {
> +  /* Ensure the jalr is emitted to the same frag as the auipc.  */
> +  frag_grow (8);
>    macro_build (ep, "auipc", "d,u", tempreg, reloc);
>    macro_build (NULL, "jalr", "d,s", destreg, tempreg);
> +  /* See comment at end of append_insn.  */
> +  frag_wane (frag_now);
> +  frag_new (0);
>  }
>
>  /* Load an integer constant into a register.  */
> @@ -3031,6 +3034,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;
>      }
>  }

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-09-26 16:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-17  8:20 PR26569, R_RISCV_RVC_JUMP results in buffer overflow Alan Modra
2020-09-17 10:10 ` Nelson Chu
2020-09-19 20:53 ` Palmer Dabbelt
2020-09-21  0:15   ` Alan Modra
2020-09-26 16:30     ` Palmer Dabbelt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).