This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
[PATCH][MIPS] Fix ext, dextm, dextu size checking
- From: Chao-Ying Fu <Chao-Ying dot Fu at imgtec dot com>
- To: "binutils at sourceware dot org" <binutils at sourceware dot org>
- Date: Tue, 23 Apr 2013 01:10:29 +0000
- Subject: [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