1573674 - Crash in [@ mozilla::CycleCollectedJSRuntime::ErrorInterceptor::interceptError]

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Liz Henry (:lizzard) (relman/hg->git project)]

This bug is for crash report bp-77c76c17-8ccc-42a2-a404-999490190812.

This signature has been showing up in low volume in 70 nightly with a little bit of a spike around 8/12 and the 20190811215021 build.

Top 9 frames of crashing thread:

0 xul.dll void mozilla::CycleCollectedJSRuntime::ErrorInterceptor::interceptError xpcom/base/CycleCollectedJSRuntime.cpp:1501
1 xul.dll js::ReportOutOfMemory js/src/vm/JSContext.cpp:308
2 xul.dll js::Atomize js/src/vm/JSAtom.cpp:1020
3 xul.dll js::NumberToAtom js/src/jsnum.cpp:1604
4 xul.dll class JSAtom* js::ToAtom<js::CanGC> js/src/vm/JSAtom.cpp
5 xul.dll bool js::ValueToId<js::CanGC> js/src/vm/JSAtom-inl.h:93
6 xul.dll js::jit::IonGetPropertyIC::update js/src/jit/IonIC.cpp:223
7  @0x3c8e944e1e 
8 xul.dll trunc 

Comment 1

[Andrew McCreight [:mccr8]]

Looks like maybe a null deref inside the ErrorInterceptor. It looks like it is Nightly-only.

Comment 3

[Jason Orendorff [:jorendorff]]

The crash reports are completely consistent. Whatever's being null-deref'd, the crash happens trying to read the field at offset 0x1a0 in 64-bit builds, 0x120 in 32-bit builds.

I don't think the aCx passed from SM to the error interceptor can be null. I think the answer must lie somewhere within the code that gets inlined here, at the top of the stack: https://hg.mozilla.org/mozilla-central/annotate/b84b36af3b1c69e8f370871bd54149c6884e48dc/xpcom/base/CycleCollectedJSRuntime.cpp#l1501 But I didn't find where any of that code has changed recently.

Since ErrorInterceptor is nightly-only and only called on errors, diagnostic asserts could find the answer here.

Comment 4

[Jan de Mooij [:jandem]]

I'm pretty sure the problem is that we're calling ErrorInterceptor::interceptError while in the atoms zone. cx->realm_ is nullptr then so we crash while trying to determine the current principal.

Comment 5

[Jan de Mooij [:jandem]]

We probably shouldn't be calling the interceptor for OOM errors.

Comment 6

[Steve Fink [:sfink] [:s:]]

Attachment: offset-416.txt

I thought this might be more useful, but it really isn't. I'm attaching a list of all 320 (!) fields in our code base at offset 0x1a0.

Ooh, wait! While eyeballing this just before posting, I happened to stumble across something very promising: nsGlobalWindowInner.mIsChrome. I'll put my money on that.

Comment 7

[Steve Fink [:sfink] [:s:]]

Oh whoops, I didn't see jandem's comments before posting.

Comment 8

[Jan de Mooij [:jandem]]

Attachment: Bug 1573674 part 1 - Move JSContext::setPendingException to the cpp file. r?Yoric!

There's a lot of code and this path is slow enough that a non-inlined call isn't
going to be an issue.

Comment 9

[Jan de Mooij [:jandem]]

Attachment: Bug 1573674 part 2 - Don't report OOM exceptions to Nightly-only error interceptor hook. r?Yoric!

Depends on D43583

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9f3f9eee5143
part 1 - Move JSContext::setPendingException to the cpp file. r=Yoric
https://hg.mozilla.org/integration/autoland/rev/1aa2c382c4ec
part 2 - Don't report OOM exceptions to Nightly-only error interceptor hook. r=Yoric

Comment 11

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/9f3f9eee5143
https://hg.mozilla.org/mozilla-central/rev/1aa2c382c4ec

Comment 12

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/releases/mozilla-beta/rev/9f3f9eee5143
https://hg.mozilla.org/releases/mozilla-beta/rev/1aa2c382c4ec