1871949 - Assertion failure: !Watchtower::watchesPropertyModification(&obj->as<NativeObject>()), at jsfriendapi.cpp:473

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[lukas.bernhard]

Steps to reproduce:

On git commit 3bd65516eb9b3a9568806d846ba8c81a9402a885 the attached sample asserts in the js-shell when invoked as obj-x86_64-pc-linux-gnu/dist/bin/js --fuzzing-safe crash.js

const v10 = ``; 
const v12 = cacheEntry(v10);
function F14() {
    function f20() {
        this.addWatchtowerTarget(v12);
    }   
    f20();
}
new F14();
const o27 = { 
    "saveIncrementalBytecode": {}, 
};
evaluate(v12, o27);

#0  0x000055555824bda7 in JS::detail::SetReservedSlotWithBarrier (obj=0x39bcd4900658, slot=1, value=...)
    at js/src/jsfriendapi.cpp:472
#1  0x00005555578d7c4c in JS::SetReservedSlot (obj=0x39bcd4900658, slot=1, value=...)
    at obj-x86_64-pc-linux-gnu/dist/include/js/Object.h:91
#2  0x000055555789224c in CacheEntry_setBytecode (cx=0x7ffff662e100, cache=..., cacheOptions=..., buffer=0x7ffff766ce00 "\f", 
    length=256) at js/src/shell/js.cpp:2416
#3  0x000055555787c593 in Evaluate (cx=0x7ffff662e100, argc=2, vp=0x7ffff55ef090)
    at js/src/shell/js.cpp:2788
#4  0x0000555557a713fc in CallJSNative (cx=0x7ffff662e100, 
    native=0x55555787a4b0 <Evaluate(JSContext*, unsigned int, JS::Value*)>, reason=js::CallReason::Call, args=...)
    at js/src/vm/Interpreter.cpp:479
#5  0x0000555557a48143 in js::InternalCallOrConstruct (cx=0x7ffff662e100, args=..., construct=js::NO_CONSTRUCT, 
    reason=js::CallReason::Call) at js/src/vm/Interpreter.cpp:573
#6  0x0000555557a488c9 in InternalCall (cx=0x7ffff662e100, args=..., reason=js::CallReason::Call)
    at js/src/vm/Interpreter.cpp:640
#7  0x0000555557a48703 in js::CallFromStack (cx=0x7ffff662e100, args=..., reason=js::CallReason::Call)
    at js/src/vm/Interpreter.cpp:645
#8  0x0000555557a56f5f in js::Interpret (cx=0x7ffff662e100, state=...) at js/src/vm/Interpreter.cpp:3060
#9  0x0000555557a47b25 in MaybeEnterInterpreterTrampoline (cx=0x7ffff662e100, state=...)
    at js/src/vm/Interpreter.cpp:393
#10 0x0000555557a477d1 in js::RunScript (cx=0x7ffff662e100, state=...) at js/src/vm/Interpreter.cpp:451
#11 0x0000555557a49d11 in js::ExecuteKernel (cx=0x7ffff662e100, script=..., envChainArg=..., evalInFrame=..., result=...)
    at js/src/vm/Interpreter.cpp:838
#12 0x0000555557a4a08c in js::Execute (cx=0x7ffff662e100, script=..., envChain=..., rval=...)
    at js/src/vm/Interpreter.cpp:870
#13 0x0000555557c2769f in ExecuteScript (cx=0x7ffff662e100, envChain=..., script=..., rval=...)
    at js/src/vm/CompilationAndEvaluation.cpp:494
#14 0x0000555557c277c5 in JS_ExecuteScript (cx=0x7ffff662e100, scriptArg=...)
    at js/src/vm/CompilationAndEvaluation.cpp:518
--Type <RET> for more, q to quit, c to continue without paging--
#15 0x000055555789fe4f in RunFile (cx=0x7ffff662e100, filename=0x7ffff6606ba0 "crash_2023_12_25.js", file=0x7ffff766a4c0, 
    compileMethod=CompileUtf8::DontInflate, compileOnly=false, fullParse=false)
    at js/src/shell/js.cpp:1220
#16 0x000055555789f71c in Process (cx=0x7ffff662e100, filename=0x7ffff6606ba0 "crash_2023_12_25.js", forceTTY=false, 
    kind=FileScript) at js/src/shell/js.cpp:1800
#17 0x0000555557879291 in ProcessArgs (cx=0x7ffff662e100, op=0x7fffffffdcf0) at js/src/shell/js.cpp:10875
#18 0x0000555557867e87 in Shell (cx=0x7ffff662e100, op=0x7fffffffdcf0) at js/src/shell/js.cpp:11137
#19 0x0000555557862d44 in main (argc=3, argv=0x7fffffffdf58) at js/src/shell/js.cpp:11541

Comment 1

[Iain Ireland [:iain]]

I think this assertion needs adjustment.

Slightly reduced testcase:

const entry = cacheEntry("");
addWatchtowerTarget(entry);
evaluate(entry, {"saveIncrementalBytecode": true});

In short, we create an object with a reserved slot, use addWatchtowerTarget to hook it up with the watchtower log, and then call a function that will call JS::SetReservedSlot, which incorrectly asserts that we're not watching the object.

Matt, thoughts on the best fix here?

Comment 2

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

Hmm. This highlights that really the infrastructure to watch reserved slots does not yet exist. As a result, removing the assertion and highlighting this fact in documentation is probably the best to be done.

Comment 3

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

Attachment: Bug 1871949 - Remove assert in SetReservedSlot path. r?iain

We simply need to make it clear that we cannot currently watch a reserved slot value

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0fdc9d8910cc
Remove assert in SetReservedSlot path. r=iain

Comment 5

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/0fdc9d8910cc