Closed Bug 1316829 Opened 3 years ago Closed 3 years ago

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

Categories

(Core :: Canvas: WebGL, defect, critical)

51 Branch
All
Unspecified
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla53
Tracking Status
firefox51 + verified
firefox52 + verified
firefox53 - verified

People

(Reporter: cbook, Assigned: jgilbert)

References

(Blocks 1 open bug, )

Details

(Keywords: crash, regression)

Attachments

(3 files)

Attached file 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]
[Tracking Requested - why for this release]:

crashes opt and debug 

https://crash-stats.mozilla.com/report/index/9e1f4c20-06e7-4e90-be90-637b82161111 opt crash
looks like a recent regression, could you take a look ?
Flags: needinfo?(evilpies)
Flags: needinfo?(bzbarsky)
Attached file gdb output
Looks like view.mTypedObj is an DataViewObject, maybe that is unexpected?
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.
Blocks: 1313541
No longer blocks: 1015798
Flags: needinfo?(bzbarsky) → needinfo?(jgilbert)
Flags: needinfo?(evilpies)
Bug 1313541 wants to get uplifted to 51, so we should track this there too.
(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!
Component: JavaScript Engine → Canvas: WebGL
Flags: needinfo?(jgilbert)
Assignee: nobody → jgilbert
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.
Attachment #8810002 - Flags: review?(jwalden+bmo) → review+
Track 51+/52+ as this is related to webgl 2 issue.
Pushed by jgilbert@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e12e761d2106
DataViews are part of ArrayBufferView. - r=waldo
We need this on aurora and beta too at this point.
https://hg.mozilla.org/mozilla-central/rev/e12e761d2106
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Hi :jgilbert,
could you please nominate patch for uplift to Aurora52 and Beta51?
Flags: needinfo?(jgilbert)
Tracking 53- since this is now resolved fixed.
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]:
Flags: needinfo?(jgilbert)
Attachment #8810002 - Flags: approval-mozilla-beta?
Attachment #8810002 - Flags: approval-mozilla-aurora?
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.
Attachment #8810002 - Flags: approval-mozilla-beta?
Attachment #8810002 - Flags: approval-mozilla-beta+
Attachment #8810002 - Flags: approval-mozilla-aurora?
Attachment #8810002 - Flags: approval-mozilla-aurora+
Flags: qe-verify+
QA Contact: bogdan.maris
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).
Version: unspecified → 51 Branch
You need to log in before you can comment on or make changes to this bug.