From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id C3D783858429 for ; Fri, 29 Jul 2022 14:09:14 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C3D783858429 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 22FCA1063; Fri, 29 Jul 2022 07:09:15 -0700 (PDT) Received: from [10.2.78.65] (unknown [10.2.78.65]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 8E6D53F73D; Fri, 29 Jul 2022 07:09:10 -0700 (PDT) Message-ID: <775561e6-1eb6-b4ba-854b-ef55cb708a22@foss.arm.com> Date: Fri, 29 Jul 2022 15:09:08 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [PATCH v2][Binutils][GAS] arm: Use DWARF numbering convention for pseudo-register representation Content-Language: en-GB To: Victor Do Nascimento , binutils@sourceware.org Cc: richard.earnshaw@arm.com References: <20220728154407.49962-1-victor.donascimento@arm.com> From: Richard Earnshaw In-Reply-To: <20220728154407.49962-1-victor.donascimento@arm.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3496.7 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, KAM_LAZY_DOMAIN_SECURITY, NICE_REPLY_A, SPF_HELO_NONE, SPF_NONE, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Jul 2022 14:09:18 -0000 On 28/07/2022 16:44, Victor Do Nascimento via Binutils wrote: > Hi all, > > 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]) and amends the unwinder .save > directive-processing code to correctly handle mixed register-type save > directives. > > The mechanism for splitting the register list is 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} > > Tested for arm-none-eabi on x86_64 and aarch64 hosts. > > Best regards, > Victor > > [1] > [2] > > 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/set/Set/ > (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 1 register at a time. s/1/one/ > * testsuite/gas/arm/unwind-pacbti-m-readelf.d: Fix test. > * testsuite/gas/arm/unwind-pacbti-m.d: Likewise. > --- > gas/config/tc-arm.c | 160 ++++++++++-------- > .../gas/arm/unwind-pacbti-m-readelf.d | 4 +- > gas/testsuite/gas/arm/unwind-pacbti-m.d | 2 +- > 3 files changed, 96 insertions(+), 70 deletions(-) > Otherwise OK. R. > diff --git a/gas/config/tc-arm.c b/gas/config/tc-arm.c > index 2e6d175482e..f2e7f73cc2d 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 no. encountered: %d\n"), regno); > } > } > > @@ -4375,6 +4364,81 @@ 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, thus 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 +4780,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 +4794,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 +4826,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 +24024,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), > + /* DWARF ABI defines RA_AUTH_CODE to 143. */ > + REGDEF(ra_auth_code,143,PSEUDO), > }; > #undef REGDEF > #undef REGNUM > @@ -27905,7 +27932,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 : @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 .*