diff mbox series

[2/3] x86: disassembling over-long insns

Message ID 65393035-8006-1a66-deae-70a88ed29077@suse.com
State Unresolved
Headers
Series x86: a little bit of disassembler tidying |

Checks

Context Check Description
snail/binutils-gdb-check warning Git am fail log

Commit Message

Jan Beulich May 19, 2023, 2:06 p.m. UTC 
  The present way of dealing with them - misusing MAX_MNEM_SIZE, which has
nothing to do with insn length - leads to inconsistent results. Since we
allow for up to MAX_CODE_LENGTH - 1 prefix bytes (which then could be
followed by another MAX_CODE_LENGTH "normal" insn bytes until we're done
decoding), size the_buffer[] accordingly.

Move struct dis_private down to be able to use MAX_CODE_LENGTH without
moving its #define. While doing this also alter the order to have the
potentially large array last.
---
Originally I meant to use just MAX_CODE_LENGTH as the buffer size. But
that broke the four "long insn" testcases, which I thought might make
sense to keep intact. Nevertheless that's certainly another way to get
things into consistent shape. It would further allow to drop the check
associated with a "Check maximum code length" comment, as then we would
never fetch more than this many bytes.

Strictly speaking what can come past suffixes is a little less than
MAX_CODE_LENGTH: Right now it could be EVEX (4 bytes), opcode (1),
ModR/M (<=1), SIB (<=1), displacement (<=4), and immediate (<=4); the
8-byte displacement and 8-byte immediate cases have, respectively, no
immediate or no displacement, and hence disp+imm <= 8 in all cases.
While this sums up to 15, there are no EVEX insns at present with 32-bit
immediate. The longest prefix-less ones currently known are those in the
oversized{16,64} gas testcases (sans their prefixes, of course), i.e. 14
bytes. But I think it is better to cover the potential case of EVEX-
with-32-bit-immediate case right away.
diff mbox series

Patch

--- a/opcodes/i386-dis.c
+++ b/opcodes/i386-dis.c
@@ -118,14 +118,6 @@  static void ATTRIBUTE_PRINTF_3 i386_dis_
 /* The maximum operand buffer size.  */
 #define MAX_OPERAND_BUFFER_SIZE 128
 
-struct dis_private {
-  /* Points to first byte not fetched.  */
-  uint8_t *max_fetched;
-  uint8_t the_buffer[MAX_MNEM_SIZE];
-  bfd_vma insn_start;
-  int orig_sizeflag;
-};
-
 enum address_mode
 {
   mode_16bit,
@@ -251,6 +243,15 @@  struct instr_info
   enum x86_64_isa isa64;
 };
 
+struct dis_private {
+  bfd_vma insn_start;
+  int orig_sizeflag;
+
+  /* Points to first byte not fetched.  */
+  uint8_t *max_fetched;
+  uint8_t the_buffer[2 * MAX_CODE_LENGTH - 1];
+};
+
 /* Mark parts used in the REX prefix.  When we are testing for
    empty prefix (for 8bit register REX extension), just mask it
    out.  Otherwise test for REX bit is excuse for existence of REX
@@ -297,7 +298,7 @@  fetch_code (struct disassemble_info *inf
   if (until <= priv->max_fetched)
     return true;
 
-  if (until <= priv->the_buffer + MAX_MNEM_SIZE)
+  if (until <= priv->the_buffer + ARRAY_SIZE (priv->the_buffer))
     status = (*info->read_memory_func) (start,
 					priv->max_fetched,
 					until - priv->max_fetched,