1316829 - Hit MOZ_CRASH(invalid scalar type) at jsfriendapi.h:1552

Status: VERIFIED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Carsten Book [:Tomcat]]

Attachment: bughunter stack

found by bughunter and reproduced on latest tinderbox windows trunk debug builds.

Steps to reproduce:
-> Load https://www.revolvermaps.com/?target=enlarge&i=7ks0jl7f453&dm=8
--> Crash on Load 

Nightly only so regression from bug 1015798 ?

Hit MOZ_CRASH(invalid scalar type) at c:\builds\moz2_slave\m-cen-w32-d-000000000000000000\build\src\obj-firefox\dist\include\jsfriendapi.h:1552
#01: mozilla_dump_image[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x166a69b]
#02: mozilla_dump_image[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x1659a93]
#03: mozilla_dump_image[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x1329459]
#04: mozilla_dump_image[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x15e5a6c]
#05: mozilla::scache::PathifyURI[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x2d32c3b]
#06: mozilla::scache::PathifyURI[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x2d3c50c]
#07: mozilla::scache::PathifyURI[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x2d3c15f]
#08: mozilla::scache::PathifyURI[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x31b7762]
#09: ??? (???:???)
#10: ??? (???:???)
#11: ??? (???:???)
#12: mozilla::scache::PathifyURI[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x31baf58]
#13: mozilla::scache::PathifyURI[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x31bb580]
#14: mozilla::scache::PathifyURI[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x2d42af6]
#15: mozilla::scache::PathifyURI[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x2d4a637]
#16: mozilla::scache::PathifyURI[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x2d3c581]
#17: mozilla::scache::PathifyURI[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x2d3c15f]
#18: mozilla::scache::PathifyURI[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x2d324f5]
#19: mozilla::scache::PathifyURI[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x2bb44ff]
#20: mozilla_dump_image[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x14665e7]
#21: mozilla_dump_image[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x16f2eeb]
#22: mozilla_dump_image[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x17007dd]
#23: mozilla_dump_image[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x1701039]
#24: mozilla_dump_image[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x1700cdf]
#25: mozilla_dump_image[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x170006d]
#26: mozilla_dump_image[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x170013b]
#27: mozilla_dump_image[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x170113a]
#28: mozilla_dump_image[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x16fb649]
#29: mozilla_dump_image[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x16fbbd0]
#30: mozilla_dump_image[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x16e27c7]
#31: mozilla_dump_image[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x1d0adbd]
#32: mozilla_dump_image[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x1d09a7e]
#33: mozilla_dump_image[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x1d110e5]
#34: mozilla::net::LoadInfo::TriggeringPrincipal[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x29bade]
#35: mozilla::net::LoadInfo::TriggeringPrincipal[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x4dd867]
#36: mozilla::net::LoadInfo::TriggeringPrincipal[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x4a1d59]
#37: mozilla::net::LoadInfo::TriggeringPrincipal[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x3f0e4b]
#38: mozilla::net::LoadInfo::TriggeringPrincipal[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x49558f]
#39: mozilla::net::LoadInfo::TriggeringPrincipal[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x4a1941]
#40: mozilla::net::LoadInfo::TriggeringPrincipal[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x4ad77f]
#41: mozilla::net::LoadInfo::TriggeringPrincipal[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x4a8593]
#42: mozilla::net::LoadInfo::TriggeringPrincipal[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x6440c7]
#43: mozilla::net::LoadInfo::TriggeringPrincipal[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x80c8b2]
#44: mozilla::net::LoadInfo::TriggeringPrincipal[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x5b3b15]
#45: mozilla::net::LoadInfo::TriggeringPrincipal[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x5b4080]
#46: mozilla::net::LoadInfo::TriggeringPrincipal[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x5bb707]
#47: mozilla::net::LoadInfo::TriggeringPrincipal[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x5bacf4]
#48: XRE_AddStaticComponent[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x1e8184]
#49: NS_StringSetIsVoid[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x211817]
#50: mozilla::net::LoadInfo::TriggeringPrincipal[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x5ba961]
#51: mozilla::net::LoadInfo::TriggeringPrincipal[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x5baa97]
#52: mozilla::net::LoadInfo::TriggeringPrincipal[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x59b09e]
#53: mozilla::net::LoadInfo::TriggeringPrincipal[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x59b056]
#54: mozilla::net::LoadInfo::TriggeringPrincipal[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x59ad9f]
#55: mozilla_dump_image[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x1d6d221]
#56: mozilla_dump_image[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x1dc21b7]
#57: XRE_RunAppShell[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x25850e4]
#58: mozilla::net::LoadInfo::TriggeringPrincipal[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x5ba9c4]
#59: mozilla::net::LoadInfo::TriggeringPrincipal[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x59b09e]
#60: mozilla::net::LoadInfo::TriggeringPrincipal[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x59b056]
#61: mozilla::net::LoadInfo::TriggeringPrincipal[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x59ad9f]
#62: XRE_InitChildProcess[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\xul.dll +0x2584c7e]
#63: ???[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\firefox.exe +0x18b5]
#64: ???[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\firefox.exe +0x1638]
#65: ???[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\firefox.exe +0x20a4]
#66: TargetNtUnmapViewOfSection[c:\bughunter\firefox-52.0a1.en-US.win32\firefox\firefox.exe +0x32bc1]
#67: BaseThreadInitThunk[C:\Windows\system32\kernel32.dll +0x4ee1c]
#68: RtlInitializeExceptionChain[C:\Windows\SYSTEM32\ntdll.dll +0x637eb]
#69: RtlInitializeExceptionChain[C:\Windows\SYSTEM32\ntdll.dll +0x637be]

Comment 1

[Carsten Book [:Tomcat]]

[Tracking Requested - why for this release]:

crashes opt and debug 

https://crash-stats.mozilla.com/report/index/9e1f4c20-06e7-4e90-be90-637b82161111 opt crash

Comment 2

[Carsten Book [:Tomcat]]

looks like a recent regression, could you take a look ?

Comment 3

[Tom S [:evilpie]]

Attachment: gdb output

Looks like view.mTypedObj is an DataViewObject, maybe that is unexpected?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Totally expected.  I'm surprised we have no tests for that in our tree.  :(

The argument to WebGL's bufferData can be an ArrayBuffer or ArrayBufferView, where ArrayBufferView is defined as:

  typedef (Int8Array or Int16Array or Int32Array or
           Uint8Array or Uint16Array or Uint32Array or Uint8ClampedArray or
           Float32Array or Float64Array or DataView) ArrayBufferView;

So a DataView is a perfectly reasonable thing to pass to bufferData.  

Looks like bug 1313541 recently completely rejiggered how we handle validation at these webgl entrypoints and it's the new code that's crashing.

view.Type() is returning Scalar::MaxTypedArrayViewType (see JS_GetArrayBufferViewType).  Then Scalar::byteSize of course falls through to the MOZ_CRASH.  I really wish new JSAPI consumers would get review from people who know JSAPI.... :(

Anyway, in terms of what the behavior should be... I have no idea.  WebGL2 adds this IDL signature:

  void bufferData(GLenum target, ArrayBufferView srcData, GLenum usage, GLuint srcOffset,
                  optional GLuint length = 0);

which is presumably what's being implemented here, but there is nothing defining its behavior and nothing documenting what the srcOffset argument means, so I have no idea what the method is _supposed_ to do.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Bug 1313541 wants to get uplifted to 51, so we should track this there too.

Comment 6

[Kelsey Gilbert [:jgilbert]]

(In reply to Boris Zbarsky [:bz] (still a bit busy) from comment #4)
> Totally expected.  I'm surprised we have no tests for that in our tree.  :(
> 
> The argument to WebGL's bufferData can be an ArrayBuffer or ArrayBufferView,
> where ArrayBufferView is defined as:
> 
>   typedef (Int8Array or Int16Array or Int32Array or
>            Uint8Array or Uint16Array or Uint32Array or Uint8ClampedArray or
>            Float32Array or Float64Array or DataView) ArrayBufferView;
> 
> So a DataView is a perfectly reasonable thing to pass to bufferData.  
> 
> Looks like bug 1313541 recently completely rejiggered how we handle
> validation at these webgl entrypoints and it's the new code that's crashing.
> 
> view.Type() is returning Scalar::MaxTypedArrayViewType (see
> JS_GetArrayBufferViewType).  Then Scalar::byteSize of course falls through
> to the MOZ_CRASH.  I really wish new JSAPI consumers would get review from
> people who know JSAPI.... :(

I talked with either :waldo or :efaust to some extent about how to do this, so it's not like it had no input from JSAPI people.

> 
> Anyway, in terms of what the behavior should be... I have no idea.  WebGL2
> adds this IDL signature:
> 
>   void bufferData(GLenum target, ArrayBufferView srcData, GLenum usage,
> GLuint srcOffset,
>                   optional GLuint length = 0);
> 
> which is presumably what's being implemented here, but there is nothing
> defining its behavior and nothing documenting what the srcOffset argument
> means, so I have no idea what the method is _supposed_ to do.

Fortunately, I know exactly what this means!

Comment 7

[Kelsey Gilbert [:jgilbert]]

Attachment: Bug 1316829 - DataViews are part of ArrayBufferView. -

Review commit: https://reviewboard.mozilla.org/r/92474/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/92474/

Comment 8

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8810002 [details]
Bug 1316829 - DataViews are part of ArrayBufferView. -

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/92474/diff/1-2/

Comment 9

[Jeff Walden [:Waldo]]

Comment on attachment 8810002 [details]
Bug 1316829 - DataViews are part of ArrayBufferView. -

https://reviewboard.mozilla.org/r/92474/#review92532

::: dom/canvas/WebGLContext.cpp:2519
(Diff revision 2)
>  ////////////////////////////////////////////////////////////////////////////////
>  
> +static inline size_t
> +SizeOfViewElem(const dom::ArrayBufferView& view)
> +{
> +    const auto& elemType = view.Type();

I'd be a lot happier if this weren't a reference and just copied, but okay.

Comment 10

[Gerry Chang [:gchang]]

Track 51+/52+ as this is related to webgl 2 issue.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e12e761d2106
DataViews are part of ArrayBufferView. - r=waldo

Comment 12

[Boris Zbarsky [:bzbarsky]]

We need this on aurora and beta too at this point.

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e12e761d2106

Comment 14

[Gerry Chang [:gchang]]

Hi :jgilbert,
could you please nominate patch for uplift to Aurora52 and Beta51?

Comment 15

[Marcia Knous [:marcia]]

Tracking 53- since this is now resolved fixed.

Comment 16

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8810002 [details]
Bug 1316829 - DataViews are part of ArrayBufferView. -

Approval Request Comment
[Feature/regressing bug #]: webgl2
[User impact if declined]:
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: 
[String/UUID change made/needed]:

Comment 17

[Gerry Chang [:gchang]]

Comment on attachment 8810002 [details]
Bug 1316829 - DataViews are part of ArrayBufferView. -

Fix an issue related to WebGL 2. Beta51+ and Aurora52+. Should be in 51 beta 2.

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/01be3fcf934f

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/97db28cecd6f

Comment 20

[Bogdan Maris, Desktop QA]

Reproduced the initial crash using Nightly tinderbox debug build from 2016-11-12. Verified that using Firefox 51 beta 10, latest Developer Edition 52.0a2, latest Nightly 53.0a1 and latest tinderbox beta debug build from 2016-12-27 across platforms (Windows 10 64-bit, Mac OS X 10.12.1 and Ubuntu 16.04 64-bit).