1236519 - Crash [@ isDerivedClassConstructor] or Assertion failure: isInterpretedLazy() && u.i.s.lazy_, at jsfun.h:452

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision d7a0ad85d9fb (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-debug, run with --fuzzing-safe --no-threads --disable-oom-functions --ion-eager --baseline-eager --ion-extra-checks):

gcPreserveCode();
setJitCompilerOption("ion.warmup.trigger", 20);
DateTimeFormat =  newGlobal().Intl.DateTimeFormat;
new DateTimeFormat;
gc();
new DateTimeFormat;



Backtrace:

Program received signal SIGSEGV, Segmentation fault.
isDerivedClassConstructor (this=0x7ffff7e97560) at js/src/jsfun.h:537
#0  isDerivedClassConstructor (this=0x7ffff7e97560) at js/src/jsfun.h:537
#1  js::jit::IonBuilder::createThis (this=this@entry=0x7ffff699c1a0, target=target@entry=0x7ffff7e97560, callee=0x7ffff69a2b90, newTarget=0x7ffff69a2b90) at js/src/jit/IonBuilder.cpp:6269
#2  0x00000000005922dc in js::jit::IonBuilder::makeCallHelper (this=this@entry=0x7ffff699c1a0, target=target@entry=0x7ffff7e97560, callInfo=...) at js/src/jit/IonBuilder.cpp:6719
#3  0x000000000059884c in js::jit::IonBuilder::makeCall (this=this@entry=0x7ffff699c1a0, target=target@entry=0x7ffff7e97560, callInfo=...) at js/src/jit/IonBuilder.cpp:6768
#4  0x00000000005ad1bb in js::jit::IonBuilder::jsop_call (this=this@entry=0x7ffff699c1a0, argc=<optimized out>, constructing=<optimized out>) at js/src/jit/IonBuilder.cpp:6585
#5  0x00000000005a8796 in js::jit::IonBuilder::inspectOpcode (this=this@entry=0x7ffff699c1a0, op=op@entry=JSOP_NEW) at js/src/jit/IonBuilder.cpp:1887
#6  0x00000000005a94a4 in js::jit::IonBuilder::traverseBytecode (this=this@entry=0x7ffff699c1a0) at js/src/jit/IonBuilder.cpp:1521
#7  0x00000000005a98c6 in js::jit::IonBuilder::build (this=0x7ffff699c1a0) at js/src/jit/IonBuilder.cpp:917
#8  0x00000000005b31e3 in js::jit::IonCompile (cx=cx@entry=0x7ffff6907800, script=<optimized out>, baselineFrame=baselineFrame@entry=0x0, osrPc=<optimized out>, constructing=<optimized out>, recompile=<optimized out>, optimizationLevel=js::jit::Normal) at js/src/jit/Ion.cpp:2213
#9  0x00000000005b3a8e in js::jit::Compile (cx=cx@entry=0x7ffff6907800, script=script@entry=..., osrFrame=osrFrame@entry=0x0, osrPc=osrPc@entry=0x0, constructing=<optimized out>, forceRecompile=forceRecompile@entry=false) at js/src/jit/Ion.cpp:2451
#10 0x00000000005b3d9b in js::jit::CanEnter (cx=cx@entry=0x7ffff6907800, state=...) at js/src/jit/Ion.cpp:2613
#11 0x0000000000870ef1 in js::RunScript (cx=cx@entry=0x7ffff6907800, state=...) at js/src/vm/Interpreter.cpp:383
#12 0x000000000087114f in js::Invoke (cx=cx@entry=0x7ffff6907800, args=..., construct=construct@entry=js::NO_CONSTRUCT) at js/src/vm/Interpreter.cpp:478
#13 0x00000000004f3581 in IntlInitialize (cx=cx@entry=0x7ffff6907800, obj=..., obj@entry=..., initializer=..., locales=..., locales@entry=..., options=options@entry=...) at js/src/builtin/Intl.cpp:486
#14 0x00000000004fb70e in DateTimeFormat (cx=cx@entry=0x7ffff6907800, args=..., construct=<optimized out>) at js/src/builtin/Intl.cpp:1679
#15 0x00000000004fba90 in DateTimeFormat (cx=cx@entry=0x7ffff6907800, argc=<optimized out>, vp=<optimized out>) at js/src/builtin/Intl.cpp:1691
#16 0x0000000000871630 in CallJSNative (args=..., native=0x4fba60 <DateTimeFormat(JSContext*, unsigned int, JS::Value*)>, cx=0x7ffff6907800) at js/src/jscntxtinlines.h:235
#17 CallJSNativeConstructor (args=..., native=0x4fba60 <DateTimeFormat(JSContext*, unsigned int, JS::Value*)>, cx=0x7ffff6907800) at js/src/jscntxtinlines.h:268
#18 InternalConstruct (cx=cx@entry=0x7ffff6907800, args=...) at js/src/vm/Interpreter.cpp:537
#19 0x0000000000871746 in js::Construct (cx=cx@entry=0x7ffff6907800, fval=..., fval@entry=..., args=..., newTarget=..., rval=...) at js/src/vm/Interpreter.cpp:586
#20 0x00000000007db7fa in js::DirectProxyHandler::construct (this=this@entry=0x183a390 <js::CrossCompartmentWrapper::singleton>, cx=cx@entry=0x7ffff6907800, proxy=..., proxy@entry=..., args=...) at js/src/proxy/DirectProxyHandler.cpp:95
#21 0x00000000007db994 in js::CrossCompartmentWrapper::construct (this=0x183a390 <js::CrossCompartmentWrapper::singleton>, cx=0x7ffff6907800, wrapper=..., args=...) at js/src/proxy/CrossCompartmentWrapper.cpp:309
#22 0x00000000007d7dda in js::Proxy::construct (cx=0x7ffff6907800, proxy=proxy@entry=..., args=...) at js/src/proxy/Proxy.cpp:410
#23 0x00000000007d87ff in js::proxy_Construct (cx=<optimized out>, argc=<optimized out>, vp=<optimized out>) at js/src/proxy/Proxy.cpp:692
#24 0x00000000008714da in CallJSNative (args=..., native=0x7d87a0 <js::proxy_Construct(JSContext*, unsigned int, JS::Value*)>, cx=0x7ffff6907800) at js/src/jscntxtinlines.h:235
#25 CallJSNativeConstructor (args=..., native=0x7d87a0 <js::proxy_Construct(JSContext*, unsigned int, JS::Value*)>, cx=0x7ffff6907800) at js/src/jscntxtinlines.h:268
#26 InternalConstruct (cx=cx@entry=0x7ffff6907800, args=...) at js/src/vm/Interpreter.cpp:549
#27 0x0000000000871746 in js::Construct (cx=cx@entry=0x7ffff6907800, fval=..., fval@entry=..., args=..., newTarget=..., newTarget@entry=..., rval=..., rval@entry=...) at js/src/vm/Interpreter.cpp:586
#28 0x00000000005265b9 in js::jit::DoCallFallback (cx=0x7ffff6907800, frame=0x7fffffffd228, stub_=0x7ffff53da258, argc=0, vp=0x7fffffffd1d8, res=...) at js/src/jit/BaselineIC.cpp:6164
#29 0x00007ffff7ff0bf4 in ?? ()
[...]
#39 0x0000000000000000 in ?? ()
rax	0x0	0
rbx	0x7ffff699c1a0	140737330659744
rcx	0x7ffff69a2b90	140737330686864
rdx	0x7ffff69a2b90	140737330686864
rsi	0x7ffff7e97560	140737352660320
rdi	0x7ffff699c1a0	140737330659744
rbp	0x7ffff69a2b90	140737330686864
rsp	0x7fffffffc290	140737488339600
r8	0x286	646
r9	0x91f0	37360
r10	0x7ffff7e97000	140737352658944
r11	0xd	13
r12	0x7ffff69a2b90	140737330686864
r13	0x7ffff69a2d08	140737330687240
r14	0x7ffff699c1a0	140737330659744
r15	0xfffffffffffffff8	-8
rip	0x591fa5 <js::jit::IonBuilder::createThis(JSFunction*, js::jit::MDefinition*, js::jit::MDefinition*)+53>
=> 0x591fa5 <js::jit::IonBuilder::createThis(JSFunction*, js::jit::MDefinition*, js::jit::MDefinition*)+53>:	movzbl 0x2f(%rax),%eax
   0x591fa9 <js::jit::IonBuilder::createThis(JSFunction*, js::jit::MDefinition*, js::jit::MDefinition*)+57>:	shr    $0x5,%al


Marking s-s because this involves the GC and in particular, GC code preservation.

Comment 1

[Jan de Mooij [:jandem]]

NI efaust because isDerivedClassConstructor, but feel free to forward if it's a pre-existing bug.

Comment 2

[Fuzzing Team]

JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/a59b5b0139b4
user:        Eric Faust
date:        Thu Oct 08 17:01:48 2015 -0700
summary:     Bug 1169740 - Implement a TDZ-like behavior for |this| in derived class constructors. (r=jandem, r=jorendorff, inputs on nit resoulution from Waldo)

This iteration took 211.224 seconds to run.

Comment 3

[Eric Faust [:efaust]]

This seems to be working OK for me? Let's see what happened.

Comment 4

[Eric Faust [:efaust]]

OK, got it. Looking.

Comment 5

[Eric Faust [:efaust]]

Adding till to CC so that I can ask him for review.

Comment 6

[Eric Faust [:efaust]]

Attachment: Fix?

OK, so we crash because we cannot ask for the lazyScript for a lazy self-hosted clone, because it hasn't got one. The only way to get a lazy self-hosted clone that's a derived class constructor is to have a clone of the default derived class constructor.

Doing that check takes a little doing, since we don't have a cx. Instead, walk all the way around to the runtime through the compartment.

Comment 7

[Till Schneidereit [:till]]

Comment on attachment 8707218 [details] [diff] [review]
Fix?

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

Not pretty, but yes, absolutely.

Comment 8

[Eric Faust [:efaust]]

Comment on attachment 8707218 [details] [diff] [review]
Fix?

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

It's gonna crash at null. I don't think it's very exploitable.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

Nope.

Which older supported branches are affected by this flaw?

Aurora 45

If not all supported branches, which bug introduced the flaw?

bug 1197932 exposed it on aurora

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

It's slightly different, but even less complicated. Backport should be trivial.

How likely is this patch to cause regressions; how much testing does it need?

Not hardly at all. We'll let it bake on central for a minute.

Comment 9

[Eric Faust [:efaust]]

Just a nullcrash. Doesn't actually need approval.

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Landed direct to m-c: https://hg.mozilla.org/mozilla-central/rev/ad1f85f172b7
Once I see that it isn't totally broken, I'll trigger new nightlies.

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Just triggered new nightlies.

Comment 12

[Eric Faust [:efaust]]

Attachment: Fix for aurora

No Self-hosted default constructors in 45, so the fix is different, but much simpler.

Comment 13

[Till Schneidereit [:till]]

Comment on attachment 8708082 [details] [diff] [review]
Fix for aurora

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

Much simpler indeed.

Comment 14

[Eric Faust [:efaust]]

Comment on attachment 8708082 [details] [diff] [review]
Fix for aurora

Approval Request Comment
[Feature/regressing bug #]:
bug 1197932 released this to aurora

[User impact if declined]:
crashes jitting self-hosted code.
[Describe test coverage new/current, TreeHerder]:
This is a port, but an analagous patch landed with test to central yesterday without incident
[Risks and why]: 
None. Just removes crash.
[String/UUID change made/needed]:
None.

Comment 15

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8708082 [details] [diff] [review]
Fix for aurora

Fix a crash, taking it.

Comment 16

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/22348f269780

Comment 17

[Fuzzing Team]

JSBugMon: This bug has been automatically verified fixed.
JSBugMon: This bug has been automatically verified fixed on Fx45