1518075 - Crash in JS::SetScriptPrivate

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, critical)

Description

[Calixte Denizet (:calixte)]

This bug was filed from the Socorro interface and is
report bp-e28c99f1-3406-4050-9aa7-220ab0190104.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll JS::SetScriptPrivate 
1 xul.dll static nsresult mozilla::dom::ExecuteCompiledScript dom/script/ScriptLoader.cpp:2451
2 xul.dll nsresult mozilla::dom::ScriptLoader::EvaluateScript dom/script/ScriptLoader.cpp:2623
3 xul.dll nsresult mozilla::dom::ScriptLoader::ProcessRequest dom/script/ScriptLoader.cpp:2185
4 xul.dll ?Run@NotifyOffThreadScriptLoadCompletedRunnable@?A0x4F5E86E9@dom@mozilla@@UEAA?AW4nsresult@@XZ$d8497f1e518fb9aee2400b22ef3826d3 dom/script/ScriptLoader.cpp:1934
5 xul.dll mozilla::SchedulerGroup::Runnable::Run xpcom/threads/SchedulerGroup.cpp:299
6 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1157
7 xul.dll NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:468
8 xul.dll mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:88
9 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:307

=============================================================

There are 58 crashes (from 40 installations) in nightly 66 starting with buildid 20190103220533. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1342012.

[1] https://hg.mozilla.org/mozilla-central/rev?node=34e605a49937

Comment 1

[Jon Coppeard (:jonco)]

This looks like a we're dereferencing a null JSScript pointer.

Comment 2

[Jon Coppeard (:jonco)]

Attachment: bug1518075-check-script

The crash is happening when LoadedScript::AssociateWithScript calls JS::SetScriptPrivate with a null script.

I think this is happening because ExecutionContext methods will return NS_OK without doing anything if scripting is disabled:

https://searchfox.org/mozilla-central/source/dom/base/nsJSUtils.cpp#146

The patch just adds a null check.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4db1d37ef3bb
Add check for null script because compilation can 'succeed' if scripting is disabled r=smaug

Comment 4

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/4db1d37ef3bb

Comment 5

[Calixte Denizet (:calixte)]

The volume of crashes really decreased but there are still some crashes.
:jonco, should I open a new bug for them ?

Comment 6

[Jon Coppeard (:jonco)]

Attachment: bug1518075-check-script-2

Ah, it seems I missed another place where this can happen.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d323b050b0ce
Add another check for null script because compilation can 'succeed' if scripting is disabled r=smaug

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b1f8fe42b1a8
Fix rooting hazard r=me on a CLOSED TREE

Comment 9

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/d323b050b0ce
https://hg.mozilla.org/mozilla-central/rev/b1f8fe42b1a8