[binutils-gdb] PR22819, powerpc gas "instruction address is not a multiple of 4"

[Alan Modra <amodra at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=a9479dc051ab00f311c04cdd5b299a70739f67ed

commit a9479dc051ab00f311c04cdd5b299a70739f67ed
Author: Alan Modra <amodra@gmail.com>
Date:   Thu Feb 8 10:18:59 2018 +1030

    PR22819, powerpc gas "instruction address is not a multiple of 4"
    
    Checks for insn alignment were hopelessly confused when misaligned
    data starts a new frag.  The real-world testcase happened to run out
    of frag space in the middle of emitting a trace-back table via
    something like:
    	.byte 0		/* VERSION=0 */
     	.byte 9		/* LANG=C++ */
    	.byte 34	/* Bits on: has_tboff, fp_present */
    	.byte 64	/* Bits on: name_present */
    	.byte 128	/* Bits on: stores_bc, FP_SAVED=0 */
    	.byte 0		/* Bits on: GP_SAVED=0 */
    	.byte 2		/* FIXEDPARMS=2 */
    	.byte 1		/* FLOATPARMS=0, parmsonstk */
    	.long 0
    	.long 768	/* tb_offset: 0x300 */
    	.hword 45	/* Function name length: 45 */
     	.long 0x334e5a5f
    	.long 0x31766f70
    	.long 0x65744932
    	.long 0x69746172
    	.long 0x7a5f6e6f
    	.long 0x64504533
    	.long 0x5f534e50
    	.long 0x72463431
    	.long 0x61746361
    	.long 0x74535f6c
    	.long 0x74637572
    	.byte 0x45
    	.byte 0
    The trigger being those misaligned .long's output for the function
    name.  A most horrible way to output a string, especially considering
    endian issues..
    
    	PR 22819
    	* config/tc-ppc.c (md_assemble): Rewrite insn alignment checking.
    	(ppc_frag_check): Likewise.
    	* testsuite/gas/ppc/misalign.d,
    	* testsuite/gas/ppc/misalign.l,
    	* testsuite/gas/ppc/misalign.s: New test.
    	* testsuite/gas/ppc/misalign2.d,
    	* testsuite/gas/ppc/misalign2.s: New test.
    	* testsuite/gas/ppc/ppc.exp: Run them.

Diff:
---
 gas/ChangeLog                     | 12 ++++++++
 gas/config/tc-ppc.c               | 64 ++++++++++++++++++---------------------
 gas/testsuite/gas/ppc/misalign.d  |  2 ++
 gas/testsuite/gas/ppc/misalign.l  |  2 ++
 gas/testsuite/gas/ppc/misalign.s  |  3 ++
 gas/testsuite/gas/ppc/misalign2.d |  7 +++++
 gas/testsuite/gas/ppc/misalign2.s | 13 ++++++++
 gas/testsuite/gas/ppc/ppc.exp     |  2 ++
 8 files changed, 70 insertions(+), 35 deletions(-)

diff --git a/gas/ChangeLog b/gas/ChangeLog
index 45ad93b..44d9682 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,15 @@
+2018-02-08  Alan Modra  <amodra@gmail.com>
+
+	PR 22819
+	* config/tc-ppc.c (md_assemble): Rewrite insn alignment checking.
+	(ppc_frag_check): Likewise.
+	* testsuite/gas/ppc/misalign.d,
+	* testsuite/gas/ppc/misalign.l,
+	* testsuite/gas/ppc/misalign.s: New test.
+	* testsuite/gas/ppc/misalign2.d,
+	* testsuite/gas/ppc/misalign2.s: New test.
+	* testsuite/gas/ppc/ppc.exp: Run them.
+
 2018-02-05  Maciej W. Rozycki  <macro@mips.com>
 
 	* config/tc-riscv.c (riscv_handle_implicit_zero_offset): Rename
diff --git a/gas/config/tc-ppc.c b/gas/config/tc-ppc.c
index 35da05c..f63003b 100644
--- a/gas/config/tc-ppc.c
+++ b/gas/config/tc-ppc.c
@@ -2747,7 +2747,7 @@ md_assemble (char *str)
   struct ppc_fixup fixups[MAX_INSN_FIXUPS];
   int fc;
   char *f;
-  int addr_mod;
+  int addr_mask;
   int i;
   unsigned int insn_length;
 
@@ -3520,31 +3520,34 @@ md_assemble (char *str)
 #endif
 
   /* Write out the instruction.  */
-  /* Differentiate between two and four byte insns.  */
+
+  addr_mask = 3;
   if ((ppc_cpu & PPC_OPCODE_VLE) != 0)
+    /* All instructions can start on a 2 byte boundary for VLE.  */
+    addr_mask = 1;
+
+  if (frag_now->insn_addr != addr_mask)
     {
-      if (PPC_OP_SE_VLE (insn))
-        insn_length = 2;
-      else
-        insn_length = 4;
-      addr_mod = frag_now_fix () & 1;
-    }
-  else
-    {
-      insn_length = 4;
-      addr_mod = frag_now_fix () & 3;
+      /* Don't emit instructions to a frag started for data, or for a
+	 CPU differing in VLE mode.  Data is allowed to be misaligned,
+	 and it's possible to start a new frag in the middle of
+	 misaligned data.  */
+      frag_wane (frag_now);
+      frag_new (0);
     }
-  /* All instructions can start on a 2 byte boundary for VLE.  */
+
+  /* Check that insns within the frag are aligned.  ppc_frag_check
+     will ensure that the frag start address is aligned.  */
+  if ((frag_now_fix () & addr_mask) != 0)
+    as_bad (_("instruction address is not a multiple of %d"), addr_mask + 1);
+
+  /* Differentiate between two and four byte insns.  */
+  insn_length = 4;
+  if ((ppc_cpu & PPC_OPCODE_VLE) != 0 && PPC_OP_SE_VLE (insn))
+    insn_length = 2;
+
   f = frag_more (insn_length);
-  if (frag_now->has_code && frag_now->insn_addr != addr_mod)
-    {
-      if ((ppc_cpu & PPC_OPCODE_VLE) != 0)
-        as_bad (_("instruction address is not a multiple of 2"));
-      else
-        as_bad (_("instruction address is not a multiple of 4"));
-    }
-  frag_now->insn_addr = addr_mod;
-  frag_now->has_code = 1;
+  frag_now->insn_addr = addr_mask;
   md_number_to_chars (f, insn, insn_length);
   last_insn = insn;
   last_seg = now_seg;
@@ -6491,19 +6494,10 @@ ppc_fix_adjustable (fixS *fix)
 void
 ppc_frag_check (struct frag *fragP)
 {
-  if (!fragP->has_code)
-    return;
-
-  if ((ppc_cpu & PPC_OPCODE_VLE) != 0)
-    {
-      if (((fragP->fr_address + fragP->insn_addr) & 1) != 0)
-        as_bad (_("instruction address is not a multiple of 2"));
-    }
-  else
-    {
-      if (((fragP->fr_address + fragP->insn_addr) & 3) != 0)
-        as_bad (_("instruction address is not a multiple of 4"));
-    }
+  if ((fragP->fr_address & fragP->insn_addr) != 0)
+    as_bad_where (fragP->fr_file, fragP->fr_line,
+		  _("instruction address is not a multiple of %d"),
+		  fragP->insn_addr + 1);
 }
 
 /* Implement HANDLE_ALIGN.  This writes the NOP pattern into an
diff --git a/gas/testsuite/gas/ppc/misalign.d b/gas/testsuite/gas/ppc/misalign.d
new file mode 100644
index 0000000..c004b17
--- /dev/null
+++ b/gas/testsuite/gas/ppc/misalign.d
@@ -0,0 +1,2 @@
+#as: 
+#error-output: misalign.l
diff --git a/gas/testsuite/gas/ppc/misalign.l b/gas/testsuite/gas/ppc/misalign.l
new file mode 100644
index 0000000..2e27323
--- /dev/null
+++ b/gas/testsuite/gas/ppc/misalign.l
@@ -0,0 +1,2 @@
+.*Assembler messages:
+.*Error: instruction address is not a multiple of 4
diff --git a/gas/testsuite/gas/ppc/misalign.s b/gas/testsuite/gas/ppc/misalign.s
new file mode 100644
index 0000000..51836dc
--- /dev/null
+++ b/gas/testsuite/gas/ppc/misalign.s
@@ -0,0 +1,3 @@
+ .text
+ .byte 1
+ nop
diff --git a/gas/testsuite/gas/ppc/misalign2.d b/gas/testsuite/gas/ppc/misalign2.d
new file mode 100644
index 0000000..369d304
--- /dev/null
+++ b/gas/testsuite/gas/ppc/misalign2.d
@@ -0,0 +1,7 @@
+#as:
+#nm: -n
+
+#...
+0+00001 T odd
+0+40001 T odd2
+0+40004 T aligned
diff --git a/gas/testsuite/gas/ppc/misalign2.s b/gas/testsuite/gas/ppc/misalign2.s
new file mode 100644
index 0000000..ada84d8
--- /dev/null
+++ b/gas/testsuite/gas/ppc/misalign2.s
@@ -0,0 +1,13 @@
+ .text
+ .global odd, odd2, aligned
+ .dc.b 1
+odd:
+ .rept 65536
+ .dc.l 0
+ .endr
+odd2:
+ .dc.b 2,3,4
+aligned:
+ .rept 65536
+ nop
+ .endr
diff --git a/gas/testsuite/gas/ppc/ppc.exp b/gas/testsuite/gas/ppc/ppc.exp
index 39bbbbd..bdce3dc 100644
--- a/gas/testsuite/gas/ppc/ppc.exp
+++ b/gas/testsuite/gas/ppc/ppc.exp
@@ -45,6 +45,8 @@ if { [istarget powerpc64*-*-*] || [istarget *-*-elf64*]} then {
 
 if { [istarget powerpc*-*-*] } then {
     run_dump_test "regnames"
+    run_dump_test "misalign"
+    run_dump_test "misalign2"
     if { [is_elf_format] } then {
 	run_dump_test "machine"
 	run_dump_test "common"