Assertion failure: int4[0] == mViewportX && int4[1] == mViewportY && int4[2] == mViewportWidth && int4[3] == mViewportHeight,

RESOLVED FIXED in Firefox 52

Status

()

defect
--
critical
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: rforbes, Assigned: kvark)

Tracking

({assertion, crash, testcase})

Trunk
mozilla53
x86_64
Windows 10
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(firefox49 affected, firefox51 wontfix, firefox52 fixed, firefox53 fixed)

Details

(Whiteboard: [gfx-noted])

Attachments

(2 attachments)

Posted file testcase
Code in question is here.

WebGLContext::AssertCachedGlobalState()
{
#ifdef DEBUG
    MakeContextCurrent();

    GetAndFlushUnderlyingGLErrors();

    ////////////////

    // Draw state
    MOZ_ASSERT(gl->fIsEnabled(LOCAL_GL_DEPTH_TEST) == mDepthTestEnabled);

Assertion failure: int4[0] == mViewportX && int4[1] == mViewportY && int4[2] == mViewportWidth && int4[3] == mViewportHeight, at c:/src/mozilla/mozilla-central/dom/canvas/WebGLContextUtils.cpp:801
#01: mozilla::WebGLContext::SetDimensions (c:\src\mozilla\mozilla-central\dom\canvas\webglcontext.cpp:1005)
#02: mozilla::WebGLContext::TryToRestoreContext (c:\src\mozilla\mozilla-central\dom\canvas\webglcontext.cpp:1602)
#03: mozilla::UpdateContextLossStatusTask::Run (c:\src\mozilla\mozilla-central\dom\canvas\webglcontext.cpp:1628)
#04: nsThread::ProcessNextEvent (c:\src\mozilla\mozilla-central\xpcom\threads\nsthread.cpp:1029)
#05: NS_ProcessNextEvent (c:\src\mozilla\mozilla-central\xpcom\glue\nsthreadutils.cpp:290)
#06: mozilla::ipc::MessagePump::Run (c:\src\mozilla\mozilla-central\ipc\glue\messagepump.cpp:100)
#07: MessageLoop::RunInternal (c:\src\mozilla\mozilla-central\ipc\chromium\src\base\message_loop.cc:235)
#08: MessageLoop::RunHandler (c:\src\mozilla\mozilla-central\ipc\chromium\src\base\message_loop.cc:229)
#09: MessageLoop::Run (c:\src\mozilla\mozilla-central\ipc\chromium\src\base\message_loop.cc:209)
#10: nsBaseAppShell::Run (c:\src\mozilla\mozilla-central\widget\nsbaseappshell.cpp:158)
#11: nsAppShell::Run (c:\src\mozilla\mozilla-central\widget\windows\nsappshell.cpp:264)
#12: nsAppStartup::Run (c:\src\mozilla\mozilla-central\toolkit\components\startup\nsappstartup.cpp:285)
#13: XREMain::XRE_mainRun (c:\src\mozilla\mozilla-central\toolkit\xre\nsapprunner.cpp:4374)
#14: XREMain::XRE_main (c:\src\mozilla\mozilla-central\toolkit\xre\nsapprunner.cpp:4478)
#15: XRE_main (c:\src\mozilla\mozilla-central\toolkit\xre\nsapprunner.cpp:4586)
#16: do_main (c:\src\mozilla\mozilla-central\browser\app\nsbrowserapp.cpp:242)
#17: NS_internal_main (c:\src\mozilla\mozilla-central\browser\app\nsbrowserapp.cpp:382)
#18: wmain (c:\src\mozilla\mozilla-central\toolkit\xre\nswindowswmain.cpp:130)
#19: invoke_main (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:79)
#20: __scrt_common_main_seh (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:255)
#21: __scrt_common_main (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:300)
#22: wmainCRTStartup (f:\dd\vctools\crt\vcstartup\src\startup\exe_wmain.cpp:17)
#23: BaseThreadInitThunk[C:\Windows\SYSTEM32\KERNEL32.DLL +0x138f4]
#24: RtlUnicodeStringToInteger[C:\Windows\SYSTEM32\ntdll.dll +0x65de3]
#25: RtlUnicodeStringToInteger[C:\Windows\SYSTEM32\ntdll.dll +0x65dae]
Severity: normal → critical
Group: core-security → gfx-core-security
Jeff: is this an assertion we should worry about wrt security? If not is there a way to fix or suppress it so it doesn't trigger false positives in our testing tools?
Flags: needinfo?(jgilbert)
(In reply to Daniel Veditz [:dveditz] from comment #1)
> Jeff: is this an assertion we should worry about wrt security? If not is
> there a way to fix or suppress it so it doesn't trigger false positives in
> our testing tools?

This should just be a correctness bug with no security repercussions.
Flags: needinfo?(jgilbert)
Group: gfx-core-security
Whiteboard: [gfx-noted]
Assignee: nobody → kvark
Status: NEW → ASSIGNED
Comment on attachment 8825547 [details] [diff] [review]
Reset viewport offset on SetDimensions and clamp to MAX_VIEWPORT_DIMS.

Review of attachment 8825547 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/canvas/WebGLContextGL.cpp
@@ +2139,5 @@
>  
>      if (width < 0 || height < 0)
>          return ErrorInvalidValue("viewport: negative size");
>  
> +    width = std::min(width, (GLsizei)mImplMaxViewportDims[0]);

std::min(uint32_t(width), mImpl...) is more correct, though for our expected ranges here it shouldn't matter.
Attachment #8825547 - Flags: review?(jgilbert) → review+
Keywords: checkin-needed
Should we be landing the testcase as a crashtest as well?
Flags: needinfo?(kvark)
(In reply to Ryan VanderMeulen [:RyanVM] from comment #6)
> Should we be landing the testcase as a crashtest as well?

Not for this, no.
Flags: needinfo?(kvark)
Flags: in-testsuite-
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/84c7a36da084
Reset viewport offset on SetDimensions and clamp to MAX_VIEWPORT_DIMS. r=jgilbert
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/84c7a36da084
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Comment on attachment 8825547 [details] [diff] [review]
Reset viewport offset on SetDimensions and clamp to MAX_VIEWPORT_DIMS.

Approval Request Comment
[Feature/Bug causing the regression]: n/a
[User impact if declined]: Spurious assert when running DEBUG builds
[Is this code covered by automated tests?]:
[Has the fix been verified in Nightly?]:
[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]: no
[Why is the change risky/not risky?]: Practically DEBUG-only.
[String changes made/needed]:
Attachment #8825547 - Flags: approval-mozilla-aurora?
Comment on attachment 8825547 [details] [diff] [review]
Reset viewport offset on SetDimensions and clamp to MAX_VIEWPORT_DIMS.

webgl viewport fix for aurora52
Attachment #8825547 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.