1882423 - [meta] Crash in [@ js::IsProxy] from various code paths

Status: NEW

(Core :: JavaScript Engine, defect, P3)

Description

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

Crash report: https://crash-stats.mozilla.org/report/index/977a98d1-2a4d-4b20-b422-532440240226

Reason: EXCEPTION_ACCESS_VIOLATION_READ

Top 10 frames of crashing thread:

0  xul.dll  js::IsProxy  js/public/Proxy.h:384
0  xul.dll  js::IsWrapper  js/public/Wrapper.h:393
0  xul.dll  js::IsCrossCompartmentWrapper  js/public/Wrapper.h:397
0  xul.dll  JS::GetNonCCWObjectGlobal  js/src/jsapi.cpp:1192
0  xul.dll  xpc::NativeGlobal  js/xpconnect/wrappers/WrapperFactory.cpp:787
1  xul.dll  mozilla::dom::AutoJSAPI::Init  dom/script/ScriptSettings.cpp:431
1  xul.dll  nsMessageManagerScriptExecutor::TryCacheLoadAndCompileScript  dom/base/nsFrameMessageManager.cpp:1273
1  xul.dll  nsMessageManagerScriptExecutor::LoadScriptInternal  dom/base/nsFrameMessageManager.cpp:1200
2  xul.dll  mozilla::dom::ContentProcessMessageManager::LoadScript  dom/base/ContentProcessMessageManager.cpp:119
3  xul.dll  mozilla::dom::ContentChild::RecvLoadProcessScript  dom/ipc/ContentChild.cpp:2320

By querying Nightly crashes reported within the last 2 months, here are some insights about the signature:

• First crash report: 2023-12-28
• Process type: Multiple distinct types
• Is startup crash: No
• Has user comments: No
• Is null crash: Yes - 14 out of 25 crashes happened on null or near null memory address

Comment 1

[Matthew Gaudet (he/him) [:mgaudet]]

So, looking at the stack here I actually think this is an implementation error in ContentProcessMessageManager::LoadScript.

The problem is that this function calls GetOrCreateWrapper, which is a fallible operation, but never actually checks to see if the wrapper allocation succeeded. If wrapper allocation fails here, then the next point where we dereference the pointer is the stack highlighted here.

Comment 2

[Andrew McCreight [:mccr8]]

That makes sense for the crash in comment 0. This signature is pretty bad, unfortunately, so I'm not sure how common that specific crash is. I looked at the most common proto signatures for this crash and didn't see any other instances of message manager thing, at least on release.

Comment 3

[Jens Stutte [:jstutte]]

This signature is pretty bad, unfortunately, so I'm not sure how common that specific crash is.

Yeah, this looks a bit like a meta crash bug with many different causes. I'd propose to remove the ContentProcessMessageManager::LoadScript from the title here and move it to some more general component while filing new bugs for concrete things to improve like this.

Comment 4

[Jens Stutte [:jstutte]]

I hope it is fair to keep the signature in the Javascript world, even though reasons might be manifold.

Comment 5

[Andrew McCreight [:mccr8]]

We could add a bunch of these JS predicate/getter helpers to the skip list though I'm not sure how useful the result would be. The volume here is just not that high.

Comment 6

[Matthew Gaudet (he/him) [:mgaudet]]

Yeah, most of these crashes don't have much actionable; the signature covers a whole range of things.