1825861 - Assertion failure: isObject(), at js/Value.h:939

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[lukas.bernhard]

Steps to reproduce:

On git commit be62edbf403e6f21e935acc7611521c09aa9a436 the attached sample crashes when invoking the js-shell as obj-x86_64-pc-linux-gnu/dist/bin/js crash.js. Bisecting did not identify a recent regression, commits from 2021 are affected as well.

function f0(a1) {
    function f3(a4, a5) {
        const v7 = this.options();
        function f8(a9, a10, a11) {
            return f3;
        }
        const v12 = f8.bind();
        v12(a5);
        v12.sameZoneAs = this;
        const v15 = this.newGlobal(v12);
        const v16 = v15.Debugger;
        const v17 = v16(f8);

        const v19 = v17.getNewestFrame(v7, f0, v15).older;
        const v20 = v19.arguments;
        const v26 = v20[0];
        v26.isExtensible(v17);

        const v27 = v26.makeDebuggeeNativeFunction(v16);

        v27.call(undefined);
        return f0;
    }
    return f3(f0, f0);
}
f0(f0);

  0x00005555573b2834 in JS::Value::toObject (this=0x7fffffff9a90)
    at obj-x86_64-pc-linux-gnu/dist/include/js/Value.h:939
#1  0x00005555573b025d in js::WrappedPtrOperations<JS::Value, JS::Rooted<JS::Value>, void>::toObject (
    this=0x7fffffff9a80) at obj-x86_64-pc-linux-gnu/dist/include/js/Value.h:1348
#2  0x00005555580920bf in js::Debugger::construct (cx=0x7ffff7430100, argc=0, vp=0x7fffffffa190)
    at js/src/debugger/Debugger.cpp:4673
#3  0x000055555757f24e in CallJSNative (cx=0x7ffff7430100, 
    native=0x555558091e30 <js::Debugger::construct(JSContext*, unsigned int, JS::Value*)>, 
    reason=js::CallReason::Call, args=...) at js/src/vm/Interpreter.cpp:459
#4  0x000055555757ea2d in js::InternalCallOrConstruct (cx=0x7ffff7430100, args=..., construct=js::NO_CONSTRUCT, 
    reason=js::CallReason::Call) at js/src/vm/Interpreter.cpp:553
#5  0x000055555757fe21 in InternalCall (cx=0x7ffff7430100, args=..., reason=js::CallReason::Call)
    at js/src/vm/Interpreter.cpp:620
#6  0x0000555557580065 in js::Call (cx=0x7ffff7430100, fval=..., thisv=..., args=..., rval=..., 
    reason=js::CallReason::Call) at js/src/vm/Interpreter.cpp:652
#7  0x0000555558155c97 in js::DebuggerObject::call (cx=0x7ffff7430100, object=..., thisv_=..., args=...)
    at js/src/debugger/Object.cpp:2434
#8  0x00005555581552d6 in js::DebuggerObject::CallData::callMethod (this=0x7fffffffa3d0)
    at js/src/debugger/Object.cpp:939
#9  0x000055555816fac4 in js::DebuggerObject::CallData::ToNative<&js::DebuggerObject::CallData::callMethod> (
    cx=0x7ffff7430100, argc=1, vp=0x7fffffffa8c0) at js/src/debugger/Object.cpp:234
#10 0x000055555757f24e in CallJSNative (cx=0x7ffff7430100, 
    native=0x55555816f910 <js::DebuggerObject::CallData::ToNative<&js::DebuggerObject::CallData::callMethod>(JSContext*, unsigned int, JS::Value*)>, reason=js::CallReason::Call, args=...)
    at js/src/vm/Interpreter.cpp:459
#11 0x000055555757ea2d in js::InternalCallOrConstruct (cx=0x7ffff7430100, args=..., construct=js::NO_CONSTRUCT, 
    reason=js::CallReason::Call) at js/src/vm/Interpreter.cpp:553
#12 0x000055555757fe21 in InternalCall (cx=0x7ffff7430100, args=..., reason=js::CallReason::Call)
    at js/src/vm/Interpreter.cpp:620
#13 0x0000555557580065 in js::Call (cx=0x7ffff7430100, fval=..., thisv=..., args=..., rval=..., 
    reason=js::CallReason::Call) at js/src/vm/Interpreter.cpp:652
#14 0x0000555558001b48 in js::ForwardingProxyHandler::call (
    this=0x55555995e9c0 <js::CrossCompartmentWrapper::singleton>, cx=0x7ffff7430100, proxy=..., args=...)
    at js/src/proxy/Wrapper.cpp:168
#15 0x0000555557fd67d5 in js::CrossCompartmentWrapper::call (
    this=0x55555995e9c0 <js::CrossCompartmentWrapper::singleton>, cx=0x7ffff7430100, wrapper=..., args=...)
    at js/src/proxy/CrossCompartmentWrapper.cpp:229
#16 0x0000555557ff1401 in js::Proxy::call (cx=0x7ffff7430100, proxy=..., args=...)
    at js/src/proxy/Proxy.cpp:687
#17 0x000055555757e6ba in js::InternalCallOrConstruct (cx=0x7ffff7430100, args=..., construct=js::NO_CONSTRUCT, 
    reason=js::CallReason::Call) at js/src/vm/Interpreter.cpp:533
#18 0x000055555757fe21 in InternalCall (cx=0x7ffff7430100, args=..., reason=js::CallReason::Call)
    at js/src/vm/Interpreter.cpp:620
#19 0x000055555757fbe5 in js::CallFromStack (cx=0x7ffff7430100, args=..., reason=js::CallReason::Call)
    at js/src/vm/Interpreter.cpp:625
#20 0x00005555575705d4 in Interpret (cx=0x7ffff7430100, state=...)
    at js/src/vm/Interpreter.cpp:3368
#21 0x00005555575626b0 in js::RunScript (cx=0x7ffff7430100, state=...)
    at js/src/vm/Interpreter.cpp:431
#22 0x00005555575819dc in js::ExecuteKernel (cx=0x7ffff7430100, script=..., envChainArg=..., evalInFrame=..., 
    result=...) at js/src/vm/Interpreter.cpp:818
#23 0x0000555557582285 in js::Execute (cx=0x7ffff7430100, script=..., envChain=..., rval=...)
    at js/src/vm/Interpreter.cpp:850
#24 0x00005555577c34a6 in ExecuteScript (cx=0x7ffff7430100, envChain=..., script=..., rval=...)
    at js/src/vm/CompilationAndEvaluation.cpp:472
#25 0x00005555577c35fd in JS_ExecuteScript (cx=0x7ffff7430100, scriptArg=...)
    at js/src/vm/CompilationAndEvaluation.cpp:496
#26 0x00005555573ac4cf in RunFile (cx=0x7ffff7430100, filename=0x7fffffffea61 "crash_2023_03_31.js",
    file=0x7ffff7769020, compileMethod=CompileUtf8::DontInflate, compileOnly=false, fullParse=false)
    at js/src/shell/js.cpp:1098
#27 0x00005555573abd75 in Process (cx=0x7ffff7430100, filename=0x7fffffffea61 "crash_2023_03_31.js", forceTTY=false,
    kind=FileScript) at js/src/shell/js.cpp:1697
#28 0x0000555557385b98 in ProcessArgs (cx=0x7ffff7430100, op=0x7fffffffe480)
    at js/src/shell/js.cpp:10591
#29 0x0000555557374b53 in Shell (cx=0x7ffff7430100, op=0x7fffffffe480)
    at js/src/shell/js.cpp:10815
#30 0x000055555736fbf6 in main (argc=2, argv=0x7fffffffe6e8)
    at js/src/shell/js.cpp:11247

Comment 1

[Bryan Thrall [:bthrall]]

A simplified script that still produces the crash:

function f0() {
    const dbgConstructor = this.newGlobal({ sameZoneAs: this }).Debugger;

    const frame = dbgConstructor(this).getNewestFrame();
    const wrappedArg = frame.arguments[0];

    const wrappedDbgNative = wrappedArg.makeDebuggeeNativeFunction(dbgConstructor);

    wrappedDbgNative.call(undefined);
}
f0({});

Comment 2

[André Bargull [:anba]]

I think we should remove makeDebuggeeNativeFunction instead of trying to fix this issue. makeDebuggeeNativeFunction seems kind of scary, because it allows you to create a copy of any native function, as long as the function isn't extended (js::FunctionExtended). That means any caller of CallArgs::callee can't be sure that the current callee is actually the original function or a copy created by makeDebuggeeNativeFunction. In this specific case, js::Debugger::construct assumes the current callee is the original Debugger constructor function, which is guaranteed to have a prototype property whose value is a DebuggerPrototypeObject. But due to makeDebuggeeNativeFunction, the current callee can also by a copied function.

Without removing makeDebuggeeNativeFunction, we'd have to go through every caller of CallArgs::callee, check if the original callee isn't an extended function, and then check how the callee is actually used.

It should be okay to remove makeDebuggeeNativeFunction, because it was added only for WebReplay (bug 1556813), but WebReplay has been removed (bug 1609815).

The test case can be simplified to just:

var g = newGlobal({newCompartment: true});
var dbg = new Debugger();
var gw = dbg.addDebuggee(g);

var fn = gw.makeDebuggeeNativeFunction(Debugger);

fn.call();

Comment 3

[Jan de Mooij [:jandem]]

I agree. This API complicates invariants around native functions and if it's not actually being used we should just remove it.

Comment 5

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1825861 - Remove makeDebuggeeNativeFunction r?anba

Comment 6

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 1825861 - Simplify EnsureNativeFunction helper r?anba

Depends on D186464

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b1ddee272101
Remove makeDebuggeeNativeFunction r=anba
https://hg.mozilla.org/integration/autoland/rev/bf6dbc976a51
Simplify EnsureNativeFunction helper r=anba

Comment 8

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/b1ddee272101
https://hg.mozilla.org/mozilla-central/rev/bf6dbc976a51