570214 - MIPS variable shift instructions corrupts registers

Status: RESOLVED FIXED

(Core Graveyard :: Nanojit, defect)

Description

[Chris Dearman]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.9) Gecko/20100402 Ubuntu/9.10 (karmic) Firefox/3.5.9
Build Identifier: 

The MIPS backend masks the shift amount before using it for a shift.
This is both unnecessary and can corrupt a register that may still be in use. 

Reproducible: Always

Steps to Reproduce:
Shows up in the random testcase as a segmentation fault:
-sh-4.0# bin/lirasm --execute --random 100000 --optimize
Segmentation fault

Comment 1

[Chris Dearman]

Attachment: Removes unnecessary mask in variable shift opcodes

Tested in nanojit-central and tamarin-redux

Comment 2

[Matthias Versen [:Matti]]

patch -> new

Comment 3

[Edwin Smith]

Comment on attachment 449339 [details] [diff] [review]
Removes unnecessary mask in variable shift opcodes

I do see how the code could corrupt a register, and how the patch won't do that anymore.  I have to take it on faith that the MIPS shift instructions only consider the low order 5 bits of the shift amount.

Comment 4

[Chris Dearman]

(In reply to comment #3)
> I have to take it on faith that the MIPS shift instructions only
> consider the low order 5 bits of the shift amount.

That's correct. The variable shift instructions only use the bottom 5 bits of the shift amount register.

Comment 5

[Steven Johnson]

(In reply to comment #4)
> That's correct. The variable shift instructions only use the bottom 5 bits of
> the shift amount register.

Perhaps we should capture that in comments in the source, to prevent someone trying to helpfully re-add masking in the future.

Comment 6

[Chris Dearman]

Attachment: Removes unnecessary mask in variable shift opcodes

Added comments to make it clear that no additional
masking of the shift amount is required as it is handled by the
individual instructions.

Comment 7

[William Maddox]

Pushed to nanojit-central:

http://hg.mozilla.org/projects/nanojit-central/rev/8986dba933c6

Comment 8

[Nicholas Nethercote [inactive]]

http://hg.mozilla.org/tracemonkey/rev/c26a5619e1b0

Comment 9

[Steven Johnson]

http://hg.mozilla.org/tamarin-redux/rev/31b1d6767e65