1412654 - Assertion failure: toStringEnd >= bufEnd, at js/src/jsscript.cpp:2706 with clone

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision c16bc8097c10 (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --enable-stdcxx-compat --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --ion-offthread-compile=off min.js):

var gv = newGlobal();
gv.f = (class get {});
gv.eval('f = clone(f);');


Backtrace:

received signal SIGSEGV, Segmentation fault.
0x0000000000a619c8 in JSScript::Create (cx=cx@entry=0x7ffff6955000, options=..., sourceObject=..., sourceObject@entry=..., bufStart=27837, bufEnd=27843, toStringStart=30, toStringEnd=42) at js/src/jsscript.cpp:2706
#0  0x0000000000a619c8 in JSScript::Create (cx=cx@entry=0x7ffff6955000, options=..., sourceObject=..., sourceObject@entry=..., bufStart=27837, bufEnd=27843, toStringStart=30, toStringEnd=42) at js/src/jsscript.cpp:2706
#1  0x0000000000a61bd5 in CreateEmptyScriptForClone (cx=0x7ffff6955000, src=src@entry=...) at js/src/jsscript.cpp:3726
#2  0x0000000000a640b3 in js::CloneScriptIntoFunction (cx=0x7ffff6955000, enclosingScope=..., enclosingScope@entry=..., fun=..., fun@entry=..., src=src@entry=...) at js/src/jsscript.cpp:3758
#3  0x00000000009f54c7 in js::CloneFunctionAndScript (cx=0x7ffff6955000, fun=..., fun@entry=..., enclosingEnv=..., newScope=..., newScope@entry=..., allocKind=<optimized out>, proto=..., proto@entry=...) at js/src/jsfun.cpp:2265
#4  0x0000000000992c18 in CloneFunctionObject (cx=cx@entry=0x7ffff6955000, funobj=..., env=..., scope=scope@entry=...) at js/src/jsapi.cpp:3752
#5  0x0000000000992ec8 in JS::CloneFunctionObject (cx=0x7ffff6955000, funobj=..., funobj@entry=..., envChain=...) at js/src/jsapi.cpp:3778
#6  0x0000000000472f88 in Clone (cx=0x7ffff6955000, argc=<optimized out>, vp=<optimized out>) at js/src/shell/js.cpp:3138
#7  0x0000000000560411 in js::CallJSNative (cx=0x7ffff6955000, native=0x472c40 <Clone(JSContext*, unsigned int, JS::Value*)>, args=...) at js/src/jscntxtinlines.h:291
[...]
#38 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:8928
rax	0x0	0
rbx	0x6cbd	27837
rcx	0x7ffff6c28a2d	140737333332525
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffffbbc0	140737488337856
rsp	0x7fffffffbb50	140737488337744
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4740	140737354024768
r10	0x58	88
r11	0x7ffff6b9f750	140737332770640
r12	0x1e	30
r13	0x6cc3	27843
r14	0x7fffffffbbf0	140737488337904
r15	0x7fffffffbf10	140737488338704
rip	0xa619c8 <JSScript::Create(JSContext*, JS::ReadOnlyCompileOptions const&, JS::Handle<JSObject*>, unsigned int, unsigned int, unsigned int, unsigned int)+616>
=> 0xa619c8 <JSScript::Create(JSContext*, JS::ReadOnlyCompileOptions const&, JS::Handle<JSObject*>, unsigned int, unsigned int, unsigned int, unsigned int)+616>:	movl   $0x0,0x0
   0xa619d3 <JSScript::Create(JSContext*, JS::ReadOnlyCompileOptions const&, JS::Handle<JSObject*>, unsigned int, unsigned int, unsigned int, unsigned int)+627>:	ud2

Comment 1

[Fuzzing Team]

JSBugMon: Bisection requested, result:
Due to skipped revisions, the first bad revision could be any of:
changeset:   https://hg.mozilla.org/mozilla-central/rev/751cc121aa3f
user:        Shu-yu Guo
date:        Mon Apr 17 19:51:34 2017 -0700
summary:     Bug 1216630 - Print class source when calling toString on the constructor. (r=Yoric)

changeset:   https://hg.mozilla.org/mozilla-central/rev/8f3e4478d23a
user:        Shu-yu Guo
date:        Mon Apr 17 19:51:35 2017 -0700
summary:     Bug 1216630 - Rename preludeStart and postludeEnd to toStringStart and toStringEnd and misc fixes. (r=Yoric)

This iteration took 247.499 seconds to run.

Comment 2

[Jan de Mooij [:jandem]]

Ah another clone() treat.

Comment 3

[Jan de Mooij [:jandem]]

The bug here is that JSScript::setDefaultClassConstructorSpan sets toStringStart/toStrindEnd, but sourceStart/sourceEnd still refer to (I assume) the offsets in the self-hosted source code. Hence this assertion failure when we try to clone() the self-hosted constructor.

There are other issues though - cloning functions with extended slots across compartments will initialize these slots to |undefined|. That also seems like a recipe for failure and something I'd like to get rid of.

I'll try locking down CloneFunctionObject more - in bug 1405766 we blocked native functions and we should probably block everything else that's not a normal function.

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Patch

Here's a patch to reject all extended functions and functions with kind() != NormalFunction in CloneFunctionObject.

I added a MOZ_CRASH() to this if-statement, and it didn't hit on any browser test. We have this (potentially cross-compartment) function cloning API just for XBL and I think it makes sense to only handle the few cases we need for that.

Comment 5

[Tom S [:evilpie]]

Comment on attachment 8923976 [details] [diff] [review]
Patch

Review of attachment 8923976 [details] [diff] [review]:
-----------------------------------------------------------------

Agreed

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/55642641fbd4
Only allow cloning normal functions in CloneFunctionObject. r=evilpie

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/55642641fbd4