RISC-V: correct alignment directive handling for text sections

.insn or data emitted inside text sections can lead to positions not
being at insn granularity. In such situations using alignment
directives should reliably enforce the requested alignment.
Specifically requests to align back to insn granularity may not be
ignored (where, as a subcase thereof, the ordering of ".option norvc"
and e.g. ".p2align 2" should not matter; so far the alignment directive
needs to come first to have any effect). Similarly ahead of emitting
NOPs alignment first needs to be forced back to insn granularity.

The new testcases actually point out a corner case issue in the
disassembler as well, which is being corrected at the same time: We
don't want to print "0x" without any subsequent digits.

diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
index 301607b0aad7e721635bcf0423aa584814f3d8d8..4a5768327903c9db89c76c37bf799c5dd8f24684 100644 (file)
--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -185,6 +185,9 @@ static enum float_abi float_abi = FLOAT_ABI_DEFAULT;
 
 static unsigned elf_flags = 0;
 
+/* Indicate whether we are already assembling any instructions.  */
+static bool start_assemble = false;
+
 static bool probing_insn_operands;
 
 /* Set the default_isa_spec.  Return 0 if the spec isn't supported.
@@ -282,6 +285,16 @@ riscv_set_rvc (bool rvc_value)
   if (rvc_value)
     elf_flags |= EF_RISCV_RVC;
 
+  if (start_assemble && subseg_text_p (now_seg)
+      && riscv_opts.rvc && !rvc_value)
+    {
+      struct riscv_segment_info_type *info
+       = &seg_info(now_seg)->tc_segment_info_data;
+
+      info->last_insn16 = true;
+      info->rvc = rvc_value;
+    }
+
   riscv_opts.rvc = rvc_value;
 }
 
@@ -354,10 +367,8 @@ riscv_set_arch (const char *s)
   riscv_set_arch_str (&file_arch_str);
   riscv_set_arch_str (&riscv_rps_as.subset_list->arch_str);
 
-  riscv_set_rvc (false);
-  if (riscv_subset_supports (&riscv_rps_as, "c")
-      || riscv_subset_supports (&riscv_rps_as, "zca"))
-    riscv_set_rvc (true);
+  riscv_set_rvc (riscv_subset_supports (&riscv_rps_as, "c")
+                || riscv_subset_supports (&riscv_rps_as, "zca"));
 
   if (riscv_subset_supports (&riscv_rps_as, "ztso"))
     riscv_set_tso ();
@@ -457,9 +468,6 @@ const char EXP_CHARS[] = "eE";
    As in 0f12.456 or 0d1.2345e12.  */
 const char FLT_CHARS[] = "rRsSfFdDxXpPhH";
 
-/* Indicate we are already assemble any instructions or not.  */
-static bool start_assemble = false;
-
 /* Indicate ELF attributes are explicitly set.  */
 static bool explicit_attr = false;
 
@@ -627,6 +635,7 @@ 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;
+  seg_info (now_seg)->tc_segment_info_data.last_insn16 = false;
   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,
@@ -4237,12 +4246,13 @@ riscv_ip_hardcode (char *str,
            generic_bignum[num],
            llen);
       memset(ip->insn_long_opcode + repr_bytes, 0, bytes - repr_bytes);
-      return NULL;
     }
-
-  if (bytes < sizeof(values[0]) && values[num - 1] >> (8 * bytes) != 0)
+  else if (bytes < sizeof(values[0]) && values[num - 1] >> (8 * bytes) != 0)
     return _("value conflicts with instruction length");
 
+  if (!riscv_opts.rvc && (bytes & 2))
+    seg_info (now_seg)->tc_segment_info_data.last_insn16 = true;
+
   return NULL;
 }
 
@@ -4929,10 +4939,8 @@ s_riscv_option (int x ATTRIBUTE_UNUSED)
       riscv_update_subset (&riscv_rps_as, name);
       riscv_set_arch_str (&riscv_rps_as.subset_list->arch_str);
 
-      riscv_set_rvc (false);
-      if (riscv_subset_supports (&riscv_rps_as, "c")
-         || riscv_subset_supports (&riscv_rps_as, "zca"))
-       riscv_set_rvc (true);
+      riscv_set_rvc (riscv_subset_supports (&riscv_rps_as, "c")
+                    || riscv_subset_supports (&riscv_rps_as, "zca"));
 
       if (riscv_subset_supports (&riscv_rps_as, "ztso"))
        riscv_set_tso ();
@@ -5040,15 +5048,27 @@ riscv_frag_align_code (int n)
   char *nops;
   expressionS ex;
 
-  /* If we are moving to a smaller alignment than the instruction size, then no
-     alignment is required. */
+  /* If we are moving to alignment no larger than the instruction size, then
+     no special alignment handling is required. */
   if (bytes <= insn_alignment)
-    return true;
+    {
+      if (bytes == insn_alignment)
+       seg_info (now_seg)->tc_segment_info_data.last_insn16 = false;
+      return false;
+    }
 
   /* When not relaxing, riscv_handle_align handles code alignment.  */
   if (!riscv_opts.relax)
     return false;
 
+  /* If the last item emitted was not an ordinary insn, first align back to
+     insn granularity.  Don't do this unconditionally, to avoid altering frags
+     when that's not actually needed.  */
+  if (seg_info (now_seg)->tc_segment_info_data.map_state != MAP_INSN
+      || seg_info (now_seg)->tc_segment_info_data.last_insn16)
+    frag_align_code (riscv_opts.rvc ? 1 : 2, 0);
+  seg_info (now_seg)->tc_segment_info_data.last_insn16 = false;
+
   /* Maybe we should use frag_var to create a new rs_align_code fragment,
      rather than just use frag_more to handle an alignment here?  So that we
      don't need to call riscv_mapping_state again later, and then only need
@@ -5365,6 +5385,18 @@ tc_riscv_regname_to_dw2regnum (char *regname)
   return -1;
 }
 
+void
+riscv_elf_section_change_hook (void)
+{
+  struct riscv_segment_info_type *info
+    = &seg_info(now_seg)->tc_segment_info_data;
+
+  if (info->rvc && !riscv_opts.rvc)
+    info->last_insn16 = true;
+
+  info->rvc = riscv_opts.rvc;
+}
+
 void
 riscv_elf_final_processing (void)
 {

diff --git a/gas/config/tc-riscv.h b/gas/config/tc-riscv.h
index 07a00b6c5f705026719b63b14120c858d7af64d5..05594cfc13e14a304bb252f902d5d09bb26cf3b3 100644 (file)
--- a/gas/config/tc-riscv.h
+++ b/gas/config/tc-riscv.h
@@ -128,6 +128,9 @@ extern int tc_riscv_regname_to_dw2regnum (char *);
 /* Even on RV64, use 4-byte alignment, as F registers may be only 32 bits.  */
 #define DWARF2_CIE_DATA_ALIGNMENT -4
 
+#define md_elf_section_change_hook riscv_elf_section_change_hook
+extern void riscv_elf_section_change_hook (void);
+
 #define elf_tc_final_processing riscv_elf_final_processing
 extern void riscv_elf_final_processing (void);
 
@@ -153,6 +156,8 @@ void riscv_mapping_state (enum riscv_seg_mstate, int, bool);
 struct riscv_segment_info_type
 {
   enum riscv_seg_mstate map_state;
+  bool rvc;
+  bool last_insn16;
   /* The current mapping symbol with architecture string.  */
   symbolS *arch_map_symbol;
 };

diff --git a/gas/testsuite/gas/riscv/relax-align-2.d b/gas/testsuite/gas/riscv/relax-align-2.d
new file mode 100644 (file)
index 0000000..36ab78e
--- /dev/null
+++ b/gas/testsuite/gas/riscv/relax-align-2.d
@@ -0,0 +1,52 @@
+#as: -mrelax
+#objdump: -drw
+
+.*:[   ]+file format .*
+
+
+Disassembly of section .text:
+
+0+000 <rvc_func>:
+[      ]+0:[   ]+8082[         ]+ret
+[      ]+2:[   ]+0001[         ]+nop
+[      ]+4:[   ]+00000013[     ]+nop[  ]+4: R_RISCV_ALIGN[     ]+\*ABS\*\+0x4
+
+0+008 <non_rvc_func>:
+[      ]+8:[   ]+00008067[     ]+ret
+
+0+00c <insn>:
+[      ]+c:[   ]+00000013[     ]+nop
+[      ]+10:[  ]+0000[         ]+\.insn        2, 0x0+
+[      ]+12:[  ]+0001[         ]+\.insn        2, 0x0*1
+[      ]+14:[  ]+00000013[     ]+nop[  ]+14: R_RISCV_ALIGN[    ]+\*ABS\*\+0x4
+[      ]+18:[  ]+00008067[     ]+ret
+
+0+001c <hword>:
+[      ]+1c:[  ]+00000013[     ]+nop
+[      ]+20:[  ]+0000[         ]+\.short       0x0+
+[      ]+22:[  ]+0001[         ]+\.insn        2, 0x0*1
+[      ]+24:[  ]+00000013[     ]+nop[  ]+24: R_RISCV_ALIGN[    ]+\*ABS\*\+0x4
+[      ]+28:[  ]+00008067[     ]+ret
+
+0+002c <byte>:
+[      ]+2c:[  ]+00000013[     ]+nop
+[      ]+30:[  ]+00[   ]+\.byte        0x0+
+[      ]+31:[  ]+00[   ]+\.byte        0x0+
+[      ]+32:[  ]+0001[         ]+\.insn        2, 0x0*1
+[      ]+34:[  ]+00000013[     ]+nop[  ]+34: R_RISCV_ALIGN[    ]+\*ABS\*\+0x4
+[      ]+38:[  ]+00008067[     ]+ret
+[      ]+3c:[  ]+00000013[     ]+nop[  ]+3c: R_RISCV_ALIGN[    ]+\*ABS\*\+0x4
+
+0+0040 <func1>:
+[      ]+40:[  ]+00000013[     ]+nop
+[      ]+44:[  ]+00008067[     ]+ret
+
+0+0048 <func2>:
+[      ]+48:[  ]+8082[         ]+ret
+[      ]+4a:[  ]+0001[         ]+nop
+[      ]+4c:[  ]+00000013[     ]+nop[  ]+4c: R_RISCV_ALIGN[    ]+\*ABS\*\+0x4
+
+0+0050 <func3>:
+[      ]+50:[  ]+00000013[     ]+nop
+[      ]+54:[  ]+00008067[     ]+ret
+#pass

diff --git a/gas/testsuite/gas/riscv/relax-align-2.s b/gas/testsuite/gas/riscv/relax-align-2.s
new file mode 100644 (file)
index 0000000..69c6128
--- /dev/null
+++ b/gas/testsuite/gas/riscv/relax-align-2.s
@@ -0,0 +1,50 @@
+       .text
+       .option rvc
+rvc_func:
+       ret
+
+       .option norvc
+       .p2align 3
+non_rvc_func:
+       ret
+
+insn:
+       nop
+       .insn 0
+       .p2align 3
+       ret
+
+hword:
+       nop
+       .hword 0
+       .p2align 3
+       ret
+
+byte:
+       nop
+       .byte 0
+       .p2align 3
+       ret
+
+       .p2align 3
+func1:
+       nop
+       ret
+
+       .pushsection .text1, "ax", @progbits
+       .option rvc
+       nop
+       .popsection
+
+func2:
+       ret
+
+       .pushsection .text1, "ax", @progbits
+       nop
+       .option norvc
+       .popsection
+
+       .p2align 3
+func3:
+       nop
+       ret

diff --git a/gas/testsuite/gas/riscv/relax-align.d b/gas/testsuite/gas/riscv/relax-align.d
new file mode 100644 (file)
index 0000000..0aed58e
--- /dev/null
+++ b/gas/testsuite/gas/riscv/relax-align.d
@@ -0,0 +1,34 @@
+#as: -mrelax
+#objdump: -dr
+
+.*:[   ]+file format .*
+
+
+Disassembly of section .text:
+
+0+000 <rvc_func>:
+[      ]+0:[   ]+8082[         ]+ret
+[      ]+2:[   ]+0001[         ]+nop
+
+0+004 <non_rvc_func>:
+[      ]+4:[   ]+00008067[     ]+ret
+
+0+008 <insn>:
+[      ]+8:[   ]+00000013[     ]+nop
+[      ]+c:[   ]+0000[         ]+\.insn        2, 0x0+
+[      ]+e:[   ]+0001[         ]+\.insn        2, 0x0*1
+[      ]+10:[  ]+00008067[     ]+ret
+
+0+0014 <hword>:
+[      ]+14:[  ]+00000013[     ]+nop
+[      ]+18:[  ]+0000[         ]+\.short       0x0+
+[      ]+1a:[  ]+0001[         ]+\.insn        2, 0x0*1
+[      ]+1c:[  ]+00008067[     ]+ret
+
+0+0020 <byte>:
+[      ]+20:[  ]+00000013[     ]+nop
+[      ]+24:[  ]+00[   ]+\.byte        0x0+
+[      ]+25:[  ]+00[   ]+\.byte        0x0+
+[      ]+26:[  ]+0001[         ]+\.insn        2, 0x0*1
+[      ]+28:[  ]+00008067[     ]+ret
+#pass

diff --git a/gas/testsuite/gas/riscv/relax-align.s b/gas/testsuite/gas/riscv/relax-align.s
new file mode 100644 (file)
index 0000000..54db102
--- /dev/null
+++ b/gas/testsuite/gas/riscv/relax-align.s
@@ -0,0 +1,27 @@
+       .text
+       .option rvc
+rvc_func:
+       ret
+
+       .option norvc
+       .p2align 2
+non_rvc_func:
+       ret
+
+insn:
+       nop
+       .insn 0
+       .p2align 2
+       ret
+
+hword:
+       nop
+       .hword 0
+       .p2align 2
+       ret
+
+byte:
+       nop
+       .byte 0
+       .p2align 2
+       ret

diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 8ab138e45cec2f87ca0f653e688b8ea39fb0affe..551d57e4cbadc4d0fb8de558b2ff985a331ee076 100644 (file)
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -1030,7 +1030,7 @@ riscv_disassemble_insn (bfd_vma memaddr,
     {
       i -= 2;
       word = bfd_get_bits (packet + i, 16, false);
-      if (!word && !printed)
+      if (!word && !printed && i)
        continue;
 
       (*info->fprintf_styled_func) (info->stream, dis_style_immediate,