1920282 - Assertion failure: observing, at debugger/Debugger.cpp:3320

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: debug stack

var x = newGlobal({ newCompartment: true });
Debugger(x).nativeTracing = 1;
Debugger(x).collectCoverageInfo = true;
[].y = x + 0;
Debugger(x).collectCoverageInfo = false;

3320        MOZ_ASSERT_IF(scripts[i]->isDebuggee(), observing);
(gdb) bt
#0  UpdateExecutionObservabilityOfScriptsInZone (cx=cx@entry=0x7ffff6f36200, zone=0x7ffff63ad000, obs=..., observing=observing@entry=js::DebugAPI::NotObserving) at /home/ubu32gx500/trees/mozilla-central/js/src/debugger/Debugger.cpp:3320                                                                              
#1  0x00005555579fa1e4 in js::Debugger::updateExecutionObservabilityOfScripts (cx=cx@entry=0x7ffff6f36200, obs=..., observing=observing@entry=js::DebugAPI::NotObserving) at /home/ubu32gx500/trees/mozilla-central/js/src/debugger/Debugger.cpp:3353
#2  0x00005555579fc0fe in js::Debugger::updateExecutionObservability (cx=0x7ffff6f36200, obs=..., observing=js::DebugAPI::NotObserving) at /home/ubu32gx500/trees/mozilla-central/js/src/debugger/Debugger.cpp:3425
#3  js::Debugger::updateObservesCoverageOnDebuggees (this=<optimized out>, cx=0x7ffff6f36200, observing=js::DebugAPI::NotObserving) at /home/ubu32gx500/trees/mozilla-central/js/src/debugger/Debugger.cpp:3592
#4  0x0000555557a03cb1 in js::Debugger::CallData::setCollectCoverageInfo (this=this@entry=0x7fffffffc570) at /home/ubu32gx500/trees/mozilla-central/js/src/debugger/Debugger.cpp:4657
#5  0x0000555557a32eb7 in js::Debugger::CallData::ToNative<&js::Debugger::CallData::setCollectCoverageInfo> (cx=cx@entry=0x7ffff6f36200, argc=<optimized out>, vp=<optimized out>) at /home/ubu32gx500/trees/mozilla-central/js/src/debugger/Debugger.cpp:4281
#6  0x00005555572c9515 in CallJSNative (cx=cx@entry=0x7ffff6f36200, native=0x555557a32e20 <js::Debugger::CallData::ToNative<&js::Debugger::CallData::setCollectCoverageInfo>(JSContext*, unsigned int, JS::Value*)>, reason=reason@entry=js::CallReason::Setter, args=...) at /home/ubu32gx500/trees/mozilla-central/js/src/vm/Interpreter.cpp:518
/snip

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/87e647b66f2e
user:        alexical
date:        Mon Aug 05 15:44:21 2024 +0000
summary:     Bug 1906719 - Implement a native backend for tracing JS execution r=arai

Run with --fuzzing-safe --no-threads --ion-eager, compile with AR=ar sh ../configure --enable-debug --enable-debug-symbols --with-ccache --enable-nspr-build --enable-ctypes --enable-gczeal --enable-rust-simd --disable-tests, tested on m-c rev b5de9d1c0820.

Setting s-s just in case. Alex, is bug 1906719 a likely regressor?

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1906719

Comment 2

[Tooru Fujisawa [:arai]]

Thanks!
The nativeTracing debugger API is getting removed in https://phabricator.services.mozilla.com/D221914 , as part of replacing the functionality with C++ API, in bug 1911021,
but the same might happen with the C++ API as well.
I'll look into this next week

Comment 3

[Steven DeTar [:sdetar]]

Moving the NI to arai since they seem to be looking into this.

Comment 4

[Tooru Fujisawa [:arai]]

Confirmed the same thing happens with the C++ API.
We should have a testing function in JS shell and test the behavior there.

Comment 5

[Tooru Fujisawa [:arai]]

So, the problem here is that both execution tracer and the code coverage want to flip the execution observability flag, and both of them overwrite the flag set by other.

Possible options would be either:

• (A) Correctly set the flag based on "OR" of those features
• (B) Make them mutually exclusive

unless there's any need to use both of them at once, making mutually exclusive would be easier and will make the situation simpler.

Comment 6

[Tooru Fujisawa [:arai]]

Unless I'm missing something, this doesn't result in any security flaw, but it just can result in some data being missing from the coverage result or the execution trace.
But let's keep this hidden for now.

Comment 7

[Tooru Fujisawa [:arai]]

Attachment: Bug 1920282 - Part 1: Make the execution tracing ang the code coverage mutualy exclusive. r?jandem!

Comment 8

[Tooru Fujisawa [:arai]]

Attachment: Bug 1920282 - Part 2: Add tests. r?jandem!

Comment 9

[Andrew McCreight [:mccr8]]

I marked this sec-other because code coverage and maybe execution tracing aren't things that a regular user would encounter. I'm leaving it hidden because arai requested that.

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1906719

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/47f451944a7a
Part 1: Make the execution tracing ang the code coverage mutualy exclusive. r=jandem
https://hg.mozilla.org/integration/autoland/rev/e753f407c791
Part 2: Add tests. r=jandem

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/47f451944a7a
https://hg.mozilla.org/mozilla-central/rev/e753f407c791