[PATCH 1/3] ddraw/tests: Disable a test that causes heap corruption on some Nvidia (v2).
Signed-off-by: Stefan Dösinger <stefan@codeweavers.com
---
Version 2: Disable the test only on Nvidia and see if we run into problems elsewhere.
The other parts of test_clipper_refcount appear to work fine on the affected system, including the invokation of Release() on a destroyed clipper and destroying the surface that has the destroyed clipper attached to it. --- dlls/ddraw/tests/ddraw1.c | 16 +++++++++++++--- dlls/ddraw/tests/ddraw2.c | 16 +++++++++++++--- dlls/ddraw/tests/ddraw4.c | 16 +++++++++++++--- dlls/ddraw/tests/ddraw7.c | 16 +++++++++++++--- 4 files changed, 52 insertions(+), 12 deletions(-)
diff --git a/dlls/ddraw/tests/ddraw1.c b/dlls/ddraw/tests/ddraw1.c index b513121fbbc..80c96fccc4c 100644 --- a/dlls/ddraw/tests/ddraw1.c +++ b/dlls/ddraw/tests/ddraw1.c @@ -12908,9 +12908,19 @@ static void test_clipper_refcount(void) IDirectDrawClipper_Release(clipper); IDirectDrawClipper_Release(clipper);
- hr = IDirectDrawSurface_GetClipper(surface, &clipper2); - ok(SUCCEEDED(hr), "Failed to get clipper, hr %#x.\n", hr); - ok(clipper == clipper2, "Got clipper %p, expected %p.\n", clipper2, clipper); + if (!ddraw_is_nvidia(ddraw)) + { + /* Disabled because it causes heap corruption (HeapValidate fails and random + * hangs in a later HeapFree) on Windows on one of my Machines: MacbookPro 10,1 + * running Windows 10 18363.535 and Nvidia driver 425.31. Driver version 441.66 + * is affected too. + * + * The same Windows and driver versions run the test without heap corruption on + * a Geforce 1060 GTX card. I have not seen the problem on AMD GPUs either. */ + hr = IDirectDrawSurface_GetClipper(surface, &clipper2); + ok(SUCCEEDED(hr), "Failed to get clipper, hr %#x.\n", hr); + ok(clipper == clipper2, "Got clipper %p, expected %p.\n", clipper2, clipper); + }
/* Show that invoking the Release method does not crash, but don't get the * vtable through the clipper pointer because it is no longer pointing to diff --git a/dlls/ddraw/tests/ddraw2.c b/dlls/ddraw/tests/ddraw2.c index 541362c7488..00937fd0a7c 100644 --- a/dlls/ddraw/tests/ddraw2.c +++ b/dlls/ddraw/tests/ddraw2.c @@ -13734,9 +13734,19 @@ static void test_clipper_refcount(void) IDirectDrawClipper_Release(clipper); IDirectDrawClipper_Release(clipper);
- hr = IDirectDrawSurface_GetClipper(surface, &clipper2); - ok(SUCCEEDED(hr), "Failed to get clipper, hr %#x.\n", hr); - ok(clipper == clipper2, "Got clipper %p, expected %p.\n", clipper2, clipper); + if (!ddraw_is_nvidia(ddraw)) + { + /* Disabled because it causes heap corruption (HeapValidate fails and random + * hangs in a later HeapFree) on Windows on one of my Machines: MacbookPro 10,1 + * running Windows 10 18363.535 and Nvidia driver 425.31. Driver version 441.66 + * is affected too. + * + * The same Windows and driver versions run the test without heap corruption on + * a Geforce 1060 GTX card. I have not seen the problem on AMD GPUs either. */ + hr = IDirectDrawSurface_GetClipper(surface, &clipper2); + ok(SUCCEEDED(hr), "Failed to get clipper, hr %#x.\n", hr); + ok(clipper == clipper2, "Got clipper %p, expected %p.\n", clipper2, clipper); + }
/* Show that invoking the Release method does not crash, but don't get the * vtable through the clipper pointer because it is no longer pointing to diff --git a/dlls/ddraw/tests/ddraw4.c b/dlls/ddraw/tests/ddraw4.c index 8cadfae52a0..29d72dfbbae 100644 --- a/dlls/ddraw/tests/ddraw4.c +++ b/dlls/ddraw/tests/ddraw4.c @@ -16386,9 +16386,19 @@ static void test_clipper_refcount(void) IDirectDrawClipper_Release(clipper); IDirectDrawClipper_Release(clipper);
- hr = IDirectDrawSurface4_GetClipper(surface, &clipper2); - ok(SUCCEEDED(hr), "Failed to get clipper, hr %#x.\n", hr); - ok(clipper == clipper2, "Got clipper %p, expected %p.\n", clipper2, clipper); + if (!ddraw_is_nvidia(ddraw)) + { + /* Disabled because it causes heap corruption (HeapValidate fails and random + * hangs in a later HeapFree) on Windows on one of my Machines: MacbookPro 10,1 + * running Windows 10 18363.535 and Nvidia driver 425.31. Driver version 441.66 + * is affected too. + * + * The same Windows and driver versions run the test without heap corruption on + * a Geforce 1060 GTX card. I have not seen the problem on AMD GPUs either. */ + hr = IDirectDrawSurface4_GetClipper(surface, &clipper2); + ok(SUCCEEDED(hr), "Failed to get clipper, hr %#x.\n", hr); + ok(clipper == clipper2, "Got clipper %p, expected %p.\n", clipper2, clipper); + }
/* Show that invoking the Release method does not crash, but don't get the * vtable through the clipper pointer because it is no longer pointing to diff --git a/dlls/ddraw/tests/ddraw7.c b/dlls/ddraw/tests/ddraw7.c index f4a709711b0..20a1bc5f3af 100644 --- a/dlls/ddraw/tests/ddraw7.c +++ b/dlls/ddraw/tests/ddraw7.c @@ -16290,9 +16290,19 @@ static void test_clipper_refcount(void) IDirectDrawClipper_Release(clipper); IDirectDrawClipper_Release(clipper);
- hr = IDirectDrawSurface7_GetClipper(surface, &clipper2); - ok(SUCCEEDED(hr), "Failed to get clipper, hr %#x.\n", hr); - ok(clipper == clipper2, "Got clipper %p, expected %p.\n", clipper2, clipper); + if (!ddraw_is_nvidia(ddraw)) + { + /* Disabled because it causes heap corruption (HeapValidate fails and random + * hangs in a later HeapFree) on Windows on one of my Machines: MacbookPro 10,1 + * running Windows 10 18363.535 and Nvidia driver 425.31. Driver version 441.66 + * is affected too. + * + * The same Windows and driver versions run the test without heap corruption on + * a Geforce 1060 GTX card. I have not seen the problem on AMD GPUs either. */ + hr = IDirectDrawSurface7_GetClipper(surface, &clipper2); + ok(SUCCEEDED(hr), "Failed to get clipper, hr %#x.\n", hr); + ok(clipper == clipper2, "Got clipper %p, expected %p.\n", clipper2, clipper); + }
/* Show that invoking the Release method does not crash, but don't get the * vtable through the clipper pointer because it is no longer pointing to
Signed-off-by: Stefan Dösinger <stefan@codeweavers.com --- dlls/ddraw/tests/ddraw1.c | 9 +++++++-- dlls/ddraw/tests/ddraw2.c | 9 +++++++-- 2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/dlls/ddraw/tests/ddraw1.c b/dlls/ddraw/tests/ddraw1.c index 80c96fccc4c..c10be66236a 100644 --- a/dlls/ddraw/tests/ddraw1.c +++ b/dlls/ddraw/tests/ddraw1.c @@ -10043,8 +10043,12 @@ static void test_getdc(void) ok(SUCCEEDED(hr), "Failed to get DC for format %s, hr %#x.\n", test_data[i].name, hr); hr = IDirectDrawSurface_ReleaseDC(surface, dc); ok(SUCCEEDED(hr), "Failed to release DC for format %s, hr %#x.\n", test_data[i].name, hr); + /* Geforce 9600, Windows 7 returns E_FAIL. The unlock still seems to work as intended, after- + * wards the surface can be locked again. ReleaseDC() does not unlock the surface, trying to + * Lock it after ReleaseDC returns DDERR_SURFACEBUSY. ddraw4 and 7 are unaffected. */ hr = IDirectDrawSurface_Unlock(surface, NULL); - ok(SUCCEEDED(hr), "Failed to unmap surface for format %s, hr %#x.\n", test_data[i].name, hr); + ok(SUCCEEDED(hr) || broken(ddraw_is_nvidia(ddraw) && hr == E_FAIL), + "Failed to unmap surface for format %s, hr %#x.\n", test_data[i].name, hr);
hr = IDirectDrawSurface_GetDC(surface, &dc); ok(SUCCEEDED(hr), "Failed to get DC for format %s, hr %#x.\n", test_data[i].name, hr); @@ -10080,7 +10084,8 @@ static void test_getdc(void) hr = IDirectDrawSurface_ReleaseDC(surface, dc); ok(SUCCEEDED(hr), "Failed to release DC for format %s, hr %#x.\n", test_data[i].name, hr); hr = IDirectDrawSurface_Unlock(surface, NULL); - ok(SUCCEEDED(hr), "Failed to unmap surface for format %s, hr %#x.\n", test_data[i].name, hr); + ok(SUCCEEDED(hr) || broken(ddraw_is_nvidia(ddraw) && hr == E_FAIL), + "Failed to unmap surface for format %s, hr %#x.\n", test_data[i].name, hr);
hr = IDirectDrawSurface_Lock(surface2, NULL, &map_desc, DDLOCK_READONLY | DDLOCK_WAIT, NULL); ok(SUCCEEDED(hr), "Failed to map surface for format %s, hr %#x.\n", test_data[i].name, hr); diff --git a/dlls/ddraw/tests/ddraw2.c b/dlls/ddraw/tests/ddraw2.c index 00937fd0a7c..800e5d8a80b 100644 --- a/dlls/ddraw/tests/ddraw2.c +++ b/dlls/ddraw/tests/ddraw2.c @@ -10742,8 +10742,12 @@ static void test_getdc(void) ok(SUCCEEDED(hr), "Failed to get DC for format %s, hr %#x.\n", test_data[i].name, hr); hr = IDirectDrawSurface_ReleaseDC(surface, dc); ok(SUCCEEDED(hr), "Failed to release DC for format %s, hr %#x.\n", test_data[i].name, hr); + /* Geforce 9600, Windows 7 returns E_FAIL. The unlock still seems to work as intended, after- + * wards the surface can be locked again. ReleaseDC() does not unlock the surface, trying to + * Lock it after ReleaseDC returns DDERR_SURFACEBUSY. ddraw4 and 7 are unaffected. */ hr = IDirectDrawSurface_Unlock(surface, NULL); - ok(SUCCEEDED(hr), "Failed to unmap surface for format %s, hr %#x.\n", test_data[i].name, hr); + ok(SUCCEEDED(hr) || broken(ddraw_is_nvidia(ddraw) && hr == E_FAIL), + "Failed to unmap surface for format %s, hr %#x.\n", test_data[i].name, hr);
hr = IDirectDrawSurface_GetDC(surface, &dc); ok(SUCCEEDED(hr), "Failed to get DC for format %s, hr %#x.\n", test_data[i].name, hr); @@ -10779,7 +10783,8 @@ static void test_getdc(void) hr = IDirectDrawSurface_ReleaseDC(surface, dc); ok(SUCCEEDED(hr), "Failed to release DC for format %s, hr %#x.\n", test_data[i].name, hr); hr = IDirectDrawSurface_Unlock(surface, NULL); - ok(SUCCEEDED(hr), "Failed to unmap surface for format %s, hr %#x.\n", test_data[i].name, hr); + ok(SUCCEEDED(hr) || broken(ddraw_is_nvidia(ddraw) && hr == E_FAIL), + "Failed to unmap surface for format %s, hr %#x.\n", test_data[i].name, hr);
hr = IDirectDrawSurface_Lock(surface2, NULL, &map_desc, DDLOCK_READONLY | DDLOCK_WAIT, NULL); ok(SUCCEEDED(hr), "Failed to map surface for format %s, hr %#x.\n", test_data[i].name, hr);
Signed-off-by: Henri Verbeet hverbeet@codeweavers.com
Signed-off-by: Stefan Dösinger <stefan@codeweavers.com
---
The way it stands it is just a repeat of the test a few lines earlier. I guess this was the original intention. And no, it does not affect the E_FAIL on the Geforce 9600. --- dlls/ddraw/tests/ddraw1.c | 8 ++++---- dlls/ddraw/tests/ddraw2.c | 8 ++++---- dlls/ddraw/tests/ddraw4.c | 8 ++++---- dlls/ddraw/tests/ddraw7.c | 8 ++++---- 4 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/dlls/ddraw/tests/ddraw1.c b/dlls/ddraw/tests/ddraw1.c index c10be66236a..8e9d8ff9ad1 100644 --- a/dlls/ddraw/tests/ddraw1.c +++ b/dlls/ddraw/tests/ddraw1.c @@ -10077,13 +10077,13 @@ static void test_getdc(void) hr = IDirectDrawSurface_Unlock(surface, NULL); ok(SUCCEEDED(hr), "Failed to unmap surface for format %s, hr %#x.\n", test_data[i].name, hr);
- hr = IDirectDrawSurface_Lock(surface, NULL, &map_desc, DDLOCK_READONLY | DDLOCK_WAIT, NULL); + hr = IDirectDrawSurface_Lock(surface2, NULL, &map_desc, DDLOCK_READONLY | DDLOCK_WAIT, NULL); ok(SUCCEEDED(hr), "Failed to map surface for format %s, hr %#x.\n", test_data[i].name, hr); - hr = IDirectDrawSurface_GetDC(surface, &dc); + hr = IDirectDrawSurface_GetDC(surface2, &dc); ok(SUCCEEDED(hr), "Failed to get DC for format %s, hr %#x.\n", test_data[i].name, hr); - hr = IDirectDrawSurface_ReleaseDC(surface, dc); + hr = IDirectDrawSurface_ReleaseDC(surface2, dc); ok(SUCCEEDED(hr), "Failed to release DC for format %s, hr %#x.\n", test_data[i].name, hr); - hr = IDirectDrawSurface_Unlock(surface, NULL); + hr = IDirectDrawSurface_Unlock(surface2, NULL); ok(SUCCEEDED(hr) || broken(ddraw_is_nvidia(ddraw) && hr == E_FAIL), "Failed to unmap surface for format %s, hr %#x.\n", test_data[i].name, hr);
diff --git a/dlls/ddraw/tests/ddraw2.c b/dlls/ddraw/tests/ddraw2.c index 800e5d8a80b..273a01c2f4c 100644 --- a/dlls/ddraw/tests/ddraw2.c +++ b/dlls/ddraw/tests/ddraw2.c @@ -10776,13 +10776,13 @@ static void test_getdc(void) hr = IDirectDrawSurface_Unlock(surface, NULL); ok(SUCCEEDED(hr), "Failed to unmap surface for format %s, hr %#x.\n", test_data[i].name, hr);
- hr = IDirectDrawSurface_Lock(surface, NULL, &map_desc, DDLOCK_READONLY | DDLOCK_WAIT, NULL); + hr = IDirectDrawSurface_Lock(surface2, NULL, &map_desc, DDLOCK_READONLY | DDLOCK_WAIT, NULL); ok(SUCCEEDED(hr), "Failed to map surface for format %s, hr %#x.\n", test_data[i].name, hr); - hr = IDirectDrawSurface_GetDC(surface, &dc); + hr = IDirectDrawSurface_GetDC(surface2, &dc); ok(SUCCEEDED(hr), "Failed to get DC for format %s, hr %#x.\n", test_data[i].name, hr); - hr = IDirectDrawSurface_ReleaseDC(surface, dc); + hr = IDirectDrawSurface_ReleaseDC(surface2, dc); ok(SUCCEEDED(hr), "Failed to release DC for format %s, hr %#x.\n", test_data[i].name, hr); - hr = IDirectDrawSurface_Unlock(surface, NULL); + hr = IDirectDrawSurface_Unlock(surface2, NULL); ok(SUCCEEDED(hr) || broken(ddraw_is_nvidia(ddraw) && hr == E_FAIL), "Failed to unmap surface for format %s, hr %#x.\n", test_data[i].name, hr);
diff --git a/dlls/ddraw/tests/ddraw4.c b/dlls/ddraw/tests/ddraw4.c index 29d72dfbbae..c674bad992c 100644 --- a/dlls/ddraw/tests/ddraw4.c +++ b/dlls/ddraw/tests/ddraw4.c @@ -12756,13 +12756,13 @@ static void test_getdc(void) hr = IDirectDrawSurface4_Unlock(surface, NULL); ok(SUCCEEDED(hr), "Failed to unmap surface for format %s, hr %#x.\n", test_data[i].name, hr);
- hr = IDirectDrawSurface4_Lock(surface, NULL, &map_desc, DDLOCK_READONLY | DDLOCK_WAIT, NULL); + hr = IDirectDrawSurface4_Lock(surface2, NULL, &map_desc, DDLOCK_READONLY | DDLOCK_WAIT, NULL); ok(SUCCEEDED(hr), "Failed to map surface for format %s, hr %#x.\n", test_data[i].name, hr); - hr = IDirectDrawSurface4_GetDC(surface, &dc); + hr = IDirectDrawSurface4_GetDC(surface2, &dc); ok(SUCCEEDED(hr), "Failed to get DC for format %s, hr %#x.\n", test_data[i].name, hr); - hr = IDirectDrawSurface4_ReleaseDC(surface, dc); + hr = IDirectDrawSurface4_ReleaseDC(surface2, dc); ok(SUCCEEDED(hr), "Failed to release DC for format %s, hr %#x.\n", test_data[i].name, hr); - hr = IDirectDrawSurface4_Unlock(surface, NULL); + hr = IDirectDrawSurface4_Unlock(surface2, NULL); ok(SUCCEEDED(hr), "Failed to unmap surface for format %s, hr %#x.\n", test_data[i].name, hr);
hr = IDirectDrawSurface4_Lock(surface2, NULL, &map_desc, DDLOCK_READONLY | DDLOCK_WAIT, NULL); diff --git a/dlls/ddraw/tests/ddraw7.c b/dlls/ddraw/tests/ddraw7.c index 20a1bc5f3af..849db47a6f4 100644 --- a/dlls/ddraw/tests/ddraw7.c +++ b/dlls/ddraw/tests/ddraw7.c @@ -12748,13 +12748,13 @@ static void test_getdc(void) hr = IDirectDrawSurface7_Unlock(surface, NULL); ok(SUCCEEDED(hr), "Failed to unmap surface for format %s, hr %#x.\n", test_data[i].name, hr);
- hr = IDirectDrawSurface7_Lock(surface, NULL, &map_desc, DDLOCK_READONLY | DDLOCK_WAIT, NULL); + hr = IDirectDrawSurface7_Lock(surface2, NULL, &map_desc, DDLOCK_READONLY | DDLOCK_WAIT, NULL); ok(SUCCEEDED(hr), "Failed to map surface for format %s, hr %#x.\n", test_data[i].name, hr); - hr = IDirectDrawSurface7_GetDC(surface, &dc); + hr = IDirectDrawSurface7_GetDC(surface2, &dc); ok(SUCCEEDED(hr), "Failed to get DC for format %s, hr %#x.\n", test_data[i].name, hr); - hr = IDirectDrawSurface7_ReleaseDC(surface, dc); + hr = IDirectDrawSurface7_ReleaseDC(surface2, dc); ok(SUCCEEDED(hr), "Failed to release DC for format %s, hr %#x.\n", test_data[i].name, hr); - hr = IDirectDrawSurface7_Unlock(surface, NULL); + hr = IDirectDrawSurface7_Unlock(surface2, NULL); ok(SUCCEEDED(hr), "Failed to unmap surface for format %s, hr %#x.\n", test_data[i].name, hr);
hr = IDirectDrawSurface7_Lock(surface2, NULL, &map_desc, DDLOCK_READONLY | DDLOCK_WAIT, NULL);
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=62968
Your paranoid android.
=== w8adm (32 bit report) ===
ddraw: ddraw1.c:3191: Test failed: Got unexpected hr 0x887601c2.
=== w8 (32 bit report) ===
ddraw: 0ea0:ddraw2: unhandled exception c0000005 at 69FF3599
=== wvistau64 (32 bit report) ===
ddraw: 0910:ddraw4: unhandled exception c0000005 at 73DE6143
=== w2008s64 (32 bit report) ===
ddraw: 0390:ddraw4: unhandled exception c0000005 at 74486143
=== wvistau64 (64 bit report) ===
ddraw: 0910:ddraw4: unhandled exception c0000005 at 000007FEF6301883
=== w864 (64 bit report) ===
ddraw: ddraw4.c:3477: Test failed: Failed to create surface, hr 0x887601c2. 08ec:ddraw4: unhandled exception c0000005 at 00000000004AEFA3
=== wvistau64 (32 bit report) ===
ddraw: 06ec:ddraw7: unhandled exception c0000005 at 74126143
=== w2008s64 (32 bit report) ===
ddraw: 0390:ddraw7: unhandled exception c0000005 at 74486143
=== wvistau64 (64 bit report) ===
ddraw: 08e4:ddraw7: unhandled exception c0000005 at 000007FEF6301883
On Tue, 7 Jan 2020 at 15:10, Stefan Dösinger stefan@codeweavers.com wrote:
The way it stands it is just a repeat of the test a few lines earlier. I guess this was the original intention. And no, it does not affect the E_FAIL on the Geforce 9600.
It was a port of the equivalent d3d9 test. In the d3d9 test the GetDC() call fails, and the second instance of this sequence tests calling ReleaseDC() after the failed GetDC(), while the first instance doesn't, mostly to avoid the possibility of the invalid ReleaseDC() breaking subsequent tests. The second instance of the sequence in the ddraw tests could have been removed, but at the time I felt it was easier to compare the ddraw and d3d9 tests if it was kept.
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=62966
Your paranoid android.
=== wvistau64 (32 bit report) ===
ddraw: 0914:ddraw4: unhandled exception c0000005 at 73DE6143
=== w2008s64 (32 bit report) ===
ddraw: 0390:ddraw4: unhandled exception c0000005 at 74486143
=== wvistau64 (64 bit report) ===
ddraw: 0910:ddraw4: unhandled exception c0000005 at 000007FEF6301883
=== wvistau64 (32 bit report) ===
ddraw: 0388:ddraw7: unhandled exception c0000005 at 74126143
=== w2008s64 (32 bit report) ===
ddraw: 0390:ddraw7: unhandled exception c0000005 at 74486143
=== wvistau64 (64 bit report) ===
ddraw: 08d8:ddraw7: unhandled exception c0000005 at 000007FEF6301883
-
Henri Verbeet -
Henri Verbeet -
Marvin -
Stefan Dösinger