Add a warning for some problematic MIPS constants

Daniel Jacobowitz drow@false.org
Wed Sep 19 14:42:00 GMT 2007


MIPS maintainers, could I please get your opinion of this patch?

A customer of ours (Wind River) has a second MIPS assembler (Diab).
Their MIPS assembler makes some different choices than gas does about
which constants should be sign extended.  Richard explained the logic
of gas's choices to me, and I entirely agree with it.  However, it's a
bit subtle and I wouldn't be surprised if some gas users expected
the opposite behavior.

The cases I'm trying to warn about are instructions that do not have a
clear explicit width.  For instance, "li" and "dli" are obvious, but
bne and sltu are not.  If you write "bne $4,0x80000000,label" gas
zero extends the constant, and Diab sign extends it.

While writing the test I turned on NewABI tests for mips-linux; they
work fine nowadays.

-- 
Daniel Jacobowitz
CodeSourcery

2007-09-19  Daniel Jacobowitz  <dan@codesourcery.com>

	* config/tc-mips.c (mips_warn_unsigned): New.
	(unsigned_warning): New.
	(set_at, macro, macro2): Call it.
	(OPTION_WARN_UNSIGNED, OPTION_NO_WARN_UNSIGNED): Define.
	(md_longopts): Add --warn-unsigned and --no-warn-unsigned.
	(md_parse_option): Likewise.
	* doc/c-mips.texi (MIPS Opts): Document --warn-unsigned.

2007-09-19  Daniel Jacobowitz  <dan@codesourcery.com>

	* gas/mips/mips.exp: Run NewABI tests for mips*-*-linux*.  Add
	unsigned-warn-1.
	* unsigned-warn-1.d, unsigned-warn-1.l, unsigned-warn-1.s: New.

Index: config/tc-mips.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-mips.c,v
retrieving revision 1.373
diff -u -p -r1.373 tc-mips.c
--- config/tc-mips.c	11 Jul 2007 15:11:15 -0000	1.373
+++ config/tc-mips.c	19 Sep 2007 14:13:34 -0000
@@ -73,6 +73,8 @@ static int mips_output_flavor (void) { r
 
 int mips_flag_mdebug = -1;
 
+static int mips_warn_unsigned = 0;
+
 /* Control generation of .pdr sections.  Off by default on IRIX: the native
    linker doesn't know about and discards them, but relocations against them
    remain, leading to rld crashes.  */
@@ -994,6 +996,7 @@ static void append_insn
 static void mips_no_prev_insn (void);
 static void mips16_macro_build
   (expressionS *, const char *, const char *, va_list);
+static void unsigned_warning (expressionS *, int);
 static void load_register (int, expressionS *, int);
 static void macro_start (void);
 static void macro_end (void);
@@ -3820,6 +3823,24 @@ macro_build_ldst_constoffset (expression
     }
 }
 
+/* Issue a warning if this constant will be treated as unsigned,
+   but the user may have expected it to be signed.  */
+static void
+unsigned_warning (expressionS *ep, int dbl)
+{
+  if (!mips_warn_unsigned)
+    return;
+
+  if (!dbl)
+    return;
+
+  if (ep->X_op == O_constant
+      && IS_ZEXT_32BIT_NUM (ep->X_add_number)
+      && !IS_SEXT_32BIT_NUM (ep->X_add_number))
+    as_warn (_("constant (0x%lx) will be zero extended"),
+	     (unsigned long) ep->X_add_number);
+}
+
 /*			set_at()
  * Generates code to set the $at register to true (one)
  * if reg is less than the immediate expression.
@@ -3834,6 +3855,7 @@ set_at (int reg, int unsignedp)
 		 AT, reg, BFD_RELOC_LO16);
   else
     {
+      unsigned_warning (&imm_expr, HAVE_64BIT_GPRS);
       load_register (AT, &imm_expr, HAVE_64BIT_GPRS);
       macro_build (NULL, unsignedp ? "sltu" : "slt", "d,v,t", AT, reg, AT);
     }
@@ -4640,6 +4662,7 @@ macro (struct mips_cl_insn *ip)
 	}
 
       used_at = 1;
+      unsigned_warning (&imm_expr, HAVE_64BIT_GPRS);
       load_register (AT, &imm_expr, HAVE_64BIT_GPRS);
       macro_build (NULL, s2, "d,v,t", treg, sreg, AT);
       break;
@@ -4680,6 +4703,7 @@ macro (struct mips_cl_insn *ip)
 	  break;
 	}
       used_at = 1;
+      unsigned_warning (&imm_expr, HAVE_64BIT_GPRS);
       load_register (AT, &imm_expr, HAVE_64BIT_GPRS);
       macro_build (&offset_expr, s, "s,t,p", sreg, AT);
       break;
@@ -7495,6 +7519,7 @@ macro2 (struct mips_cl_insn *ip)
 	}
       else
 	{
+	  unsigned_warning (&imm_expr, HAVE_64BIT_GPRS);
 	  load_register (AT, &imm_expr, HAVE_64BIT_GPRS);
 	  macro_build (NULL, "xor", "d,v,t", dreg, sreg, AT);
 	  used_at = 1;
@@ -7523,6 +7548,7 @@ macro2 (struct mips_cl_insn *ip)
 	}
       else
 	{
+	  unsigned_warning (&imm_expr, HAVE_64BIT_GPRS);
 	  load_register (AT, &imm_expr, HAVE_64BIT_GPRS);
 	  macro_build (NULL, mask == M_SGE_I ? "slt" : "sltu", "d,v,t",
 		       dreg, sreg, AT);
@@ -7547,6 +7573,7 @@ macro2 (struct mips_cl_insn *ip)
       s = "sltu";
     sgti:
       used_at = 1;
+      unsigned_warning (&imm_expr, HAVE_64BIT_GPRS);
       load_register (AT, &imm_expr, HAVE_64BIT_GPRS);
       macro_build (NULL, s, "d,v,t", dreg, AT, sreg);
       break;
@@ -7568,6 +7595,7 @@ macro2 (struct mips_cl_insn *ip)
       s = "sltu";
     slei:
       used_at = 1;
+      unsigned_warning (&imm_expr, HAVE_64BIT_GPRS);
       load_register (AT, &imm_expr, HAVE_64BIT_GPRS);
       macro_build (NULL, s, "d,v,t", dreg, AT, sreg);
       macro_build (&expr1, "xori", "t,r,i", dreg, dreg, BFD_RELOC_LO16);
@@ -7582,6 +7610,7 @@ macro2 (struct mips_cl_insn *ip)
 	  break;
 	}
       used_at = 1;
+      unsigned_warning (&imm_expr, HAVE_64BIT_GPRS);
       load_register (AT, &imm_expr, HAVE_64BIT_GPRS);
       macro_build (NULL, "slt", "d,v,t", dreg, sreg, AT);
       break;
@@ -7596,6 +7625,7 @@ macro2 (struct mips_cl_insn *ip)
 	  break;
 	}
       used_at = 1;
+      unsigned_warning (&imm_expr, HAVE_64BIT_GPRS);
       load_register (AT, &imm_expr, HAVE_64BIT_GPRS);
       macro_build (NULL, "sltu", "d,v,t", dreg, sreg, AT);
       break;
@@ -7642,6 +7672,7 @@ macro2 (struct mips_cl_insn *ip)
 	}
       else
 	{
+	  unsigned_warning (&imm_expr, HAVE_64BIT_GPRS);
 	  load_register (AT, &imm_expr, HAVE_64BIT_GPRS);
 	  macro_build (NULL, "xor", "d,v,t", dreg, sreg, AT);
 	  used_at = 1;
@@ -7702,6 +7733,7 @@ macro2 (struct mips_cl_insn *ip)
       s = "tne";
     trap:
       used_at = 1;
+      unsigned_warning (&imm_expr, HAVE_64BIT_GPRS);
       load_register (AT, &imm_expr, HAVE_64BIT_GPRS);
       macro_build (NULL, s, "s,t", sreg, AT);
       break;
@@ -10963,10 +10995,14 @@ struct option md_longopts[] =
 #define OPTION_MNO_SYM32 (OPTION_MISC_BASE + 15)
   {"msym32", no_argument, NULL, OPTION_MSYM32},
   {"mno-sym32", no_argument, NULL, OPTION_MNO_SYM32},
+#define OPTION_WARN_UNSIGNED (OPTION_MISC_BASE + 16)
+#define OPTION_NO_WARN_UNSIGNED (OPTION_MISC_BASE + 17)
+  {"warn-unsigned", no_argument, NULL, OPTION_WARN_UNSIGNED},
+  {"no-warn-unsigned", no_argument, NULL, OPTION_NO_WARN_UNSIGNED},
 
   /* ELF-specific options.  */
 #ifdef OBJ_ELF
-#define OPTION_ELF_BASE    (OPTION_MISC_BASE + 16)
+#define OPTION_ELF_BASE    (OPTION_MISC_BASE + 18)
 #define OPTION_CALL_SHARED (OPTION_ELF_BASE + 0)
   {"KPIC",        no_argument, NULL, OPTION_CALL_SHARED},
   {"call_shared", no_argument, NULL, OPTION_CALL_SHARED},
@@ -11236,6 +11272,14 @@ md_parse_option (int c, char *arg)
       mips_opts.sym32 = FALSE;
       break;
 
+    case OPTION_WARN_UNSIGNED:
+      mips_warn_unsigned = 1;
+      break;
+
+    case OPTION_NO_WARN_UNSIGNED:
+      mips_warn_unsigned = 0;
+      break;
+
 #ifdef OBJ_ELF
       /* When generating ELF code, we permit -KPIC and -call_shared to
 	 select SVR4_PIC, and -non_shared to select no PIC.  This is
Index: doc/c-mips.texi
===================================================================
RCS file: /cvs/src/src/gas/doc/c-mips.texi,v
retrieving revision 1.43
diff -u -p -r1.43 c-mips.texi
--- doc/c-mips.texi	4 Jul 2007 19:55:18 -0000	1.43
+++ doc/c-mips.texi	19 Sep 2007 14:13:34 -0000
@@ -343,6 +343,13 @@ tells gas to generate code which uses th
 not go into a shared library.  The resulting code is slightly more
 efficient.  This option only affects the handling of the
 @samp{.cpload} and @samp{.cpsetup} pseudo-ops.
+
+@item --warn-unsigned
+@itemx --no-warn-unsigned
+Warn when an instruction takes 64-bit constant which the user may have
+expected to be 32-bit.  Warnings are generated only macro instructions
+which do not have an explicit size, e.g., @samp{slt} and and @samp{bne},
+and only for constants which differ when sign-extended versus zero-extended.
 @end table
 
 @node MIPS Object
Index: testsuite/gas/mips/mips.exp
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/mips/mips.exp,v
retrieving revision 1.128
diff -u -p -r1.128 mips.exp
--- testsuite/gas/mips/mips.exp	5 Jun 2007 17:00:32 -0000	1.128
+++ testsuite/gas/mips/mips.exp	19 Sep 2007 14:13:34 -0000
@@ -379,7 +379,7 @@ if { [istarget mips*-*-vxworks*] } {
     set ilocks [istarget mipstx39*-*-*]
     set gpr_ilocks [expr [istarget mipstx39*-*-*]]
     set addr32 [expr [istarget mipstx39*-*-*] || [istarget mips-*-linux*] || [istarget mipsel-*-linux*]]
-    set has_newabi [expr [istarget *-*-irix6*] || [istarget mips64*-*-linux*]]
+    set has_newabi [expr [istarget *-*-irix6*] || [istarget mips*-*-linux*]]
 
     if { [istarget "mips*-*-*linux*"] || [istarget "mips*-sde-elf*"] } then {
 	set tmips "t"
@@ -774,4 +774,8 @@ if { [istarget mips*-*-vxworks*] } {
     run_dump_test "vxworks1-xgot-el"
 
     run_dump_test "noreorder"
+
+    if { $has_newabi } {
+	run_dump_test "unsigned-warn-1"
+    }
 }
Index: testsuite/gas/mips/unsigned-warn-1.d
===================================================================
RCS file: testsuite/gas/mips/unsigned-warn-1.d
diff -N testsuite/gas/mips/unsigned-warn-1.d
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gas/mips/unsigned-warn-1.d	19 Sep 2007 14:13:34 -0000
@@ -0,0 +1,5 @@
+#as: -mgp64 --n32 --warn-unsigned
+#source: unsigned-warn-1.s
+#stderr: unsigned-warn-1.l
+#objdump: -p
+#pass
Index: testsuite/gas/mips/unsigned-warn-1.l
===================================================================
RCS file: testsuite/gas/mips/unsigned-warn-1.l
diff -N testsuite/gas/mips/unsigned-warn-1.l
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gas/mips/unsigned-warn-1.l	19 Sep 2007 14:13:34 -0000
@@ -0,0 +1,12 @@
+.*: Assembler messages:
+.*:5: Warning: constant \(0xa0000000\) will be zero extended
+.*:6: Warning: constant \(0xa0000000\) will be zero extended
+.*:7: Warning: constant \(0xa0000000\) will be zero extended
+.*:9: Warning: constant \(0xc0000000\) will be zero extended
+.*:10: Warning: constant \(0xc0000000\) will be zero extended
+.*:12: Warning: constant \(0xf0000000\) will be zero extended
+.*:13: Warning: constant \(0xf0000000\) will be zero extended
+.*:14: Warning: constant \(0xf0000000\) will be zero extended
+.*:15: Warning: constant \(0xf0000000\) will be zero extended
+.*:16: Warning: constant \(0xf0000000\) will be zero extended
+.*:18: Warning: constant \(0xb0000000\) will be zero extended
Index: testsuite/gas/mips/unsigned-warn-1.s
===================================================================
RCS file: testsuite/gas/mips/unsigned-warn-1.s
diff -N testsuite/gas/mips/unsigned-warn-1.s
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gas/mips/unsigned-warn-1.s	19 Sep 2007 14:13:34 -0000
@@ -0,0 +1,18 @@
+	li	$8, (~(7 << 29))
+	li	$4, 0xa0000000
+	subu	$4, 64
+
+1:	bne	$4, 0xa0000000, 1b
+1:	bge	$4, 0xa0000000, 1b
+1:	bgeu	$4, 0xa0000000, 1b
+
+	and	$8, 0xc0000000
+	or	$8, 0xc0000000
+
+	seq	$8, 0xf0000000
+	sltu	$8, 0xf0000000
+	slt	$8, 0xf0000000
+	sgt	$8, 0xf0000000
+	sle	$8, 0xf0000000
+
+	teq	$8, 0xb0000000



More information about the Binutils mailing list