public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: Binutils <binutils@sourceware.org>
Cc: "H.J. Lu" <hjl.tools@gmail.com>, Alan Modra <amodra@gmail.com>
Subject: [PATCH 6/8] x86: change fetch error handling for get<N>()
Date: Tue, 4 Apr 2023 09:00:46 +0200	[thread overview]
Message-ID: <a9e13e17-ecf2-8e96-5b48-870801faa750@suse.com> (raw)
In-Reply-To: <5dac45a8-cd5f-ee4d-52fc-7d283fc29ec4@suse.com>

Make them return boolean and convert FETCH_DATA() uses to fetch_code().
With this no further users of FETCH_DATA() remain, so the macro and its
backing function are dropped as well.

Leave value types as they were for the helper functions, even if I don't
think that beyond get64() use of bfd_{,signed_}vma is really necessary.
With type change of "disp" in OP_E_memory(), change the 2nd parameter of
print_displacement() to a signed type as well, though (eliminating the
need for a local variable of signed type). This also eliminates the need
for custom printing of '-' in Intel syntax displacement expressions.

While there drop forward declarations which aren't really needed.

--- a/opcodes/i386-dis.c
+++ b/opcodes/i386-dis.c
@@ -48,10 +48,8 @@ static void oappend_with_style (instr_in
 				enum disassembler_style);
 static void oappend (instr_info *, const char *);
 static void append_seg (instr_info *);
-static bfd_vma get64 (instr_info *);
-static bfd_signed_vma get32 (instr_info *);
-static bfd_signed_vma get32s (instr_info *);
-static int get16 (instr_info *);
+static bool get32s (instr_info *, bfd_signed_vma *);
+static bool get16 (instr_info *, int *);
 static void set_op (instr_info *, bfd_vma, bool);
 
 static bool OP_E (instr_info *, int, int);
@@ -295,41 +293,8 @@ struct instr_info
 #define PREFIX_FWAIT 0x800
 
 /* Make sure that bytes from INFO->PRIVATE_DATA->BUFFER (inclusive)
-   to ADDR (exclusive) are valid.  Returns 1 for success, longjmps
+   to ADDR (exclusive) are valid.  Returns true for success, false
    on error.  */
-#define FETCH_DATA(info, addr) \
-  ((addr) <= ((struct dis_private *) (info->private_data))->max_fetched \
-   ? 1 : fetch_data ((info), (addr)))
-
-static int
-fetch_data (struct disassemble_info *info, bfd_byte *addr)
-{
-  int status;
-  struct dis_private *priv = (struct dis_private *) info->private_data;
-  bfd_vma start = priv->insn_start + (priv->max_fetched - priv->the_buffer);
-
-  if (addr <= priv->the_buffer + MAX_MNEM_SIZE)
-    status = (*info->read_memory_func) (start,
-					priv->max_fetched,
-					addr - priv->max_fetched,
-					info);
-  else
-    status = -1;
-  if (status != 0)
-    {
-      /* If we did manage to read at least one byte, then
-	 print_insn_i386 will do something sensible.  Otherwise, print
-	 an error.  We do that here because this is where we know
-	 STATUS.  */
-      if (priv->max_fetched == priv->the_buffer)
-	(*info->memory_error_func) (status, start, info);
-      OPCODES_SIGLONGJMP (priv->bailout, 1);
-    }
-  else
-    priv->max_fetched = addr;
-  return 1;
-}
-
 static bool
 fetch_code (struct disassemble_info *info, bfd_byte *until)
 {
@@ -11412,15 +11377,14 @@ oappend_immediate (instr_info *ins, bfd_
 /* Put DISP in BUF as signed hex number.  */
 
 static void
-print_displacement (instr_info *ins, bfd_vma disp)
+print_displacement (instr_info *ins, bfd_signed_vma val)
 {
-  bfd_signed_vma val = disp;
   char tmp[30];
 
   if (val < 0)
     {
       oappend_char_with_style (ins, '-', dis_style_address_offset);
-      val = -disp;
+      val = (bfd_vma) 0 - val;
 
       /* Check for possible overflow.  */
       if (val < 0)
@@ -11830,7 +11794,6 @@ print_register (instr_info *ins, unsigne
 static bool
 OP_E_memory (instr_info *ins, int bytemode, int sizeflag)
 {
-  bfd_vma disp = 0;
   int add = (ins->rex & REX_B) ? 8 : 0;
   int riprel = 0;
   int shift;
@@ -11939,6 +11902,7 @@ OP_E_memory (instr_info *ins, int bytemo
   if ((sizeflag & AFLAG) || ins->address_mode == mode_64bit)
     {
       /* 32/64 bit address mode */
+      bfd_signed_vma disp = 0;
       int havedisp;
       int havebase;
       int needindex;
@@ -12030,7 +11994,8 @@ OP_E_memory (instr_info *ins, int bytemo
 	      havebase = 0;
 	      if (ins->address_mode == mode_64bit && !ins->has_sib)
 		riprel = 1;
-	      disp = get32s (ins);
+	      if (!get32s (ins, &disp))
+		return false;
 	      if (riprel && bytemode == v_bndmk_mode)
 		{
 		  oappend (ins, "(bad)");
@@ -12048,7 +12013,8 @@ OP_E_memory (instr_info *ins, int bytemo
 	    disp <<= shift;
 	  break;
 	case 2:
-	  disp = get32s (ins);
+	  if (!get32s (ins, &disp))
+	    return false;
 	  break;
 	}
 
@@ -12154,14 +12120,8 @@ OP_E_memory (instr_info *ins, int bytemo
 	  if (ins->intel_syntax
 	      && (disp || ins->modrm.mod != 0 || base == 5))
 	    {
-	      if (!havedisp || (bfd_signed_vma) disp >= 0)
+	      if (!havedisp || disp >= 0)
 		  oappend_char (ins, '+');
-	      else if (ins->modrm.mod != 1 && disp != -disp)
-		{
-		  oappend_char (ins, '-');
-		  disp = -disp;
-		}
-
 	      if (havedisp)
 		print_displacement (ins, disp);
 	      else
@@ -12209,13 +12169,17 @@ OP_E_memory (instr_info *ins, int bytemo
   else
     {
       /* 16 bit address mode */
+      int disp = 0;
+
       ins->used_prefixes |= ins->prefixes & PREFIX_ADDR;
       switch (ins->modrm.mod)
 	{
 	case 0:
 	  if (ins->modrm.rm == 6)
 	    {
-	      disp = get16 (ins);
+	case 2:
+	      if (!get16 (ins, &disp))
+		return false;
 	      if ((disp & 0x8000) != 0)
 		disp -= 0x10000;
 	    }
@@ -12229,11 +12193,6 @@ OP_E_memory (instr_info *ins, int bytemo
 	  if (ins->vex.evex && shift > 0)
 	    disp <<= shift;
 	  break;
-	case 2:
-	  disp = get16 (ins);
-	  if ((disp & 0x8000) != 0)
-	    disp -= 0x10000;
-	  break;
 	}
 
       if (!ins->intel_syntax)
@@ -12248,14 +12207,8 @@ OP_E_memory (instr_info *ins, int bytemo
 	  if (ins->intel_syntax
 	      && (disp || ins->modrm.mod != 0 || ins->modrm.rm == 6))
 	    {
-	      if ((bfd_signed_vma) disp >= 0)
+	      if (disp >= 0)
 		oappend_char (ins, '+');
-	      else if (ins->modrm.mod != 1)
-		{
-		  oappend_char (ins, '-');
-		  disp = -disp;
-		}
-
 	      print_displacement (ins, disp);
 	    }
 
@@ -12382,14 +12335,14 @@ OP_G (instr_info *ins, int bytemode, int
 }
 
 #ifdef BFD64
-static bfd_vma
-get64 (instr_info *ins)
+static bool
+get64 (instr_info *ins, bfd_vma *res)
 {
-  bfd_vma x;
   unsigned int a;
   unsigned int b;
 
-  FETCH_DATA (ins->info, ins->codep + 8);
+  if (!fetch_code (ins->info, ins->codep + 8))
+    return false;
   a = *ins->codep++ & 0xff;
   a |= (*ins->codep++ & 0xff) << 8;
   a |= (*ins->codep++ & 0xff) << 16;
@@ -12398,56 +12351,49 @@ get64 (instr_info *ins)
   b |= (*ins->codep++ & 0xff) << 8;
   b |= (*ins->codep++ & 0xff) << 16;
   b |= (*ins->codep++ & 0xffu) << 24;
-  x = a + ((bfd_vma) b << 32);
-  return x;
+  *res = a + ((bfd_vma) b << 32);
+  return true;
 }
 #else
-static bfd_vma
-get64 (instr_info *ins ATTRIBUTE_UNUSED)
+static bool
+get64 (instr_info *ins ATTRIBUTE_UNUSED, bfd_vma *res ATTRIBUTE_UNUSED)
 {
   abort ();
-  return 0;
+  return false;
 }
 #endif
 
-static bfd_signed_vma
-get32 (instr_info *ins)
+static bool
+get32 (instr_info *ins, bfd_signed_vma *res)
 {
-  bfd_vma x = 0;
-
-  FETCH_DATA (ins->info, ins->codep + 4);
-  x = *ins->codep++ & (bfd_vma) 0xff;
-  x |= (*ins->codep++ & (bfd_vma) 0xff) << 8;
-  x |= (*ins->codep++ & (bfd_vma) 0xff) << 16;
-  x |= (*ins->codep++ & (bfd_vma) 0xff) << 24;
-  return x;
+  if (!fetch_code (ins->info, ins->codep + 4))
+    return false;
+  *res = *ins->codep++ & (bfd_vma) 0xff;
+  *res |= (*ins->codep++ & (bfd_vma) 0xff) << 8;
+  *res |= (*ins->codep++ & (bfd_vma) 0xff) << 16;
+  *res |= (*ins->codep++ & (bfd_vma) 0xff) << 24;
+  return true;
 }
 
-static bfd_signed_vma
-get32s (instr_info *ins)
+static bool
+get32s (instr_info *ins, bfd_signed_vma *res)
 {
-  bfd_vma x = 0;
-
-  FETCH_DATA (ins->info, ins->codep + 4);
-  x = *ins->codep++ & (bfd_vma) 0xff;
-  x |= (*ins->codep++ & (bfd_vma) 0xff) << 8;
-  x |= (*ins->codep++ & (bfd_vma) 0xff) << 16;
-  x |= (*ins->codep++ & (bfd_vma) 0xff) << 24;
+  if (!get32 (ins, res))
+    return false;
 
-  x = (x ^ ((bfd_vma) 1 << 31)) - ((bfd_vma) 1 << 31);
+  *res = (*res ^ ((bfd_vma) 1 << 31)) - ((bfd_vma) 1 << 31);
 
-  return x;
+  return true;
 }
 
-static int
-get16 (instr_info *ins)
+static bool
+get16 (instr_info *ins, int *res)
 {
-  int x = 0;
-
-  FETCH_DATA (ins->info, ins->codep + 2);
-  x = *ins->codep++ & 0xff;
-  x |= (*ins->codep++ & 0xff) << 8;
-  return x;
+  if (!fetch_code (ins->info, ins->codep + 2))
+    return false;
+  *res = *ins->codep++ & 0xff;
+  *res |= (*ins->codep++ & 0xff) << 8;
+  return true;
 }
 
 static void
@@ -12587,30 +12533,32 @@ OP_I (instr_info *ins, int bytemode, int
     case v_mode:
       USED_REX (REX_W);
       if (ins->rex & REX_W)
-	op = get32s (ins);
+	{
+	  if (!get32s (ins, &op))
+	    return false;
+	}
       else
 	{
+	  ins->used_prefixes |= (ins->prefixes & PREFIX_DATA);
 	  if (sizeflag & DFLAG)
 	    {
-	      op = get32 (ins);
+    case d_mode:
+	      if (!get32 (ins, &op))
+		return false;
 	      mask = 0xffffffff;
 	    }
 	  else
 	    {
-	      op = get16 (ins);
+	      int num;
+
+    case w_mode:
+	      if (!get16 (ins, &num))
+		return false;
+	      op = num;
 	      mask = 0xfffff;
 	    }
-	  ins->used_prefixes |= (ins->prefixes & PREFIX_DATA);
 	}
       break;
-    case d_mode:
-      mask = 0xffffffff;
-      op = get32 (ins);
-      break;
-    case w_mode:
-      mask = 0xfffff;
-      op = get16 (ins);
-      break;
     case const_1_mode:
       if (ins->intel_syntax)
 	oappend (ins, "1");
@@ -12628,13 +12576,18 @@ OP_I (instr_info *ins, int bytemode, int
 static bool
 OP_I64 (instr_info *ins, int bytemode, int sizeflag)
 {
+  bfd_vma op;
+
   if (bytemode != v_mode || ins->address_mode != mode_64bit
       || !(ins->rex & REX_W))
     return OP_I (ins, bytemode, sizeflag);
 
   USED_REX (REX_W);
 
-  oappend_immediate (ins, get64 (ins));
+  if (!get64 (ins, &op))
+    return false;
+
+  oappend_immediate (ins, op);
   return true;
 }
 
@@ -12677,10 +12630,16 @@ OP_sI (instr_info *ins, int bytemode, in
       break;
     case v_mode:
       /* The operand-size prefix is overridden by a REX prefix.  */
-      if ((sizeflag & DFLAG) || (ins->rex & REX_W))
-	op = get32s (ins);
-      else
-	op = get16 (ins);
+      if (!(sizeflag & DFLAG) && !(ins->rex & REX_W))
+	{
+	  int val;
+
+	  if (!get16 (ins, &val))
+	    return false;
+	  op = val;
+	}
+      else if (!get32s (ins, &op))
+	return false;
       break;
     default:
       oappend (ins, INTERNAL_DISASSEMBLER_ERROR);
@@ -12713,12 +12672,19 @@ OP_J (instr_info *ins, int bytemode, int
 	  || (ins->address_mode == mode_64bit
 	      && ((ins->isa64 == intel64 && bytemode != dqw_mode)
 		  || (ins->rex & REX_W))))
-	disp = get32s (ins);
+	{
+	  bfd_signed_vma val;
+
+	  if (!get32s (ins, &val))
+	    return false;
+	  disp = val;
+	}
       else
 	{
-	  disp = get16 (ins);
-	  if ((disp & 0x8000) != 0)
-	    disp -= 0x10000;
+	  int val;
+
+	  get16 (ins, &val);
+	  disp = val & 0x8000 ? val - 0x10000 : val;
 	  /* In 16bit mode, address is wrapped around at 64k within
 	     the same segment.  Otherwise, a data16 prefix on a jump
 	     instruction means that the pc is masked to 16 bits after
@@ -12762,14 +12728,16 @@ OP_DIR (instr_info *ins, int dummy ATTRI
 
   if (sizeflag & DFLAG)
     {
-      offset = get32 (ins);
-      seg = get16 (ins);
-    }
-  else
-    {
-      offset = get16 (ins);
-      seg = get16 (ins);
+      bfd_signed_vma val;
+
+      if (!get32 (ins, &val))
+	return false;;
+      offset = val;
     }
+  else if (!get16 (ins, &offset))
+    return false;
+  if (!get16 (ins, &seg))
+    return false;;
   ins->used_prefixes |= (ins->prefixes & PREFIX_DATA);
 
   res = snprintf (scratch, ARRAY_SIZE (scratch),
@@ -12791,9 +12759,21 @@ OP_OFF (instr_info *ins, int bytemode, i
   append_seg (ins);
 
   if ((sizeflag & AFLAG) || ins->address_mode == mode_64bit)
-    off = get32 (ins);
+    {
+      bfd_signed_vma val;
+
+      if (!get32 (ins, &val))
+	return false;
+      off = val;
+    }
   else
-    off = get16 (ins);
+    {
+      int val;
+
+      if (!get16 (ins, &val))
+	return false;
+      off = val;
+    }
 
   if (ins->intel_syntax)
     {
@@ -12820,7 +12800,8 @@ OP_OFF64 (instr_info *ins, int bytemode,
     intel_operand_size (ins, bytemode, sizeflag);
   append_seg (ins);
 
-  off = get64 (ins);
+  if (!get64 (ins, &off))
+    return false;
 
   if (ins->intel_syntax)
     {


  parent reply	other threads:[~2023-04-04  7:00 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-04  6:56 [PATCH 0/8] x86: do away with (ab)using setjmp/longjmp for error handling Jan Beulich
2023-04-04  6:58 ` [PATCH 1/8] x86: move fetch error handling into a helper function Jan Beulich
2023-04-04  6:58 ` [PATCH 2/8] x86: change fetch error handling in top-level function Jan Beulich
2023-04-04  6:59 ` [PATCH 3/8] x86: change fetch error handling in ckprefix() Jan Beulich
2023-04-04  6:59 ` [PATCH 4/8] x86: change fetch error handling in get_valid_dis386() Jan Beulich
2023-04-04  7:00 ` [PATCH 5/8] x86: change fetch error handling when processing operands Jan Beulich
2023-04-04  7:00 ` Jan Beulich [this message]
2023-04-04  7:01 ` [PATCH 7/8] x86: drop use of setjmp() from disassembler Jan Beulich
2023-04-04  7:01 ` [PATCH 8/8] x86: drop (explicit) BFD64 dependency " Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a9e13e17-ecf2-8e96-5b48-870801faa750@suse.com \
    --to=jbeulich@suse.com \
    --cc=amodra@gmail.com \
    --cc=binutils@sourceware.org \
    --cc=hjl.tools@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).