1046205 - crash in TextureD3D11::Update

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

In TextureD3D11.cpp, from bug 1035227:


>+    if (aDestRegion) {
>+      nsIntRegionRectIterator iter(*aDestRegion);
>+      const nsIntRect *iterRect;
>+      while ((iterRect = iter.Next())) {
>+        D3D11_BOX box;
>+        box.front = 0;
>+        box.back = 1;
>+        box.left = iterRect->x;
>+        box.top = iterRect->y;
>+        box.right = iterRect->XMost();
>+        box.bottom = iterRect->YMost();
>+
>+        void* data = map.mData + map.mStride * iterRect->y + BytesPerPixel(aSurface->GetFormat()) * iterRect->x;
>+
>+        mCompositor->GetDC()->UpdateSubresource(mTexture, 0, &box, data, map.mStride, map.mStride * mSize.height);

Pretty sure that the last param is incorrect; mSize is the size of the entire surface, but we're only updating part of it.  This should be map.mStride * iterRect->width, no?  Otherwise we're already offsetting the data pointer, but updating the full byte size of the surface.

This is triggering a D3D debug layer error:

D3D11 CORRUPTION: ID3D11DeviceContext::UpdateSubresource: Fourth parameter is corrupt or NULL. [ MISCELLANEOUS CORRUPTION #16: CORRUPTED_PARAMETER4]

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: fix UpdateSubresource call

This might just be a D3D11 debug layer correctness issue, but we should fix those... this patch does fix it.

I believe that the data pointer needs to be valid for [data, data + rowStride) and [data, data + depthStride).  The latter isn't true and fails the check, even though data + depthStride will never be read.

If that's the case, technically we could end up in trouble even with data + rowStride -- for example, updating the last N pixels of the bottommost row (rect of height 1).  "data" will point at the end of the mapping minus N, so data + rowStride won't even be valid.

Comment 2

[Bas Schouten (:bas.schouten)]

Comment on attachment 8464771 [details] [diff] [review]
fix UpdateSubresource call

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

Yes, I believe this is just a debug layer correctness checking issue. I do not believe the depth pitch is ever used in practice when copying only a single plane. However as you say, we might as well make sure it's correct!

Comment 3

[Mason Chang [Inactive] [:mchang]]

Attachment: rebase.patch

Carrying r+, rebased on master.

Any particular reason this didn't land? Also, how do you get the d3d debug output in Firefox? I added the D3D11_CREATE_DEVICE_DEBUG flag to the d3d device creation flags, but I never see any output. It has worked for me outside of gecko.

Comment 5

[Mason Chang [Inactive] [:mchang]]

Try - https://treeherder.mozilla.org/#/jobs?repo=try&revision=3465538f5d8f

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a267539a757f
Fix depth pitch read out of bounds. r=bas

Comment 7

[Carsten Book [:Tomcat]]

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