1874293 - Assertion failure: false (We had an exception; we should not have), at /builds/worker/checkouts/gecko/dom/script/ScriptSettings.cpp:384

Status: VERIFIED FIXED

(Core :: Audio/Video: Web Codecs, defect, P2)

Description

[Tyson Smith [:tsmith]]

Attachment: testcase.zip

Found while fuzzing m-c 20231218-1d2ed0b29473 (--enable-debug --enable-fuzzing)

To reproduce via Grizzly Replay:

$ pip install fuzzfetch grizzly-framework
$ python -m fuzzfetch -d --fuzzing -n firefox
$ python -m grizzly.replay.bugzilla ./firefox/firefox <bugid>

Assertion failure: false (We had an exception; we should not have), at /builds/worker/checkouts/gecko/dom/script/ScriptSettings.cpp:384

#0 0x7f23b650e3f6 in mozilla::dom::AutoJSAPI::InitInternal(nsIGlobalObject*, JSObject*, JSContext*, bool) /builds/worker/checkouts/gecko/dom/script/ScriptSettings.cpp:384:5
#1 0x7f23b64eaab5 in mozilla::dom::AutoEntryScript::AutoEntryScript(nsIGlobalObject*, char const*, bool) /builds/worker/checkouts/gecko/dom/script/AutoEntryScript.cpp:53:7
#2 0x7f23b13af493 in void mozilla::dom::Promise::MaybeSomething<mozilla::ErrorResult>(mozilla::ErrorResult&&, void (mozilla::dom::Promise::*)(JSContext*, JS::Handle<JS::Value>)) /builds/worker/workspace/obj-build/dist/include/mozilla/dom/Promise.h:426:21
#3 0x7f23b2f57f67 in MaybeReject /builds/worker/workspace/obj-build/dist/include/mozilla/dom/Promise.h:116:5
#4 0x7f23b2f57f67 in mozilla::dom::Promise::MaybeRejectWithTypeError(nsTSubstring<char> const&) /builds/worker/workspace/obj-build/dist/include/mozilla/dom/Promise.h:154:5
#5 0x7f23b56879cc in MaybeRejectWithTypeError<35> /builds/worker/workspace/obj-build/dist/include/mozilla/dom/Promise.h:159:5
#6 0x7f23b56879cc in mozilla::dom::VideoDecoder::IsConfigSupported(mozilla::dom::GlobalObject const&, mozilla::dom::VideoDecoderConfig const&, mozilla::ErrorResult&) /builds/worker/checkouts/gecko/dom/media/webcodecs/VideoDecoder.cpp:905:8
#7 0x7f23b3cc19a7 in mozilla::dom::VideoDecoder_Binding::isConfigSupported(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/obj-build/dom/bindings/./VideoDecoderBinding.cpp:1114:39
#8 0x7f23b43acaf1 in mozilla::dom::StaticMethodPromiseWrapper(JSContext*, unsigned int, JS::Value*) /builds/worker/checkouts/gecko/dom/bindings/BindingUtils.cpp:3297:13
#9 0x7f23b88d3d74 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:479:13
#10 0x7f23b88d36cb in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:573:12
#11 0x7f23b88e2fc8 in CallFromStack /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:645:10
#12 0x7f23b88e2fc8 in js::Interpret(JSContext*, js::RunState&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:3060:16
#13 0x7f23b88d2c52 in js::RunScript(JSContext*, js::RunState&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:451:13
#14 0x7f23b88d36e8 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:605:13
#15 0x7f23b88d499d in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:672:8
#16 0x7f23b8c3aee7 in js::CallSelfHostedFunction(JSContext*, JS::Handle<js::PropertyName*>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/checkouts/gecko/js/src/vm/SelfHosting.cpp:1520:10
#17 0x7f23b89968d0 in AsyncFunctionResume(JSContext*, JS::Handle<js::AsyncFunctionGeneratorObject*>, ResumeKind, JS::Handle<JS::Value>) /builds/worker/checkouts/gecko/js/src/vm/AsyncFunction.cpp:149:8
#18 0x7f23b8ba81f3 in AsyncFunctionPromiseReactionJob /builds/worker/checkouts/gecko/js/src/builtin/Promise.cpp:2115:12
#19 0x7f23b8ba81f3 in PromiseReactionJob(JSContext*, unsigned int, JS::Value*) /builds/worker/checkouts/gecko/js/src/builtin/Promise.cpp:2178:12
#20 0x7f23b88d3d74 in CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:479:13
#21 0x7f23b88d36cb in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:573:12
#22 0x7f23b88d499d in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) /builds/worker/checkouts/gecko/js/src/vm/Interpreter.cpp:672:8
#23 0x7f23b89c75d4 in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/worker/checkouts/gecko/js/src/vm/CallAndConstruct.cpp:119:10
#24 0x7f23b36d92bc in mozilla::dom::PromiseJobCallback::Call(mozilla::dom::BindingCallContext&, JS::Handle<JS::Value>, mozilla::ErrorResult&) /builds/worker/workspace/obj-build/dom/bindings/./PromiseBinding.cpp:83:8
#25 0x7f23b1142275 in mozilla::dom::PromiseJobCallback::Call(mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JS::Realm*) /builds/worker/workspace/obj-build/dist/include/mozilla/dom/PromiseBinding.h:198:12
#26 0x7f23b1141b75 in Call /builds/worker/workspace/obj-build/dist/include/mozilla/dom/PromiseBinding.h:211:12
#27 0x7f23b1141b75 in mozilla::PromiseJobRunnable::Run(mozilla::AutoSlowOperation&) /builds/worker/checkouts/gecko/xpcom/base/CycleCollectedJSContext.cpp:210:18
#28 0x7f23b112de28 in mozilla::CycleCollectedJSContext::PerformMicroTaskCheckPoint(bool) /builds/worker/checkouts/gecko/xpcom/base/CycleCollectedJSContext.cpp:712:17
#29 0x7f23b112ee49 in mozilla::CycleCollectedJSContext::AfterProcessTask(unsigned int) /builds/worker/checkouts/gecko/xpcom/base/CycleCollectedJSContext.cpp:499:3
#30 0x7f23b2105c16 in XPCJSContext::AfterProcessTask(unsigned int) /builds/worker/checkouts/gecko/js/xpconnect/src/XPCJSContext.cpp:1494:28
#31 0x7f23b12674b3 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/checkouts/gecko/xpcom/threads/nsThread.cpp:1237:24
#32 0x7f23b126e1bd in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/checkouts/gecko/xpcom/threads/nsThreadUtils.cpp:480:10
#33 0x7f23b1f41f93 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:107:5
#34 0x7f23b1e5b6b1 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:363:3
#35 0x7f23b1e5b6b1 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:345:3
#36 0x7f23b6792748 in nsBaseAppShell::Run() /builds/worker/checkouts/gecko/widget/nsBaseAppShell.cpp:148:27
#37 0x7f23b684f698 in nsAppShell::Run() /builds/worker/checkouts/gecko/widget/gtk/nsAppShell.cpp:470:33
#38 0x7f23b869a04b in XRE_RunAppShell() /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:721:20
#39 0x7f23b1f42ec6 in mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) /builds/worker/checkouts/gecko/ipc/glue/MessagePump.cpp:235:9
#40 0x7f23b1e5b6b1 in RunHandler /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:363:3
#41 0x7f23b1e5b6b1 in MessageLoop::Run() /builds/worker/checkouts/gecko/ipc/chromium/src/base/message_loop.cc:345:3
#42 0x7f23b86998b2 in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/checkouts/gecko/toolkit/xre/nsEmbedFunctions.cpp:656:34
#43 0x55bc77bf1156 in content_process_main /builds/worker/checkouts/gecko/browser/app/../../ipc/contentproc/plugin-container.cpp:57:28
#44 0x55bc77bf1156 in main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:375:18
#45 0x7f23c5829d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#46 0x7f23c5829e3f in __libc_start_main csu/../csu/libc-start.c:392:3
#47 0x55bc77bc6e88 in _start (/home/user/workspace/browsers/m-c-20240109162901-fuzzing-debug/firefox-bin+0x58e88) (BuildId: e4c62efaf5851b0d60578cde9670049ea317e982)

Comment 1

[Bugmon [:jkratzer for issues]]

Verified bug as reproducible on mozilla-central 20240111210611-44d7819ed242.
The bug appears to have been introduced in the following build range:

Start: 969e577218dca8273024d399b50d5fe18b9dfeff (20230810212049)
End: 05da0ca6db46baad5f6e45489d80d5097053bb42 (20230811030844)
Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=969e577218dca8273024d399b50d5fe18b9dfeff&tochange=05da0ca6db46baad5f6e45489d80d5097053bb42

Comment 2

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

This bug has been marked as a regression. Setting status flag for Nightly to affected.

Comment 3

[Bob Hood [:bhood]]

Andrew, can we get a Severity applied to this, please?

Also, soft freeze for Fx123 is this Thursday. Is this something we feel comfortable letting go to Beta?

Comment 4

[Andrew McCreight [:mccr8]]

I think this is an issue with VideoDecoder::IsConfigSupported(). It looks like that block of code has no coverage. Maybe in the if (r.isErr()) { block the aRv.Throw(e); needs to happen before the MaybeRejectWithTypeError? Somebody in the DOM channel on Element would know better than me what this means.

In any event, the regression range indicates this regressed in August 2023, so I think it is fine if it isn't immediately fixed. I'm guessing bug 1846102 from that range, although this particular block of code in IsConfigSupported is a little older, having landed in bug 1831451. Maybe chunmin can take a look.

Comment 5

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

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

Comment 6

[Peter Van der Beken [:peterv]]

CloneBuffer is probably leaving exceptions on the JSContext. Both CloneBuffer and CloneConfiguration should probably take an ErrorResult instead of returning a nsresult (through Result<Ok, nsresult>), as otherwise you can't return the exception to the caller. CloneBuffer can then move the exception through StealExceptionFromJSContext to the ErrorResult after any calls to JSAPI.

This looks very underspecified in the spec. For one, I don't think any spec defines what assign a copy of config[m] to clone[m] means. And either that can throw exceptions, and it should be specified what needs to happen, or it doesn't and we need to figure out what to do in our implementation.

Comment 7

[Paul Adenot (:padenot)]

Thanks both for the analysis here.

I think we want to align with Chrome, that has a behaviour that I find sensible. Simplifying the test case to:

var buf = new ArrayBuffer(4);
buf.transferToFixedLength()
VideoDecoder.isConfigSupported({"codec": "፝a", "description":buf})

it says:

Uncaught (in promise) TypeError: Failed to execute 'isConfigSupported' on 'VideoDecoder': description is detached.

I've filed https://github.com/w3c/webcodecs/issues/762 to make the spec clear, and I'll write a patch for our implementation to align.

Comment 8

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

:chunmin, since you are the author of the regressor, bug 1846102, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 9

[Paul Adenot (:padenot)]

Hm no this looks off.

Comment 10

[Paul Adenot (:padenot)]

Attachment: Bug 1874293 - Properly propagate errors when the description is detached in isConfigSupported. r?peterv

Comment 11

[Bob Hood [:bhood]]

Paul, could you please assign Priority/Severity ratings to this report when you get a chance?

Comment 12

[Paul Adenot (:padenot)]

https://github.com/w3c/webcodecs/pull/764 is the spec fix for this.

Comment 13

[Frederik Braun [:freddy]]

This is being tracked for Firefox 124 still. Now that the spec change has merged, is there anything else to wait for Paul?

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/019488709104
Properly propagate errors when the description is detached in isConfigSupported. r=peterv

Comment 15

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/45057 for changes under testing/web-platform/tests

Comment 16

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/019488709104

Comment 17

[Bugmon [:jkratzer for issues]]

Verified bug as fixed on rev mozilla-central 20240312214346-019488709104.
Removing bugmon keyword as no further action possible. Please review the bug and re-add the keyword for further analysis.

Comment 18

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot