Crash in mozilla::gl::GLBlitHelper::BlitDescriptor

RESOLVED FIXED in Firefox 57

Status

()

P1
critical
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: philipp, Assigned: daoshengmu)

Tracking

({crash, regression})

57 Branch
mozilla58
All
Windows
crash, regression
Points:
---
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox55 unaffected, firefox56 unaffected, firefox57 fixed, firefox58 fixed)

Details

(Whiteboard: [gfx-noted], crash signature)

Attachments

(1 attachment)

(Reporter)

Description

a year ago
This bug was filed from the Socorro interface and is 
report bp-7100bacc-79b0-4bb7-80c7-186fb0170929.
=============================================================
Crashing Thread (0)
Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::gl::GLBlitHelper::BlitDescriptor(mozilla::layers::SurfaceDescriptorD3D10 const&, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gl::OriginPos) 	gfx/gl/GLBlitHelperD3D.cpp:284
1 	xul.dll 	mozilla::gl::GLBlitHelper::BlitImage(mozilla::layers::GPUVideoImage*, mozilla::gfx::IntSizeTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gl::OriginPos) 	gfx/gl/GLBlitHelperD3D.cpp:193
2 	xul.dll 	mozilla::webgl::TexUnpackImage::TexOrSubImage(bool, bool, char const*, mozilla::WebGLTexture*, StrongGLenum<TexImageTargetDetails>, int, mozilla::webgl::DriverUnpackInfo const*, int, int, int, mozilla::webgl::PackingInfo const&, unsigned int* const) 	dom/canvas/TexUnpackBlob.cpp:708
3 	xul.dll 	mozilla::WebGLTexture::TexImage(char const*, StrongGLenum<TexImageTargetDetails>, int, unsigned int, mozilla::webgl::PackingInfo const&, mozilla::webgl::TexUnpackBlob const*) 	dom/canvas/WebGLTextureUpload.cpp:1306
4 	xul.dll 	mozilla::WebGLTexture::TexImage(char const*, StrongGLenum<TexImageTargetDetails>, int, unsigned int, int, int, int, int, mozilla::webgl::PackingInfo const&, mozilla::TexImageSource const&) 	dom/canvas/WebGLTextureUpload.cpp:474
5 	xul.dll 	mozilla::WebGLContext::TexImage(char const*, unsigned char, unsigned int, int, unsigned int, int, int, int, int, unsigned int, unsigned int, mozilla::TexImageSource const&) 	dom/canvas/WebGLContextTextures.cpp:394
6 	xul.dll 	mozilla::WebGLContext::TexImage2D<mozilla::dom::HTMLImageElement>(unsigned int, int, unsigned int, int, int, int, unsigned int, unsigned int, mozilla::dom::HTMLImageElement const&, mozilla::ErrorResult&) 	dom/canvas/WebGLContext.h:1178
7 	xul.dll 	mozilla::WebGLContext::TexImage2D<mozilla::dom::HTMLCanvasElement>(unsigned int, int, unsigned int, unsigned int, unsigned int, mozilla::dom::HTMLCanvasElement const&, mozilla::ErrorResult&) 	dom/canvas/WebGLContext.h:1155
8 	xul.dll 	mozilla::dom::WebGLRenderingContextBinding::texImage2D 	dom/bindings/WebGLRenderingContextBinding.cpp:13221
9 	xul.dll 	mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) 	dom/bindings/BindingUtils.cpp:3055
10 	xul.dll 	js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp:495
11 	xul.dll 	Interpret 	js/src/vm/Interpreter.cpp:3084
12 	xul.dll 	js::RunScript(JSContext*, js::RunState&) 	js/src/vm/Interpreter.cpp:435
13 	xul.dll 	js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp:513
14 	xul.dll 	js::fun_call(JSContext*, unsigned int, JS::Value*) 	js/src/jsfun.cpp:1236
15 	xul.dll 	js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp:495
16 	xul.dll 	Interpret 	js/src/vm/Interpreter.cpp:3084
17 	xul.dll 	js::RunScript(JSContext*, js::RunState&) 	js/src/vm/Interpreter.cpp:435
18 	xul.dll 	js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp:513
19 	xul.dll 	js::jit::DoCallFallback 	js/src/jit/BaselineIC.cpp:2589
20 		@0x15a991cd08d

crashes with this signature are starting to show up in 57.0b, in rather a low volume though - so far only from windows 8 & 10 users. it's in a codepath from bug 1322746, so ni to jeff to take a quick look here.
Flags: needinfo?(jgilbert)
Daosheng, can you help on this? Although the crash volume is really low.
Flags: needinfo?(dmu)
Comment hidden (mozreview-request)
(Assignee)

Updated

a year ago
Flags: needinfo?(dmu)
Comment hidden (mozreview-request)
Assignee: nobody → dmu
status-firefox58: ? → affected
Priority: -- → P1
Whiteboard: [gfx-noted]

Comment 4

a year ago
mozreview-review
Comment on attachment 8914205 [details]
Bug 1404518 - Return when OpenSharedTexture is false;

https://reviewboard.mozilla.org/r/185524/#review190828

::: gfx/gl/GLBlitHelperD3D.cpp:62
(Diff revision 2)
>  {
>      RefPtr<ID3D11Texture2D> tex;
>      auto hr = d3d->OpenSharedResource((HANDLE)handle, __uuidof(ID3D11Texture2D),
>                                        (void**)(ID3D11Texture2D**)getter_AddRefs(tex));
>      if (FAILED(hr)) {
> -        MOZ_ASSERT(false, "OpenSharedResource should not fail");
> +        gfxCriticalError() << "Error code from OpenSharedResource: " << hr;

Use gfx::hexa(hr).
Attachment #8914205 - Flags: review?(jgilbert) → review+
Comment hidden (mozreview-request)

Comment 6

a year ago
Pushed by dmu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f936f29ab415
Return when OpenSharedTexture is false; r=jgilbert
https://hg.mozilla.org/mozilla-central/rev/f936f29ab415
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox58: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Please request Beta approval on this when you get a chance.
Flags: needinfo?(jgilbert) → needinfo?(dmu)
(Assignee)

Comment 9

a year ago
Comment on attachment 8914205 [details]
Bug 1404518 - Return when OpenSharedTexture is false;

Approval Request Comment
[Feature/Bug causing the regression]: Fix for getting a nullptr from OpenSharedTexture()
[User impact if declined]: Crashes continue to happen.
[Is this code covered by automated tests?]: Our webgl conformance tests cover this.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]:  nope.
[List of other uplifts needed for the feature/fix]: nope.
[Is the change risky?]: nope.
[Why is the change risky/not risky?]: This is just a check for nullptr.
[String changes made/needed]:
Flags: needinfo?(dmu)
Attachment #8914205 - Flags: approval-mozilla-beta?
Comment on attachment 8914205 [details]
Bug 1404518 - Return when OpenSharedTexture is false;

Though the crash volume is low, the fix seems very low risk, Beta57+
Attachment #8914205 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 11

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/978e6322ec4c
status-firefox57: affected → fixed
(In reply to Daosheng Mu[:daoshengmu] from comment #9)
> [Is this code covered by automated tests?]: Our webgl conformance tests
> cover this.
> [Has the fix been verified in Nightly?]: Yes.
> [Needs manual test from QE? If yes, steps to reproduce]:  nope.

Setting qe-verify- based on Daosheng Mu's assessment on manual testing needs and the fact that this fix has automated coverage.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.