1517540 - Crash in mozilla::perfecthash::Lookup<T>

Status: RESOLVED WORKSFORME

(Core :: DOM: Bindings (WebIDL), defect, P3)

Description

[Marcia Knous [:marcia]]

This bug was filed from the Socorro interface and is
report bp-bf87b378-b595-4056-bf14-732300190103.
=============================================================

Small volume regression which goes back to when nightly was in 65: https://bit.ly/2Rth9Fb. Looks as if code was touched in Bug 1501124. ni on nika

All the crashes except the Android crash have crash reason EXCEPTION_ACCESS_VIOLATION_READ

Top 10 frames of crashing thread:

0 xul.dll mozilla::perfecthash::Lookup<char16_t, unsigned short, 256, mozilla::dom::WebIDLNameTableEntry, 745> xpcom/ds/PerfectHash.h:43
1 xul.dll mozilla::dom::WebIDLGlobalNameHash::GetEntry dom/bindings/RegisterBindings.cpp:6628
2 xul.dll mozilla::dom::WebIDLGlobalNameHash::ResolveForSystemGlobal dom/bindings/WebIDLGlobalNameHash.cpp:225
3 xul.dll BackstagePass::Resolve js/xpconnect/src/XPCRuntimeService.cpp:57
4 xul.dll XPC_WN_Helper_Resolve js/xpconnect/src/XPCWrappedNativeJSOps.cpp:790
5 xul.dll js::LookupProperty js/src/vm/JSObject.cpp:2405
6 xul.dll js::LookupName js/src/vm/JSObject.cpp:2414
7 xul.dll bool js::GetEnvironmentName<js::GetNameMode::Normal> js/src/vm/Interpreter-inl.h:252
8 xul.dll static bool js::jit::DoGetNameFallback js/src/jit/BaselineIC.cpp:2610
9  @0x34fe6686075 

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

Comment 1

[Nika Layzell [:nika] (ni? for response)]

Hmm, interesting. I don't know how that access could possibly be out of bounds, given that the crash is a read from a statically allocated table of fixed size after performing a modulo operation.

These all appear to be using the wide character lookups, so perhaps this has something to do with that? I'm not sure what exactly the impact would be however. Perhaps I should explicitly use two overloads of the Hash method to make sure we follow explicitly what types are used for every subexpression in case there is some signed-ness issue, although reading the code again I can't see it.

One interesting thing to note is that these errors seem to all be fairly highly aligned. The crash addresses (except for the Android one) all appear to have at least 3 trailing '0's, I'm also not sure what exactly to make of that.

It might be useful to take a look at the minidump for one of these, if that's avaliable. I don't know if it will have enough info though.

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

The volume of this crash is very low and as such we're marking this as wontfix for 65.

Comment 3

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

Not seeing any crashes yet in beta 66 or in 67. This may be fixed or just a somewhat rare crash.

Comment 4

[Marcia Knous [:marcia]]

Closing this one out as WFM based on the fact there is very trivial volume in Fennec and no crashes at all in 69 Desktop.