public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] avoid strange arithmetic with strings in i386-dis.c
@ 2013-10-10 23:15 Roland McGrath
  2013-10-11  0:12 ` H.J. Lu
  2013-10-11  0:47 ` Alan Modra
  0 siblings, 2 replies; 6+ messages in thread
From: Roland McGrath @ 2013-10-10 23:15 UTC (permalink / raw)
  To: binutils

This makes the code a bit less baroque, and also avoids warnings from some
compilers.  After the various changes I've committed in the last couple of
days, this is enough to get binutils (excluding gold) to build warning-free
with a recent Clang.

OK for trunk and 2.24?

Thanks,
Roland


opcodes/
	* i386-dis.c (oappend_maybe_intel): New function.
	(OP_ST, OP_STi, append_seg, OP_I, OP_I64, OP_sI, OP_ESreg): Use it.
	(OP_C, OP_T, CMP_Fixup, OP_EX_VexImmW): Likewise.
	(VCMP_Fixup, VPCMP_Fixup, PCLMUL_Fixup): Likewise.

--- a/opcodes/i386-dis.c
+++ b/opcodes/i386-dis.c
@@ -1,6 +1,6 @@
 /* Print i386 instructions for GDB, the GNU debugger.
    Copyright 1988, 1989, 1991, 1993, 1994, 1995, 1996, 1997, 1998, 1999,
-   2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012
+   2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013
    Free Software Foundation, Inc.

    This file is part of the GNU opcodes library.
@@ -13095,17 +13095,27 @@ dofloat (int sizeflag)
     }
 }

+/* Like oappend (below), but S is a string starting with '%'.
+   In Intel syntax, the '%' is elided.  */
+static void
+oappend_maybe_intel (const char *s)
+{
+  if (intel_syntax)
+    ++s;
+  oappend (s);
+}
+
 static void
 OP_ST (int bytemode ATTRIBUTE_UNUSED, int sizeflag ATTRIBUTE_UNUSED)
 {
-  oappend ("%st" + intel_syntax);
+  oappend_maybe_intel ("%st");
 }

 static void
 OP_STi (int bytemode ATTRIBUTE_UNUSED, int sizeflag ATTRIBUTE_UNUSED)
 {
   sprintf (scratchbuf, "%%st(%d)", modrm.rm);
-  oappend (scratchbuf + intel_syntax);
+  oappend_maybe_intel (scratchbuf);
 }

 /* Capital letters in template are macros.  */
@@ -13629,32 +13639,32 @@ append_seg (void)
   if (prefixes & PREFIX_CS)
     {
       used_prefixes |= PREFIX_CS;
-      oappend ("%cs:" + intel_syntax);
+      oappend_maybe_intel ("%cs:");
     }
   if (prefixes & PREFIX_DS)
     {
       used_prefixes |= PREFIX_DS;
-      oappend ("%ds:" + intel_syntax);
+      oappend_maybe_intel ("%ds:");
     }
   if (prefixes & PREFIX_SS)
     {
       used_prefixes |= PREFIX_SS;
-      oappend ("%ss:" + intel_syntax);
+      oappend_maybe_intel ("%ss:");
     }
   if (prefixes & PREFIX_ES)
     {
       used_prefixes |= PREFIX_ES;
-      oappend ("%es:" + intel_syntax);
+      oappend_maybe_intel ("%es:");
     }
   if (prefixes & PREFIX_FS)
     {
       used_prefixes |= PREFIX_FS;
-      oappend ("%fs:" + intel_syntax);
+      oappend_maybe_intel ("%fs:");
     }
   if (prefixes & PREFIX_GS)
     {
       used_prefixes |= PREFIX_GS;
-      oappend ("%gs:" + intel_syntax);
+      oappend_maybe_intel ("%gs:");
     }
 }

@@ -14904,7 +14914,7 @@ OP_I (int bytemode, int sizeflag)
   op &= mask;
   scratchbuf[0] = '$';
   print_operand_value (scratchbuf + 1, 1, op);
-  oappend (scratchbuf + intel_syntax);
+  oappend_maybe_intel (scratchbuf);
   scratchbuf[0] = '\0';
 }

@@ -14958,7 +14968,7 @@ OP_I64 (int bytemode, int sizeflag)
   op &= mask;
   scratchbuf[0] = '$';
   print_operand_value (scratchbuf + 1, 1, op);
-  oappend (scratchbuf + intel_syntax);
+  oappend_maybe_intel (scratchbuf);
   scratchbuf[0] = '\0';
 }

@@ -15012,7 +15022,7 @@ OP_sI (int bytemode, int sizeflag)

   scratchbuf[0] = '$';
   print_operand_value (scratchbuf + 1, 1, op);
-  oappend (scratchbuf + intel_syntax);
+  oappend_maybe_intel (scratchbuf);
 }

 static void
@@ -15194,7 +15204,7 @@ OP_ESreg (int code, int sizeflag)
 	  intel_operand_size (b_mode, sizeflag);
 	}
     }
-  oappend ("%es:" + intel_syntax);
+  oappend_maybe_intel ("%es:");
   ptr_reg (code, sizeflag);
 }

@@ -15247,7 +15257,7 @@ OP_C (int dummy ATTRIBUTE_UNUSED, int sizeflag
ATTRIBUTE_UNUSED)
   else
     add = 0;
   sprintf (scratchbuf, "%%cr%d", modrm.reg + add);
-  oappend (scratchbuf + intel_syntax);
+  oappend_maybe_intel (scratchbuf);
 }

 static void
@@ -15270,7 +15280,7 @@ static void
 OP_T (int dummy ATTRIBUTE_UNUSED, int sizeflag ATTRIBUTE_UNUSED)
 {
   sprintf (scratchbuf, "%%tr%d", modrm.reg);
-  oappend (scratchbuf + intel_syntax);
+  oappend_maybe_intel (scratchbuf);
 }

 static void
@@ -15715,7 +15725,7 @@ CMP_Fixup (int bytemode ATTRIBUTE_UNUSED, int
sizeflag ATTRIBUTE_UNUSED)
       /* We have a reserved extension byte.  Output it directly.  */
       scratchbuf[0] = '$';
       print_operand_value (scratchbuf + 1, 1, cmp_type);
-      oappend (scratchbuf + intel_syntax);
+      oappend_maybe_intel (scratchbuf);
       scratchbuf[0] = '\0';
     }
 }
@@ -16240,7 +16250,7 @@ OP_EX_VexImmW (int bytemode, int sizeflag)
       /* Output the imm8 directly.  */
       scratchbuf[0] = '$';
       print_operand_value (scratchbuf + 1, 1, vex_imm8 & 0xf);
-      oappend (scratchbuf + intel_syntax);
+      oappend_maybe_intel (scratchbuf);
       scratchbuf[0] = '\0';
       codep++;
     }
@@ -16469,7 +16479,7 @@ VCMP_Fixup (int bytemode ATTRIBUTE_UNUSED, int
sizeflag ATTRIBUTE_UNUSED)
       /* We have a reserved extension byte.  Output it directly.  */
       scratchbuf[0] = '$';
       print_operand_value (scratchbuf + 1, 1, cmp_type);
-      oappend (scratchbuf + intel_syntax);
+      oappend_maybe_intel (scratchbuf);
       scratchbuf[0] = '\0';
     }
 }
@@ -16516,7 +16526,7 @@ VPCMP_Fixup (int bytemode ATTRIBUTE_UNUSED,
       /* We have a reserved extension byte.  Output it directly.  */
       scratchbuf[0] = '$';
       print_operand_value (scratchbuf + 1, 1, cmp_type);
-      oappend (scratchbuf + intel_syntax);
+      oappend_maybe_intel (scratchbuf);
       scratchbuf[0] = '\0';
     }
 }
@@ -16564,7 +16574,7 @@ PCLMUL_Fixup (int bytemode ATTRIBUTE_UNUSED,
       /* We have a reserved extension byte.  Output it directly.  */
       scratchbuf[0] = '$';
       print_operand_value (scratchbuf + 1, 1, pclmul_type);
-      oappend (scratchbuf + intel_syntax);
+      oappend_maybe_intel (scratchbuf);
       scratchbuf[0] = '\0';
     }
 }

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] avoid strange arithmetic with strings in i386-dis.c
  2013-10-10 23:15 [PATCH] avoid strange arithmetic with strings in i386-dis.c Roland McGrath
@ 2013-10-11  0:12 ` H.J. Lu
  2013-10-11  1:01   ` Roland McGrath
  2013-10-11  0:47 ` Alan Modra
  1 sibling, 1 reply; 6+ messages in thread
From: H.J. Lu @ 2013-10-11  0:12 UTC (permalink / raw)
  To: Roland McGrath; +Cc: binutils

On Thu, Oct 10, 2013 at 4:15 PM, Roland McGrath <mcgrathr@google.com> wrote:
> This makes the code a bit less baroque, and also avoids warnings from some
> compilers.  After the various changes I've committed in the last couple of
> days, this is enough to get binutils (excluding gold) to build warning-free
> with a recent Clang.
>
> OK for trunk and 2.24?
>
> Thanks,
> Roland
>
>
> opcodes/
>         * i386-dis.c (oappend_maybe_intel): New function.
>         (OP_ST, OP_STi, append_seg, OP_I, OP_I64, OP_sI, OP_ESreg): Use it.
>         (OP_C, OP_T, CMP_Fixup, OP_EX_VexImmW): Likewise.
>         (VCMP_Fixup, VPCMP_Fixup, PCLMUL_Fixup): Likewise.
>
> --- a/opcodes/i386-dis.c
> +++ b/opcodes/i386-dis.c
> @@ -1,6 +1,6 @@
>  /* Print i386 instructions for GDB, the GNU debugger.
>     Copyright 1988, 1989, 1991, 1993, 1994, 1995, 1996, 1997, 1998, 1999,
> -   2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012
> +   2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013
>     Free Software Foundation, Inc.
>
>     This file is part of the GNU opcodes library.
> @@ -13095,17 +13095,27 @@ dofloat (int sizeflag)
>      }
>  }
>
> +/* Like oappend (below), but S is a string starting with '%'.
> +   In Intel syntax, the '%' is elided.  */
> +static void
> +oappend_maybe_intel (const char *s)
> +{
> +  if (intel_syntax)
> +    ++s;
> +  oappend (s);
> +}
> +

Please use

oappend (s + intel_syntax);

instead.  OK for trunk with this change.

Thanks.

-- 
H.J.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] avoid strange arithmetic with strings in i386-dis.c
  2013-10-10 23:15 [PATCH] avoid strange arithmetic with strings in i386-dis.c Roland McGrath
  2013-10-11  0:12 ` H.J. Lu
@ 2013-10-11  0:47 ` Alan Modra
  2013-10-11  6:30   ` Jan Beulich
  1 sibling, 1 reply; 6+ messages in thread
From: Alan Modra @ 2013-10-11  0:47 UTC (permalink / raw)
  To: Roland McGrath; +Cc: binutils

On Thu, Oct 10, 2013 at 04:15:00PM -0700, Roland McGrath wrote:
> This makes the code a bit less baroque, and also avoids warnings from some
> compilers.  After the various changes I've committed in the last couple of
> days, this is enough to get binutils (excluding gold) to build warning-free
> with a recent Clang.

Baroque?  I think you meant to say "Inexperienced programmers, minds
corrupted by the bizarre and perverse nature of C++, ignorant of the
fact that a C string is an array, find a time-honoured C trick from
days when life was simple and compilers non-optimising, confusing.
Clang, catering to these users and written by mad C++ programmers,
warns about perfectly good code." 

> 	* i386-dis.c (oappend_maybe_intel): New function.
> 	(OP_ST, OP_STi, append_seg, OP_I, OP_I64, OP_sI, OP_ESreg): Use it.
> 	(OP_C, OP_T, CMP_Fixup, OP_EX_VexImmW): Likewise.
> 	(VCMP_Fixup, VPCMP_Fixup, PCLMUL_Fixup): Likewise.

OK.

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] avoid strange arithmetic with strings in i386-dis.c
  2013-10-11  0:12 ` H.J. Lu
@ 2013-10-11  1:01   ` Roland McGrath
  0 siblings, 0 replies; 6+ messages in thread
From: Roland McGrath @ 2013-10-11  1:01 UTC (permalink / raw)
  To: H.J. Lu; +Cc: binutils

Committed.

Thanks,
Roland

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] avoid strange arithmetic with strings in i386-dis.c
  2013-10-11  0:47 ` Alan Modra
@ 2013-10-11  6:30   ` Jan Beulich
  2013-10-11 22:49     ` Alan Modra
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2013-10-11  6:30 UTC (permalink / raw)
  To: Alan Modra, Roland McGrath; +Cc: binutils

>>> On 11.10.13 at 02:46, Alan Modra <amodra@gmail.com> wrote:
> On Thu, Oct 10, 2013 at 04:15:00PM -0700, Roland McGrath wrote:
>> This makes the code a bit less baroque, and also avoids warnings from some
>> compilers.  After the various changes I've committed in the last couple of
>> days, this is enough to get binutils (excluding gold) to build warning-free
>> with a recent Clang.
> 
> Baroque?  I think you meant to say "Inexperienced programmers, minds
> corrupted by the bizarre and perverse nature of C++, ignorant of the
> fact that a C string is an array, find a time-honoured C trick from
> days when life was simple and compilers non-optimising, confusing.
> Clang, catering to these users and written by mad C++ programmers,
> warns about perfectly good code." 

+1

>> 	* i386-dis.c (oappend_maybe_intel): New function.
>> 	(OP_ST, OP_STi, append_seg, OP_I, OP_I64, OP_sI, OP_ESreg): Use it.
>> 	(OP_C, OP_T, CMP_Fixup, OP_EX_VexImmW): Likewise.
>> 	(VCMP_Fixup, VPCMP_Fixup, PCLMUL_Fixup): Likewise.
> 
> OK.

I'm surprised you even approved of the change considering the
above...

Jan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] avoid strange arithmetic with strings in i386-dis.c
  2013-10-11  6:30   ` Jan Beulich
@ 2013-10-11 22:49     ` Alan Modra
  0 siblings, 0 replies; 6+ messages in thread
From: Alan Modra @ 2013-10-11 22:49 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Roland McGrath, binutils

On Fri, Oct 11, 2013 at 07:30:33AM +0100, Jan Beulich wrote:
> I'm surprised you even approved of the change considering the
> above...

There wasn't anything wrong with the patch, especially HJ's suggested
change.  Really, I was just amusing myself giving the grumpy old C
programmer's perspective..

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-10-11 22:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-10 23:15 [PATCH] avoid strange arithmetic with strings in i386-dis.c Roland McGrath
2013-10-11  0:12 ` H.J. Lu
2013-10-11  1:01   ` Roland McGrath
2013-10-11  0:47 ` Alan Modra
2013-10-11  6:30   ` Jan Beulich
2013-10-11 22:49     ` Alan Modra

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).