Closed
Bug 1364648
Opened 7 years ago
Closed 7 years ago
Assertion failure: begin + len <= length(), at js/src/jsscript.cpp:1691 with columnNumber property
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla55
People
(Reporter: decoder, Assigned: shu)
Details
(4 keywords, Whiteboard: [jsbugmon:update,bisect])
Attachments
(1 file)
13.39 KB,
patch
|
jimb
:
review+
|
Details | Diff | Splinter Review |
The following testcase crashes on mozilla-central revision 120d8562d4a5 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-stdcxx-compat --disable-profiling --enable-debug --without-intl-api --enable-optimize --target=i686-pc-linux-gnu, run with --fuzzing-safe --no-threads): assertEq(evaluate(`var f = x=>class { }; f()`, { columnNumber: 1729 }), 1); Backtrace: received signal SIGSEGV, Segmentation fault. 0x08638fc9 in js::ScriptSource::chars (this=0xf686bdc0, cx=0xf7951800, holder=..., begin=1740, len=9) at js/src/jsscript.cpp:1691 #0 0x08638fc9 in js::ScriptSource::chars (this=0xf686bdc0, cx=0xf7951800, holder=..., begin=1740, len=9) at js/src/jsscript.cpp:1691 #1 0x0863922b in js::ScriptSource::PinnedChars::PinnedChars (len=9, begin=1740, holder=..., source=0xf686bdc0, cx=0xf7951800, this=0xffffc99c) at js/src/jsscript.cpp:1653 #2 js::ScriptSource::substring (this=0xf686bdc0, cx=0xf7951800, start=1740, stop=1749) at js/src/jsscript.cpp:1774 #3 0x08639392 in JSScript::sourceDataForToString (cx=0xf7951800, script=...) at js/src/jsscript.cpp:1486 #4 0x085c95da in js::FunctionToString (cx=0xf7951800, fun=..., prettyPrint=false) at js/src/jsfun.cpp:1073 #5 0x085c98b8 in fun_toStringHelper (cx=0xf7951800, obj=..., indent=32768) at js/src/jsfun.cpp:1123 #6 0x085e7342 in fun_toSource (cx=0xf7951800, argc=0, vp=0xffffcd90) at js/src/jsfun.cpp:1176 #7 0x081707e6 in js::CallJSNative (cx=0xf7951800, native=0x85e71c0 <fun_toSource(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:293 #8 0x08166473 in js::InternalCallOrConstruct (cx=0xf7951800, args=..., construct=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:470 #9 0x0816687f in InternalCall (cx=cx@entry=0xf7951800, args=...) at js/src/vm/Interpreter.cpp:515 #10 0x08166a1a in js::Call (cx=0xf7951800, fval=..., thisv=..., args=..., rval=...) at js/src/vm/Interpreter.cpp:534 #11 0x0862a555 in js::Call (rval=..., thisObj=<optimized out>, fval=..., cx=0xf7951800) at js/src/vm/Interpreter.h:94 #12 js::ValueToSource (cx=0xf7951800, v=...) at js/src/jsstr.cpp:3543 #13 0x08530bb0 in JS_ValueToSource (cx=0xf7951800, value=...) at js/src/jsapi.cpp:380 #14 0x08082fb2 in ToSource (cx=cx@entry=0xf7951800, vp=..., vp@entry=..., bytes=bytes@entry=0xffffceb8) at js/src/shell/js.cpp:2342 #15 0x08086a81 in AssertEq (cx=0xf7951800, argc=2, vp=0xf686c058) at js/src/shell/js.cpp:2372 #16 0x081707e6 in js::CallJSNative (cx=0xf7951800, native=0x8086950 <AssertEq(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:293 [...] #30 main (argc=4, argv=0xffffda14, envp=0xffffda28) at js/src/shell/js.cpp:8682 eax 0x0 0 ebx 0x9 9 ecx 0xf7d9c864 -136722332 edx 0x0 0 esi 0x6cc 1740 edi 0x6d5 1749 ebp 0xffffc948 4294953288 esp 0xffffc8c0 4294953152 eip 0x8638fc9 <js::ScriptSource::chars(JSContext*, js::UncompressedSourceCache::AutoHoldEntry&, unsigned int, unsigned int)+953> => 0x8638fc9 <js::ScriptSource::chars(JSContext*, js::UncompressedSourceCache::AutoHoldEntry&, unsigned int, unsigned int)+953>: movl $0x0,0x0 0x8638fd3 <js::ScriptSource::chars(JSContext*, js::UncompressedSourceCache::AutoHoldEntry&, unsigned int, unsigned int)+963>: ud2 Still pops up in fuzzing for me, even several days after bug 1161312 has been fixed.
Reporter | ||
Comment 1•7 years ago
|
||
Needinfo from :shu because he also fixed bug 1161312.
Flags: needinfo?(shu)
Assignee | ||
Comment 2•7 years ago
|
||
CompileOptions's column field has been conflating two meanings: the starting column of the ScriptSource, and the starting column of the current thing being compiled. In the shell's evaluate() function, when the "column" option is passed, it's fine to conflate the two meanings above. When delazifying functions, it is incorrect to conflate the two meanings. This is observable when generating SRC_CLASS_SPAN, which is a srcnote used to save the toString offsets for a default class constructor. (Since default class constructors aren't syntactically present, there's no JSFunction made ahead of time. And since class constructors must be toString'd as the class source instead of the function source, we save the offsets in a srcnote to use when we actually create the constructor at runtime.) When we save these offsets, these are offsets into the ScriptSource buffer, and must be de-offset. But it's incorrect to subtract the starting column of the lazy function, which is itself offset from the starting column of the underlying ScriptSource.
Attachment #8867926 -
Flags: review?(jimb)
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(shu)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → shu
Comment 3•7 years ago
|
||
Comment on attachment 8867926 [details] [diff] [review] Fix OOB column handling for default class constructors' toString offsets. Review of attachment 8867926 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/frontend/SharedContext.h @@ +565,5 @@ > // CompileOptions. bufStart and toStringStart, however, refer to > // absolute positions within the ScriptSource buffer, and need to > // de-offset from the starting column. > uint32_t offset = tokenStream.currentToken().pos.begin; > + uint32_t subtrahend = tokenStream.options().sourceStartColumn; `subtrahend`, here and below, is not a very helpful name for this variable. `sourceStartColumn`?
Attachment #8867926 -
Flags: review?(jimb) → review+
Pushed by shu@rfrn.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/20fd2a3c8039 Fix OOB column handling for default class constructors' toString offsets. (r=jimb)
Comment 5•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/20fd2a3c8039
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Comment 6•7 years ago
|
||
Doesn't graft cleanly to 54 - probably not worth taking the time to rebase given where we are in the cycle.
You need to log in
before you can comment on or make changes to this bug.
Description
•