Closed Bug 594205 Opened 9 years ago Closed 9 years ago

TM: safeguard against RegExp.prototype.test changes in exec-to-test conversions

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: njn, Assigned: njn)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

From bug 594108 comment 12, w.r.t. the replacement of calls to
RegExp.prototype.exec() with calls to RegExp.prototype.test() in some circumstances:

> Argh, what if someone messes with RegExp.prototype.test behind your back
> (overwrites it with some other function)?
>
> Instead we could save the original value of RegExp.prototype.test in a
> global-reserved slot. See JSCLASS_GLOBAL_FLAGS. Or we could link from
> RegExp.prototype.exec to its peer RegExp.prototype.test via a slot
> reserved by the Function class.
Would this alternative work:  before making the change, check if RegExp.prototype.test() is still equal to js_regexp_test()?  That way you'd lose the optimization if the user overwrites RegExp.prototype.test() but that's gonna be very rare.  I like the idea because we're already looking up 'test' in RegExp.prototype, so why not look up 'exec' at the same time?
That would work -- you'd have to check that the value is a function object, then that the function is native, before you could safely test u.n.native.

I was thinking of a scheme that avoids all property accesses, instead linking exec to test via one of the two native function object reserved slots (these are up for grabs to the code that creates the native function object). But you would still need to verify that exec was exec to pull this off safely. On the upside it would get rid of testAtom as well as the getProperty call, and have the smallest recording time overhead.

/be
(In reply to comment #2)
> That would work -- you'd have to check that the value is a function object,
> then that the function is native, before you could safely test u.n.native.

I went with this approach.  I particularly liked that it kept all the code relating to this exec-to-test conversion in a single place.  I measured v8-regexp, which contains 466 static calls to exec() that are convertible;  the cost of the property accesses is negligible.
Attachment #473296 - Flags: review?(brendan)
Summary: TM: stash a copy of RegExp.prototype.test to safeguard exec-to-test conversions → TM: safeguard against RegExp.prototype.test changes in exec-to-test conversions
Comment on attachment 473296 [details] [diff] [review]
patch (against TM 53102:82e35dc17b71)

Great!

/be
Attachment #473296 - Flags: review?(brendan) → review+
http://hg.mozilla.org/tracemonkey/rev/858168a1a3a7
Whiteboard: fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/858168a1a3a7
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.