Closed Bug 1518075 Opened 8 months ago Closed 7 months ago

Crash in JS::SetScriptPrivate

Categories

(Core :: JavaScript Engine, defect, critical)

Unspecified
Windows 10
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox-esr60 --- unaffected
firefox64 --- unaffected
firefox65 --- unaffected
firefox66 --- fixed

People

(Reporter: calixte, Assigned: jonco)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(2 files)

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
Flags: needinfo?(jcoppeard)
This looks like a we're dereferencing a null JSScript pointer.
Assignee: nobody → jcoppeard
Flags: needinfo?(jcoppeard)
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.
Attachment #9034710 - Flags: review?(bugs)
Attachment #9034710 - Flags: review?(bugs) → review+
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4db1d37ef3bb
Add check for null script because compilation can 'succeed' if scripting is disabled r=smaug
Status: NEW → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66

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

Status: RESOLVED → REOPENED
Flags: needinfo?(jcoppeard)
Resolution: FIXED → ---

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

Flags: needinfo?(jcoppeard)
Attachment #9037563 - Flags: review?(bugs)
Attachment #9037563 - Flags: review?(bugs) → review+
Pushed by jcoppeard@mozilla.com:
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
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b1f8fe42b1a8
Fix rooting hazard r=me on a CLOSED TREE
Status: REOPENED → RESOLVED
Closed: 8 months ago7 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.