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

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

[Victor L. Do Nascimento]

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

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

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

[Nick Clifton]

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