public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Victor Do Nascimento <victor.donascimento@arm.com>
To: <binutils@sourceware.org>
Cc: <richard.earnshaw@arm.com>,
	Victor Do Nascimento <victor.donascimento@arm.com>
Subject: [PATCH][Binutils][GAS] arm: Fix DWARF pseudo-register numbering convention
Date: Fri, 22 Jul 2022 13:12:49 +0100	[thread overview]
Message-ID: <20220722121249.22131-1-victor.donascimento@arm.com> (raw)

Hi all,

This patch restores the assembler internal numbering of DWARF pseudo-
registers to the preferred modulo 134 convention (see "4.1 DWARF register
names" in [1]) and amends the unwinder .save directive processing code to
handle this.

Issue:
1.  DWARF pseudo-registers to be assigned their own reg_entry numbering
    system based on modulo 134. Pseudo-reg no. 143 thus gets assigned
    to (143 % 134) = 9.
    Not orthogonal to core-register numbering.
2.  We must have a way to to map pseudo-register numbers to
    corresponding physical register numbers and vice-versa. More
    concretely, issues first arise when invoking s_arm_unwind_save_mixed.

    Consider parsing the hypothetical directive:

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

    As `.save`-ing ra_auth_code requires a special DWARF opcode, this
    directive must be split into several sub-statements, i.e.

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

    we pass s_arm_unwind_save_mixed 2 bitmaps - one of physical register
    numbers and one of pseudo-register numbers.

        core-register bitmap:   000011110010001
        pseudo-register bitmap: 000000000100000

    Under proposed modulo 134 numbering system, this would erroneously
    produce:

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

    the pseudo-reg bitmap must be cast to physical register space to
    know how to split .save directives correctly. So something like:

        core-register bitmap:           000011110010001
        recast pseudo-register bitmap:  000000000000100

    would give us the correct split of the .save statements.

3.  so our bitmap of pseudo-registers cast in physical register space
    tells us that bit 12 came from a pseudo-register, but with no
    guarantee (or indeed any reason to expect) that there is
    preservation of register ordering in mapping between DWARF register
    space and core-register space, in the presence of multiple
    pseudo-registers, there is no immediate way of inferring which bit
    in the DWARF bitmap mapped onto bit 12. This therefore creates the
    need for a function to convert DWARF bitmaps into core-register
    bitmaps for splitting the register list and then another for
    translating core-register numbers back to DWARF register numbers to
    correctly handle the pseudo-registers.
    This is the least invasive implementation for handling the proposed
    DWARF modulo 134 numbering system, but also seems to involve a great
    deal of mapping to and fro numbering conventions.

I've not yet thought of an alternative that is better and doesn't
require too many invasive alterations to the existing codebase (though
I'm sure it may well be out there).

Therefore, any feedback/advice/suggestions would be most welcome.

Proposed changes:
1.  Implement a function to cast a bitmap of pseudo-registers to
    corresponding physical register numbers: tc_dw2range_to_arm_range.
2.  When emitting an unwinder save instruction for a pseudo-register
    have a function that maps back physical register ranges back to the
    correct pseudo-register numbers:  tc_arm_range_to_dw2range

[1] <https://github.com/ARM-software/abi-aa/blob/main/aadwarf32/aadwarf32.rst>

Many thanks,
Victor

gas/Changelog:

    * config/tc-arm.c:
    (reg_names): Renumber RA_AUTH_CODE from 12 to 9.
    (parse_reg_list): Ensure correct range-handling.
    (s_arm_unwind_save_pseudo): Fix register number.
    (s_arm_unwind_save_mixed): Add number system interconversion.
    (tc_dw2range_to_arm_range): New.
    (tc_arm_range_to_dw2range): New.
    * config/tc-arm.h: Add new function prototypes.
---
 gas/config/tc-arm.c | 64 ++++++++++++++++++++++++++++++++++++++++-----
 gas/config/tc-arm.h |  2 ++
 2 files changed, 60 insertions(+), 6 deletions(-)

diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c
index 2e6d175482e..f66d2acca0e 100644
--- a/gas/config/tc-arm.c
+++ b/gas/config/tc-arm.c
@@ -1948,13 +1948,13 @@ parse_reg_list (char ** strp, enum reg_list_els etype)
 	      if (reg == FAIL && rt == REG_TYPE_PSEUDO
 		  && ((reg = arm_reg_parse (&str, REG_TYPE_RN)) != FAIL))
 		{
-		  cur_reg = reg;
+		  in_range = 0;
 		  continue;
 		}
 	      else if (reg == FAIL && rt == REG_TYPE_RN
 		       && ((reg = arm_reg_parse (&str, REG_TYPE_PSEUDO)) != FAIL))
 		{
-		  cur_reg = reg;
+		  in_range = 0;
 		  continue;
 		}
 
@@ -4301,7 +4301,7 @@ s_arm_unwind_save_pseudo (long range)
 {
   valueT op;
 
-  if (range & (1 << 12))
+  if (range & (1 << 9))
     {
       /* Opcode for restoring RA_AUTH_CODE.  */
       op = 0xb4;
@@ -4724,15 +4724,19 @@ s_arm_unwind_save_mmxwcg (void)
 static void
 s_arm_unwind_save_mixed (long range, long mask_range)
 {
+
+  mask_range = tc_dw2range_to_arm_range (mask_range);
+
   while (mask_range)
     {
       long mask_bit = mask_range & -mask_range;
+      long pseudo_reg = tc_arm_range_to_dw2range (mask_bit);
       long subrange = range & (mask_bit - 1);
 
       if (subrange)
-	s_arm_unwind_save_core (subrange);
+        s_arm_unwind_save_core (subrange);
 
-      s_arm_unwind_save_pseudo (mask_bit);
+      s_arm_unwind_save_pseudo (pseudo_reg);
       range &= ~subrange;
       mask_range &= ~mask_bit;
     }
@@ -23998,7 +24002,7 @@ static const struct reg_entry reg_names[] =
      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),
+  REGDEF(ra_auth_code,9,PSEUDO),
 };
 #undef REGDEF
 #undef REGNUM
@@ -27936,6 +27940,54 @@ tc_arm_regname_to_dw2regnum (char *regname)
   return FAIL;
 }
 
+/* Cast a bitmap representation of a register set from DWARF-2 modulo 134
+   pseudo-register numbering to the corresponding physical reg. numbers. */
+
+long
+tc_dw2range_to_arm_range (long pseudo_range)
+{
+  long phys_range = 0;
+  long reg = 0;
+  while (pseudo_range)
+    {
+      reg = pseudo_range & -pseudo_range;
+      switch (reg)
+        {
+          case (1 << 9):
+            phys_range |= (1 << 12);
+            break;
+          default:
+            break;
+        }
+      pseudo_range &= ~reg;
+    }
+  return phys_range;
+}
+
+
+/* from DWARF-2 modulo 134 pseudo-register numbering retrieve physical regs. */
+
+long
+tc_arm_range_to_dw2range (long phys_range)
+{
+  long pseudo_range = 0;
+  long reg = 0;
+  while (phys_range)
+    {
+      reg = phys_range & -phys_range;
+      switch (reg)
+        {
+          case (1 << 12):
+            pseudo_range |= (1 << 9);
+            break;
+          default:
+            break;
+        }
+      phys_range &= ~reg;
+    }
+  return pseudo_range;
+}
+
 #ifdef TE_PE
 void
 tc_pe_dwarf2_emit_offset (symbolS *symbol, unsigned int size)
diff --git a/gas/config/tc-arm.h b/gas/config/tc-arm.h
index d2176e03c98..8a8cb15d702 100644
--- a/gas/config/tc-arm.h
+++ b/gas/config/tc-arm.h
@@ -359,6 +359,8 @@ extern void arm_handle_align (struct frag *);
 extern bool arm_fix_adjustable (struct fix *);
 extern int arm_elf_section_type (const char *, size_t);
 extern int tc_arm_regname_to_dw2regnum (char *regname);
+extern long tc_dw2range_to_arm_range (long);
+extern long tc_arm_range_to_dw2range (long);
 extern void tc_arm_frame_initial_instructions (void);
 
 #ifdef TE_PE
-- 
2.36.1


             reply	other threads:[~2022-07-22 12:13 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-22 12:12 Victor Do Nascimento [this message]
2022-07-22 15:25 ` Richard Earnshaw

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220722121249.22131-1-victor.donascimento@arm.com \
    --to=victor.donascimento@arm.com \
    --cc=binutils@sourceware.org \
    --cc=richard.earnshaw@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).