This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH][MIPS] Fix ext, dextm, dextu size checking


Hi All,

  We found out that GAS doesn't check "size" operands correctly for ext, dextm, dextu instructions.
Note that dext is ok, because it is handled by macro expansion at M_DEXT.
Ex:
linux:~/dev/test> cat ext.s
test:
        ext $2,$3,1,0
        dextm $2,$3,31,2
        dextm $2,$3,1,32
        dextu $2,$3,33,0

linux:~/dev/test> ~/dev/build-binutils/gas/as-new ext.s -o ext.o -mips64r2
linux:~/dev/test> ~/dev/build-binutils/binutils/objdump -d ext.o

ext.o:     file format elf32-tradbigmips


Disassembly of section .text:

00000000 <test>:
   0:   7c62f840        ext     v0,v1,0x1,0x20 <-- WRONG
   4:   7c620fc1        dextm   v0,v1,0x1f,0x22 <-- WRONG
   8:   7c62f841        dextm   v0,v1,0x1,0x40 <-- WRONG
   c:   7c62f842        dextu   v0,v1,0x21,0x20 <-- WRONG

Case 1: ext
GAS checks:
A. 0 <= pos <= 31
B. size >= 0
C. 1 <= pos + size <= 32

This allows 0 <= size <= 32.  However, 0 should not be allowed.

Case 2: dextm
GAS checks:
A. 0 <= pos <= 31
B. size >= 0
C. 33 <= pos + size <= 64

This allows 2 <= size <= 64.  However, 2 to 32 should not be allowed.

Case 3: dextu
GAS checks:
A. 32 <= pos <= 63
B. size >= 0
C. 33 <= pos + size <= 64

This allows 0 <= size <= 32.  However, 0 should not be allowed.

A possible fix is as follows.  We just need to check if the lower bound of "size" is correct, instead of checking it against 0.
(Note that there is no issue at the upper bound of "size".)
Ex:
2013-04-22  Chao-ying Fu  <Chao-ying.Fu@imgtec.com>

	* config/tc-mips.c (mips_ip): Add sizelo.
	For "+C", "+G", and "+H", set sizelo and compare against it.

Index: tc-mips.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-mips.c,v
retrieving revision 1.527
diff -u -p -r1.527 tc-mips.c
--- tc-mips.c	18 Feb 2013 23:27:23 -0000	1.527
+++ tc-mips.c	22 Apr 2013 23:59:33 -0000
@@ -10790,6 +10790,7 @@ mips_ip (char *str, struct mips_cl_insn 
   unsigned int destregno = 0;
   unsigned int lastpos = 0;
   unsigned int limlo, limhi;
+  int sizelo;
   char *s_reset;
   offsetT min_range, max_range;
   long opend;
@@ -11374,23 +11375,25 @@ mips_ip (char *str, struct mips_cl_insn 
 		case 'C':		/* ext size, becomes MSBD.  */
 		  limlo = 1;
 		  limhi = 32;
+		  sizelo = 1;
 		  goto do_msbd;
 		case 'G':
 		  limlo = 33;
 		  limhi = 64;
+		  sizelo = 33;
 		  goto do_msbd;
 		case 'H':
 		  limlo = 33;
 		  limhi = 64;
+		  sizelo = 1;
 		  goto do_msbd;
 		do_msbd:
 		  my_getExpression (&imm_expr, s);
 		  check_absolute_expr (ip, &imm_expr);
-		  /* Check for negative input so that small negative numbers
-		     will not succeed incorrectly.  The checks against
-		     (pos+size) transitively check "size" itself,
-		     assuming that "pos" is reasonable.  */
-		  if ((long) imm_expr.X_add_number < 0
+		  /* The checks against (pos+size) don't transitively check
+		     "size" itself, assuming that "pos" is reasonable.
+		     We also need to check the lower bound of size.  */
+		  if ((long) imm_expr.X_add_number < sizelo
 		      || ((unsigned long) imm_expr.X_add_number
 			  + lastpos) < limlo
 		      || ((unsigned long) imm_expr.X_add_number

The new GAS can detect wrong "size" as follows.
Ex:
linux:~/dev/test> ~/dev/build-binutils/gas/as-new ext.s -o ext.o -mips64r2
ext.s: Assembler messages:
ext.s:2: Error: Improper extract size (0, position 1)
ext.s:3: Error: Improper extract size (2, position 31)
ext.s:4: Error: Improper extract size (32, position 1)
ext.s:5: Error: Improper extract size (0, position 33)

  The GAS regression test for mips-sde-elf is ok (as before).
Ex:

                === gas Summary ===

# of expected passes            2619
# of expected failures          1
../as-new 2.23.52.20130422

  Any feedback?  Thanks!

Regards,
Chao-ying


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]