[Binutils-2.39,backport,GAS] arm: Use DWARF numbering convention for pseudo-register representation

Message ID yw8jbkpqk6vw.fsf@arm.com
State Unresolved
Headers
Series [Binutils-2.39,backport,GAS] arm: Use DWARF numbering convention for pseudo-register representation |

Checks

Context Check Description
snail/binutils-gdb-check warning Git am fail log

Commit Message

Victor Do Nascimento Nov. 1, 2022, 3:54 p.m. UTC
  Hi all, 

Having missed the deadline at the time for the 2.39 release, would it
be possible to backport the following patch to Binutils 2.39?

Initially submitted to trunk in
https://sourceware.org/pipermail/binutils/2022-July/122092.html
This backport is a fix of the erroneous functionality that had been
introduced to 2.39 by
https://sourceware.org/pipermail/binutils/2022-May/120672.html.

This patch modifies the internal `struct reg_entry' numbering of DWARF
pseudo-registers to match values assigned in DWARF standards (see "4.1
DWARF register names" in [1])so ra_auth_code goes from 12 to 143 and
amends the unwinder .save directive-processing code to correctly handle
mixed register-type save directives.

The mechanism for splitting the register list is also re-written to
comply with register ordering on push statements, being that registers
are stored on the stack in numerical order, with the lowest numbered
register at the lowest address [2].

Consequently, the parsing of the hypothetical directive

        .save{r4-r7, r10, ra_auth_core, lr}

has been changed such as rather than producing

        .save{r4-r7, r10}
        .save{ra_auth_code}
        .save{lr}

as was the case with previous implementation, now produces:

        .save{lr}
        .save{ra_auth_code}
        .save{r4-r7, r10}

[1] <https://github.com/ARM-software/abi-aa/blob/main/aadwarf32/aadwarf32.rst>
[2] <https://developer.arm.com/documentation/dui0473/j/arm-and-thumb-instructions/push>


Backport regtested on x86_64 Ubuntu 18.04 and armhf Ubuntu 20.04 with
no issues.

Thanks,
Victor

gas/Changelog:

	* config/tc-arm.c (REG_RA_AUTH_CODE): New.
	(parse_dot_save): Likewise.
	(parse_reg_list): Remove obsolete code.
	(reg_names): Set ra_auth_code to 143.
	(s_arm_unwind_save): Handle core and pseudo-register lists via
	parse_dot_save.
	(s_arm_unwind_save_mixed): Deleted.
	(s_arm_unwind_save_pseudo): Handle one register at a time.
	* testsuite/gas/arm/unwind-pacbti-m-readelf.d: Fix test.
	* testsuite/gas/arm/unwind-pacbti-m.d: Likewise.

(cherry picked from commit 3a368c4c248f6e9f4bda3a5369befa17a4560293)
---
 gas/config/tc-arm.c                           | 159 ++++++++++--------
 .../gas/arm/unwind-pacbti-m-readelf.d         |   4 +-
 gas/testsuite/gas/arm/unwind-pacbti-m.d       |   2 +-
 3 files changed, 95 insertions(+), 70 deletions(-)
  

Comments

Nick Clifton Nov. 2, 2022, 2:22 p.m. UTC | #1
Hi Victor,

> Having missed the deadline at the time for the 2.39 release, would it
> be possible to backport the following patch to Binutils 2.39?

Backport approved - please apply.

Cheers
   Nick
  

Patch

diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
index 2e6d175482e..7da76793d89 100644
--- a/gas/config/tc-arm.c
+++ b/gas/config/tc-arm.c
@@ -742,6 +742,7 @@  const char * const reg_expected_msgs[] =
 #define REG_SP	13
 #define REG_LR	14
 #define REG_PC	15
+#define REG_RA_AUTH_CODE 143
 
 /* ARM instructions take 4bytes in the object file, Thumb instructions
    take 2:  */
@@ -1943,21 +1944,6 @@  parse_reg_list (char ** strp, enum reg_list_els etype)
 
 	      reg = arm_reg_parse (&str, rt);
 
-	      /* Skip over allowed registers of alternative types in mixed-type
-	         register lists.  */
-	      if (reg == FAIL && rt == REG_TYPE_PSEUDO
-		  && ((reg = arm_reg_parse (&str, REG_TYPE_RN)) != FAIL))
-		{
-		  cur_reg = reg;
-		  continue;
-		}
-	      else if (reg == FAIL && rt == REG_TYPE_RN
-		       && ((reg = arm_reg_parse (&str, REG_TYPE_PSEUDO)) != FAIL))
-		{
-		  cur_reg = reg;
-		  continue;
-		}
-
 	      if (etype == REGLIST_CLRM)
 		{
 		  if (reg == REG_SP || reg == REG_PC)
@@ -4139,7 +4125,6 @@  s_arm_unwind_fnstart (int ignored ATTRIBUTE_UNUSED)
   unwind.sp_restored = 0;
 }
 
-
 /* Parse a handlerdata directive.  Creates the exception handling table entry
    for the function.  */
 
@@ -4297,15 +4282,19 @@  s_arm_unwind_personality (int ignored ATTRIBUTE_UNUSED)
 /* Parse a directive saving pseudo registers.  */
 
 static void
-s_arm_unwind_save_pseudo (long range)
+s_arm_unwind_save_pseudo (int regno)
 {
   valueT op;
 
-  if (range & (1 << 12))
+  switch (regno)
     {
+    case REG_RA_AUTH_CODE:
       /* Opcode for restoring RA_AUTH_CODE.  */
       op = 0xb4;
       add_unwind_opcode (op, 1);
+      break;
+    default:
+      as_bad (_("Unknown register %d encountered\n"), regno);
     }
 }
 
@@ -4375,6 +4364,80 @@  s_arm_unwind_save_core (long range)
     }
 }
 
+/* Implement correct handling of .save lists enabling the split into
+sublists where necessary, while preserving correct sublist ordering.  */
+
+static void
+parse_dot_save (char **str_p, int prev_reg)
+{
+  long core_regs = 0;
+  int reg;
+  int in_range = 0;
+
+  if (**str_p == ',')
+    *str_p += 1;
+  if (**str_p == '}')
+    {
+      *str_p += 1;
+      return;
+    }
+
+  while ((reg = arm_reg_parse (str_p, REG_TYPE_RN)) != FAIL)
+    {
+      if (!in_range)
+	{
+	  if (core_regs & (1 << reg))
+	    as_tsktsk (_("Warning: duplicated register (r%d) in register list"),
+		       reg);
+	  else if (reg <= prev_reg)
+	    as_tsktsk (_("Warning: register list not in ascending order"));
+
+	  core_regs |= (1 << reg);
+	  prev_reg = reg;
+	  if (skip_past_char(str_p, '-') != FAIL)
+	    in_range = 1;
+	  else if (skip_past_comma(str_p) == FAIL)
+	    first_error (_("bad register list"));
+	}
+      else
+	{
+	  int i;
+	  if (reg <= prev_reg)
+	    first_error (_("bad range in register list"));
+	  for (i = prev_reg + 1; i <= reg; i++)
+	    {
+	      if (core_regs & (1 << i))
+		as_tsktsk (_("Warning: duplicated register (r%d) in register list"),
+			   i);
+	      else
+		core_regs |= 1 << i;
+	    }
+	  in_range = 0;
+	}
+    }
+  if (core_regs)
+    {
+      /* Higher register numbers go in higher memory addresses.  When splitting a list,
+	 right-most sublist should therefore be .saved first.  Use recursion for this.  */
+      parse_dot_save (str_p, reg);
+      /* We're back from recursion, so emit .save insn for sublist.  */
+      s_arm_unwind_save_core (core_regs);
+      return;
+    }
+  /* Handle pseudo-regs, under assumption these are emitted singly.  */
+  else if ((reg = arm_reg_parse (str_p, REG_TYPE_PSEUDO)) != FAIL)
+    {
+      /* Recurse for remainder of input.  Note: No assumption is made regarding which
+	 register in core register set holds pseudo-register.  It's not considered in
+	 ordering check beyond ensuring it's not sandwiched between 2 consecutive
+	 registers.  */
+      parse_dot_save (str_p, prev_reg + 1);
+      s_arm_unwind_save_pseudo (reg);
+      return;
+    }
+  else
+    as_bad (BAD_SYNTAX);
+}
 
 /* Parse a directive saving FPA registers.  */
 
@@ -4716,39 +4779,13 @@  s_arm_unwind_save_mmxwcg (void)
   ignore_rest_of_line ();
 }
 
-/* Convert range and mask_range into a sequence of s_arm_unwind_core
-   and s_arm_unwind_pseudo operations.  We assume that mask_range will
-   not have consecutive bits set, or that one operation per bit is
-   acceptable.  */
-
-static void
-s_arm_unwind_save_mixed (long range, long mask_range)
-{
-  while (mask_range)
-    {
-      long mask_bit = mask_range & -mask_range;
-      long subrange = range & (mask_bit - 1);
-
-      if (subrange)
-	s_arm_unwind_save_core (subrange);
-
-      s_arm_unwind_save_pseudo (mask_bit);
-      range &= ~subrange;
-      mask_range &= ~mask_bit;
-    }
-
-  if (range)
-    s_arm_unwind_save_core (range);
-}
-
 /* Parse an unwind_save directive.
    If the argument is non-zero, this is a .vsave directive.  */
 
 static void
 s_arm_unwind_save (int arch_v6)
 {
-  char *peek, *mask_peek;
-  long range, mask_range;
+  char *peek;
   struct reg_entry *reg;
   bool had_brace = false;
 
@@ -4756,7 +4793,7 @@  s_arm_unwind_save (int arch_v6)
     as_bad (MISSING_FNSTART);
 
   /* Figure out what sort of save we have.  */
-  peek = mask_peek = input_line_pointer;
+  peek = input_line_pointer;
 
   if (*peek == '{')
     {
@@ -4788,20 +4825,13 @@  s_arm_unwind_save (int arch_v6)
 
     case REG_TYPE_PSEUDO:
     case REG_TYPE_RN:
-      mask_range = parse_reg_list (&mask_peek, REGLIST_PSEUDO);
-      range = parse_reg_list (&input_line_pointer, REGLIST_RN);
-
-      if (range == FAIL || mask_range == FAIL)
-	{
-	  as_bad (_("expected register list"));
-	  ignore_rest_of_line ();
-	  return;
-	}
-
-      demand_empty_rest_of_line ();
-
-      s_arm_unwind_save_mixed (range, mask_range);
-      return;
+      {
+	if (had_brace)
+	  input_line_pointer++;
+	parse_dot_save (&input_line_pointer, -1);
+	demand_empty_rest_of_line ();
+	return;
+      }
 
     case REG_TYPE_VFD:
       if (arch_v6)
@@ -23993,12 +24023,8 @@  static const struct reg_entry reg_names[] =
   /* XScale accumulator registers.  */
   REGNUM(acc,0,XSCALE), REGNUM(ACC,0,XSCALE),
 
-  /* DWARF ABI defines RA_AUTH_CODE to 143. It also reserves 134-142 for future
-     expansion.  RA_AUTH_CODE here is given the value 143 % 134 to make it easy
-     for tc_arm_regname_to_dw2regnum to translate to DWARF reg number using
-     134 + reg_number should the range 134 to 142 be used for more pseudo regs
-     in the future.  This also helps fit RA_AUTH_CODE into a bitmask.  */
-  REGDEF(ra_auth_code,12,PSEUDO),
+  /* AADWARF32 defines RA_AUTH_CODE to 143.  */
+  REGDEF(ra_auth_code,143,PSEUDO),
 };
 #undef REGDEF
 #undef REGNUM
@@ -27905,7 +27931,6 @@  create_unwind_entry (int have_data)
   return 0;
 }
 
-
 /* Initialize the DWARF-2 unwind information for this procedure.  */
 
 void
diff --git a/gas/testsuite/gas/arm/unwind-pacbti-m-readelf.d b/gas/testsuite/gas/arm/unwind-pacbti-m-readelf.d
index d8d647bb7f0..c40544a5a94 100644
--- a/gas/testsuite/gas/arm/unwind-pacbti-m-readelf.d
+++ b/gas/testsuite/gas/arm/unwind-pacbti-m-readelf.d
@@ -10,11 +10,11 @@  Unwind section '.ARM.exidx' at offset 0x60 contains 1 entry:
 
 0x0 <foo>: @0x0
   Compact model index: 1
-  0x84 0x00 pop {r14}
   0xb4      pop {ra_auth_code}
   0x84 0x00 pop {r14}
-  0xb4      pop {ra_auth_code}
   0xa3      pop {r4, r5, r6, r7}
   0xb4      pop {ra_auth_code}
+  0x84 0x00 pop {r14}
+  0xb4      pop {ra_auth_code}
   0xa8      pop {r4, r14}
   0xb0      finish
diff --git a/gas/testsuite/gas/arm/unwind-pacbti-m.d b/gas/testsuite/gas/arm/unwind-pacbti-m.d
index d178e4c0a44..b021c007b33 100644
--- a/gas/testsuite/gas/arm/unwind-pacbti-m.d
+++ b/gas/testsuite/gas/arm/unwind-pacbti-m.d
@@ -8,4 +8,4 @@ 
 .*:     file format.*
 
 Contents of section .ARM.extab:
- 0000 (00840281 b40084b4 b0a8b4a3|81028400 b48400b4 a3b4a8b0) 00000000  .*
+ 0000 (84b40281 84b4a300 b0a8b400|8102b484 00a3b484 00b4a8b0) 00000000  .*