[PATCH v3 3/6] MIPS: microMIPS: Fix decoding of addiusp instruction

2017-08-08 Thread Matt Redfearn
Commit 34c2f668d0f6 ("MIPS: microMIPS: Add unaligned access support.")
added handling of microMIPS instructions to manipulate the stack
pointer. Unfortunately the decoding of the addiusp instruction was
incorrect, and performed a left shift by 2 bits to the raw immediate,
rather than decoding the immediate and then performing the shift, as
documented in the ISA.

This led to incomplete stack traces, due to incorrect frame sizes being
calculated. For example the instruction:
801faee0 :
801faee0:   4e25addiu   sp,sp,-952

As decoded by objdump, would be interpreted by the existing code as
having manipulated the stack pointer by +1096.

Fix this by changing the order of decoding the immediate and applying
the left shift. Also change to accessing the instuction through the
union to avoid the endianness problem of accesing halfword[0], which
will fail on big endian systems.

Cope with the special behaviour of immediates 0x0, 0x1, 0x1fe and 0x1ff
by XORing with 0x100 again if mod(immediate) < 4. This logic was tested
with the following test code:

int main(int argc, char **argv)
{
unsigned int enc;
int imm;

for (enc = 0; enc < 512; ++enc) {
int tmp = enc << 2;
imm = -(signed short)(tmp | ((tmp & 0x100) ? 0xfe00 : 0));
unsigned short tmp = enc;
tmp = (tmp ^ 0x100) - 0x100;
if ((unsigned short)(tmp + 2) < 4)
tmp ^= 0x100;
imm = -(signed short)(tmp << 2);
printf("%#x\t%d\t->\t(%#x\t%d)\t%#x\t%d\n",
   enc, enc,
   (short)tmp, (short)tmp,
   imm, imm);
}
return EXIT_SUCCESS;
}

Which generates the table:

input encoding  ->  tmp (matching manual)   frame size
---
0   0   ->  (0x100  256)0xfc00  -1024
0x1 1   ->  (0x101  257)0xfbfc  -1028
0x2 2   ->  (0x22)  0xfff8  -8
0x3 3   ->  (0x33)  0xfff4  -12
...
0xfe254 ->  (0xfe   254)0xfc08  -1016
0xff255 ->  (0xff   255)0xfc04  -1020
0x100   256 ->  (0xff00 -256)   0x400   1024
0x101   257 ->  (0xff01 -255)   0x3fc   1020
...
0x1fc   508 ->  (0xfffc -4) 0x1016
0x1fd   509 ->  (0xfffd -3) 0xc 12
0x1fe   510 ->  (0xfefe -258)   0x408   1032
0x1ff   511 ->  (0xfeff -257)   0x404   1028

Thanks to James Hogan for the test code & verifying the logic.

Fixes: 34c2f668d0f6 ("MIPS: microMIPS: Add unaligned access support.")
Suggested-by: James Hogan 
Signed-off-by: Matt Redfearn 

---

Changes in v3:
- Deal with special behaviour of addiusp immediates 0x0,0x1,0x1fe & 0x1ff

Changes in v2:
- Replace conditional with xor and subtract

 arch/mips/kernel/process.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
index 9d38faf01055..288a79bd0e72 100644
--- a/arch/mips/kernel/process.c
+++ b/arch/mips/kernel/process.c
@@ -386,10 +386,14 @@ static int get_frame_info(struct mips_frame_info *info)
{
unsigned short tmp;
 
-   if (ip->halfword[0] & mm_addiusp_func)
+   if (ip->mm16_r3_format.simmediate & 
mm_addiusp_func)
{
-   tmp = (((ip->halfword[0] >> 1) 
& 0x1ff) << 2);
-   info->frame_size = -(signed 
short)(tmp | ((tmp & 0x100) ? 0xfe00 : 0));
+   tmp = 
ip->mm_b0_format.simmediate >> 1;
+   tmp = ((tmp & 0x1ff) ^ 0x100) - 
0x100;
+   /* 0x0,0x1,0x1fe,0x1ff are 
special */
+   if ((tmp + 2) < 4)
+   tmp ^= 0x100;
+   info->frame_size = -(signed 
short)(tmp << 2);
} else {
tmp = (ip->halfword[0] >> 1);
info->frame_size = -(signed 
short)(tmp & 0xf);
-- 
2.7.4



[PATCH v3 3/6] MIPS: microMIPS: Fix decoding of addiusp instruction

2017-08-08 Thread Matt Redfearn
Commit 34c2f668d0f6 ("MIPS: microMIPS: Add unaligned access support.")
added handling of microMIPS instructions to manipulate the stack
pointer. Unfortunately the decoding of the addiusp instruction was
incorrect, and performed a left shift by 2 bits to the raw immediate,
rather than decoding the immediate and then performing the shift, as
documented in the ISA.

This led to incomplete stack traces, due to incorrect frame sizes being
calculated. For example the instruction:
801faee0 :
801faee0:   4e25addiu   sp,sp,-952

As decoded by objdump, would be interpreted by the existing code as
having manipulated the stack pointer by +1096.

Fix this by changing the order of decoding the immediate and applying
the left shift. Also change to accessing the instuction through the
union to avoid the endianness problem of accesing halfword[0], which
will fail on big endian systems.

Cope with the special behaviour of immediates 0x0, 0x1, 0x1fe and 0x1ff
by XORing with 0x100 again if mod(immediate) < 4. This logic was tested
with the following test code:

int main(int argc, char **argv)
{
unsigned int enc;
int imm;

for (enc = 0; enc < 512; ++enc) {
int tmp = enc << 2;
imm = -(signed short)(tmp | ((tmp & 0x100) ? 0xfe00 : 0));
unsigned short tmp = enc;
tmp = (tmp ^ 0x100) - 0x100;
if ((unsigned short)(tmp + 2) < 4)
tmp ^= 0x100;
imm = -(signed short)(tmp << 2);
printf("%#x\t%d\t->\t(%#x\t%d)\t%#x\t%d\n",
   enc, enc,
   (short)tmp, (short)tmp,
   imm, imm);
}
return EXIT_SUCCESS;
}

Which generates the table:

input encoding  ->  tmp (matching manual)   frame size
---
0   0   ->  (0x100  256)0xfc00  -1024
0x1 1   ->  (0x101  257)0xfbfc  -1028
0x2 2   ->  (0x22)  0xfff8  -8
0x3 3   ->  (0x33)  0xfff4  -12
...
0xfe254 ->  (0xfe   254)0xfc08  -1016
0xff255 ->  (0xff   255)0xfc04  -1020
0x100   256 ->  (0xff00 -256)   0x400   1024
0x101   257 ->  (0xff01 -255)   0x3fc   1020
...
0x1fc   508 ->  (0xfffc -4) 0x1016
0x1fd   509 ->  (0xfffd -3) 0xc 12
0x1fe   510 ->  (0xfefe -258)   0x408   1032
0x1ff   511 ->  (0xfeff -257)   0x404   1028

Thanks to James Hogan for the test code & verifying the logic.

Fixes: 34c2f668d0f6 ("MIPS: microMIPS: Add unaligned access support.")
Suggested-by: James Hogan 
Signed-off-by: Matt Redfearn 

---

Changes in v3:
- Deal with special behaviour of addiusp immediates 0x0,0x1,0x1fe & 0x1ff

Changes in v2:
- Replace conditional with xor and subtract

 arch/mips/kernel/process.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
index 9d38faf01055..288a79bd0e72 100644
--- a/arch/mips/kernel/process.c
+++ b/arch/mips/kernel/process.c
@@ -386,10 +386,14 @@ static int get_frame_info(struct mips_frame_info *info)
{
unsigned short tmp;
 
-   if (ip->halfword[0] & mm_addiusp_func)
+   if (ip->mm16_r3_format.simmediate & 
mm_addiusp_func)
{
-   tmp = (((ip->halfword[0] >> 1) 
& 0x1ff) << 2);
-   info->frame_size = -(signed 
short)(tmp | ((tmp & 0x100) ? 0xfe00 : 0));
+   tmp = 
ip->mm_b0_format.simmediate >> 1;
+   tmp = ((tmp & 0x1ff) ^ 0x100) - 
0x100;
+   /* 0x0,0x1,0x1fe,0x1ff are 
special */
+   if ((tmp + 2) < 4)
+   tmp ^= 0x100;
+   info->frame_size = -(signed 
short)(tmp << 2);
} else {
tmp = (ip->halfword[0] >> 1);
info->frame_size = -(signed 
short)(tmp & 0xf);
-- 
2.7.4