The default bug view has changed. See this FAQ.

IonMonkey: Crash at weird location of 0x00007ffff7f60d58 with testcase

VERIFIED FIXED

Status

()

Core
JavaScript Engine
--
critical
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: gkw, Assigned: dvander)

Tracking

(Blocks: 2 bugs, {crash, testcase})

Other Branch
x86_64
Linux
crash, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fuzzblocker])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Created attachment 625272 [details]
partially reduced testcase

The attached testcase crashes 64-bit js debug and opt shell on IonMonkey changeset 47b283284868 without any CLI arguments at 0x00007ffff7f60d58

The crash becomes intermittent as more reduction is done. s-s because this is crashing at a weird memory address.
(Reporter)

Comment 1

5 years ago
Nicolas is looking at this on my computer now.
First thing is that we end-up in a non-readable code or a garbage collected and erased code.

tl;dr  We forgot to mark something which is shared among multiple IonScript, I don't know what yet.


Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7efc720 in ?? ()
(gdb) x /i $pc
=> 0x7ffff7efc720:	Cannot access memory at address 0x7ffff7efc720

Looking at the stack, we can see 2 Ion JS-Frames that we can identify with the frame descriptor 0x180 and 0x101.  0x101 indicates that this is the first frame because the previous frame is an entry frame.  I annotated the stack with the information that I found with the return address and the calleeToken.

The invalidated annotation mean that the script->ion is reset to NULL.

(gdb) x /100x $sp
0x7fffffffc950:	0xf7ec57ee	0x00007fff	0x00f67610	0x00000000
0x7fffffffc960:	0xf7ec57cb	0x00007fff	0xffffc970	0x00007fff
0x7fffffffc970:	0x00000000	0xfff90000	0xf6763340	0xfffaffff
0x7fffffffc980:	0xf6763340	0x00007fff	0xf7ec539d	0x00007fff (2)
0x7fffffffc990:	0x00000180	0x00000000	0xf6710ac	0x00007fff
… (JSFunction*) 0x7ffff6710ac0: w17-orig.js:28 <invalidated>  …
0x7fffffffc9a0:	0x00000000	0xfff90000	0xf6708400	0xfffbffff
0x7fffffffc9b0:	0x00000000	0x7ff00000	0x00000000	0xfff90000
0x7fffffffc9c0:	0x00d6a420	0x00000000	0xf6796400	0xfffbffff
0x7fffffffc9d0:	0xf7ff7fbc	0x00007fff	0x00000101	0x00000000 (1)
0x7fffffffc9e0:	0xf6710a80	0x00007fff	0x00000000	0xfff90000
… (JSFunction*) 0x7ffff6710a80: w17-orig.js:27 <invalidated> …
0x7fffffffc9f0:	0x00000000	0xfff90000	0xffffcaec	0x00007fff
0x7fffffffca00:	0xffffcaf0	0x00007fff	0x00000000	0x00000000
0x7fffffffca10:	0x00000000	0x00000000	0xffffe0a0	0x00007fff
0x7fffffffca20:	0x00de4967	0x00000000	0xffffcb54	0x00007fff
0x7fffffffca30:	0xffffcb70	0x00007fff	0x007c3ba6	0x00000000
… 0x7c3ba6 <EnterIon(JSContext*, js::StackFrame*, void*)+802>:	lea    -0x90(%rbp),%rax …
0x7fffffffca40:	0x00000000	0x7ff00000	0xf7ec519f	0x00007fff
0x7fffffffca50:	0xf69240b8	0x00007fff	0x00db75a0	0x00000000
0x7fffffffca60:	0x00db75a0	0x00000000	0x00db7d60	0x00000000
0x7fffffffca70:	0x00000000	0x00000000	0xf69240b8	0x00007fff
0x7fffffffca80:	0x00000000	0x00000000	0x00000000	0x00000000
0x7fffffffca90:	0x00000000	0x00000000	0x00405538	0x00000000
0x7fffffffcaa0:	0x00db75a0	0x00000000	0x00db7d60	0x00000000
0x7fffffffcab0:	0x00000001	0x00000000	0xf6732128	0x00007fff
0x7fffffffcac0:	0x00db75a0	0x00000000	0x00000000	0x00000000
0x7fffffffcad0:	0x00000000	0x00000000	0xf6732128	0x00007fff

(1) Bottom of the generateEnterJIT
(gdb) x/30i 0x7ffff7ff7fbc
   0x7ffff7ff7fbc:	pop    %r14
   0x7ffff7ff7fbe:	shr    $0x3,%r14
   0x7ffff7ff7fc2:	add    %r14,%rsp
   0x7ffff7ff7fc5:	pop    %r12
   0x7ffff7ff7fc7:	mov    %rcx,(%r12)
   0x7ffff7ff7fcb:	pop    %r15
   0x7ffff7ff7fcd:	pop    %r14
   0x7ffff7ff7fcf:	pop    %r13
   0x7ffff7ff7fd1:	pop    %r12
   0x7ffff7ff7fd3:	pop    %rbx
   0x7ffff7ff7fd4:	pop    %rbp
   0x7ffff7ff7fd5:	retq   


(2) Generated code for … ? …
(gdb) x/30i 0x7ffff7ec539d
   0x7ffff7ec539d:	add    $0x10,%rsp
   0x7ffff7ec53a1:	jmpq   0x7ffff7ec53b7   (3d)
   0x7ffff7ec53a6:	push   %rsp
   0x7ffff7ec53a7:	pushq  $0x0
   0x7ffff7ec53ac:	push   %rax
   0x7ffff7ec53ad:	pushq  $0x240
   0x7ffff7ec53b2:	callq  0x7ffff7f41420   (3a)
   0x7ffff7ec53b7:	callq  0x7ffff7ec5554   (3b)
   0x7ffff7ec53bc:	add    %al,(%rax)
   0x7ffff7ec53be:	add    %bh,%dl
   0x7ffff7ec53c0:	decl   -0x77(%rax)
   0x7ffff7ec53c3:	add    $0x24,%al
   0x7ffff7ec53c5:	movabs $0xfffbfffff673b480,%r11
   0x7ffff7ec53cf:	push   %r11
   0x7ffff7ec53d1:	movabs $0x7ffff7fa2010,%rax
   0x7ffff7ec53db:	mov    0x34b60(%rax),%rax
   0x7ffff7ec53e2:	xor    %edi,%edi
   0x7ffff7ec53e4:	mov    %rsp,%rbx
   0x7ffff7ec53e7:	callq  0x7ffff7ec53f8
   0x7ffff7ec53ec:	jmpq   0x7ffff7ec53fd
   0x7ffff7ec53f1:	hlt    
   0x7ffff7ec53f2:	hlt    
   0x7ffff7ec53f3:	hlt    
   0x7ffff7ec53f4:	hlt    (3c)
   0x7ffff7ec53f5:	hlt    
   0x7ffff7ec53f6:	hlt    
   0x7ffff7ec53f7:	hlt    
   0x7ffff7ec53f8:	mov    (%rsp),%rcx
   0x7ffff7ec53fc:	retq   
   0x7ffff7ec53fd:	pushq  $0x1c0

Looking before is helpless because the invalidation erase the call, and thus we have no mean to recover the address of the callee which later jump to the current $pc.

(3a) This is a VM Call to the wrapper for InvokeFunction.

(gdb) x/30i 0x7ffff7f41420
   …   
   0x7ffff7f41494:	movabs $0x85201c,%rax
   0x7ffff7f4149e:	callq  *%rax
   …

(gdb) x/i 0x85201c
   0x85201c <js::ion::InvokeFunction(JSContext*, JSFunction*, unsigned int, JS::Value*, JS::Value*)>:	push   %rbp


(3b) This is weird, we have no reason to generate a second calls stick to the first one.  This is not a VM call because a VM call expect at least to have a frame descriptor.


(gdb) x/30i 0x7ffff7ec5554
   0x7ffff7ec5554:	movabs $0xe2e570,%r11
   0x7ffff7ec555e:	push   %r11
   0x7ffff7ec5560:	callq  0x7ffff7efc720  (4)
   0x7ffff7ec5565:	int3   
   0x7ffff7ec5566:	pushq  $0x0
   0x7ffff7ec556b:	jmpq   0x7ffff7ec55cb
   0x7ffff7ec5570:	pushq  $0x180
   0x7ffff7ec5575:	callq  0x7ffff7efc9f0
   0x7ffff7ec557a:	int3   
   0x7ffff7ec557b:	pushq  $0x32
   0x7ffff7ec5580:	jmpq   0x7ffff7ec55cb
   0x7ffff7ec5585:	sub    $0x8,%rsp
   0x7ffff7ec5589:	mov    %rcx,(%rsp)
   0x7ffff7ec558d:	push   %rcx
   0x7ffff7ec558e:	pushq  $0x0
   0x7ffff7ec5593:	pushq  $0x240
   0x7ffff7ec5598:	callq  0x7ffff7f2c000
   0x7ffff7ec559d:	mov    %rcx,%rax
   0x7ffff7ec55a0:	mov    (%rsp),%rcx
   0x7ffff7ec55a4:	add    $0x8,%rsp
   0x7ffff7ec55a8:	jmpq   0x7ffff7ec518b
   0x7ffff7ec55ad:	pushq  $0x7d
   0x7ffff7ec55b2:	jmpq   0x7ffff7ec55cb
   0x7ffff7ec55b7:	pushq  $0x97
   0x7ffff7ec55bc:	jmpq   0x7ffff7ec55cb
   0x7ffff7ec55c1:	pushq  $0xe4
   0x7ffff7ec55c6:	jmpq   0x7ffff7ec55cb
   0x7ffff7ec55cb:	pushq  $0x30
   0x7ffff7ec55d0:	jmpq   0x7ffff7efcac0
   0x7ffff7ec55d5:	hlt 


(3c) If the code has been invalidated, this space should be full, right ?

(3d) An unconditional jump, what for ?


(4) Ok, this one is interesting, our current $pc location was also reachable by the parent frame.  This means we forgot to mark one of the shared generated code which got garbage collected.
The issue is coming from the invalidation(In reply to Nicolas B. Pierron [:pierron] from comment #2)
> First thing is that we end-up in a non-readable code or a garbage collected
> and erased code.
> 
> tl;dr  We forgot to mark something which is shared among multiple IonScript,
> I don't know what yet.
> 

> (gdb) x/30i 0x7ffff7ec539d
>    0x7ffff7ec539d:	add    $0x10,%rsp
>    0x7ffff7ec53a1:	jmpq   0x7ffff7ec53b7   (3d)
>    0x7ffff7ec53a6:	push   %rsp
>    0x7ffff7ec53a7:	pushq  $0x0
>    0x7ffff7ec53ac:	push   %rax
>    0x7ffff7ec53ad:	pushq  $0x240
>    0x7ffff7ec53b2:	callq  0x7ffff7f41420   (3a)
>    0x7ffff7ec53b7:	callq  0x7ffff7ec5554   (3b)

This code is generated by CodeGenerator::visitCallGeneric(LCallGeneric *call)

    masm.callIon(objreg);
    masm.adjustStack(prefixGarbage - unusedStack);
    masm.jump(&end);

    {
        masm.bind(&invoke);
        masm.freeStack(unusedStack);
        pushArg(StackPointer);                 // argv.
        pushArg(Imm32(call->bytecodeArgc()));  // argc.
        pushArg(calleereg);                    // JSFunction *.
        callVM(InvokeFunctionInfo, call);
        masm.reserveStack(unusedStack);
    }

    masm.bind(&end);

Where unusedStack is 0.  The second call is created by the invalidation which is patching the OsiPoint.
(Reporter)

Comment 4

5 years ago
Created attachment 625326 [details]
135-line testcase
Attachment #625272 - Attachment is obsolete: true
Seeing this as well, and causing lots of heap crashes that cannot be determined to be dups automatically.
Whiteboard: [fuzzblocker]
(Assignee)

Updated

5 years ago
Assignee: general → dvander
Status: NEW → ASSIGNED
(Assignee)

Comment 6

5 years ago
Created attachment 625833 [details] [diff] [review]
fix
Attachment #625833 - Flags: review?(nicolas.b.pierron)
Attachment #625833 - Flags: review?(nicolas.b.pierron) → review+
(Assignee)

Comment 7

5 years ago
http://hg.mozilla.org/projects/ionmonkey/rev/d80602d38aa8
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
JSBugMon: This bug has been automatically verified fixed.
Status: RESOLVED → VERIFIED
Group: core-security
(Reporter)

Comment 9

5 years ago
Large fragile testcase -> in-testsuite-
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.