495028 - nanojit: add opcode checks to all LIR get/set methods

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

Attachment: patch adding opcode checks to LIns get/set methods

Nanojit currently checks the opcode for many but not all of the LIns get/set.  This patch adds checks for those methods that lack one.  This makes things safer because calling an unchecked method on the wrong instruction kind will lead to total memory bogosity.

Comment 1

[Edwin Smith]

Comment on attachment 379838 [details] [diff] [review]
patch adding opcode checks to LIns get/set methods

looks nice.

minor suggestion:

#ifdef _DEBUG around isOp1() and isOp2(), if theyre only used by asserts.  or, something table driven, if they're really used in releasable code.

Comment 2

[Nicholas Nethercote [inactive]]

Attachment: updated patch

Updated patch which incorporates Ed's suggestion.

Comment 3

[Robert Sayre]

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

Comment 4

[Nicholas Nethercote [inactive]]

This was marked as RESOLVED FIXED and "checkin-needed" despite being checked into tracemonkey but not m-c.  Changing it to "REOPENED" and "fixed-in-tracemonkey" because I think that's what it's supposed to be.

Comment 5

[Graydon Hoare :graydon]

I'm curious what happened to LIR_ldc here.

Comment 6

[Graydon Hoare :graydon]

comment #5 was fixed by bug 497784. this bug's back to 'finished'.

Comment 7

[Nicholas Nethercote [inactive]]

Sayre, did this one ever make it into mozilla-central?

Comment 8

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/d11ca213ecdf

Comment 9

[Robert Sayre]

forgot to note it here.