Closed
Bug 607659
Opened 15 years ago
Closed 15 years ago
js_GetProperty on RegExp.prototype from TraceRecorder can reenter VM, crash
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
People
(Reporter: luke, Assigned: luke)
Details
(Whiteboard: [sg:critical?] fixed-in-tracemonkey)
Attachments
(1 file)
6.13 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
The RegExp.exec -> test tjit optimization does a js_GetProperty for 'test' on RegExp.prototype. This is bad since it can cause visible side effects during compilation. It also kills the current trace recorder which crashes compilation.
var g = 0;
Object.defineProperty(RegExp.prototype, 'test', { get:function() { ++g } });
function f() {
for (var i = 0; i < 100; ++i)
/a/.exec('a');
}
f();
assertEq(g, 0);
I think the solution is to do something more like ClassMethodIsNative and use JSObject::nativeLookup.
![]() |
Assignee | |
Comment 1•15 years ago
|
||
Comment 2•15 years ago
|
||
Comment on attachment 486367 [details] [diff] [review]
fix
Looks good. Two nits:
>+JSObject *
>+HasNativeMethod(JSContext *cx, JSObject *obj, jsid methodid, Native native)
Looks like cx is unused in this. Delete it?
This function should assert that obj is native. Better, JSObject::nativeSearch should assert isNative().
r=me with that.
Attachment #486367 -
Flags: review?(jorendorff) → review+
Comment 3•15 years ago
|
||
(In reply to comment #2)
> Comment on attachment 486367 [details] [diff] [review]
> fix
>
> Looks good. Two nits:
>
> >+JSObject *
> >+HasNativeMethod(JSContext *cx, JSObject *obj, jsid methodid, Native native)
>
> Looks like cx is unused in this. Delete it?
+1
> This function should assert that obj is native. Better, JSObject::nativeSearch
> should assert isNative().
+2 ;-)
/be
Updated•15 years ago
|
Whiteboard: [sg:critical?]
![]() |
Assignee | |
Comment 4•15 years ago
|
||
Whiteboard: [sg:critical?] → [sg:critical?] fixed-in-tracemonkey
Comment 5•15 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Updated•10 years ago
|
Group: core-security
You need to log in
before you can comment on or make changes to this bug.
Description
•