Closed Bug 1046205 Opened 5 years ago Closed 3 years ago

crash in TextureD3D11::Update

Categories

(Core :: Graphics: Layers, defect)

x86_64
Windows 8
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox49 --- fixed

People

(Reporter: vlad, Assigned: mchang)

References

Details

Attachments

(1 file, 1 obsolete file)

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]
Attached patch fix UpdateSubresource call (obsolete) — Splinter Review
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.
Attachment #8464771 - Flags: review?(bas)
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!
Attachment #8464771 - Flags: review?(bas) → review+
Attached patch rebase.patchSplinter Review
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.
Assignee: nobody → mchang
Attachment #8464771 - Attachment is obsolete: true
Flags: needinfo?(vladimir)
Attachment #8757414 - Flags: review+
Duplicate of this bug: 1275030
Pushed by mchang@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a267539a757f
Fix depth pitch read out of bounds. r=bas
https://hg.mozilla.org/mozilla-central/rev/a267539a757f
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Flags: needinfo?(vladimir)
You need to log in before you can comment on or make changes to this bug.