From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NelsondeMBP.localdomain (114-25-98-25.dynamic-ip.hinet.net [114.25.98.25]) by sourceware.org (Postfix) with ESMTP id 356803858C00 for ; Wed, 2 Nov 2022 05:04:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 356803858C00 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=rivosinc.com Authentication-Results: sourceware.org; spf=none smtp.mailfrom=NelsondeMBP.localdomain Received: by NelsondeMBP.localdomain (Postfix, from userid 501) id 078733BD6B4; Wed, 2 Nov 2022 13:04:33 +0800 (CST) From: Nelson Chu To: binutils@sourceware.org Cc: Nelson Chu Subject: [committed] RISC-V: Fixed the missing $x+arch when adding odd paddings for alignment. Date: Wed, 2 Nov 2022 13:04:30 +0800 Message-Id: <20221102050430.1053-1-nelson@rivosinc.com> X-Mailer: git-send-email 2.37.0 (Apple Git-136) MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-5.4 required=5.0 tests=BAYES_00,GIT_PATCH_0,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,KAM_LAZY_DOMAIN_SECURITY,KHOP_HELO_FCRDNS,NO_DNS_FOR_FROM,PDS_RDNS_DYNAMIC_FP,RCVD_IN_BARRACUDACENTRAL,RCVD_IN_PBL,RCVD_IN_SORBS_DUL,RDNS_DYNAMIC,SPF_HELO_NONE,SPF_NONE,TXREP,T_FILL_THIS_FORM_SHORT autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Consider the case, .option arch, rv32i .option norelax .option arch, +c .byte 1 .align 2 addi a0, zero, 1 Assembler adds $d for the odd .byte, and then adds $x+arch for the alignment. Since norelax, riscv_add_odd_padding_symbol will add the $d and $x for the odd alignment, but accidently remove the $x+arch because it has the same address as $d. Therefore, we will get the unexpected result before applying this patch, .byte 1 # $d .align 2 # odd alignment, $xrv32ic replaced by $d + $x After this patch, the expected result should be, .byte 1 # $d .align 2 # odd alignment, $xrv32ic replaced by $d + $xrv32ic gas/ * config/tc-riscv.c (make_mapping_symbol): If we are adding mapping symbol for odd alignment, then we probably will remove the $x+arch by accidently when it has the same address of $d. Try to add the removed $x+arch back after the $d rather than just $x. (riscv_mapping_state): Updated since parameters of make_mapping_symbol are changed. (riscv_add_odd_padding_symbol): Likewise. (riscv_remove_mapping_symbol): Removed and moved the code into the riscv_check_mapping_symbols. (riscv_check_mapping_symbols): Updated. * testsuite/gas/riscv/mapping-dis.d: Updated and added new testcase. * testsuite/gas/riscv/mapping-symbols.d: Likewise. * testsuite/gas/riscv/mapping.s: Likewise. --- gas/config/tc-riscv.c | 65 +++++++++++++---------- gas/testsuite/gas/riscv/mapping-dis.d | 13 ++++- gas/testsuite/gas/riscv/mapping-symbols.d | 16 +++--- gas/testsuite/gas/riscv/mapping.s | 11 +++- 4 files changed, 67 insertions(+), 38 deletions(-) diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c index b664116af6d..2dc92ecd3c3 100644 --- a/gas/config/tc-riscv.c +++ b/gas/config/tc-riscv.c @@ -476,7 +476,8 @@ static void make_mapping_symbol (enum riscv_seg_mstate state, valueT value, fragS *frag, - bool reset_seg_arch_str) + const char *arch_str, + bool odd_data_padding) { const char *name; char *buff = NULL; @@ -486,12 +487,11 @@ make_mapping_symbol (enum riscv_seg_mstate state, name = "$d"; break; case MAP_INSN: - if (reset_seg_arch_str) + if (arch_str != NULL) { - const char *isa = riscv_rps_as.subset_list->arch_str; - size_t size = strlen (isa) + 3; /* "rv" + '\0' */ + size_t size = strlen (arch_str) + 3; /* "$x" + '\0' */ buff = xmalloc (size); - snprintf (buff, size, "$x%s", isa); + snprintf (buff, size, "$x%s", arch_str); name = buff; } else @@ -503,7 +503,7 @@ make_mapping_symbol (enum riscv_seg_mstate state, symbolS *symbol = symbol_new (name, now_seg, frag, value); symbol_get_bfdsym (symbol)->flags |= (BSF_NO_FLAGS | BSF_LOCAL); - if (reset_seg_arch_str) + if (arch_str != NULL) { /* Store current $x+arch into tc_segment_info. */ seg_info (now_seg)->tc_segment_info_data.arch_map_symbol = symbol; @@ -517,6 +517,7 @@ make_mapping_symbol (enum riscv_seg_mstate state, and .text.zero.fill.last. */ symbolS *first = frag->tc_frag_data.first_map_symbol; symbolS *last = frag->tc_frag_data.last_map_symbol; + symbolS *removed = NULL; if (value == 0) { if (first != NULL) @@ -524,7 +525,7 @@ make_mapping_symbol (enum riscv_seg_mstate state, know (S_GET_VALUE (first) == S_GET_VALUE (symbol) && first == last); /* Remove the old one. */ - symbol_remove (first, &symbol_rootP, &symbol_lastP); + removed = first; } frag->tc_frag_data.first_map_symbol = symbol; } @@ -534,9 +535,23 @@ make_mapping_symbol (enum riscv_seg_mstate state, know (S_GET_VALUE (last) <= S_GET_VALUE (symbol)); /* Remove the old one. */ if (S_GET_VALUE (last) == S_GET_VALUE (symbol)) - symbol_remove (last, &symbol_rootP, &symbol_lastP); + removed = last; } frag->tc_frag_data.last_map_symbol = symbol; + + if (removed == NULL) + return; + + if (odd_data_padding) + { + /* If the removed mapping symbol is $x+arch, then add it back to + the next $x. */ + const char *str = strncmp (S_GET_NAME (removed), "$xrv", 4) == 0 + ? S_GET_NAME (removed) + 2 : NULL; + make_mapping_symbol (MAP_INSN, frag->fr_fix + 1, frag, str, + false/* odd_data_padding */); + } + symbol_remove (removed, &symbol_rootP, &symbol_lastP); } /* Set the mapping state for frag_now. */ @@ -580,7 +595,10 @@ riscv_mapping_state (enum riscv_seg_mstate to_state, valueT value = (valueT) (frag_now_fix () - max_chars); seg_info (now_seg)->tc_segment_info_data.map_state = to_state; - make_mapping_symbol (to_state, value, frag_now, reset_seg_arch_str); + const char *arch_str = reset_seg_arch_str + ? riscv_rps_as.subset_list->arch_str : NULL; + make_mapping_symbol (to_state, value, frag_now, arch_str, + false/* odd_data_padding */); } /* Add the odd bytes of paddings for riscv_handle_align. */ @@ -591,25 +609,9 @@ riscv_add_odd_padding_symbol (fragS *frag) /* If there was already a mapping symbol, it should be removed in the make_mapping_symbol. - Please see gas/testsuite/gas/riscv/mapping.s: .text.odd.align. */ - make_mapping_symbol (MAP_DATA, frag->fr_fix, frag, false); - make_mapping_symbol (MAP_INSN, frag->fr_fix + 1, frag, false); -} - -/* If previous and current mapping symbol have same value, then remove the - current $x only if the previous is $x+arch; Otherwise, always remove the - previous. */ - -static void -riscv_remove_mapping_symbol (symbolS *pre, symbolS *cur) -{ - know (pre != NULL && cur != NULL - && S_GET_VALUE (pre) == S_GET_VALUE (cur)); - symbolS *removed = pre; - if (strncmp (S_GET_NAME (pre), "$xrv", 4) == 0 - && strcmp (S_GET_NAME (cur), "$x") == 0) - removed = cur; - symbol_remove (removed, &symbol_rootP, &symbol_lastP); + Please see gas/testsuite/gas/riscv/mapping.s: .text.odd.align.*. */ + make_mapping_symbol (MAP_DATA, frag->fr_fix, frag, + NULL/* arch_str */, true/* odd_data_padding */); } /* Remove any excess mapping symbols generated for alignment frags in @@ -654,7 +656,12 @@ riscv_check_mapping_symbols (bfd *abfd ATTRIBUTE_UNUSED, Please see the gas/testsuite/gas/riscv/mapping.s: .text.zero.fill.align.A and .text.zero.fill.align.B. */ - riscv_remove_mapping_symbol (last, next_first); + know (S_GET_VALUE (last) == S_GET_VALUE (next_first)); + symbolS *removed = last; + if (strncmp (S_GET_NAME (last), "$xrv", 4) == 0 + && strcmp (S_GET_NAME (next_first), "$x") == 0) + removed = next_first; + symbol_remove (removed, &symbol_rootP, &symbol_lastP); break; } diff --git a/gas/testsuite/gas/riscv/mapping-dis.d b/gas/testsuite/gas/riscv/mapping-dis.d index 246f3672ade..b1a26fbd151 100644 --- a/gas/testsuite/gas/riscv/mapping-dis.d +++ b/gas/testsuite/gas/riscv/mapping-dis.d @@ -26,9 +26,9 @@ Disassembly of section .text.data: [ ]+[0-9a-f]+:[ ]+4509[ ]+li[ ]+a0,2 [ ]+[0-9a-f]+:[ ]+05000302[ ]+.word[ ]+0x05000302 -Disassembly of section .text.odd.align: +Disassembly of section .text.odd.align.start.insn: -0+000 <.text.odd.align>: +0+000 <.text.odd.align.start.insn>: [ ]+[0-9a-f]+:[ ]+4505[ ]+li[ ]+a0,1 [ ]+[0-9a-f]+:[ ]+01[ ]+.byte[ ]+0x01 [ ]+[0-9a-f]+:[ ]+00[ ]+.byte[ ]+0x00 @@ -36,6 +36,15 @@ Disassembly of section .text.odd.align: [ ]+[0-9a-f]+:[ ]+00200513[ ]+li[ ]+a0,2 [ ]+[0-9a-f]+:[ ]+00000013[ ]+nop +Disassembly of section .text.odd.align.start.data: + +0+000 <.text.odd.align.start.data>: +[ ]+[0-9a-f]+:[ ]+01[ ]+.byte[ ]+0x01 +[ ]+[0-9a-f]+:[ ]+00[ ]+.byte[ ]+0x00 +[ ]+[0-9a-f]+:[ ]+0001[ ]+nop +[ ]+[0-9a-f]+:[ ]+4505[ ]+li[ ]+a0,1 +[ ]+[0-9a-f]+:[ ]+0001[ ]+nop + Disassembly of section .text.zero.fill.first: 0+000 <.text.zero.fill.first>: diff --git a/gas/testsuite/gas/riscv/mapping-symbols.d b/gas/testsuite/gas/riscv/mapping-symbols.d index db9a8dd8030..40df3409736 100644 --- a/gas/testsuite/gas/riscv/mapping-symbols.d +++ b/gas/testsuite/gas/riscv/mapping-symbols.d @@ -17,10 +17,12 @@ SYMBOL TABLE: 0+00 l .text.data 0+00 \$d 0+08 l .text.data 0+00 \$xrv32i2p1_c2p0 0+0c l .text.data 0+00 \$d -0+00 l d .text.odd.align 0+00 .text.odd.align -0+00 l .text.odd.align 0+00 \$xrv32i2p1_c2p0 -0+02 l .text.odd.align 0+00 \$d -0+08 l .text.odd.align 0+00 \$xrv32i2p1 +0+00 l d .text.odd.align.start.insn 0+00 .text.odd.align.start.insn +0+00 l .text.odd.align.start.insn 0+00 \$xrv32i2p1_c2p0 +0+02 l .text.odd.align.start.insn 0+00 \$d +0+08 l .text.odd.align.start.insn 0+00 \$xrv32i2p1 +0+00 l d .text.odd.align.start.data 0+00 .text.odd.align.start.data +0+00 l .text.odd.align.start.data 0+00 \$d 0+00 l d .text.zero.fill.first 0+00 .text.zero.fill.first 0+00 l .text.zero.fill.first 0+00 \$xrv32i2p1_c2p0 0+00 l d .text.zero.fill.last 0+00 .text.zero.fill.last @@ -41,8 +43,10 @@ SYMBOL TABLE: 0+00 l .text.relax.align 0+00 \$xrv32i2p1_c2p0 0+08 l .text.relax.align 0+00 \$xrv32i2p1 0+0a l .text.section.padding 0+00 \$x -0+03 l .text.odd.align 0+00 \$d -0+04 l .text.odd.align 0+00 \$x +0+03 l .text.odd.align.start.insn 0+00 \$d +0+04 l .text.odd.align.start.insn 0+00 \$x +0+01 l .text.odd.align.start.data 0+00 \$d +0+02 l .text.odd.align.start.data 0+00 \$xrv32i2p1_c2p0 0+00 l d .riscv.attributes 0+00 .riscv.attributes 0+00 g .text.cross.section.A 0+00 funcA 0+00 g .text.corss.section.B 0+00 funcB diff --git a/gas/testsuite/gas/riscv/mapping.s b/gas/testsuite/gas/riscv/mapping.s index a0e6c744107..3014a69e792 100644 --- a/gas/testsuite/gas/riscv/mapping.s +++ b/gas/testsuite/gas/riscv/mapping.s @@ -32,7 +32,7 @@ addi a0, zero, 2 # $x, but same as previous addi, so removed .byte 5 .option pop -.section .text.odd.align, "ax" +.section .text.odd.align.start.insn, "ax" .option push .option norelax .option arch, +c @@ -43,6 +43,15 @@ addi a0, zero, 1 # $xrv32ic addi a0, zero, 2 # $xrv32i .option pop +.section .text.odd.align.start.data, "ax" +.option push +.option norelax +.option arch, +c +.byte 1 # $d +.align 2 # odd alignment, $xrv32ic replaced by $d + $xrv32ic +addi a0, zero, 1 +.option pop + .section .text.zero.fill.first, "ax" .option push .option norelax -- 2.37.0 (Apple Git-136)