public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] MSP430: Support relocations for subtract expressions in .uleb128 directives
@ 2020-09-02 12:21 Jozef Lawrynowicz
  2020-09-03 14:41 ` Nick Clifton
  0 siblings, 1 reply; 11+ messages in thread
From: Jozef Lawrynowicz @ 2020-09-02 12:21 UTC (permalink / raw)
  To: binutils

[-- Attachment #1: Type: text/plain, Size: 1119 bytes --]

Link-time relaxations of branches are common for MSP430, given that GCC
can generate pessimal branch instructions, and the
-mcode-region=either/-mdata-region=either options to shuffle sections
can further change the type of branch instruction required.

These relaxations can result in invalid code when .uleb128
directives, used in the .gcc_except_table section, are used to calculate
the distance between two labels. A value for the .uleb128 directive is
calculated at assembly-time, and can't be updated at link-time, even if
relaxation causes the distance between the labels to change.

This patch adds relocations for subtract expressions in .uleb128
directives, to allow the linker to re-calculate the value of these
expressions after relaxation has been performed.

Successfully regtested the GCC, G++, Binutils, GAS and LD testsuites
for the msp430-elf target in the default, -mcpu=msp430, -mlarge and
-mlarge/-mdata-region=either/-mcode-region=either configurations.
The patch fixes a number of failures in the G++ testsuite for 
-mlarge/-mdata-region=either/-mcode-region=either.

Ok to apply?

Thanks,
Jozef

[-- Attachment #2: 0001-MSP430-Support-relocations-for-subtract-expressions-.patch --]
[-- Type: text/plain, Size: 20075 bytes --]

From 2aa6c6ec5a603d1b65b3775a6c48720650ddc83f Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
Date: Mon, 31 Aug 2020 10:28:06 +0100
Subject: [PATCH] MSP430: Support relocations for subtract expressions in
 .uleb128 directives

Link-time relaxations of branches are common for MSP430, given that GCC
can generate pessimal branch instructions, and the
-mcode-region=either/-mdata-region=either options to shuffle sections
can further change the type of branch instruction required.

These relaxations can result in invalid code when .uleb128
directives, used in the .gcc_except_table section, are used to calculate
the distance between two labels. A value for the .uleb128 directive is
calculated at assembly-time, and can't be updated at link-time, even if
relaxation causes the distance between the labels to change.

This patch adds relocations for subtract expressions in .uleb128
directives, to allow the linker to re-calculate the value of these
expressions after relaxation has been performed.

bfd/ChangeLog:

2020-09-02  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
	Kuan-Lin Chen  <kuanlinchentw@gmail.com>

	* bfd-in2.h (bfd_reloc_code_real): Add
	BFD_RELOC_MSP430_{SET,SUB}_ULEB128.
	* elf32-msp430.c (msp430_elf_ignore_reloc): New.
	(elf_msp430_howto_table): Add R_MSP430{,X}_{SET,SUB}_ULEB128.
	(msp430_reloc_map): Add R_MSP430_{SET,SUB}_ULEB128.
	(msp430x_reloc_map): Add R_MSP430X_{SET,SUB}_ULEB128.
	(write_uleb128): New.
	(msp430_final_link_relocate): Handle R_MSP430{,X}_{SET,SUB}_ULEB128.
	* libbfd.h: Add BFD_RELOC_MSP430_{SET,SUB}_ULEB128.
	* reloc.c: Document BFD_RELOC_MSP430_{SET,SUB}_ULEB128.

binutils/ChangeLog:

2020-09-02  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
	Kuan-Lin Chen  <kuanlinchentw@gmail.com>

	* readelf.c (target_specific_reloc_handling): Handle
	R_MSP430{,X}_{SET,SUB}_ULEB128.

gas/ChangeLog:

2020-09-02  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
	Kuan-Lin Chen  <kuanlinchentw@gmail.com>

	* config/tc-msp430.c (msp430_insert_uleb128_fixes): New.
	(msp430_md_end): Call msp430_insert_uleb128_fixes.

include/ChangeLog:

2020-09-02  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
	Kuan-Lin Chen  <kuanlinchentw@gmail.com>

	* elf/msp430.h (elf_msp430_reloc_type): Add R_MSP430_{SET,SUB}_ULEB128.
	(elf_msp430x_reloc_type): Add R_MSP430X_{SET,SUB}_ULEB128.

ld/ChangeLog:

2020-09-02  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	* testsuite/ld-msp430-elf/msp430-elf.exp: Run new tests.
	* testsuite/ld-msp430-elf/uleb128.s: New test.
	* testsuite/ld-msp430-elf/uleb128_430.d: New test.
	* testsuite/ld-msp430-elf/uleb128_430x.d: New test.

---
 bfd/bfd-in2.h                             |   2 +
 bfd/elf32-msp430.c                        | 152 +++++++++++++++++++++-
 bfd/libbfd.h                              |   2 +
 bfd/reloc.c                               |   5 +
 binutils/readelf.c                        |  27 +++-
 gas/config/tc-msp430.c                    |  54 +++++++-
 include/elf/msp430.h                      |   4 +
 ld/testsuite/ld-msp430-elf/msp430-elf.exp |   3 +
 ld/testsuite/ld-msp430-elf/uleb128.s      |  35 +++++
 ld/testsuite/ld-msp430-elf/uleb128_430.d  |  10 ++
 ld/testsuite/ld-msp430-elf/uleb128_430x.d |  10 ++
 11 files changed, 294 insertions(+), 10 deletions(-)
 create mode 100644 ld/testsuite/ld-msp430-elf/uleb128.s
 create mode 100644 ld/testsuite/ld-msp430-elf/uleb128_430.d
 create mode 100644 ld/testsuite/ld-msp430-elf/uleb128_430x.d

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 3c85b073013..74d3a8ab53e 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -5113,6 +5113,8 @@ then it may be truncated to 8 bits.  */
   BFD_RELOC_MSP430_ABS_HI16,
   BFD_RELOC_MSP430_PREL31,
   BFD_RELOC_MSP430_SYM_DIFF,
+  BFD_RELOC_MSP430_SET_ULEB128,
+  BFD_RELOC_MSP430_SUB_ULEB128,
 
 /* Relocations used by the Altera Nios II core.  */
   BFD_RELOC_NIOS2_S16,
diff --git a/bfd/elf32-msp430.c b/bfd/elf32-msp430.c
index 59e54ecbc9a..afbd61a215e 100644
--- a/bfd/elf32-msp430.c
+++ b/bfd/elf32-msp430.c
@@ -56,6 +56,20 @@ rl78_sym_diff_handler (bfd * abfd,
   return bfd_reloc_continue;
 }
 
+/* Special handler for relocations which don't have to be relocated.
+   This function just simply returns bfd_reloc_ok.  */
+static bfd_reloc_status_type
+msp430_elf_ignore_reloc (bfd *abfd ATTRIBUTE_UNUSED, arelent *reloc_entry,
+			asymbol *symbol ATTRIBUTE_UNUSED,
+			void *data ATTRIBUTE_UNUSED, asection *input_section,
+			bfd *output_bfd, char **error_message ATTRIBUTE_UNUSED)
+{
+  if (output_bfd != NULL)
+    reloc_entry->address += input_section->output_offset;
+
+  return bfd_reloc_ok;
+}
+
 static reloc_howto_type elf_msp430_howto_table[] =
 {
   HOWTO (R_MSP430_NONE,		/* type */
@@ -220,7 +234,40 @@ static reloc_howto_type elf_msp430_howto_table[] =
 	 FALSE,			/* partial_inplace */
 	 0xffffffff,		/* src_mask */
 	 0xffffffff,		/* dst_mask */
-	 FALSE)			/* pcrel_offset */
+	 FALSE),		/* pcrel_offset */
+
+  /* The length of unsigned-leb128 is variable, just assume the
+     size is one byte here.  */
+  HOWTO (R_MSP430_SET_ULEB128,		/* type */
+	 0,				/* rightshift */
+	 0,				/* size */
+	 0,				/* bitsize */
+	 FALSE,				/* pc_relative */
+	 0,				/* bitpos */
+	 complain_overflow_dont,	/* complain_on_overflow */
+	 msp430_elf_ignore_reloc,	/* special handler.  */
+	 "R_MSP430_SET_ULEB128",	/* name */
+	 FALSE,				/* partial_inplace */
+	 0,				/* src_mask */
+	 0,				/* dst_mask */
+	 FALSE),			/* pcrel_offset */
+
+  /* The length of unsigned-leb128 is variable, just assume the
+     size is one byte here.  */
+  HOWTO (R_MSP430_SUB_ULEB128,		/* type */
+	 0,				/* rightshift */
+	 0,				/* size */
+	 0,				/* bitsize */
+	 FALSE,				/* pc_relative */
+	 0,				/* bitpos */
+	 complain_overflow_dont,	/* complain_on_overflow */
+	 msp430_elf_ignore_reloc,	/* special handler.  */
+	 "R_MSP430_SUB_ULEB128",	/* name */
+	 FALSE,				/* partial_inplace */
+	 0,				/* src_mask */
+	 0,				/* dst_mask */
+	 FALSE),			/* pcrel_offset */
+
 };
 
 static reloc_howto_type elf_msp430x_howto_table[] =
@@ -523,7 +570,40 @@ static reloc_howto_type elf_msp430x_howto_table[] =
 	 FALSE,			/* partial_inplace */
 	 0xffffffff,		/* src_mask */
 	 0xffffffff,		/* dst_mask */
-	 FALSE)			/* pcrel_offset */
+	 FALSE),		/* pcrel_offset */
+
+  /* The length of unsigned-leb128 is variable, just assume the
+     size is one byte here.  */
+  HOWTO (R_MSP430X_SET_ULEB128,		/* type */
+	 0,				/* rightshift */
+	 0,				/* size */
+	 0,				/* bitsize */
+	 FALSE,				/* pc_relative */
+	 0,				/* bitpos */
+	 complain_overflow_dont,	/* complain_on_overflow */
+	 msp430_elf_ignore_reloc,	/* special handler.  */
+	 "R_MSP430X_SET_ULEB128",	/* name */
+	 FALSE,				/* partial_inplace */
+	 0,				/* src_mask */
+	 0,				/* dst_mask */
+	 FALSE),			/* pcrel_offset */
+
+  /* The length of unsigned-leb128 is variable, just assume the
+     size is one byte here.  */
+  HOWTO (R_MSP430X_SUB_ULEB128,		/* type */
+	 0,				/* rightshift */
+	 0,				/* size */
+	 0,				/* bitsize */
+	 FALSE,				/* pc_relative */
+	 0,				/* bitpos */
+	 complain_overflow_dont,	/* complain_on_overflow */
+	 msp430_elf_ignore_reloc,	/* special handler.  */
+	 "R_MSP430X_SUB_ULEB128",	/* name */
+	 FALSE,				/* partial_inplace */
+	 0,				/* src_mask */
+	 0,				/* dst_mask */
+	 FALSE),			/* pcrel_offset */
+
 };
 
 /* Map BFD reloc types to MSP430 ELF reloc types.  */
@@ -547,7 +627,9 @@ static const struct msp430_reloc_map msp430_reloc_map[] =
   {BFD_RELOC_MSP430_2X_PCREL,	   R_MSP430_2X_PCREL},
   {BFD_RELOC_MSP430_RL_PCREL,	   R_MSP430_RL_PCREL},
   {BFD_RELOC_8,			   R_MSP430_8},
-  {BFD_RELOC_MSP430_SYM_DIFF,	   R_MSP430_SYM_DIFF}
+  {BFD_RELOC_MSP430_SYM_DIFF,	   R_MSP430_SYM_DIFF},
+  {BFD_RELOC_MSP430_SET_ULEB128,   R_MSP430_SET_ULEB128 },
+  {BFD_RELOC_MSP430_SUB_ULEB128,   R_MSP430_SUB_ULEB128 }
 };
 
 static const struct msp430_reloc_map msp430x_reloc_map[] =
@@ -573,7 +655,9 @@ static const struct msp430_reloc_map msp430x_reloc_map[] =
   {BFD_RELOC_MSP430_10_PCREL,	      R_MSP430X_10_PCREL},
   {BFD_RELOC_MSP430_2X_PCREL,	      R_MSP430X_2X_PCREL},
   {BFD_RELOC_MSP430_RL_PCREL,	      R_MSP430X_PCR16},
-  {BFD_RELOC_MSP430_SYM_DIFF,	      R_MSP430X_SYM_DIFF}
+  {BFD_RELOC_MSP430_SYM_DIFF,	      R_MSP430X_SYM_DIFF},
+  {BFD_RELOC_MSP430_SET_ULEB128,      R_MSP430X_SET_ULEB128 },
+  {BFD_RELOC_MSP430_SUB_ULEB128,      R_MSP430X_SUB_ULEB128 }
 };
 
 static inline bfd_boolean
@@ -711,6 +795,26 @@ elf32_msp430_check_relocs (bfd * abfd, struct bfd_link_info * info,
   return TRUE;
 }
 
+/* Write VAL in uleb128 format to P, returning a pointer to the
+   following byte.
+   This code is copied from elf-attr.c.  */
+
+static bfd_byte *
+write_uleb128 (bfd_byte *p, unsigned int val)
+{
+  bfd_byte c;
+  do
+    {
+      c = val & 0x7f;
+      val >>= 7;
+      if (val)
+	c |= 0x80;
+      *(p++) = c;
+    }
+  while (val);
+  return p;
+}
+
 /* Perform a single relocation.  By default we use the standard BFD
    routines, but a few relocs, we have to do them ourselves.  */
 
@@ -755,6 +859,9 @@ msp430_final_link_relocate (reloc_howto_type *	   howto,
      if (uses_msp430x_relocs (input_bfd))
        switch (howto->type)
 	 {
+	 case R_MSP430X_SET_ULEB128:
+	   relocation += (!is_rel_reloc ? rel->r_addend : 0);
+	   /* Fall through.  */
 	 case R_MSP430_ABS32:
 	  /* If we are computing a 32-bit value for the location lists
 	     and the result is 0 then we add one to the value.  A zero
@@ -780,6 +887,9 @@ msp430_final_link_relocate (reloc_howto_type *	   howto,
      else
        switch (howto->type)
 	 {
+	 case R_MSP430_SET_ULEB128:
+	   relocation += (!is_rel_reloc ? rel->r_addend : 0);
+	   /* Fall through.  */
 	 case R_MSP430_32:
 	 case R_MSP430_16:
 	 case R_MSP430_16_BYTE:
@@ -794,16 +904,42 @@ msp430_final_link_relocate (reloc_howto_type *	   howto,
       sym_diff_section = NULL;
     }
 
-  if (uses_msp430x_relocs (input_bfd))
+  if ((uses_msp430x_relocs (input_bfd) && howto->type == R_MSP430X_SET_ULEB128)
+      || (!uses_msp430x_relocs (input_bfd)
+	  && howto->type == R_MSP430_SET_ULEB128))
+    {
+      unsigned int len = 0;
+      bfd_byte *endp, *p;
+
+      _bfd_read_unsigned_leb128 (input_bfd, contents + rel->r_offset, &len);
+
+      /* Clean the contents value to zero.  Do not reduce the length.  */
+      p = contents + rel->r_offset;
+      endp = p + len -1;
+      memset (p, 0x80, len - 1);
+      *(endp) = 0;
+      p = write_uleb128 (p, relocation) - 1;
+      if (p < endp)
+	*p |= 0x80;
+      else if (p > endp)
+	_bfd_error_handler
+	  (_("error: final size of uleb128 value in %pA from %pB exceeds "
+	     "available space"), input_section, input_bfd);
+
+      return bfd_reloc_ok;
+    }
+  else if (uses_msp430x_relocs (input_bfd))
     switch (howto->type)
       {
       case R_MSP430X_SYM_DIFF:
+      case R_MSP430X_SUB_ULEB128:
 	/* Cache the input section and value.
 	   The offset is unreliable, since relaxation may
 	   have reduced the following reloc's offset.  */
 	BFD_ASSERT (! is_rel_reloc);
 	sym_diff_section = input_section;
-	sym_diff_value = relocation;
+	sym_diff_value = relocation + (howto->type == R_MSP430X_SUB_ULEB128
+				       ? rel->r_addend : 0);
 	return bfd_reloc_ok;
 
       case R_MSP430_ABS16:
@@ -1254,11 +1390,13 @@ msp430_final_link_relocate (reloc_howto_type *	   howto,
       break;
 
     case R_MSP430_SYM_DIFF:
+    case R_MSP430_SUB_ULEB128:
       /* Cache the input section and value.
 	 The offset is unreliable, since relaxation may
 	 have reduced the following reloc's offset.  */
       sym_diff_section = input_section;
-      sym_diff_value = relocation;
+      sym_diff_value = relocation + (howto->type == R_MSP430_SUB_ULEB128
+				     ? rel->r_addend : 0);
       return bfd_reloc_ok;
 
       default:
diff --git a/bfd/libbfd.h b/bfd/libbfd.h
index b97534fc9fe..d4df99d151b 100644
--- a/bfd/libbfd.h
+++ b/bfd/libbfd.h
@@ -2802,6 +2802,8 @@ static const char *const bfd_reloc_code_real_names[] = { "@@uninitialized@@",
   "BFD_RELOC_MSP430_ABS_HI16",
   "BFD_RELOC_MSP430_PREL31",
   "BFD_RELOC_MSP430_SYM_DIFF",
+  "BFD_RELOC_MSP430_SET_ULEB128",
+  "BFD_RELOC_MSP430_SUB_ULEB128",
   "BFD_RELOC_NIOS2_S16",
   "BFD_RELOC_NIOS2_U16",
   "BFD_RELOC_NIOS2_CALL26",
diff --git a/bfd/reloc.c b/bfd/reloc.c
index 7d3479acef4..7f402257188 100644
--- a/bfd/reloc.c
+++ b/bfd/reloc.c
@@ -6366,6 +6366,11 @@ ENUMX
   BFD_RELOC_MSP430_PREL31
 ENUMX
   BFD_RELOC_MSP430_SYM_DIFF
+ENUMX
+  BFD_RELOC_MSP430_SET_ULEB128
+ENUMX
+  BFD_RELOC_MSP430_SUB_ULEB128
+
 ENUMDOC
   msp430 specific relocation codes
 
diff --git a/binutils/readelf.c b/binutils/readelf.c
index f02848e4681..7ac1461f194 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -12586,10 +12586,12 @@ target_specific_reloc_handling (Filedata *           filedata,
 	switch (reloc_type)
 	  {
 	  case 10: /* R_MSP430_SYM_DIFF */
+	  case 12: /* R_MSP430_SUB_ULEB128 */
 	    if (uses_msp430x_relocs (filedata))
 	      break;
 	    /* Fall through.  */
 	  case 21: /* R_MSP430X_SYM_DIFF */
+	  case 23: /* R_MSP430X_SUB_ULEB128 */
 	    /* PR 21139.  */
 	    if (sym_index >= num_syms)
 	      error (_("MSP430 SYM_DIFF reloc contains invalid symbol index %lu\n"),
@@ -12604,12 +12606,14 @@ target_specific_reloc_handling (Filedata *           filedata,
 
 	  case 5: /* R_MSP430_16_BYTE */
 	  case 9: /* R_MSP430_8 */
+	  case 11: /* R_MSP430_SET_ULEB128 */
 	    if (uses_msp430x_relocs (filedata))
 	      break;
 	    goto handle_sym_diff;
 
 	  case 2: /* R_MSP430_ABS16 */
 	  case 15: /* R_MSP430X_ABS16 */
+	  case 22: /* R_MSP430X_SET_ULEB128 */
 	    if (! uses_msp430x_relocs (filedata))
 	      break;
 	    goto handle_sym_diff;
@@ -12617,10 +12621,29 @@ target_specific_reloc_handling (Filedata *           filedata,
 	  handle_sym_diff:
 	    if (saved_sym != NULL)
 	      {
-		int reloc_size = reloc_type == 1 ? 4 : 2;
 		bfd_vma value;
+		unsigned int reloc_size;
+		int leb_ret = 0;
+		switch (reloc_type)
+		  {
+		  case 1: /* R_MSP430_32 or R_MSP430_ABS32 */
+		    reloc_size = 4;
+		    break;
+		  case 11: /* R_MSP430_SET_ULEB128 */
+		  case 22: /* R_MSP430X_SET_ULEB128 */
+		    read_leb128 (start + reloc->r_offset, end, FALSE,
+				 &reloc_size, &leb_ret);
+		    break;
+		  default:
+		    reloc_size = 2;
+		    break;
+		  }
 
-		if (sym_index >= num_syms)
+		if (leb_ret != 0)
+		  error (_("MSP430 ULEB128 field at 0x%lx contains invalid "
+			   "ULEB128 value\n"),
+			 (long) reloc->r_offset);
+		else if (sym_index >= num_syms)
 		  error (_("MSP430 reloc contains invalid symbol index %lu\n"),
 			 sym_index);
 		else
diff --git a/gas/config/tc-msp430.c b/gas/config/tc-msp430.c
index 2738937b112..6d1803202ce 100644
--- a/gas/config/tc-msp430.c
+++ b/gas/config/tc-msp430.c
@@ -5048,8 +5048,56 @@ msp430_fix_adjustable (struct fix *fixp ATTRIBUTE_UNUSED)
   return FALSE;
 }
 
-/* Set the contents of the .MSP430.attributes and .GNU.attributes sections.  */
+/* Scan uleb128 subtraction expressions and insert fixups for them.
+   e.g., .uleb128 .L1 - .L0
+   Because relaxation may change the value of the subtraction, we
+   must resolve them at link-time.  */
 
+static void
+msp430_insert_uleb128_fixes (bfd *abfd ATTRIBUTE_UNUSED,
+			    asection *sec, void *xxx ATTRIBUTE_UNUSED)
+{
+  segment_info_type *seginfo = seg_info (sec);
+  struct frag *fragP;
+
+  subseg_set (sec, 0);
+
+  for (fragP = seginfo->frchainP->frch_root;
+       fragP; fragP = fragP->fr_next)
+    {
+      expressionS *exp, *exp_dup;
+
+      if (fragP->fr_type != rs_leb128  || fragP->fr_symbol == NULL)
+	continue;
+
+      exp = symbol_get_value_expression (fragP->fr_symbol);
+
+      if (exp->X_op != O_subtract)
+	continue;
+
+      /* FIXME: Skip for .sleb128.  */
+      if (fragP->fr_subtype != 0)
+	continue;
+
+      exp_dup = xmemdup (exp, sizeof (*exp), sizeof (*exp));
+      exp_dup->X_op = O_symbol;
+      exp_dup->X_op_symbol = NULL;
+
+      /* Emit the SUB relocation first, since the SET relocation will write out
+	 the final value.  */
+      exp_dup->X_add_symbol = exp->X_op_symbol;
+      fix_new_exp (fragP, fragP->fr_fix, 0,
+		   exp_dup, 0, BFD_RELOC_MSP430_SUB_ULEB128);
+
+      exp_dup->X_add_symbol = exp->X_add_symbol;
+      /* Insert relocations to resolve the subtraction at link-time.  */
+      fix_new_exp (fragP, fragP->fr_fix, 0,
+		   exp_dup, 0, BFD_RELOC_MSP430_SET_ULEB128);
+
+    }
+}
+
+/* Called after all assembly has been done.  */
 void
 msp430_md_end (void)
 {
@@ -5065,6 +5113,10 @@ msp430_md_end (void)
 	as_warn (_(WARN_NOP_AT_EOF));
     }
 
+  /* Insert relocations for uleb128 directives, so the values can be recomputed
+     at link time.  */
+  bfd_map_over_sections (stdoutput, msp430_insert_uleb128_fixes, NULL);
+
   /* We have already emitted an error if any of the following attributes
      disagree with the attributes in the input assembly file.  See
      msp430_object_attribute.  */
diff --git a/include/elf/msp430.h b/include/elf/msp430.h
index 536f71452f6..6169801f3fa 100644
--- a/include/elf/msp430.h
+++ b/include/elf/msp430.h
@@ -113,6 +113,8 @@ START_RELOC_NUMBERS (elf_msp430_reloc_type)
      RELOC_NUMBER (R_MSP430_RL_PCREL,		8)
      RELOC_NUMBER (R_MSP430_8,			9)
      RELOC_NUMBER (R_MSP430_SYM_DIFF,		10)
+     RELOC_NUMBER (R_MSP430_SET_ULEB128, 11) /* GNU only.  */
+     RELOC_NUMBER (R_MSP430_SUB_ULEB128, 12) /* GNU only.  */
 END_RELOC_NUMBERS (R_MSP430_max)
 
 START_RELOC_NUMBERS (elf_msp430x_reloc_type)
@@ -137,6 +139,8 @@ START_RELOC_NUMBERS (elf_msp430x_reloc_type)
      RELOC_NUMBER (R_MSP430X_10_PCREL, 19)	/* Red Hat invention.  Used for Jump instructions.  */
      RELOC_NUMBER (R_MSP430X_2X_PCREL, 20)	/* Red Hat invention.  Used for relaxing jumps.  */
      RELOC_NUMBER (R_MSP430X_SYM_DIFF, 21)	/* Red Hat invention.  Used for relaxing debug info.  */
+     RELOC_NUMBER (R_MSP430X_SET_ULEB128, 22) /* GNU only.  */
+     RELOC_NUMBER (R_MSP430X_SUB_ULEB128, 23) /* GNU only.  */
 END_RELOC_NUMBERS (R_MSP430x_max)
 
 #endif /* _ELF_MSP430_H */
diff --git a/ld/testsuite/ld-msp430-elf/msp430-elf.exp b/ld/testsuite/ld-msp430-elf/msp430-elf.exp
index a2fa4db48d4..875b413c149 100644
--- a/ld/testsuite/ld-msp430-elf/msp430-elf.exp
+++ b/ld/testsuite/ld-msp430-elf/msp430-elf.exp
@@ -176,6 +176,9 @@ set msp430arraytests {
 
 run_ld_link_tests $msp430arraytests
 
+run_dump_test uleb128_430
+run_dump_test uleb128_430x
+
 # Don't run further tests when msp430 ISA is selected
 if {[string match "*-mcpu=msp430 *" [board_info [target_info name] multilib_flags]]
   || [string match "*-mcpu=msp430" [board_info [target_info name] multilib_flags]]} {
diff --git a/ld/testsuite/ld-msp430-elf/uleb128.s b/ld/testsuite/ld-msp430-elf/uleb128.s
new file mode 100644
index 00000000000..93d745ed426
--- /dev/null
+++ b/ld/testsuite/ld-msp430-elf/uleb128.s
@@ -0,0 +1,35 @@
+.data
+	.global	bar
+	.balign 2
+bar:
+	.short	42
+	.short	43
+
+	.global foo
+foo:
+.skip 0xff
+
+	.global	foo2
+	.balign 2
+foo2:
+	.short	4
+
+.text
+
+  .balign 2
+  .global byte
+byte:
+  .word foo-bar
+  .word foo2-bar
+
+  .global uleb
+  .balign 2
+uleb:
+	.uleb128 foo-bar  ; this value can be stored in one byte
+	.uleb128 foo2-bar ; this value requires 2 bytes
+
+  .balign 2
+  .global _start
+  _start:
+  nop
+
diff --git a/ld/testsuite/ld-msp430-elf/uleb128_430.d b/ld/testsuite/ld-msp430-elf/uleb128_430.d
new file mode 100644
index 00000000000..5104552e7a9
--- /dev/null
+++ b/ld/testsuite/ld-msp430-elf/uleb128_430.d
@@ -0,0 +1,10 @@
+#source: uleb128.s
+#as: -mcpu=msp430
+#ld:
+#objdump: -sj.text
+
+.*:[ 	]+file format .*
+
+Contents of section .text:
+ [0-9a-f]+ 04000401 04840200.*
+#pass
diff --git a/ld/testsuite/ld-msp430-elf/uleb128_430x.d b/ld/testsuite/ld-msp430-elf/uleb128_430x.d
new file mode 100644
index 00000000000..e808a53bedf
--- /dev/null
+++ b/ld/testsuite/ld-msp430-elf/uleb128_430x.d
@@ -0,0 +1,10 @@
+#source: uleb128.s
+#as: -mcpu=msp430x
+#ld:
+#objdump: -sj.text
+
+.*:[ 	]+file format .*
+
+Contents of section .text:
+ [0-9a-f]+ 04000401 04840200.*
+#pass
-- 
2.28.0

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

* Re: [PATCH] MSP430: Support relocations for subtract expressions in .uleb128 directives
  2020-09-02 12:21 [PATCH] MSP430: Support relocations for subtract expressions in .uleb128 directives Jozef Lawrynowicz
@ 2020-09-03 14:41 ` Nick Clifton
  2020-09-03 20:22   ` Jozef Lawrynowicz
  0 siblings, 1 reply; 11+ messages in thread
From: Nick Clifton @ 2020-09-03 14:41 UTC (permalink / raw)
  To: binutils

Hi Jozef,

  I have a few questions/comments about this patch:

  * Have these new relocations been agreed with TI ?
    Ie are they now part of the MSP430 EABI ?  If not, then
    they probably ought to include "GNU" as part of their names...

  * Paranoia here:  The write_uleb128() function really should
    take an end pointer parameter, and check to make sure that it
    is not overwritten.  I know that you do check the return pointer
    in the code, but I would not be at all surprised that a fuzzed
    MSP430 binary could persuade the new code to write beyond the
    end of the section buffer.

    Also the function should be renamed to _bfd_write_uleb128
    (or something similar) and moved into bfd/elf-eh-frame.c.
    It is a generic function after all, and should live with the
    other LEB128 functions.

Other than that though the patch looks good to me.

Cheers
  Nick


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

* Re: [PATCH] MSP430: Support relocations for subtract expressions in .uleb128 directives
  2020-09-03 14:41 ` Nick Clifton
@ 2020-09-03 20:22   ` Jozef Lawrynowicz
  2020-09-04  7:13     ` Nick Clifton
  0 siblings, 1 reply; 11+ messages in thread
From: Jozef Lawrynowicz @ 2020-09-03 20:22 UTC (permalink / raw)
  To: Nick Clifton; +Cc: binutils

Hi Nick,

On Thu, Sep 03, 2020 at 03:41:59PM +0100, Nick Clifton via Binutils wrote:
> Hi Jozef,
> 
>   I have a few questions/comments about this patch:
> 
>   * Have these new relocations been agreed with TI ?
>     Ie are they now part of the MSP430 EABI ?  If not, then
>     they probably ought to include "GNU" as part of their names...

No, these relocations are GNU specific, they are not part of MSP430 EABI.
I'll update the names.

> 
>   * Paranoia here:  The write_uleb128() function really should
>     take an end pointer parameter, and check to make sure that it
>     is not overwritten.  I know that you do check the return pointer
>     in the code, but I would not be at all surprised that a fuzzed
>     MSP430 binary could persuade the new code to write beyond the
>     end of the section buffer.
> 
>     Also the function should be renamed to _bfd_write_uleb128
>     (or something similar) and moved into bfd/elf-eh-frame.c.
>     It is a generic function after all, and should live with the
>     other LEB128 functions.

Thanks for the feedback, I'm working on an updated patch. I'm actually
going to put the generic version into bfd/libbfd.c, as there are
functions in there to read leb128 values as well, and it doesn't need to
be ELF specific.

Since relaxation can actually grow the size of branches on MSP430,
I was thinking about how to handle a possible unfortunate situation
where the result of the subtract expression increases past a certain point
and so the uleb128 value requires an extra byte of space. The patch
currently has an error message for this case, but the user wouldn't
really be able to do anything about it, except maybe go back and
manually change some JMP to BR in the assembly code.

Would the reservation of an extra byte by the assembler when it inserts
the SET/SUB relocations be the only way to really protect against this?

Technically we might even require the size of the field reserved for the
relocated value to be large enough to hold the maximum difference
between two pointers.

Thanks,
Jozef

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

* Re: [PATCH] MSP430: Support relocations for subtract expressions in .uleb128 directives
  2020-09-03 20:22   ` Jozef Lawrynowicz
@ 2020-09-04  7:13     ` Nick Clifton
  2020-09-04 16:52       ` Jozef Lawrynowicz
  0 siblings, 1 reply; 11+ messages in thread
From: Nick Clifton @ 2020-09-04  7:13 UTC (permalink / raw)
  To: binutils

Hi Jozef,

> No, these relocations are GNU specific, they are not part of MSP430 EABI.
> I'll update the names.

Oh - just a thought - I did not check the numbers that you had chosen for these
relocations, but you ought to make sure that they are not adjacent to the already
existing reloc values.  If TI does create some more relocs for the MSP430 they 
are likely to select values close to the current range, and it would be best to
avoid a collision with your new values.


> The patch
> currently has an error message for this case, but the user wouldn't
> really be able to do anything about it, except maybe go back and
> manually change some JMP to BR in the assembly code.

Hmmm - does the patch allow to the link to complete ?  I think that it
should, so at least the user will have a binary that can execute, even
if its debugging information is not 100% accurate.


> Would the reservation of an extra byte by the assembler when it inserts
> the SET/SUB relocations be the only way to really protect against this?

Hmm, that does sound like the only way.  Which presumably means that the
sections containing these expressions are going to be larger than they
really need to be.  (I am right in thinking that even when you are able
to reduce the length of a leb128 value you do not actually change the 
size of the section in which it resides, right ?)  Possibly this is
something that a tool like dwz could mitigate.

Cheers
  Nick


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

* Re: [PATCH] MSP430: Support relocations for subtract expressions in .uleb128 directives
  2020-09-04  7:13     ` Nick Clifton
@ 2020-09-04 16:52       ` Jozef Lawrynowicz
  2020-09-07 12:28         ` Jozef Lawrynowicz
  0 siblings, 1 reply; 11+ messages in thread
From: Jozef Lawrynowicz @ 2020-09-04 16:52 UTC (permalink / raw)
  To: Nick Clifton; +Cc: binutils

On Fri, Sep 04, 2020 at 08:13:05AM +0100, Nick Clifton via Binutils wrote:
> Hi Jozef,
> 
> > No, these relocations are GNU specific, they are not part of MSP430 EABI.
> > I'll update the names.
> 
> Oh - just a thought - I did not check the numbers that you had chosen for these
> relocations, but you ought to make sure that they are not adjacent to the already
> existing reloc values.  If TI does create some more relocs for the MSP430 they 
> are likely to select values close to the current range, and it would be best to
> avoid a collision with your new values.

I would agree, but there are already existing GNU-only relocs which begin
immediately after the TI ABI reloc numbers:
     RELOC_NUMBER (R_MSP430_PREL31, 17)
     RELOC_NUMBER (R_MSP430_EHTYPE, 18)   /* Mentioned in ABI.  */
     RELOC_NUMBER (R_MSP430X_10_PCREL, 19)  /* Red Hat invention.  Used for Jump instructions.  */
     RELOC_NUMBER (R_MSP430X_2X_PCREL, 20)  /* Red Hat invention.  Used for relaxing jumps.  */
     RELOC_NUMBER (R_MSP430X_SYM_DIFF, 21)  /* Red Hat invention.  Used for relaxing debug info.  */

I don't think there's any benefit to leaving a gap after R_MSP430X_SYM_DIFF
and I would prefer not changing the existing GNU relocs, so compatibility
with older GNU tools is maintained.

> 
> > The patch
> > currently has an error message for this case, but the user wouldn't
> > really be able to do anything about it, except maybe go back and
> > manually change some JMP to BR in the assembly code.
> 
> Hmmm - does the patch allow to the link to complete ?  I think that it
> should, so at least the user will have a binary that can execute, even
> if its debugging information is not 100% accurate.
> 

Since .uleb128 directives are also used in the .gcc_except_table
section, it could break exception handling. But at least the user will
be warned, and this seems like it would be a very rare case to actually
trigger. So I'm not going to try and handle this case at this time.

> 
> > Would the reservation of an extra byte by the assembler when it inserts
> > the SET/SUB relocations be the only way to really protect against this?
> 
> Hmm, that does sound like the only way.  Which presumably means that the
> sections containing these expressions are going to be larger than they
> really need to be.  (I am right in thinking that even when you are able
> to reduce the length of a leb128 value you do not actually change the 
> size of the section in which it resides, right ?)  Possibly this is
> something that a tool like dwz could mitigate.

Yes the field size never changes, even when the new value requires fewer
bytes. My new patch tweaks this a bit and pads the new value with
uleb128 zeros from the MSB. This should avoid any additional potential
for breakage when the value following the field is a regular integer and
not a uleb128 value.

Thanks,
Jozef

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

* Re: [PATCH] MSP430: Support relocations for subtract expressions in .uleb128 directives
  2020-09-04 16:52       ` Jozef Lawrynowicz
@ 2020-09-07 12:28         ` Jozef Lawrynowicz
  2020-09-08 10:01           ` Nick Clifton
  0 siblings, 1 reply; 11+ messages in thread
From: Jozef Lawrynowicz @ 2020-09-07 12:28 UTC (permalink / raw)
  To: Nick Clifton; +Cc: binutils

[-- Attachment #1: Type: text/plain, Size: 3418 bytes --]

Hi Nick,

On Fri, Sep 04, 2020 at 05:52:40PM +0100, Jozef Lawrynowicz wrote:
> On Fri, Sep 04, 2020 at 08:13:05AM +0100, Nick Clifton via Binutils wrote:
> > Hi Jozef,
> > 
> > > No, these relocations are GNU specific, they are not part of MSP430 EABI.
> > > I'll update the names.
> > 
> > Oh - just a thought - I did not check the numbers that you had chosen for these
> > relocations, but you ought to make sure that they are not adjacent to the already
> > existing reloc values.  If TI does create some more relocs for the MSP430 they 
> > are likely to select values close to the current range, and it would be best to
> > avoid a collision with your new values.
> 
> I would agree, but there are already existing GNU-only relocs which begin
> immediately after the TI ABI reloc numbers:
>      RELOC_NUMBER (R_MSP430_PREL31, 17)
>      RELOC_NUMBER (R_MSP430_EHTYPE, 18)   /* Mentioned in ABI.  */
>      RELOC_NUMBER (R_MSP430X_10_PCREL, 19)  /* Red Hat invention.  Used for Jump instructions.  */
>      RELOC_NUMBER (R_MSP430X_2X_PCREL, 20)  /* Red Hat invention.  Used for relaxing jumps.  */
>      RELOC_NUMBER (R_MSP430X_SYM_DIFF, 21)  /* Red Hat invention.  Used for relaxing debug info.  */
> 
> I don't think there's any benefit to leaving a gap after R_MSP430X_SYM_DIFF
> and I would prefer not changing the existing GNU relocs, so compatibility
> with older GNU tools is maintained.
> 
> > 
> > > The patch
> > > currently has an error message for this case, but the user wouldn't
> > > really be able to do anything about it, except maybe go back and
> > > manually change some JMP to BR in the assembly code.
> > 
> > Hmmm - does the patch allow to the link to complete ?  I think that it
> > should, so at least the user will have a binary that can execute, even
> > if its debugging information is not 100% accurate.
> > 
> 
> Since .uleb128 directives are also used in the .gcc_except_table
> section, it could break exception handling. But at least the user will
> be warned, and this seems like it would be a very rare case to actually
> trigger. So I'm not going to try and handle this case at this time.
> 
> > 
> > > Would the reservation of an extra byte by the assembler when it inserts
> > > the SET/SUB relocations be the only way to really protect against this?
> > 
> > Hmm, that does sound like the only way.  Which presumably means that the
> > sections containing these expressions are going to be larger than they
> > really need to be.  (I am right in thinking that even when you are able
> > to reduce the length of a leb128 value you do not actually change the 
> > size of the section in which it resides, right ?)  Possibly this is
> > something that a tool like dwz could mitigate.
> 
> Yes the field size never changes, even when the new value requires fewer
> bytes. My new patch tweaks this a bit and pads the new value with
> uleb128 zeros from the MSB. This should avoid any additional potential
> for breakage when the value following the field is a regular integer and
> not a uleb128 value.

I've attached an updated patch with the following changes:
- "GNU" used in new reloc names
- uleb128 write function moved to bfd/libbfd.c
- uleb128 write function takes an end pointer argument and will not
  overwrite memory beyond this
- relocated uleb128 value is "right aligned" within the available space

Successfully re-tested the patch.

Ok to apply?

Thanks,
Jozef

[-- Attachment #2: 0001-MSP430-Support-relocations-for-subtract-expressions-.patch --]
[-- Type: text/plain, Size: 21638 bytes --]

From c57431b98b1b022ad52fc045544f82c10d034308 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
Date: Fri, 4 Sep 2020 18:26:47 +0100
Subject: [PATCH] MSP430: Support relocations for subtract expressions in
 .uleb128 directives

Link-time relaxations of branches are common for MSP430, given that GCC
can generate pessimal branch instructions, and the
-mcode-region=either/-mdata-region=either options to shuffle sections
can further change the type of branch instruction required.

These relaxations can result in invalid code when .uleb128
directives, used in the .gcc_except_table section, are used to calculate
the distance between two labels. A value for the .uleb128 directive is
calculated at assembly-time, and can't be updated at link-time, even if
relaxation causes the distance between the labels to change.

This patch adds relocations for subtract expressions in .uleb128
directives, to allow the linker to re-calculate the value of these
expressions after relaxation has been performed.

bfd/ChangeLog:

2020-09-07  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
	Kuan-Lin Chen  <kuanlinchentw@gmail.com>

	* bfd-in2.h (bfd_reloc_code_real): Add
	BFD_RELOC_MSP430_{SET,SUB}_ULEB128.
	* elf32-msp430.c (msp430_elf_ignore_reloc): New.
	(elf_msp430_howto_table): Add R_MSP430{,X}_GNU_{SET,SUB}_ULEB128.
	(msp430_reloc_map): Add R_MSP430_GNU_{SET,SUB}_ULEB128.
	(msp430x_reloc_map): Add R_MSP430X_GNU_{SET,SUB}_ULEB128.
	(write_uleb128): New.
	(msp430_final_link_relocate): Handle R_MSP430{,X}_GNU_{SET,SUB}_ULEB128.
	* libbfd.c (_bfd_write_unsigned_leb128): New.
	* libbfd.h (_bfd_write_unsigned_leb128): New prototype.
	Add BFD_RELOC_MSP430_{SET,SUB}_ULEB128.
	* reloc.c: Document BFD_RELOC_MSP430_{SET,SUB}_ULEB128.

binutils/ChangeLog:

2020-09-07  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
	Kuan-Lin Chen  <kuanlinchentw@gmail.com>

	* readelf.c (target_specific_reloc_handling): Handle
	R_MSP430{,X}_GNU_{SET,SUB}_ULEB128.

gas/ChangeLog:

2020-09-07  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
	Kuan-Lin Chen  <kuanlinchentw@gmail.com>

	* config/tc-msp430.c (msp430_insert_uleb128_fixes): New.
	(msp430_md_end): Call msp430_insert_uleb128_fixes.

include/ChangeLog:

2020-09-07  Jozef Lawrynowicz  <jozef.l@mittosystems.com>
	Kuan-Lin Chen  <kuanlinchentw@gmail.com>

	* elf/msp430.h (elf_msp430_reloc_type): Add
	R_MSP430_GNU_{SET,SUB}_ULEB128.
	(elf_msp430x_reloc_type): Add R_MSP430X_GNU_{SET,SUB}_ULEB128.

ld/ChangeLog:

2020-09-07  Jozef Lawrynowicz  <jozef.l@mittosystems.com>

	* testsuite/ld-msp430-elf/msp430-elf.exp: Run new tests.
	* testsuite/ld-msp430-elf/uleb128.s: New test.
	* testsuite/ld-msp430-elf/uleb128_430.d: New test.
	* testsuite/ld-msp430-elf/uleb128_430x.d: New test.

---
 bfd/bfd-in2.h                             |   2 +
 bfd/elf32-msp430.c                        | 153 +++++++++++++++++++++-
 bfd/libbfd.c                              |  24 ++++
 bfd/libbfd.h                              |   4 +
 bfd/reloc.c                               |   5 +
 binutils/readelf.c                        |  27 +++-
 gas/config/tc-msp430.c                    |  54 +++++++-
 include/elf/msp430.h                      |   4 +
 ld/testsuite/ld-msp430-elf/msp430-elf.exp |   3 +
 ld/testsuite/ld-msp430-elf/uleb128.s      |  34 +++++
 ld/testsuite/ld-msp430-elf/uleb128_430.d  |  10 ++
 ld/testsuite/ld-msp430-elf/uleb128_430x.d |  10 ++
 12 files changed, 320 insertions(+), 10 deletions(-)
 create mode 100644 ld/testsuite/ld-msp430-elf/uleb128.s
 create mode 100644 ld/testsuite/ld-msp430-elf/uleb128_430.d
 create mode 100644 ld/testsuite/ld-msp430-elf/uleb128_430x.d

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 3c85b07301..74d3a8ab53 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -5113,6 +5113,8 @@ then it may be truncated to 8 bits.  */
   BFD_RELOC_MSP430_ABS_HI16,
   BFD_RELOC_MSP430_PREL31,
   BFD_RELOC_MSP430_SYM_DIFF,
+  BFD_RELOC_MSP430_SET_ULEB128,
+  BFD_RELOC_MSP430_SUB_ULEB128,
 
 /* Relocations used by the Altera Nios II core.  */
   BFD_RELOC_NIOS2_S16,
diff --git a/bfd/elf32-msp430.c b/bfd/elf32-msp430.c
index 59e54ecbc9..a4e30e7fe7 100644
--- a/bfd/elf32-msp430.c
+++ b/bfd/elf32-msp430.c
@@ -56,6 +56,20 @@ rl78_sym_diff_handler (bfd * abfd,
   return bfd_reloc_continue;
 }
 
+/* Special handler for relocations which don't have to be relocated.
+   This function just simply returns bfd_reloc_ok.  */
+static bfd_reloc_status_type
+msp430_elf_ignore_reloc (bfd *abfd ATTRIBUTE_UNUSED, arelent *reloc_entry,
+			asymbol *symbol ATTRIBUTE_UNUSED,
+			void *data ATTRIBUTE_UNUSED, asection *input_section,
+			bfd *output_bfd, char **error_message ATTRIBUTE_UNUSED)
+{
+  if (output_bfd != NULL)
+    reloc_entry->address += input_section->output_offset;
+
+  return bfd_reloc_ok;
+}
+
 static reloc_howto_type elf_msp430_howto_table[] =
 {
   HOWTO (R_MSP430_NONE,		/* type */
@@ -220,7 +234,40 @@ static reloc_howto_type elf_msp430_howto_table[] =
 	 FALSE,			/* partial_inplace */
 	 0xffffffff,		/* src_mask */
 	 0xffffffff,		/* dst_mask */
-	 FALSE)			/* pcrel_offset */
+	 FALSE),		/* pcrel_offset */
+
+  /* The length of unsigned-leb128 is variable, just assume the
+     size is one byte here.  */
+  HOWTO (R_MSP430_GNU_SET_ULEB128,	/* type */
+	 0,				/* rightshift */
+	 0,				/* size */
+	 0,				/* bitsize */
+	 FALSE,				/* pc_relative */
+	 0,				/* bitpos */
+	 complain_overflow_dont,	/* complain_on_overflow */
+	 msp430_elf_ignore_reloc,	/* special handler.  */
+	 "R_MSP430_GNU_SET_ULEB128",	/* name */
+	 FALSE,				/* partial_inplace */
+	 0,				/* src_mask */
+	 0,				/* dst_mask */
+	 FALSE),			/* pcrel_offset */
+
+  /* The length of unsigned-leb128 is variable, just assume the
+     size is one byte here.  */
+  HOWTO (R_MSP430_GNU_SUB_ULEB128,	/* type */
+	 0,				/* rightshift */
+	 0,				/* size */
+	 0,				/* bitsize */
+	 FALSE,				/* pc_relative */
+	 0,				/* bitpos */
+	 complain_overflow_dont,	/* complain_on_overflow */
+	 msp430_elf_ignore_reloc,	/* special handler.  */
+	 "R_MSP430_GNU_SUB_ULEB128",	/* name */
+	 FALSE,				/* partial_inplace */
+	 0,				/* src_mask */
+	 0,				/* dst_mask */
+	 FALSE),			/* pcrel_offset */
+
 };
 
 static reloc_howto_type elf_msp430x_howto_table[] =
@@ -523,7 +570,40 @@ static reloc_howto_type elf_msp430x_howto_table[] =
 	 FALSE,			/* partial_inplace */
 	 0xffffffff,		/* src_mask */
 	 0xffffffff,		/* dst_mask */
-	 FALSE)			/* pcrel_offset */
+	 FALSE),		/* pcrel_offset */
+
+  /* The length of unsigned-leb128 is variable, just assume the
+     size is one byte here.  */
+  HOWTO (R_MSP430X_GNU_SET_ULEB128,	/* type */
+	 0,				/* rightshift */
+	 0,				/* size */
+	 0,				/* bitsize */
+	 FALSE,				/* pc_relative */
+	 0,				/* bitpos */
+	 complain_overflow_dont,	/* complain_on_overflow */
+	 msp430_elf_ignore_reloc,	/* special handler.  */
+	 "R_MSP430X_GNU_SET_ULEB128",	/* name */
+	 FALSE,				/* partial_inplace */
+	 0,				/* src_mask */
+	 0,				/* dst_mask */
+	 FALSE),			/* pcrel_offset */
+
+  /* The length of unsigned-leb128 is variable, just assume the
+     size is one byte here.  */
+  HOWTO (R_MSP430X_GNU_SUB_ULEB128,	/* type */
+	 0,				/* rightshift */
+	 0,				/* size */
+	 0,				/* bitsize */
+	 FALSE,				/* pc_relative */
+	 0,				/* bitpos */
+	 complain_overflow_dont,	/* complain_on_overflow */
+	 msp430_elf_ignore_reloc,	/* special handler.  */
+	 "R_MSP430X_GNU_SUB_ULEB128",	/* name */
+	 FALSE,				/* partial_inplace */
+	 0,				/* src_mask */
+	 0,				/* dst_mask */
+	 FALSE),			/* pcrel_offset */
+
 };
 
 /* Map BFD reloc types to MSP430 ELF reloc types.  */
@@ -547,7 +627,9 @@ static const struct msp430_reloc_map msp430_reloc_map[] =
   {BFD_RELOC_MSP430_2X_PCREL,	   R_MSP430_2X_PCREL},
   {BFD_RELOC_MSP430_RL_PCREL,	   R_MSP430_RL_PCREL},
   {BFD_RELOC_8,			   R_MSP430_8},
-  {BFD_RELOC_MSP430_SYM_DIFF,	   R_MSP430_SYM_DIFF}
+  {BFD_RELOC_MSP430_SYM_DIFF,	   R_MSP430_SYM_DIFF},
+  {BFD_RELOC_MSP430_SET_ULEB128,   R_MSP430_GNU_SET_ULEB128 },
+  {BFD_RELOC_MSP430_SUB_ULEB128,   R_MSP430_GNU_SUB_ULEB128 }
 };
 
 static const struct msp430_reloc_map msp430x_reloc_map[] =
@@ -573,7 +655,9 @@ static const struct msp430_reloc_map msp430x_reloc_map[] =
   {BFD_RELOC_MSP430_10_PCREL,	      R_MSP430X_10_PCREL},
   {BFD_RELOC_MSP430_2X_PCREL,	      R_MSP430X_2X_PCREL},
   {BFD_RELOC_MSP430_RL_PCREL,	      R_MSP430X_PCR16},
-  {BFD_RELOC_MSP430_SYM_DIFF,	      R_MSP430X_SYM_DIFF}
+  {BFD_RELOC_MSP430_SYM_DIFF,	      R_MSP430X_SYM_DIFF},
+  {BFD_RELOC_MSP430_SET_ULEB128,      R_MSP430X_GNU_SET_ULEB128 },
+  {BFD_RELOC_MSP430_SUB_ULEB128,      R_MSP430X_GNU_SUB_ULEB128 }
 };
 
 static inline bfd_boolean
@@ -755,6 +839,9 @@ msp430_final_link_relocate (reloc_howto_type *	   howto,
      if (uses_msp430x_relocs (input_bfd))
        switch (howto->type)
 	 {
+	 case R_MSP430X_GNU_SET_ULEB128:
+	   relocation += (!is_rel_reloc ? rel->r_addend : 0);
+	   /* Fall through.  */
 	 case R_MSP430_ABS32:
 	  /* If we are computing a 32-bit value for the location lists
 	     and the result is 0 then we add one to the value.  A zero
@@ -780,6 +867,9 @@ msp430_final_link_relocate (reloc_howto_type *	   howto,
      else
        switch (howto->type)
 	 {
+	 case R_MSP430_GNU_SET_ULEB128:
+	   relocation += (!is_rel_reloc ? rel->r_addend : 0);
+	   /* Fall through.  */
 	 case R_MSP430_32:
 	 case R_MSP430_16:
 	 case R_MSP430_16_BYTE:
@@ -794,16 +884,63 @@ msp430_final_link_relocate (reloc_howto_type *	   howto,
       sym_diff_section = NULL;
     }
 
-  if (uses_msp430x_relocs (input_bfd))
+  if ((uses_msp430x_relocs (input_bfd)
+       && howto->type == R_MSP430X_GNU_SET_ULEB128)
+      || (!uses_msp430x_relocs (input_bfd)
+	  && howto->type == R_MSP430_GNU_SET_ULEB128))
+    {
+      unsigned int len, new_len = 0;
+      bfd_byte *endp, *p;
+      unsigned int val = relocation;
+
+      _bfd_read_unsigned_leb128 (input_bfd, contents + rel->r_offset, &len);
+
+      /* Clean the contents value to zero.  Do not reduce the length.  */
+      p = contents + rel->r_offset;
+      endp = (p + len) - 1;
+      memset (p, 0x80, len - 1);
+      *(endp) = 0;
+
+      /* Get the length of the new uleb128 value.  */
+      do
+	{
+	  new_len++;
+	  val >>= 7;
+	} while (val);
+
+      if (new_len > len)
+	{
+	  _bfd_error_handler
+	    (_("error: final size of uleb128 value at offset 0x%lx in %pA "
+	       "from %pB exceeds available space"),
+	     rel->r_offset, input_section, input_bfd);
+	}
+      else
+	{
+	  /* If the number of bytes required to store the new value has
+	     decreased, "right align" the new value within the available space,
+	     so the MSB side is padded with uleb128 zeros (0x80).  */
+	  p = _bfd_write_unsigned_leb128 (p + (len - new_len), endp,
+					  relocation);
+	  /* We checked there is enough space for the new value above, so this
+	     should never be NULL.  */
+	  BFD_ASSERT (p);
+	}
+
+      return bfd_reloc_ok;
+    }
+  else if (uses_msp430x_relocs (input_bfd))
     switch (howto->type)
       {
       case R_MSP430X_SYM_DIFF:
+      case R_MSP430X_GNU_SUB_ULEB128:
 	/* Cache the input section and value.
 	   The offset is unreliable, since relaxation may
 	   have reduced the following reloc's offset.  */
 	BFD_ASSERT (! is_rel_reloc);
 	sym_diff_section = input_section;
-	sym_diff_value = relocation;
+	sym_diff_value = relocation + (howto->type == R_MSP430X_GNU_SUB_ULEB128
+				       ? rel->r_addend : 0);
 	return bfd_reloc_ok;
 
       case R_MSP430_ABS16:
@@ -1254,11 +1391,13 @@ msp430_final_link_relocate (reloc_howto_type *	   howto,
       break;
 
     case R_MSP430_SYM_DIFF:
+    case R_MSP430_GNU_SUB_ULEB128:
       /* Cache the input section and value.
 	 The offset is unreliable, since relaxation may
 	 have reduced the following reloc's offset.  */
       sym_diff_section = input_section;
-      sym_diff_value = relocation;
+      sym_diff_value = relocation + (howto->type == R_MSP430_GNU_SUB_ULEB128
+				     ? rel->r_addend : 0);
       return bfd_reloc_ok;
 
       default:
diff --git a/bfd/libbfd.c b/bfd/libbfd.c
index efe10d22e8..8046e4873b 100644
--- a/bfd/libbfd.c
+++ b/bfd/libbfd.c
@@ -1151,6 +1151,30 @@ _bfd_read_signed_leb128 (bfd *abfd ATTRIBUTE_UNUSED,
   return result;
 }
 
+/* Write VAL in uleb128 format to P.
+   END indicates the last byte of allocated space for the uleb128 value to fit
+   in.
+   Return a pointer to the byte following the last byte that was written, or
+   NULL if the uleb128 value does not fit in the allocated space between P and
+   END.  */
+bfd_byte *
+_bfd_write_unsigned_leb128 (bfd_byte *p, bfd_byte *end, bfd_vma val)
+{
+  bfd_byte c;
+  do
+    {
+      if (p > end)
+	return NULL;
+      c = val & 0x7f;
+      val >>= 7;
+      if (val)
+	c |= 0x80;
+      *(p++) = c;
+    }
+  while (val);
+  return p;
+}
+
 bfd_boolean
 _bfd_generic_init_private_section_data (bfd *ibfd ATTRIBUTE_UNUSED,
 					asection *isec ATTRIBUTE_UNUSED,
diff --git a/bfd/libbfd.h b/bfd/libbfd.h
index b97534fc9f..7587e8f3c7 100644
--- a/bfd/libbfd.h
+++ b/bfd/libbfd.h
@@ -901,6 +901,8 @@ extern bfd_signed_vma _bfd_read_signed_leb128
 extern bfd_vma _bfd_safe_read_leb128
   (bfd *, bfd_byte *, unsigned int *, bfd_boolean, const bfd_byte * const)
   ATTRIBUTE_HIDDEN;
+extern bfd_byte * _bfd_write_unsigned_leb128
+  (bfd_byte *p, bfd_byte *end, bfd_vma val) ATTRIBUTE_HIDDEN;
 
 #if GCC_VERSION >= 7000
 #define _bfd_mul_overflow(a, b, res) __builtin_mul_overflow (a, b, res)
@@ -2802,6 +2804,8 @@ static const char *const bfd_reloc_code_real_names[] = { "@@uninitialized@@",
   "BFD_RELOC_MSP430_ABS_HI16",
   "BFD_RELOC_MSP430_PREL31",
   "BFD_RELOC_MSP430_SYM_DIFF",
+  "BFD_RELOC_MSP430_SET_ULEB128",
+  "BFD_RELOC_MSP430_SUB_ULEB128",
   "BFD_RELOC_NIOS2_S16",
   "BFD_RELOC_NIOS2_U16",
   "BFD_RELOC_NIOS2_CALL26",
diff --git a/bfd/reloc.c b/bfd/reloc.c
index b17c5e64ec..dc923fe39c 100644
--- a/bfd/reloc.c
+++ b/bfd/reloc.c
@@ -6367,6 +6367,11 @@ ENUMX
   BFD_RELOC_MSP430_PREL31
 ENUMX
   BFD_RELOC_MSP430_SYM_DIFF
+ENUMX
+  BFD_RELOC_MSP430_SET_ULEB128
+ENUMX
+  BFD_RELOC_MSP430_SUB_ULEB128
+
 ENUMDOC
   msp430 specific relocation codes
 
diff --git a/binutils/readelf.c b/binutils/readelf.c
index 94aa876b5a..cb4208f7b9 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -12586,10 +12586,12 @@ target_specific_reloc_handling (Filedata *           filedata,
 	switch (reloc_type)
 	  {
 	  case 10: /* R_MSP430_SYM_DIFF */
+	  case 12: /* R_MSP430_GNU_SUB_ULEB128 */
 	    if (uses_msp430x_relocs (filedata))
 	      break;
 	    /* Fall through.  */
 	  case 21: /* R_MSP430X_SYM_DIFF */
+	  case 23: /* R_MSP430X_GNU_SUB_ULEB128 */
 	    /* PR 21139.  */
 	    if (sym_index >= num_syms)
 	      error (_("MSP430 SYM_DIFF reloc contains invalid symbol index %lu\n"),
@@ -12604,12 +12606,14 @@ target_specific_reloc_handling (Filedata *           filedata,
 
 	  case 5: /* R_MSP430_16_BYTE */
 	  case 9: /* R_MSP430_8 */
+	  case 11: /* R_MSP430_GNU_SET_ULEB128 */
 	    if (uses_msp430x_relocs (filedata))
 	      break;
 	    goto handle_sym_diff;
 
 	  case 2: /* R_MSP430_ABS16 */
 	  case 15: /* R_MSP430X_ABS16 */
+	  case 22: /* R_MSP430X_GNU_SET_ULEB128 */
 	    if (! uses_msp430x_relocs (filedata))
 	      break;
 	    goto handle_sym_diff;
@@ -12617,10 +12621,29 @@ target_specific_reloc_handling (Filedata *           filedata,
 	  handle_sym_diff:
 	    if (saved_sym != NULL)
 	      {
-		int reloc_size = reloc_type == 1 ? 4 : 2;
 		bfd_vma value;
+		unsigned int reloc_size;
+		int leb_ret = 0;
+		switch (reloc_type)
+		  {
+		  case 1: /* R_MSP430_32 or R_MSP430_ABS32 */
+		    reloc_size = 4;
+		    break;
+		  case 11: /* R_MSP430_GNU_SET_ULEB128 */
+		  case 22: /* R_MSP430X_GNU_SET_ULEB128 */
+		    read_leb128 (start + reloc->r_offset, end, FALSE,
+				 &reloc_size, &leb_ret);
+		    break;
+		  default:
+		    reloc_size = 2;
+		    break;
+		  }
 
-		if (sym_index >= num_syms)
+		if (leb_ret != 0)
+		  error (_("MSP430 ULEB128 field at 0x%lx contains invalid "
+			   "ULEB128 value\n"),
+			 (long) reloc->r_offset);
+		else if (sym_index >= num_syms)
 		  error (_("MSP430 reloc contains invalid symbol index %lu\n"),
 			 sym_index);
 		else
diff --git a/gas/config/tc-msp430.c b/gas/config/tc-msp430.c
index 2738937b11..6d1803202c 100644
--- a/gas/config/tc-msp430.c
+++ b/gas/config/tc-msp430.c
@@ -5048,8 +5048,56 @@ msp430_fix_adjustable (struct fix *fixp ATTRIBUTE_UNUSED)
   return FALSE;
 }
 
-/* Set the contents of the .MSP430.attributes and .GNU.attributes sections.  */
+/* Scan uleb128 subtraction expressions and insert fixups for them.
+   e.g., .uleb128 .L1 - .L0
+   Because relaxation may change the value of the subtraction, we
+   must resolve them at link-time.  */
 
+static void
+msp430_insert_uleb128_fixes (bfd *abfd ATTRIBUTE_UNUSED,
+			    asection *sec, void *xxx ATTRIBUTE_UNUSED)
+{
+  segment_info_type *seginfo = seg_info (sec);
+  struct frag *fragP;
+
+  subseg_set (sec, 0);
+
+  for (fragP = seginfo->frchainP->frch_root;
+       fragP; fragP = fragP->fr_next)
+    {
+      expressionS *exp, *exp_dup;
+
+      if (fragP->fr_type != rs_leb128  || fragP->fr_symbol == NULL)
+	continue;
+
+      exp = symbol_get_value_expression (fragP->fr_symbol);
+
+      if (exp->X_op != O_subtract)
+	continue;
+
+      /* FIXME: Skip for .sleb128.  */
+      if (fragP->fr_subtype != 0)
+	continue;
+
+      exp_dup = xmemdup (exp, sizeof (*exp), sizeof (*exp));
+      exp_dup->X_op = O_symbol;
+      exp_dup->X_op_symbol = NULL;
+
+      /* Emit the SUB relocation first, since the SET relocation will write out
+	 the final value.  */
+      exp_dup->X_add_symbol = exp->X_op_symbol;
+      fix_new_exp (fragP, fragP->fr_fix, 0,
+		   exp_dup, 0, BFD_RELOC_MSP430_SUB_ULEB128);
+
+      exp_dup->X_add_symbol = exp->X_add_symbol;
+      /* Insert relocations to resolve the subtraction at link-time.  */
+      fix_new_exp (fragP, fragP->fr_fix, 0,
+		   exp_dup, 0, BFD_RELOC_MSP430_SET_ULEB128);
+
+    }
+}
+
+/* Called after all assembly has been done.  */
 void
 msp430_md_end (void)
 {
@@ -5065,6 +5113,10 @@ msp430_md_end (void)
 	as_warn (_(WARN_NOP_AT_EOF));
     }
 
+  /* Insert relocations for uleb128 directives, so the values can be recomputed
+     at link time.  */
+  bfd_map_over_sections (stdoutput, msp430_insert_uleb128_fixes, NULL);
+
   /* We have already emitted an error if any of the following attributes
      disagree with the attributes in the input assembly file.  See
      msp430_object_attribute.  */
diff --git a/include/elf/msp430.h b/include/elf/msp430.h
index 536f71452f..8d047cd18b 100644
--- a/include/elf/msp430.h
+++ b/include/elf/msp430.h
@@ -113,6 +113,8 @@ START_RELOC_NUMBERS (elf_msp430_reloc_type)
      RELOC_NUMBER (R_MSP430_RL_PCREL,		8)
      RELOC_NUMBER (R_MSP430_8,			9)
      RELOC_NUMBER (R_MSP430_SYM_DIFF,		10)
+     RELOC_NUMBER (R_MSP430_GNU_SET_ULEB128, 11) /* GNU only.  */
+     RELOC_NUMBER (R_MSP430_GNU_SUB_ULEB128, 12) /* GNU only.  */
 END_RELOC_NUMBERS (R_MSP430_max)
 
 START_RELOC_NUMBERS (elf_msp430x_reloc_type)
@@ -137,6 +139,8 @@ START_RELOC_NUMBERS (elf_msp430x_reloc_type)
      RELOC_NUMBER (R_MSP430X_10_PCREL, 19)	/* Red Hat invention.  Used for Jump instructions.  */
      RELOC_NUMBER (R_MSP430X_2X_PCREL, 20)	/* Red Hat invention.  Used for relaxing jumps.  */
      RELOC_NUMBER (R_MSP430X_SYM_DIFF, 21)	/* Red Hat invention.  Used for relaxing debug info.  */
+     RELOC_NUMBER (R_MSP430X_GNU_SET_ULEB128, 22) /* GNU only.  */
+     RELOC_NUMBER (R_MSP430X_GNU_SUB_ULEB128, 23) /* GNU only.  */
 END_RELOC_NUMBERS (R_MSP430x_max)
 
 #endif /* _ELF_MSP430_H */
diff --git a/ld/testsuite/ld-msp430-elf/msp430-elf.exp b/ld/testsuite/ld-msp430-elf/msp430-elf.exp
index a2fa4db48d..875b413c14 100644
--- a/ld/testsuite/ld-msp430-elf/msp430-elf.exp
+++ b/ld/testsuite/ld-msp430-elf/msp430-elf.exp
@@ -176,6 +176,9 @@ set msp430arraytests {
 
 run_ld_link_tests $msp430arraytests
 
+run_dump_test uleb128_430
+run_dump_test uleb128_430x
+
 # Don't run further tests when msp430 ISA is selected
 if {[string match "*-mcpu=msp430 *" [board_info [target_info name] multilib_flags]]
   || [string match "*-mcpu=msp430" [board_info [target_info name] multilib_flags]]} {
diff --git a/ld/testsuite/ld-msp430-elf/uleb128.s b/ld/testsuite/ld-msp430-elf/uleb128.s
new file mode 100644
index 0000000000..598ee0c1b9
--- /dev/null
+++ b/ld/testsuite/ld-msp430-elf/uleb128.s
@@ -0,0 +1,34 @@
+.data
+	.global	bar
+	.balign 2
+bar:
+	.short	42
+	.short	43
+
+	.global foo
+foo:
+.skip 0xff
+
+	.global	foo2
+	.balign 2
+foo2:
+	.short	4
+
+.text
+
+  .balign 2
+  .global byte
+byte:
+  .word foo-bar
+  .word foo2-bar
+
+  .global uleb
+  .balign 2
+uleb:
+	.uleb128 foo-bar  ; this value can be stored in one byte
+	.uleb128 foo2-bar ; this value requires 2 bytes
+
+  .balign 2
+  .global _start
+  _start:
+  nop
diff --git a/ld/testsuite/ld-msp430-elf/uleb128_430.d b/ld/testsuite/ld-msp430-elf/uleb128_430.d
new file mode 100644
index 0000000000..5104552e7a
--- /dev/null
+++ b/ld/testsuite/ld-msp430-elf/uleb128_430.d
@@ -0,0 +1,10 @@
+#source: uleb128.s
+#as: -mcpu=msp430
+#ld:
+#objdump: -sj.text
+
+.*:[ 	]+file format .*
+
+Contents of section .text:
+ [0-9a-f]+ 04000401 04840200.*
+#pass
diff --git a/ld/testsuite/ld-msp430-elf/uleb128_430x.d b/ld/testsuite/ld-msp430-elf/uleb128_430x.d
new file mode 100644
index 0000000000..e808a53bed
--- /dev/null
+++ b/ld/testsuite/ld-msp430-elf/uleb128_430x.d
@@ -0,0 +1,10 @@
+#source: uleb128.s
+#as: -mcpu=msp430x
+#ld:
+#objdump: -sj.text
+
+.*:[ 	]+file format .*
+
+Contents of section .text:
+ [0-9a-f]+ 04000401 04840200.*
+#pass
-- 
2.28.0

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

* Re: [PATCH] MSP430: Support relocations for subtract expressions in .uleb128 directives
  2020-09-07 12:28         ` Jozef Lawrynowicz
@ 2020-09-08 10:01           ` Nick Clifton
  2020-09-08 15:07             ` Jozef Lawrynowicz
  0 siblings, 1 reply; 11+ messages in thread
From: Nick Clifton @ 2020-09-08 10:01 UTC (permalink / raw)
  To: binutils

Hi Jozef,

> I've attached an updated patch with the following changes:
> - "GNU" used in new reloc names
> - uleb128 write function moved to bfd/libbfd.c
> - uleb128 write function takes an end pointer argument and will not
>   overwrite memory beyond this
> - relocated uleb128 value is "right aligned" within the available space
> 
> Successfully re-tested the patch.
> 
> Ok to apply?

Thanks for making these adjustments.  The patch is approved, but
it needs one small change before applying:

	  _bfd_error_handler
	    (_("error: final size of uleb128 value at offset 0x%lx in %pA "
	       "from %pB exceeds available space"),
	     rel->r_offset, input_section, input_bfd);

This line generates an error when compiling for a 64-bit target (eg --enable-targets=all)
because rel->r_offset is a long long but the print type is %0lx.  So if you could
add a cast, that would be great:

	  _bfd_error_handler
	    (_("error: final size of uleb128 value at offset 0x%lx in %pA "
	       "from %pB exceeds available space"),
	     (long) rel->r_offset, input_section, input_bfd);

Cheers
  Nick


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

* Re: [PATCH] MSP430: Support relocations for subtract expressions in .uleb128 directives
  2020-09-08 10:01           ` Nick Clifton
@ 2020-09-08 15:07             ` Jozef Lawrynowicz
  2020-09-08 15:23               ` Nick Clifton
  0 siblings, 1 reply; 11+ messages in thread
From: Jozef Lawrynowicz @ 2020-09-08 15:07 UTC (permalink / raw)
  To: Nick Clifton; +Cc: binutils

On Tue, Sep 08, 2020 at 11:01:17AM +0100, Nick Clifton via Binutils wrote:
> Hi Jozef,
> 
> > I've attached an updated patch with the following changes:
> > - "GNU" used in new reloc names
> > - uleb128 write function moved to bfd/libbfd.c
> > - uleb128 write function takes an end pointer argument and will not
> >   overwrite memory beyond this
> > - relocated uleb128 value is "right aligned" within the available space
> > 
> > Successfully re-tested the patch.
> > 
> > Ok to apply?
> 
> Thanks for making these adjustments.  The patch is approved, but
> it needs one small change before applying:
> 
> 	  _bfd_error_handler
> 	    (_("error: final size of uleb128 value at offset 0x%lx in %pA "
> 	       "from %pB exceeds available space"),
> 	     rel->r_offset, input_section, input_bfd);
> 
> This line generates an error when compiling for a 64-bit target (eg --enable-targets=all)
> because rel->r_offset is a long long but the print type is %0lx.  So if you could
> add a cast, that would be great:
> 
> 	  _bfd_error_handler
> 	    (_("error: final size of uleb128 value at offset 0x%lx in %pA "
> 	       "from %pB exceeds available space"),
> 	     (long) rel->r_offset, input_section, input_bfd);

Thanks for spotting that.

However, I'm not able to reproduce the error. I'd like to add this
configuration to my build system to spot any issues in the future.
I tried the following configure lines, but the build completes successfully
in both cases:

  ../configure --enable-targets=all --enable-werror
  ../configure --enable-targets=all --enable-werror --target=msp430-elf

Are there any additional options I need to add?

Thanks,
Jozef

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

* Re: [PATCH] MSP430: Support relocations for subtract expressions in .uleb128 directives
  2020-09-08 15:07             ` Jozef Lawrynowicz
@ 2020-09-08 15:23               ` Nick Clifton
  2020-09-08 16:46                 ` Jozef Lawrynowicz
  0 siblings, 1 reply; 11+ messages in thread
From: Nick Clifton @ 2020-09-08 15:23 UTC (permalink / raw)
  To: binutils

Hi Jozef,

> I tried the following configure lines, but the build completes successfully
> in both cases:
> 
>   ../configure --enable-targets=all --enable-werror
>   ../configure --enable-targets=all --enable-werror --target=msp430-elf
> 
> Are there any additional options I need to add?

Yes, and sorry for not noticing this.  The actual configuration command I used was:

  configure --enable-targets=all --enable-gold --enable-plugins CFLAGS=-m32 CXXFLAGS=-m32

The gold and plugin enablement are probably not important for this particular case,
but the 32-bit building flags are.  So it is not the case of building for a 64-bit target
that causes the problem, but rather building for a 32-bit host.  Sorry for getting that
wrong.

Cheers
  Nick


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

* Re: [PATCH] MSP430: Support relocations for subtract expressions in .uleb128 directives
  2020-09-08 15:23               ` Nick Clifton
@ 2020-09-08 16:46                 ` Jozef Lawrynowicz
  2020-09-08 23:23                   ` Alan Modra
  0 siblings, 1 reply; 11+ messages in thread
From: Jozef Lawrynowicz @ 2020-09-08 16:46 UTC (permalink / raw)
  To: Nick Clifton; +Cc: binutils

On Tue, Sep 08, 2020 at 04:23:43PM +0100, Nick Clifton via Binutils wrote:
> Hi Jozef,
> 
> > I tried the following configure lines, but the build completes successfully
> > in both cases:
> > 
> >   ../configure --enable-targets=all --enable-werror
> >   ../configure --enable-targets=all --enable-werror --target=msp430-elf
> > 
> > Are there any additional options I need to add?
> 
> Yes, and sorry for not noticing this.  The actual configuration command I used was:
> 
>   configure --enable-targets=all --enable-gold --enable-plugins CFLAGS=-m32 CXXFLAGS=-m32
> 
> The gold and plugin enablement are probably not important for this particular case,
> but the 32-bit building flags are.  So it is not the case of building for a 64-bit target
> that causes the problem, but rather building for a 32-bit host.  Sorry for getting that
> wrong.

That did the trick, thanks!

Jozef

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

* Re: [PATCH] MSP430: Support relocations for subtract expressions in .uleb128 directives
  2020-09-08 16:46                 ` Jozef Lawrynowicz
@ 2020-09-08 23:23                   ` Alan Modra
  0 siblings, 0 replies; 11+ messages in thread
From: Alan Modra @ 2020-09-08 23:23 UTC (permalink / raw)
  To: Nick Clifton, binutils

Put the prototype where it won't disappear on the next regen of libbfd.h.

	* libbfd-in.h (_bfd_write_unsigned_leb128): Declare.
	* libbfd.h: Regenerate.

diff --git a/bfd/libbfd-in.h b/bfd/libbfd-in.h
index 5d24efbeb2..84dba221e1 100644
--- a/bfd/libbfd-in.h
+++ b/bfd/libbfd-in.h
@@ -896,6 +896,8 @@ extern bfd_signed_vma _bfd_read_signed_leb128
 extern bfd_vma _bfd_safe_read_leb128
   (bfd *, bfd_byte *, unsigned int *, bfd_boolean, const bfd_byte * const)
   ATTRIBUTE_HIDDEN;
+extern bfd_byte * _bfd_write_unsigned_leb128
+  (bfd_byte *, bfd_byte *, bfd_vma) ATTRIBUTE_HIDDEN;
 
 #if GCC_VERSION >= 7000
 #define _bfd_mul_overflow(a, b, res) __builtin_mul_overflow (a, b, res)

-- 
Alan Modra
Australia Development Lab, IBM

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

end of thread, other threads:[~2020-09-08 23:23 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-02 12:21 [PATCH] MSP430: Support relocations for subtract expressions in .uleb128 directives Jozef Lawrynowicz
2020-09-03 14:41 ` Nick Clifton
2020-09-03 20:22   ` Jozef Lawrynowicz
2020-09-04  7:13     ` Nick Clifton
2020-09-04 16:52       ` Jozef Lawrynowicz
2020-09-07 12:28         ` Jozef Lawrynowicz
2020-09-08 10:01           ` Nick Clifton
2020-09-08 15:07             ` Jozef Lawrynowicz
2020-09-08 15:23               ` Nick Clifton
2020-09-08 16:46                 ` Jozef Lawrynowicz
2020-09-08 23:23                   ` Alan Modra

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).