Closed Bug 1364648 Opened 3 years ago Closed 3 years ago

Assertion failure: begin + len <= length(), at js/src/jsscript.cpp:1691 with columnNumber property

Categories

(Core :: JavaScript Engine, defect, critical)

x86
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox-esr52 --- wontfix
firefox53 --- wontfix
firefox54 --- wontfix
firefox55 --- fixed

People

(Reporter: decoder, Assigned: shu)

References

(Blocks 1 open bug)

Details

(4 keywords, Whiteboard: [jsbugmon:update,bisect])

Attachments

(1 file)

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.
Needinfo from :shu because he also fixed bug 1161312.
Flags: needinfo?(shu)
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)
Flags: needinfo?(shu)
Assignee: nobody → shu
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)
https://hg.mozilla.org/mozilla-central/rev/20fd2a3c8039
Status: NEW → RESOLVED
Closed: 3 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
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.