xtensa: use property tables for correct disassembly

Message ID 20180530183023.8447-1-jcmvbkbc@gmail.com
State New
Headers show
Series
  • xtensa: use property tables for correct disassembly
Related show

Commit Message

Max Filippov May 30, 2018, 6:30 p.m.
xtensa disassembler does not use information from the .xt.prop sections
to switch between code/data disassembly in text sections. This may
result in incorrect disassembly when data is interpreted as code and
disassembler loses synchronization with instruction stream. Use .xt.prop
section information to correctly interpret code and data and synchronize
with instruction stream.

2018-05-30  Max Filippov  <jcmvbkbc@gmail.com>
bfd/
	* elf32-xtensa.c (xtensa_read_table_entries): Make global.
	(compute_fill_extra_space): Drop declaration. Rename function to
	xtensa_compute_fill_extra_space.
	(compute_ebb_actions, remove_dead_literal): Update references to
	compute_fill_extra_space.

include/
	* elf/xtensa.h (xtensa_read_table_entries)
	(xtensa_compute_fill_extra_space): New declarations.

opcodes/
	* xtensa-dis.c (bfd.h, elf/xtensa.h): New includes.
	(dis_private): Add new fields for property section tracking.
	(xtensa_coalesce_insn_tables, xtensa_find_table_entry)
	(xtensa_instruction_fits): New functions.
	(fetch_data): Bump minimal fetch size to 4.
	(print_insn_xtensa): Make struct dis_private static.
	Load and prepare property table on section change.
	Don't disassemble literals. Don't disassemble instructions that
	cross property table boundaries.
---
 bfd/elf32-xtensa.c   |   9 +--
 include/elf/xtensa.h |   8 ++
 opcodes/xtensa-dis.c | 216 +++++++++++++++++++++++++++++++++++++++++++++------
 3 files changed, 206 insertions(+), 27 deletions(-)

-- 
2.11.0

Comments

augustine.sterling@gmail.com June 4, 2018, 5:20 p.m. | #1
On Wed, May 30, 2018 at 11:30 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:

> 2018-05-30  Max Filippov  <jcmvbkbc@gmail.com>

> bfd/

>         * elf32-xtensa.c (xtensa_read_table_entries): Make global.

>         (compute_fill_extra_space): Drop declaration. Rename function to

>         xtensa_compute_fill_extra_space.

>         (compute_ebb_actions, remove_dead_literal): Update references to

>         compute_fill_extra_space.

>

> include/

>         * elf/xtensa.h (xtensa_read_table_entries)

>         (xtensa_compute_fill_extra_space): New declarations.

>

> opcodes/

>         * xtensa-dis.c (bfd.h, elf/xtensa.h): New includes.

>         (dis_private): Add new fields for property section tracking.

>         (xtensa_coalesce_insn_tables, xtensa_find_table_entry)

>         (xtensa_instruction_fits): New functions.

>         (fetch_data): Bump minimal fetch size to 4.

>         (print_insn_xtensa): Make struct dis_private static.

>         Load and prepare property table on section change.

>         Don't disassemble literals. Don't disassemble instructions that

>         cross property table boundaries.

>


Approved. Thanks.
Max Filippov June 4, 2018, 5:41 p.m. | #2
On Mon, Jun 4, 2018 at 10:20 AM, augustine.sterling@gmail.com
<augustine.sterling@gmail.com> wrote:
> On Wed, May 30, 2018 at 11:30 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:

>>

>> 2018-05-30  Max Filippov  <jcmvbkbc@gmail.com>

>> bfd/

>>         * elf32-xtensa.c (xtensa_read_table_entries): Make global.

>>         (compute_fill_extra_space): Drop declaration. Rename function to

>>         xtensa_compute_fill_extra_space.

>>         (compute_ebb_actions, remove_dead_literal): Update references to

>>         compute_fill_extra_space.

>>

>> include/

>>         * elf/xtensa.h (xtensa_read_table_entries)

>>         (xtensa_compute_fill_extra_space): New declarations.

>>

>> opcodes/

>>         * xtensa-dis.c (bfd.h, elf/xtensa.h): New includes.

>>         (dis_private): Add new fields for property section tracking.

>>         (xtensa_coalesce_insn_tables, xtensa_find_table_entry)

>>         (xtensa_instruction_fits): New functions.

>>         (fetch_data): Bump minimal fetch size to 4.

>>         (print_insn_xtensa): Make struct dis_private static.

>>         Load and prepare property table on section change.

>>         Don't disassemble literals. Don't disassemble instructions that

>>         cross property table boundaries.

>

>

> Approved. Thanks.


Thank you. Applied to master.

-- Max

Patch

diff --git a/bfd/elf32-xtensa.c b/bfd/elf32-xtensa.c
index db3c8f4baab6..44c1074446e8 100644
--- a/bfd/elf32-xtensa.c
+++ b/bfd/elf32-xtensa.c
@@ -802,7 +802,7 @@  property_table_matches (const void *ap, const void *bp)
    section.  Sets TABLE_P and returns the number of entries.  On
    error, returns a negative value.  */
 
-static int
+int
 xtensa_read_table_entries (bfd *abfd,
 			   asection *section,
 			   property_table_entry **table_p,
@@ -6730,7 +6730,6 @@  static bfd_boolean check_section_ebb_pcrels_fit
 static bfd_boolean check_section_ebb_reduces (const ebb_constraint *);
 static void text_action_add_proposed
   (text_action_list *, const ebb_constraint *, asection *);
-static int compute_fill_extra_space (property_table_entry *);
 
 /* First pass: */
 static bfd_boolean compute_removed_literals
@@ -8136,7 +8135,7 @@  compute_ebb_actions (ebb_constraint *ebb_table)
       BFD_ASSERT (action->action == ta_fill);
       BFD_ASSERT (ebb->ends_unreachable->flags & XTENSA_PROP_UNREACHABLE);
 
-      extra_space = compute_fill_extra_space (ebb->ends_unreachable);
+      extra_space = xtensa_compute_fill_extra_space (ebb->ends_unreachable);
       br = action->removed_bytes + removed_bytes + extra_space;
       br = br & ((1 << ebb->sec->alignment_power ) - 1);
 
@@ -8560,7 +8559,7 @@  text_action_add_proposed (text_action_list *l,
 
 
 int
-compute_fill_extra_space (property_table_entry *entry)
+xtensa_compute_fill_extra_space (property_table_entry *entry)
 {
   int fill_extra_space;
 
@@ -8841,7 +8840,7 @@  remove_dead_literal (bfd *abfd,
 	 do not add fill.  */
       the_add_entry = elf_xtensa_find_property_entry (prop_table, ptblsize,
 						      entry_sec_offset);
-      fill_extra_space = compute_fill_extra_space (the_add_entry);
+      fill_extra_space = xtensa_compute_fill_extra_space (the_add_entry);
 
       fa = find_fill_action (&relax_info->action_list, sec, entry_sec_offset);
       removed_diff = compute_removed_action_diff (fa, sec, entry_sec_offset,
diff --git a/include/elf/xtensa.h b/include/elf/xtensa.h
index 6278e21e5750..632d9e0c8d19 100644
--- a/include/elf/xtensa.h
+++ b/include/elf/xtensa.h
@@ -210,6 +210,14 @@  typedef struct property_table_entry_t
 #define XTENSA_PROP_INSN_ABSLIT        0x00020000
 
 extern asection *xtensa_make_property_section (asection *, const char *);
+extern int
+xtensa_read_table_entries (bfd *abfd,
+			   asection *section,
+			   property_table_entry **table_p,
+			   const char *sec_name,
+			   bfd_boolean output_addr);
+extern int
+xtensa_compute_fill_extra_space (property_table_entry *entry);
 
 #ifdef __cplusplus
 }
diff --git a/opcodes/xtensa-dis.c b/opcodes/xtensa-dis.c
index d78f9d5f9490..c11cf064ce1d 100644
--- a/opcodes/xtensa-dis.c
+++ b/opcodes/xtensa-dis.c
@@ -27,6 +27,8 @@ 
 #include "xtensa-isa.h"
 #include "ansidecl.h"
 #include "libiberty.h"
+#include "bfd.h"
+#include "elf/xtensa.h"
 #include "disassemble.h"
 
 #include <setjmp.h>
@@ -43,8 +45,119 @@  struct dis_private
 {
   bfd_byte *byte_buf;
   OPCODES_SIGJMP_BUF bailout;
+  /* Persistent fields, valid for last_section only.  */
+  asection *last_section;
+  property_table_entry *insn_table_entries;
+  int insn_table_entry_count;
+  /* Cached property table search position.  */
+  bfd_vma insn_table_cur_addr;
+  int insn_table_cur_idx;
 };
 
+static void
+xtensa_coalesce_insn_tables (struct dis_private *priv)
+{
+  const int mask = ~(XTENSA_PROP_DATA | XTENSA_PROP_NO_TRANSFORM);
+  int count = priv->insn_table_entry_count;
+  int i, j;
+
+  /* Loop over all entries, combining adjacent ones that differ only in
+     the flag bits XTENSA_PROP_DATA and XTENSA_PROP_NO_TRANSFORM.  */
+
+  for (i = j = 0; j < count; ++i)
+    {
+      property_table_entry *entry = priv->insn_table_entries + i;
+
+      *entry = priv->insn_table_entries[j];
+
+      for (++j; j < count; ++j)
+	{
+	  property_table_entry *next = priv->insn_table_entries + j;
+	  int fill = xtensa_compute_fill_extra_space (entry);
+	  int size = entry->size + fill;
+
+	  if (entry->address + size == next->address)
+	    {
+	      int entry_flags = entry->flags & mask;
+	      int next_flags = next->flags & mask;
+
+	      if (next_flags == entry_flags)
+		entry->size = next->address - entry->address + next->size;
+	      else
+		break;
+	    }
+	  else
+	    {
+	      break;
+	    }
+	}
+    }
+  priv->insn_table_entry_count = i;
+}
+
+static property_table_entry *
+xtensa_find_table_entry (bfd_vma memaddr, struct disassemble_info *info)
+{
+  struct dis_private *priv = (struct dis_private *) info->private_data;
+  int i;
+
+  if (priv->insn_table_entries == NULL
+      || priv->insn_table_entry_count < 0)
+    return NULL;
+
+  if (memaddr < priv->insn_table_cur_addr)
+    priv->insn_table_cur_idx = 0;
+
+  for (i = priv->insn_table_cur_idx; i < priv->insn_table_entry_count; ++i)
+    {
+      property_table_entry *block = priv->insn_table_entries + i;
+
+      if (block->size != 0)
+	{
+	  if ((memaddr >= block->address
+	       && memaddr < block->address + block->size)
+	      || memaddr < block->address)
+	    {
+	      priv->insn_table_cur_addr = memaddr;
+	      priv->insn_table_cur_idx = i;
+	      return block;
+	    }
+	}
+    }
+  return NULL;
+}
+
+/* Check whether an instruction crosses an instruction block boundary
+   (according to property tables).
+   If it does, return 0 (doesn't fit), else return 1.  */
+
+static int
+xtensa_instruction_fits (bfd_vma memaddr, int size,
+			 property_table_entry *insn_block)
+{
+  unsigned max_size;
+
+  /* If no property table info, assume it fits.  */
+  if (insn_block == NULL || size <= 0)
+    return 1;
+
+  /* If too high, limit nextstop by the next insn address.  */
+  if (insn_block->address > memaddr)
+    {
+      /* memaddr is not in an instruction block, but is followed by one.  */
+      max_size = insn_block->address - memaddr;
+    }
+  else
+    {
+      /* memaddr is in an instruction block, go no further than the end.  */
+      max_size = insn_block->address + insn_block->size - memaddr;
+    }
+
+  /* Crossing a boundary, doesn't "fit".  */
+  if ((unsigned)size > max_size)
+    return 0;
+  return 1;
+}
 
 static int
 fetch_data (struct disassemble_info *info, bfd_vma memaddr)
@@ -53,6 +166,8 @@  fetch_data (struct disassemble_info *info, bfd_vma memaddr)
   struct dis_private *priv = (struct dis_private *) info->private_data;
   int insn_size = xtensa_isa_maxlength (xtensa_default_isa);
 
+  insn_size = MAX (insn_size, 4);
+
   /* Read the maximum instruction size, padding with zeros if we go past
      the end of the text section.  This code will automatically adjust
      length when we hit the end of the buffer.  */
@@ -140,11 +255,12 @@  print_insn_xtensa (bfd_vma memaddr, struct disassemble_info *info)
   xtensa_isa isa;
   xtensa_opcode opc;
   xtensa_format fmt;
-  struct dis_private priv;
+  static struct dis_private priv;
   static bfd_byte *byte_buf = NULL;
   static xtensa_insnbuf insn_buffer = NULL;
   static xtensa_insnbuf slot_buffer = NULL;
-  int first, first_slot, valid_insn;
+  int first, first_slot, valid_insn = 0;
+  property_table_entry *insn_block;
 
   if (!xtensa_default_isa)
     xtensa_default_isa = xtensa_isa_init (0, 0);
@@ -175,6 +291,49 @@  print_insn_xtensa (bfd_vma memaddr, struct disassemble_info *info)
   priv.byte_buf = byte_buf;
 
   info->private_data = (void *) &priv;
+
+  /* Prepare instruction tables.  */
+
+  if (info->section != NULL)
+    {
+      asection *section = info->section;
+
+      if (priv.last_section != section)
+	{
+	  bfd *abfd = section->owner;
+
+	  if (priv.last_section != NULL)
+	    {
+	      /* Reset insn_table_entries.  */
+	      priv.insn_table_entry_count = 0;
+	      if (priv.insn_table_entries)
+		free (priv.insn_table_entries);
+	      priv.insn_table_entries = NULL;
+	    }
+	  priv.last_section = section;
+
+	  /* Read insn_table_entries.  */
+	  priv.insn_table_entry_count =
+	    xtensa_read_table_entries (abfd, section,
+				       &priv.insn_table_entries,
+				       XTENSA_PROP_SEC_NAME, FALSE);
+	  if (priv.insn_table_entry_count == 0)
+	    {
+	      if (priv.insn_table_entries)
+		free (priv.insn_table_entries);
+	      priv.insn_table_entries = NULL;
+	      /* Backwards compatibility support.  */
+	      priv.insn_table_entry_count =
+		xtensa_read_table_entries (abfd, section,
+					   &priv.insn_table_entries,
+					   XTENSA_INSN_SEC_NAME, FALSE);
+	    }
+	  priv.insn_table_cur_idx = 0;
+	  xtensa_coalesce_insn_tables (&priv);
+	}
+      /* Else nothing to do, same section as last time.  */
+    }
+
   if (OPCODES_SIGSETJMP (priv.bailout) != 0)
       /* Error return.  */
       return -1;
@@ -187,36 +346,49 @@  print_insn_xtensa (bfd_vma memaddr, struct disassemble_info *info)
   /* Fetch the maximum size instruction.  */
   bytes_fetched = fetch_data (info, memaddr);
 
-  /* Copy the bytes into the decode buffer.  */
-  memset (insn_buffer, 0, (xtensa_insnbuf_size (isa) *
-			   sizeof (xtensa_insnbuf_word)));
-  xtensa_insnbuf_from_chars (isa, insn_buffer, priv.byte_buf, bytes_fetched);
+  insn_block = xtensa_find_table_entry (memaddr, info);
 
-  fmt = xtensa_format_decode (isa, insn_buffer);
-  if (fmt == XTENSA_UNDEFINED
-      || ((size = xtensa_format_length (isa, fmt)) > bytes_fetched))
-    valid_insn = 0;
-  else
+  if (!insn_block || (insn_block->flags & XTENSA_PROP_INSN))
     {
-      /* Make sure all the opcodes are valid.  */
-      valid_insn = 1;
-      nslots = xtensa_format_num_slots (isa, fmt);
-      for (n = 0; n < nslots; n++)
+      /* Copy the bytes into the decode buffer.  */
+      memset (insn_buffer, 0, (xtensa_insnbuf_size (isa) *
+			       sizeof (xtensa_insnbuf_word)));
+      xtensa_insnbuf_from_chars (isa, insn_buffer, priv.byte_buf,
+				 bytes_fetched);
+
+      fmt = xtensa_format_decode (isa, insn_buffer);
+      if (fmt != XTENSA_UNDEFINED
+	  && ((size = xtensa_format_length (isa, fmt)) <= bytes_fetched)
+	  && xtensa_instruction_fits (memaddr, size, insn_block))
 	{
-	  xtensa_format_get_slot (isa, fmt, n, insn_buffer, slot_buffer);
-	  if (xtensa_opcode_decode (isa, fmt, n, slot_buffer)
-	      == XTENSA_UNDEFINED)
+	  /* Make sure all the opcodes are valid.  */
+	  valid_insn = 1;
+	  nslots = xtensa_format_num_slots (isa, fmt);
+	  for (n = 0; n < nslots; n++)
 	    {
-	      valid_insn = 0;
-	      break;
+	      xtensa_format_get_slot (isa, fmt, n, insn_buffer, slot_buffer);
+	      if (xtensa_opcode_decode (isa, fmt, n, slot_buffer)
+		  == XTENSA_UNDEFINED)
+		{
+		  valid_insn = 0;
+		  break;
+		}
 	    }
 	}
     }
 
   if (!valid_insn)
     {
-      (*info->fprintf_func) (info->stream, ".byte %#02x", priv.byte_buf[0]);
-      return 1;
+      if (insn_block && (insn_block->flags & XTENSA_PROP_LITERAL)
+	  && (memaddr & 3) == 0 && bytes_fetched >= 4)
+	{
+	  return 4;
+	}
+      else
+	{
+	  (*info->fprintf_func) (info->stream, ".byte %#02x", priv.byte_buf[0]);
+	  return 1;
+	}
     }
 
   if (nslots > 1)