Crash in mozilla::WebGLContext::Clear

RESOLVED FIXED in Firefox 60

Status

()

defect
P2
critical
RESOLVED FIXED
Last year
Last year

People

(Reporter: marcia, Assigned: jgilbert)

Tracking

({crash, regression})

Trunk
mozilla61
Unspecified
All
Points:
---

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox59 wontfix, firefox60 fixed, firefox61 fixed)

Details

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

Attachments

(2 attachments)

This bug was filed from the Socorro interface and is
report bp-6f3c4721-23b9-4ca2-8540-d749b0180304.
=============================================================

Seen while looking at nightly Android crash stats - crashes may have started using Build ID 20180111100641 (when nightly was in 59): http://bit.ly/2oR47kv. 

Comments:
* again siteline maps goes down again before it completely loads, I did get as far as template this time 
* closing a popup and scrolling down the page seems to trigger this. even if the popup isn't closed I've had this crash a LOT scrolling halfway down normal pages 

Top 10 frames of crashing thread:

0 libxul.so mozilla::WebGLContext::Clear dom/canvas/WebGLContextFramebufferOperations.cpp:39
1 libxul.so mozilla::dom::WebGLRenderingContextBinding::clear dom/bindings/WebGLRenderingContextBinding.cpp:15467
2 libxul.so mozilla::dom::GenericBindingMethod dom/bindings/BindingUtils.cpp:3037
3 libxul.so js::InternalCallOrConstruct js/src/jscntxtinlines.h:291
4 libxul.so <name omitted> js/src/vm/Interpreter.cpp:522
5 libxul.so Interpret js/src/vm/Interpreter.cpp:528
6 libxul.so js::RunScript js/src/vm/Interpreter.cpp:423
7 libxul.so js::ExecuteKernel js/src/vm/Interpreter.cpp:706
8 libxul.so js::Execute js/src/vm/Interpreter.cpp:738
9 libxul.so ExecuteScript js/src/jsapi.cpp:4734

=============================================================
Flags: needinfo?(jnicol)
Flags: needinfo?(jgilbert)
Priority: -- → P2
Whiteboard: [gfx-noted]
Can't reproduce on siteline.co.uk. Is that the correct url, or do we have any other urls?

Jeff, the commented out lines in WebGLFBAttachPoint::IsDefined() look like they would probably prevent this. Is there a reason these are commented out and we have a less strict check?
(In reply to Jamie Nicol [:jnicol] from comment #1)
> Can't reproduce on siteline.co.uk. Is that the correct url, or do we have
> any other urls?
> 
> Jeff, the commented out lines in WebGLFBAttachPoint::IsDefined() look like
> they would probably prevent this. Is there a reason these are commented out
> and we have a less strict check?

That logic is the logic from HasImage. It's possible when I split these two, that I missed some s/IsDefined/HasImage/ cases. I will audit these.
Assignee: nobody → jgilbert
Flags: needinfo?(jgilbert)
Flags: needinfo?(jnicol)
(In reply to Jamie Nicol [:jnicol] from comment #1)
> Can't reproduce on siteline.co.uk. Is that the correct url, or do we have
> any other urls?
> 
> Jeff, the commented out lines in WebGLFBAttachPoint::IsDefined() look like
> they would probably prevent this. Is there a reason these are commented out
> and we have a less strict check?

Here are a few more URLs:

*https://poly.google.com/view/4UYuuqgX-xQ
*https://www.google.com/maps 
*http://gdchillers.com/gd-7-h-micro-chil
*https://threejs.org/examples/#webgl_gpgpu_birds 
*http://haxiomic.github.io/GPU-Fluid-Experiments/html5/?q=UltraHigh 
*https://emnh.github.io/rts-reasonml/
I was just able to reproduce on my Pixel - here is my crash report: https://crash-stats.mozilla.com/report/index/e90ea878-82e3-48b6-9971-202ae0180309. I loaded https://poly.google.com/view/9zLa6J13KAv and just scrolled the page.
Bughunter can reproduce opt/debug crashes on Fedora 27 x86_64 at
<https://viewer.autodesk.com/id/dXJuOmFkc2sub2JqZWN0czpvcy5vYmplY3Q6YTM2MHZpZXdlci90MTUxODE4MTkzNjcxN18wODE5NzI3NzI1ODQzNDQ4OF8xNTE4MTgxOTM2NzE3LmR3Zng?designtype=dwfx> but not Ubuntu 17.10 x86_64.

Operating system: Linux
                  0.0.0 Linux 4.15.6-300.fc27.x86_64 #1 SMP Mon Feb 26 18:43:03 UTC 2018 x86_64
CPU: amd64
     family 6 model 45 stepping 2
     2 CPUs

GPU: UNKNOWN

Crash reason:  SIGSEGV
Crash address: 0x0
Process uptime: not available

Thread 0 (crashed)
 0  libxul.so!mozilla::WebGLContext::Clear [WebGLContextFramebufferOperations.cpp:415e9b18ca2a1532086d5e2d5d21343cd004b5fd : 39 + 0x8]
    rax = 0x0000000000000000   rdx = 0x00007fec0832e000
    rcx = 0x0000000000000000   rbx = 0x00007fec08c73390
    rsi = 0x0000000000004000   rdi = 0x00007fec08319cf0
    rbp = 0x00007ffc907d47d0   rsp = 0x00007ffc907d47a0
     r8 = 0x0000000000003ce2    r9 = 0x0000000000000010
    r10 = 0x00007fec20d55800   r11 = 0x000000000001fff1
    r12 = 0x00007fec0cdd6000   r13 = 0x0000000000004000
    r14 = 0x00007fec08c73398   r15 = 0x0000000000000000
    rip = 0x00007fec306d617d
    Found by: given as instruction pointer in context
 1  libxul.so!mozilla::dom::WebGLRenderingContextBinding::clear [WebGLRenderingContextBinding.cpp: : 15595 + 0x8]
    rbx = 0x00007ffc907d4830   rbp = 0x00007ffc907d4810
    rsp = 0x00007ffc907d47e0   r12 = 0x00007fec23222000
    r13 = 0x00007fec0cdd6000   r14 = 0x00000000000002e9
    r15 = 0x0000000000000000   rip = 0x00007fec3031dcea
    Found by: call frame info
Confirmed I can reliably reproduce on https://poly.google.com/view/9zLa6J13KAv and that changing to HasImage() fixes the crash.
Posted file trigger.html
Testcase found while fuzzing mozilla-central rev f0d13136b358.

==5807==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x7fd720af06ff bp 0x7ffea0a5c810 sp 0x7ffea0a5c740 T0)
==5807==The signal is caused by a READ memory access.
==5807==Hint: address points to the zero page.
    #0 0x7fd720af06fe in mozilla::WebGLContext::Clear(unsigned int) /builds/worker/workspace/build/src/dom/canvas/WebGLContextFramebufferOperations.cpp:39:40
    #1 0x7fd71fb54867 in mozilla::dom::WebGLRenderingContextBinding::clear(JSContext*, JS::Handle<JSObject*>, mozilla::WebGLContext*, JSJitMethodCallArgs const&) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/WebGLRenderingContextBinding.cpp:14125:9
    #2 0x7fd7208f9d51 in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /builds/worker/workspace/build/src/dom/bindings/BindingUtils.cpp:3031:13
    #3 0x7fd727217aee in CallJSNative /builds/worker/workspace/build/src/js/src/vm/JSContext-inl.h:290:15
    #4 0x7fd727217aee in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:467
    #5 0x7fd727200fad in CallFromStack /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:522:12
    #6 0x7fd727200fad in Interpret(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:3084
    #7 0x7fd7271e23e4 in js::RunScript(JSContext*, js::RunState&) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:417:12
    #8 0x7fd7272178e7 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:489:15
    #9 0x7fd727218653 in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/vm/Interpreter.cpp:535:10
    #10 0x7fd727e74ffa in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /builds/worker/workspace/build/src/js/src/jsapi.cpp:3011:12
    #11 0x7fd71ffcb8c5 in mozilla::dom::EventListener::HandleEvent(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, mozilla::ErrorResult&) /builds/worker/workspace/build/src/obj-firefox/dom/bindings/EventListenerBinding.cpp:47:8
    #12 0x7fd72101c831 in HandleEvent<mozilla::dom::EventTarget *> /builds/worker/workspace/build/src/obj-firefox/dist/include/mozilla/dom/EventListenerBinding.h:66:12
    #13 0x7fd72101c831 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1087
    #14 0x7fd72101e0f5 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) /builds/worker/workspace/build/src/dom/events/EventListenerManager.cpp:1259:20
    #15 0x7fd721008907 in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /builds/worker/workspace/build/src/dom/events/EventDispatcher.cpp:527:16
    #16 0x7fd72100c6a3 in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) /builds/worker/workspace/build/src/dom/events/EventDispatcher.cpp:917:9
    #17 0x7fd72100e9bc in mozilla::EventDispatcher::DispatchDOMEvent(nsISupports*, mozilla::WidgetEvent*, nsIDOMEvent*, nsPresContext*, nsEventStatus*) /builds/worker/workspace/build/src/dom/events/EventDispatcher.cpp:996:12
    #18 0x7fd71e306774 in nsINode::DispatchEvent(nsIDOMEvent*, bool*) /builds/worker/workspace/build/src/dom/base/nsINode.cpp:1171:5
    #19 0x7fd71de38cee in nsContentUtils::DispatchEvent(nsIDocument*, nsISupports*, nsTSubstring<char16_t> const&, bool, bool, bool, bool*, bool) /builds/worker/workspace/build/src/dom/base/nsContentUtils.cpp:4608:18
    #20 0x7fd71de38aa4 in nsContentUtils::DispatchTrustedEvent(nsIDocument*, nsISupports*, nsTSubstring<char16_t> const&, bool, bool, bool*) /builds/worker/workspace/build/src/dom/base/nsContentUtils.cpp:4576:10
    #21 0x7fd71e235ed8 in nsIDocument::DispatchContentLoadedEvents() /builds/worker/workspace/build/src/dom/base/nsDocument.cpp:5255:3
    #22 0x7fd71e34d5d4 in applyImpl<nsIDocument, void (nsIDocument::*)()> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:1164:12
    #23 0x7fd71e34d5d4 in apply<nsIDocument, void (nsIDocument::*)()> /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:1170
    #24 0x7fd71e34d5d4 in mozilla::detail::RunnableMethodImpl<nsIDocument*, void (nsIDocument::*)(), true, (mozilla::RunnableKind)0>::Run() /builds/worker/workspace/build/src/obj-firefox/dist/include/nsThreadUtils.h:1215
    #25 0x7fd71b0cccf4 in mozilla::SchedulerGroup::Runnable::Run() /builds/worker/workspace/build/src/xpcom/threads/SchedulerGroup.cpp:415:25
    #26 0x7fd71b0ec458 in nsThread::ProcessNextEvent(bool, bool*) /builds/worker/workspace/build/src/xpcom/threads/nsThread.cpp:1096:14
    #27 0x7fd71b1087c0 in NS_ProcessNextEvent(nsIThread*, bool) /builds/worker/workspace/build/src/xpcom/threads/nsThreadUtils.cpp:519:10
    #28 0x7fd71bfca84a in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /builds/worker/workspace/build/src/ipc/glue/MessagePump.cpp:97:21
    #29 0x7fd71bf1a799 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:326:10
    #30 0x7fd71bf1a799 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:319
    #31 0x7fd71bf1a799 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:299
    #32 0x7fd722c514ca in nsBaseAppShell::Run() /builds/worker/workspace/build/src/widget/nsBaseAppShell.cpp:157:27
    #33 0x7fd726f0406b in XRE_RunAppShell() /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:893:22
    #34 0x7fd71bf1a799 in RunInternal /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:326:10
    #35 0x7fd71bf1a799 in RunHandler /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:319
    #36 0x7fd71bf1a799 in MessageLoop::Run() /builds/worker/workspace/build/src/ipc/chromium/src/base/message_loop.cc:299
    #37 0x7fd726f03a4a in XRE_InitChildProcess(int, char**, XREChildData const*) /builds/worker/workspace/build/src/toolkit/xre/nsEmbedFunctions.cpp:719:34
    #38 0x4f6f2c in content_process_main /builds/worker/workspace/build/src/browser/app/../../ipc/contentproc/plugin-container.cpp:50:30
    #39 0x4f6f2c in main /builds/worker/workspace/build/src/browser/app/nsBrowserApp.cpp:280
    #40 0x7fd73b51882f in __libc_start_main /build/glibc-Cl5G7W/glibc-2.23/csu/../csu/libc-start.c:291
the signature is regressing on desktop on all platforms (win, macos, linux) since 59 as well.
OS: Android → All
can we move forward with a review for the patch?
Flags: needinfo?(milan)
Flags: needinfo?(milan)
Attachment #8958870 - Flags: review?(jgilbert) → review?(jmuizelaar)
:jgilbert is on PTO for another week, moving review.
Comment on attachment 8958870 [details]
Bug 1443149 - Ensure WebGLFBAttachPoint::HasImage() before dereferencing Format().

https://reviewboard.mozilla.org/r/227744/#review242314

This seems reasonable to me. We can land now, but it would be good for jgilbert to confirm on his return.
Attachment #8958870 - Flags: review?(jmuizelaar) → review+
Okay, I've tested this still works and i've hit the autoland button so it should land soon. It won't be a hard one to back out if we need to. Needinfoing jgilbert so he sees this when he gets back.
Flags: needinfo?(jgilbert)
Pushed by jnicol@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ebb9f83337a2
Ensure WebGLFBAttachPoint::HasImage() before dereferencing Format(). r=jrmuizel
https://hg.mozilla.org/mozilla-central/rev/ebb9f83337a2
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Please request Beta approval on this when you get a chance.
Flags: needinfo?(jgilbert) → needinfo?(jnicol)
Comment on attachment 8958870 [details]
Bug 1443149 - Ensure WebGLFBAttachPoint::HasImage() before dereferencing Format().

Approval Request Comment
[Feature/Bug causing the regression]: Not sure
[User impact if declined]: Crashes
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: N/A
[Is the change risky?]: No
[Why is the change risky/not risky?]: Minor change, enforces stricter rules, so will just skip clearing a framebuffer rather than crash.
[String changes made/needed]: N/A
Flags: needinfo?(jnicol)
Attachment #8958870 - Flags: approval-mozilla-beta?
Comment on attachment 8958870 [details]
Bug 1443149 - Ensure WebGLFBAttachPoint::HasImage() before dereferencing Format().

crash fix, seems to be effective in nightly, approved for 60.0b14
Attachment #8958870 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.