Christian Holler (:decoder) Reporter
	Description • 6 years ago

The following testcase crashes on mozilla-central revision 277e1562ee9c (build with --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-profiling --enable-debug --enable-optimize, run with --fuzzing-safe --ion-eager --ion-offthread-compile=off):

const USE_ASM = '"use asm";';
function asmCompile(){
    var f = Function.apply(null, arguments);
    return f;
}
function asmLink(f) {
    return f.apply(null, Array.slice(arguments, 1));
}
g = newGlobal()
g.parent = this
g.eval("(" + function() {
    Debugger(parent).onExceptionUnwind = function(frame) {
        frame.older
    }
} + ")()")
function maybeThrow(i, j) {
    if (i == 0) throw j;
    return f(i - 1, j);
}
var f = asmLink(asmCompile('glob', 'imp', USE_ASM + `
  var ffi=imp.ffi;
  function f(i, j) {
    return ffi(i|0, (j+1)|0)|0 
  } 
  return f
`), null, {
    ffi: maybeThrow
});
f(3, 0)


Backtrace:

received signal SIGSEGV, Segmentation fault.
0x000000000063d386 in JSScript::ionScript (this=<optimized out>) at js/src/vm/JSScript.h:1567
#0  0x000000000063d386 in JSScript::ionScript (this=<optimized out>) at js/src/vm/JSScript.h:1567
#1  0x000000000071f0e8 in js::jit::IonScript::invalidate (this=0x7ffff4943000, cx=cx@entry=0x7ffff5f15000, script=0x7ffff4c933a0, resetUses=resetUses@entry=false, reason=reason@entry=0x12907b8 "Observe recovered instruction.") at js/src/jit/Ion.cpp:3046
#2  0x0000000000792a8e in js::jit::SnapshotIterator::initInstructionResults (this=this@entry=0x7fffffff6740, fallback=...) at js/src/jit/JitFrames.cpp:1898
#3  0x0000000000792b2e in js::jit::SnapshotIterator::maybeRead (this=this@entry=0x7fffffff6740, a=..., fallback=...) at js/src/jit/JitFrames.cpp:1746
#4  0x0000000000792cb9 in js::jit::SnapshotIterator::maybeRead (this=this@entry=0x7fffffff6740, a=..., fallback=...) at js/src/jit/JitFrames.cpp:1742
#5  0x00000000008fb669 in js::jit::SnapshotIterator::maybeRead (fallback=..., this=0x7fffffff6740) at js/src/jit/JSJitFrameIter.h:588
#6  js::jit::SnapshotIterator::readFunctionFrameArgs<CopyValueToRematerializedFrame> (fallback=..., script=<optimized out>, end=2, start=0, thisv=<optimized out>, argsObj=<optimized out>, op=..., this=0x7fffffff6740) at js/src/jit/JSJitFrameIter.h:624
#7  js::jit::InlineFrameIterator::readFrameArgsAndLocals<CopyValueToRematerializedFrame, CopyValueToRematerializedFrame> (this=this@entry=0x7fffffff6f10, cx=cx@entry=0x7ffff5f15000, argOp=..., localOp=..., envChain=envChain@entry=0x7ffff4950700, hasInitialEnv=hasInitialEnv@entry=0x7ffff49506d2, rval=0x7ffff4950718, argsObj=0x7ffff4950710, thisv=0x7ffff4950720, newTarget=0x7ffff4950728, behavior=js::jit::ReadFrame_Actuals, fallback=...) at js/src/jit/JSJitFrameIter.h:758
#8  0x00000000008e38e0 in js::jit::RematerializedFrame::RematerializedFrame (this=0x7ffff49506d0, cx=0x7ffff5f15000, top=<optimized out>, numActualArgs=<optimized out>, iter=..., fallback=...) at js/src/jit/RematerializedFrame.cpp:53
#9  0x00000000008e3a3c in js::jit::RematerializedFrame::New (cx=cx@entry=0x7ffff5f15000, top=top@entry=0x7fffffff9290 "a7\016Eo\027", iter=..., fallback=...) at js/src/jit/RematerializedFrame.cpp:78
#10 0x00000000008f1d36 in js::jit::RematerializedFrame::RematerializeInlineFrames (cx=cx@entry=0x7ffff5f15000, top=<optimized out>, iter=..., fallback=..., frames=...) at js/src/jit/RematerializedFrame.cpp:93
#11 0x0000000000c2d331 in js::jit::JitActivation::getRematerializedFrame (this=0x7fffffff9470, cx=cx@entry=0x7ffff5f15000, iter=..., inlineDepth=inlineDepth@entry=0) at js/src/vm/Stack.cpp:1625
#12 0x0000000000c2d73b in js::FrameIter::ensureHasRematerializedFrame (this=this@entry=0x7fffffff7330, cx=cx@entry=0x7ffff5f15000) at js/src/vm/Stack.cpp:1064
#13 0x0000000000ac8803 in js::DebuggerFrame::getOlder (cx=0x7ffff5f15000, frame=..., frame@entry=..., result=..., result@entry=...) at js/src/vm/Debugger.cpp:7671
#14 0x0000000000ac88de in js::DebuggerFrame::olderGetter (cx=<optimized out>, cx@entry=0x7ffff5f15000, argc=argc@entry=0, vp=vp@entry=0x7fffffff7890) at js/src/vm/Debugger.cpp:8304
#15 0x000000000093af14 in js::jit::CallNativeGetter (cx=0x7ffff5f15000, callee=..., obj=..., result=...) at js/src/jit/VMFunctions.cpp:1555
#16 0x0000176f45094f64 in ?? ()
[...]
#29 0x0000000000003821 in ?? ()
#30 0x0000000000a25c63 in js::CheckThreadLocal::check (this=0x7ffff4cb4880) at js/src/threading/ProtectedData.cpp:46
rax	0x0	0
rbx	0x7ffff4943000	140737296740352
rcx	0x7ffff6c282ad	140737333330605
rdx	0x0	0
rsi	0x7ffff6ef7770	140737336276848
rdi	0x7ffff6ef6540	140737336272192
rbp	0x7fffffff61e0	140737488314848
rsp	0x7fffffff61e0	140737488314848
r8	0x7ffff6ef7770	140737336276848
r9	0x7ffff7fe4780	140737354024832
r10	0x58	88
r11	0x7ffff6b9e7a0	140737332766624
r12	0x7ffff4c933a0	140737300214688
r13	0x0	0
r14	0x7ffff5f15000	140737319620608
r15	0x7ffff5f3f000	140737319792640
rip	0x63d386 <JSScript::ionScript() const+38>
=> 0x63d386 <JSScript::ionScript() const+38>:	movl   $0x0,0x0
   0x63d391 <JSScript::ionScript() const+49>:	ud2


This test uses Debugger but I have seen another test using gczeal and enableShellAllocationMetadataBuilder instead. Filing this s-s until it has been confirmed that this is a shell-only problem.

	Fuzzing Team
	Comment 1 • 6 years ago

JSBugMon: Bisection requested, failed due to error (try manually).

	Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)
	Comment 2 • 6 years ago

Here's another testcase that only uses enableShellAllocationMetadataBuilder() from jsfunfuzz.

	Jan de Mooij [:jandem] Assignee
	Comment 3 • 6 years ago

This will hit a release assert in opt builds.

	Jan de Mooij [:jandem] Assignee
	Comment 4 • 6 years ago

I added this release assert to IonScript::invalidate, but it can fail for already-invalidated scripts.

This patch just fixes the assert. We could also return early if invalidated() but it doesn't matter much.

	Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)
	Comment 5 • 6 years ago

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/dae25f5b42df
user:        Jan de Mooij
date:        Fri Apr 06 10:55:49 2018 +0200
summary:     Bug 1451443 - Remove CompilerOutput and simplify Ion code invalidation. r=tcampbell

Guessing related to bug 1451443?

	Nicolas B. Pierron [:nbp]
	Comment 6 • 6 years ago

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

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

::: js/src/jit/Ion.cpp
@@ +2934,5 @@
>  
>  void
>  jit::IonScript::invalidate(JSContext* cx, JSScript* script, bool resetUses, const char* reason)
>  {
> +    MOZ_RELEASE_ASSERT(invalidated() || script->ionScript() == this);

As a optimization, we should probably short-cut this function if the IonScript already got invalidated, as this literally implies that we already went through the JitActivations.

	Jan de Mooij [:jandem] Assignee
	Comment 7 • 6 years ago

(In reply to Nicolas B. Pierron [:nbp] {backlog: ~36} from comment #6)
> As a optimization, we should probably short-cut this function if the
> IonScript already got invalidated, as this literally implies that we already
> went through the JitActivations.

We could do that and it's probably fine, but jit::Invalidate also cancels off-thread compilations in this case (the cancelOffThread argument) so it's not 100% guaranteed to be a no-op. I'll add a comment. (numInvalidations will be 0 in jit::Invalidate when this happens so we don't walk the stack anyway.)

	Pulsebot
	Comment 8 • 6 years ago

Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/db2973e7667c
Fix assert in IonScript::invalidate to account for already-invalidated scripts. r=nbp

	Raul Gurzau (:RaulG)
	Comment 9 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/db2973e7667c