Closed
Bug 618362
Opened 14 years ago
Closed 13 years ago
Assertion failure: JSID_BITS(id) == JSID_TYPE_VOID
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | beta9+ |
People
(Reporter: jandem, Assigned: billm)
Details
(Whiteboard: [sg:critical?] [fixed-in-tracemonkey])
Attachments
(4 files)
Attached test case triggers this with -j in debug mode: Assertion failure: JSID_BITS(id) == JSID_TYPE_VOID, at ../jsapi.h:452 In release mode this triggers some scary errors before it crashes: js(86322) malloc: *** error for object 0x50a090: pointer being freed was not allocated *** set a breakpoint in malloc_error_break to debug js(86322) malloc: *** error for object 0x50a0d8: pointer being freed was not allocated *** set a breakpoint in malloc_error_break to debug js(86322) malloc: *** error for object 0x50a120: pointer being freed was not allocated ...repeated many times... The weird part is that this seems to depend on the full path to the script. If I store the file as /tmp/test.js, I get the crash in release mode but not the messages and I don't get the assert in a debug build. If I store the file as /tmp/test1234567891234567890.js (length 31) I get the assert and error messages. I hope somebody will be able to reproduce.
Reporter | ||
Comment 1•14 years ago
|
||
Reporter | ||
Comment 2•14 years ago
|
||
Attached is the stack trace for the assert. Stack trace in release build: Program received signal EXC_BAD_ACCESS, Could not access memory. Reason: KERN_INVALID_ADDRESS at address: 0xb7e70010 0x001289f9 in js_script_filename_sweeper () (gdb) bt #0 0x001289f9 in js_script_filename_sweeper () #1 0x00080ec2 in JS_HashTableEnumerateEntries () This might explain why it depends on the file name.
Reporter | ||
Updated•14 years ago
|
Attachment #496854 -
Attachment mime type: application/octet-stream → text/plain
Comment 3•14 years ago
|
||
Do you suppose you could run this under valgrind (--tool=memcheck) and hopefully catch some backtraces earlier than the crash, when the initial memory corruption occurs?
Reporter | ||
Comment 4•14 years ago
|
||
I was able to reduce this to this using Valgrind (great idea!): --- function f(p) { "use strict"; return arguments; } for (var i = 0; i < 10; i++) { f({}); } --- This triggers 2 messages like this: ==8341== Invalid write of size 4 ==8341== at 0x685CFB8: ??? ==8341== by 0x1CB1D4: js::ExecuteTrace(JSContext*, nanojit::Fragment*, js::TracerState&) (in ./js) ==8341== by 0x1D8FA9: js::ExecuteTree(JSContext*, js::TreeFragment*, unsigned int&, js::VMSideExit**, js::VMSideExit**) (in ./js) ==8341== by 0x1FFD75: js::RecordLoopEdge(JSContext*, unsigned int&) (in ./js) ==8341== by 0x1FFF5B: js::MonitorLoopEdge(JSContext*, unsigned int&) (in ./js) ==8341== by 0xC5E1E: js::Interpret(JSContext*, JSStackFrame*, unsigned int, JSInterpMode) (in ./js) ==8341== by 0xE40D3: js::RunScript(JSContext*, JSScript*, JSStackFrame*) (in ./js) ==8341== by 0xE4675: js::Execute(JSContext*, JSObject*, JSScript*, JSStackFrame*, unsigned int, js::Value*) (in ./js) ==8341== by 0x22FD8: JS_ExecuteScript (in ./js) ==8341== by 0x1562B: Process(JSContext*, JSObject*, char*, int) (in ./js) ==8341== by 0x1651D: ProcessArgs(JSContext*, JSObject*, char**, int) (in ./js) ==8341== by 0x1665B: Shell(JSContext*, int, char**, char**) (in ./js) ==8341== Address 0x620b1c0 is not stack'd, malloc'd or (recently) free'd So there are two invalid writes in tracejit-generated code. Any way to find out where these happen?
Reporter | ||
Comment 5•14 years ago
|
||
I used TMFLAGS=full and annotated (with >>>) the lines where the invalid writes happen. Is there anything different about the arguments object of ES5 strict mode functions?
Reporter | ||
Updated•14 years ago
|
Attachment #496884 -
Attachment mime type: application/octet-stream → text/plain
Reporter | ||
Comment 6•14 years ago
|
||
From TraceRecoder::newArguments: --- if (strict) { LIns* argsData_ins = w.getObjPrivatizedSlot(argsobj_ins, JSObject::JSSLOT_ARGS_DATA); ptrdiff_t slotsOffset = offsetof(ArgumentsData, slots); cx->fp()->forEachCanonicalActualArg(BoxArg(this, ArgsSlotsAddress(argsData_ins, slotsOffset))); } --- The "invalid writes" disappear when I comment out the last line.
Comment 7•14 years ago
|
||
Waldo might know something here.
Updated•14 years ago
|
Assignee: general → jwalden+bmo
Reporter | ||
Comment 8•14 years ago
|
||
This script: --- function f() { "use strict"; eval(""); var a = 0; for (var i = 0; i < 13; i++) { a--; } } f(); --- Asserts with (-j): Assertion failure: !argsobj.getPrivate(), at ../jsfun.cpp:272 I think it's related, maybe this one is easier to debug.
Assignee | ||
Comment 9•14 years ago
|
||
I happened on this bug today. The problem is in TraceRecorder::newArguments. The code looks like this: if (strict) { LIns* argsData_ins = w.getObjPrivatizedSlot(...); ptrdiff_t slotsOffset = offsetof(ArgumentsData, slots); cx->fp()->forEachCanonicalActualArg(BoxArg(this, ArgsSlotsAddress(argsData_ins, slotsOffset))); } The problem is that slotsOffset is an offset, but the ArgsSlotsAddress constructor expects it to be a slot index, so it multiplies by sizeof(Value). I just removed the multiply here. I can imagine other ways of fixing it though. The problem ought to show up in the arguments/strict-args.js test. However, the loop wasn't going high enough to trigger the tracer. It looks like there are a lot of other tests that only go up to 5. We really need to fix these.
Updated•14 years ago
|
blocking2.0: --- → ?
OS: Mac OS X → Windows 7
Whiteboard: [sg:critical?]
Reporter | ||
Comment 10•14 years ago
|
||
With that patch, the script in comment 8 still asserts; filed bug 620335.
Assignee | ||
Updated•14 years ago
|
OS: Windows 7 → All
Assignee | ||
Updated•14 years ago
|
Attachment #498424 -
Flags: review? → review?(nnethercote)
Comment 11•14 years ago
|
||
Comment on attachment 498424 [details] [diff] [review] fix There are several XyzSlotsAddress structs, and the constructor for each one takes a slot number, so your patch introduces an inconsistency. Probably the easiest fix is to rename ArgsSlotsAddress as ArgsSlotOffsetAddress. r=me with that.
Attachment #498424 -
Flags: review?(nnethercote) → review+
Comment 12•14 years ago
|
||
It might be good to add jitstats asserts to that test as well, so if we ever reenable them it'll fail if not traced...
Assignee | ||
Comment 13•14 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/563a9221c6c5
Reporter | ||
Updated•14 years ago
|
Severity: normal → critical
Assignee | ||
Updated•14 years ago
|
Whiteboard: [sg:critical?] → [sg:critical?] [fixed-in-tracemonkey]
Updated•14 years ago
|
blocking2.0: ? → beta9+
Comment 14•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/563a9221c6c5
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Group: core-security
Updated•11 years ago
|
Flags: sec-bounty+
You need to log in
before you can comment on or make changes to this bug.
Description
•