Closed
Bug 355834
Opened 18 years ago
Closed 18 years ago
(new Function('yield'))(1) causes "yield from closing generator" or crash [@ js_LookupPropertyWithFlags]
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
VERIFIED
FIXED
People
(Reporter: jruderman, Assigned: igor)
Details
(Keywords: crash, testcase, verified1.8.1, Whiteboard: [sg:critical?] js1.7)
Crash Data
Attachments
(2 files, 3 obsolete files)
3.56 KB,
patch
|
igor
:
review+
beltzner
:
approval1.8.1+
|
Details | Diff | Splinter Review |
2.33 KB,
text/plain
|
Details |
js> (new Function('yield'))(1) Opt: gives an error message that doesn't make sense to me in this context, "TypeError: yield from closing generator (function anonymous() {yield;})" Debug: js_Interpret calls js_DecompileValueGenerator in an attempt to create the error message above. A few frames deeper, js_LookupPropertyWithFlags crashes dereferencing 0xdadadada. The number passed as an argument matters. Passing 1 makes it output the strange error or crash, but passing 0 or 2 does not. Security-sensitive because of the 0xdadadada.
Reporter | ||
Updated•18 years ago
|
Severity: normal → critical
Whiteboard: [sg:critical?]
Reporter | ||
Comment 1•18 years ago
|
||
Thread 0 Crashed: 0 js 0x0004903c js_LookupPropertyWithFlags + 684 (jsobj.c:3159) 1 js 0x00048d74 js_LookupProperty + 68 (jsobj.c:3086) 2 js 0x00049d4c js_GetProperty + 256 (jsobj.c:3371) 3 js 0x0004e2f0 js_TryMethod + 324 (jsobj.c:4529) 4 js 0x0007d8b0 js_ValueToSource + 404 (jsstr.c:2701) 5 js 0x0003f194 js_DecompileValueGenerator + 3868 (jsopcode.c:4764) 6 js 0x000b2cd8 js_Interpret + 119864 (jsinterp.c:5932) 7 js 0x00094368 js_Execute + 936 (jsinterp.c:1622) 8 js 0x000210d8 JS_ExecuteScript + 64 (jsapi.c:4256) 9 js 0x00002be4 Process + 904 (js.c:265) 10 js 0x000037ac ProcessArgs + 2304 (js.c:487) 11 js 0x00009b9c main + 640 (js.c:3088) 12 js 0x00001f48 _start + 340 (crt.c:272) 13 js 0x00001df0 start + 60
Reporter | ||
Comment 2•18 years ago
|
||
INT_TO_JSVAL(1) is represented in memory as 0x3. JSGEN_CLOSING is the fourth element in an enum, so it is also represented in memory as 0x3. Maybe that has something to do with why the value 1 is special in triggering the error/crash? http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/js/src/jsinterp.c&rev=3.297#5931
Reporter | ||
Comment 3•18 years ago
|
||
"new Function" omits the JSOP_GENERATOR that's supposed to be at the top of each generator function. js> dis(new Function("yield 3;")) flags: LAMBDA main: 00000: uint16 3 00003: yield 00004: pop 00005: stop The JSGenerator object that the JSOP_YIELD case looks for is created in the JSOP_GENERATOR case. I guess since the JSGenerator isn't where the JSOP_YIELD case expects it to be, it ends up looking at the first parameter instead.
Assignee | ||
Comment 4•18 years ago
|
||
Function('yield') is not marked as generator: js> dis(function() { yield; }) dis(function() { yield; }) flags: LAMBDA main: 00000: generator 00001: push 00002: yield 00003: pop 00004: stop Source notes: js> dis(Function('yield')) dis(Function('yield')) flags: LAMBDA main: 00000: push 00001: yield 00002: pop 00003: stop Source notes:
Assignee | ||
Comment 5•18 years ago
|
||
Using common code for tree byte code generation. AFAICS js_EmitFunctionBytecode can also take care about js_AllocTryNotes, but I am not 100% sure.
Comment 6•18 years ago
|
||
Comment on attachment 241543 [details] [diff] [review] Fix v1 Yeah, js_AllocTryNotes can be commoned too. /be
Comment 7•18 years ago
|
||
Putting on radar, simple fix to fuzzer-found bug in sight. /be
Flags: blocking1.8.1?
Assignee | ||
Comment 8•18 years ago
|
||
Attachment #241543 -
Attachment is obsolete: true
Attachment #241571 -
Flags: review?(brendan)
Attachment #241543 -
Flags: review?(brendan)
Comment 9•18 years ago
|
||
Comment on attachment 241571 [details] [diff] [review] Fix v2 > /* >+ * Emit code into cg for the tree rooted at body. >+ */ >+extern JSBool >+js_EmitFunctionBytecode(JSContext *cx, JSCodeGenerator *cg, JSParseNode *body); Comment could say "Emit function code ...". r+ me with that. /be
Attachment #241571 -
Flags: review?(brendan) → review+
Assignee | ||
Comment 10•18 years ago
|
||
Patch to commit with updated comments.
Attachment #241571 -
Attachment is obsolete: true
Attachment #241576 -
Flags: review+
Assignee | ||
Comment 11•18 years ago
|
||
I committed the patch from comment 10 to the trunk: Checking in jsemit.c; /cvsroot/mozilla/js/src/jsemit.c,v <-- jsemit.c new revision: 3.224; previous revision: 3.223 done Checking in jsemit.h; /cvsroot/mozilla/js/src/jsemit.h,v <-- jsemit.h new revision: 3.69; previous revision: 3.68 done Checking in jsparse.c; /cvsroot/mozilla/js/src/jsparse.c,v <-- jsparse.c new revision: 3.256; previous revision: 3.255 done
Assignee | ||
Updated•18 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Updated•18 years ago
|
Attachment #241576 -
Flags: approval1.8.1.1?
Updated•18 years ago
|
Attachment #241576 -
Flags: approval1.8.1?
Updated•18 years ago
|
Attachment #241576 -
Flags: approval1.8.1.1?
Updated•18 years ago
|
Flags: blocking1.8.1? → blocking1.8.1+
Comment 12•18 years ago
|
||
Be I'm assuming we want for 181?
Comment 13•18 years ago
|
||
I can't reproduce the problems with this test. Jesse can you verify?
Updated•18 years ago
|
Flags: in-testsuite+
Comment 14•18 years ago
|
||
This is a very safe patch to refactor slightly so Function produces well-formed bytecode. Jesse's fuzzer produces Function invocations and injects yield into generated source, so it will likely produce travesties that this bug bites. We should fix it now. /be
Comment 15•18 years ago
|
||
Is this needed for 1.8.0.8 as well? "yield" is js1.7, but the emit patch looks pretty generic and might apply.
Flags: blocking1.8.0.8?
Comment 16•18 years ago
|
||
Nevermind, no TCF_FUN_IS_GENERATOR on the 1.8.0 branch
Flags: blocking1.8.0.8? → blocking1.8.0.8-
Whiteboard: [sg:critical?] → [sg:critical?] js1.7
Reporter | ||
Comment 17•18 years ago
|
||
Verified fixed on Mac, using opt and debug jsshell: js> (new Function('yield'))(1) [object Generator] bc, if you can't reproduce the crash in older builds in the regression test framework, can you at least reproduce the lack of a returned generator?
Status: RESOLVED → VERIFIED
Comment 18•18 years ago
|
||
In reply to comment #17) > bc, if you can't reproduce the crash in older builds in the regression test > framework, can you at least reproduce the lack of a returned generator? yes. Modified test attached. Thanks.
Attachment #241709 -
Attachment is obsolete: true
Comment 19•18 years ago
|
||
Comment on attachment 241576 [details] [diff] [review] Fix v2b a=beltzner on behalf of drivers for 1.8.1 RC 3
Attachment #241576 -
Flags: approval1.8.1? → approval1.8.1+
Assignee | ||
Comment 20•18 years ago
|
||
I committed the patch from comment 10 to MOZILLA_1_8_BRANCH: Checking in jsemit.c; /cvsroot/mozilla/js/src/jsemit.c,v <-- jsemit.c new revision: 3.128.2.55; previous revision: 3.128.2.54 done Checking in jsemit.h; /cvsroot/mozilla/js/src/jsemit.h,v <-- jsemit.h new revision: 3.38.20.18; previous revision: 3.38.20.17 done Checking in jsparse.c; /cvsroot/mozilla/js/src/jsparse.c,v <-- jsparse.c new revision: 3.142.2.66; previous revision: 3.142.2.65 done
Keywords: fixed1.8.1
Comment 21•18 years ago
|
||
verified fixed 1.8 20061010 windows/mac*/linux
Keywords: fixed1.8.1 → verified1.8.1
Updated•18 years ago
|
Group: security
Comment 22•17 years ago
|
||
/cvsroot/mozilla/js/tests/js1_7/geniter/regress-355834.js,v <-- regress-355834.js
Updated•13 years ago
|
Crash Signature: [@ js_LookupPropertyWithFlags]
You need to log in
before you can comment on or make changes to this bug.
Description
•