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

RESOLVED FIXED in Firefox 55

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
5 months ago
5 months ago

People

(Reporter: decoder, Assigned: shu)

Tracking

(Blocks: 1 bug, 4 keywords)

Trunk
mozilla55
x86
Linux
assertion, jsbugmon, regression, testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr52 wontfix, firefox53 wontfix, firefox54 wontfix, firefox55 fixed)

Details

(Whiteboard: [jsbugmon:update,bisect])

Attachments

(1 attachment)

(Reporter)

Description

5 months ago
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

5 months ago
Needinfo from :shu because he also fixed bug 1161312.
Flags: needinfo?(shu)
(Assignee)

Comment 2

5 months ago
Created attachment 8867926 [details] [diff] [review]
Fix OOB column handling for default class constructors' toString offsets.

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

5 months ago
Flags: needinfo?(shu)
(Assignee)

Updated

5 months ago
Assignee: nobody → shu

Comment 3

5 months 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+

Comment 4

5 months ago
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

5 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/20fd2a3c8039
Status: NEW → RESOLVED
Last Resolved: 5 months ago
status-firefox55: affected → fixed
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.
status-firefox53: --- → wontfix
status-firefox54: --- → wontfix
status-firefox-esr52: --- → wontfix
You need to log in before you can comment on or make changes to this bug.