Closed Bug 583672 Opened 11 years ago Closed 11 years ago

JM: "Assertion failure: masm.differenceBetween(pic.storeBack, dbgInlineStoreType)-4 == SETPROP_INLINE_STORE_CONST_TYPE,"

Categories

(Core :: JavaScript Engine, defect)

x86
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: gkw, Unassigned)

References

Details

(Keywords: assertion, regression, testcase)

Attachments

(1 file, 2 obsolete files)

(function () {
    function f()
    {
        this.y = w
        this.y = (void 0)
        Object
    }
    for (a in [0, 0, 0, 0])
    {
        new f
    }
    let w = {}
})()

crashes js debug and opt shell on JM tip with -m at js::mjit::JaegerShot


Program received signal SIGSEGV, Segmentation fault.
0xf76a85ac in ?? ()
(gdb) bt
#0  0xf76a85ac in ?? ()
#1  0x08212004 in js::mjit::JaegerShot (cx=0x833bcf0) at ../methodjit/MethodJIT.cpp:696
#2  0x0827225f in js::Interpret (cx=0x833bcf0, entryFrame=0xf77b0120, inlineCallCount=2) at ../jsinterp.cpp:4833
#3  0x081d7b9a in js::MonitorTracePoint (cx=0x833bcf0, inlineCallCount=@0xffffcfdc, blacklist=@0xffffcfef) at ../jstracer.cpp:16315
#4  0x08257812 in RunTracer (f=..., mic=...) at ../methodjit/InvokeHelpers.cpp:784
#5  0x08257beb in js::mjit::stubs::InvokeTracer (f=..., index=0) at ../methodjit/InvokeHelpers.cpp:895
#6  0xf76a850c in ?? ()
#7  0x08212004 in js::mjit::JaegerShot (cx=0x833bcf0) at ../methodjit/MethodJIT.cpp:696
#8  0x080d803c in js::RunScript (cx=0x833bcf0, script=0x8340e28, fun=0x0, scopeChain=0xf7502000) at ../jsinterp.cpp:461
#9  0x080d8e35 in js::Execute (cx=0x833bcf0, chain=0xf7502000, script=0x8340e28, down=0x0, flags=0, result=0xffffd200) at ../jsinterp.cpp:949
#10 0x0806f778 in JS_ExecuteScript (cx=0x833bcf0, obj=0xf7502000, script=0x8340e28, rval=0xffffd200) at ../jsapi.cpp:4736
#11 0x0804c167 in Process (cx=0x833bcf0, obj=0xf7502000, filename=0x0, forceTTY=0) at ../../shell/js.cpp:533
#12 0x0804ccf9 in ProcessArgs (cx=0x833bcf0, obj=0xf7502000, argv=0xffffd408, argc=2) at ../../shell/js.cpp:860
#13 0x08055314 in shell (cx=0x833bcf0, argc=2, argv=0xffffd408, envp=0xffffd414) at ../../shell/js.cpp:4981
#14 0x08055430 in main (argc=2, argv=0xffffd408, envp=0xffffd414) at ../../shell/js.cpp:5077
(gdb) x/i $eip
=> 0xf76a85ac:	invd
... where tip is JM changeset 951a3dbd5541.
With the latest tip, 2ee92d697741, the crash is different:

Assertion failure: masm.differenceBetween(pic.storeBack, dbgInlineStoreType)-4 == SETPROP_INLINE_STORE_CONST_TYPE, at ../methodjit/Compiler.cpp:2531
(In reply to comment #2)
> With the latest tip, 2ee92d697741, the crash is different:
> 
> Assertion failure: masm.differenceBetween(pic.storeBack, dbgInlineStoreType)-4
> == SETPROP_INLINE_STORE_CONST_TYPE, at ../methodjit/Compiler.cpp:2531

Yep.
Keywords: crashassertion
Summary: JM: Crash [@ js::mjit::JaegerShot] → JM: "Assertion failure: masm.differenceBetween(pic.storeBack, dbgInlineStoreType)-4 == SETPROP_INLINE_STORE_CONST_TYPE,"
If SETPROP encounters a constant Undefined value in the inline path, it calls storeValue() which optimizes away the payload store, since Undefined values do not bear payloads.

But then during repatching, the type repatch target is not at SETPROP_INLINE_CONST_TYPE, but rather SETPROP_INLINE_CONST_DATA, which is very unintuitive. This is the source of the SIGSEGV.

This patch removes the Undefined store optimization for PICs, so everything functions the way you'd expect.
Attachment #462004 - Flags: review?(dvander)
Comment on attachment 462004 [details] [diff] [review]
Remove Undefined optimization for PICs.

This bug and your assertion patch could not have been timed better. Nice work.
Attachment #462004 - Flags: review?(dvander) → review+
Attached patch Fix bug for PICs and MICs. (obsolete) — Splinter Review
Dvander pointed out that the same bug may occur for MICs. Indeed it does. This second version of the patch fixes the bug for both caches.
Attachment #462004 - Attachment is obsolete: true
Attachment #462008 - Flags: review?(dvander)
MICs do not have the same bug, since they patch in the reverse direction from PICs. So the type offset is still valid.

This version fixes the bug (for PICs), and includes a modified testcase to assert that MICs won't suffer from the same bug in the future.
Attachment #462008 - Attachment is obsolete: true
Attachment #462015 - Flags: review?(dvander)
Attachment #462008 - Flags: review?(dvander)
Attachment #462015 - Flags: review?(dvander) → review+
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
A testcase for this bug was automatically identified at js/src/jit-test/tests/jaeger/bug583672.js.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.