[PATCH 1/1] d3d8: Don't assert on invalid IDirect3DBaseTexture8 interfaces.

[Rico Schüller]

On 15.04.2013 13:50, Stefan Dösinger wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Am 2013-04-15 10:53, schrieb Rico Schüller:
>> I'm not sure what GetTexture does, a test might show it (I'll have
>> a look). The problem might be, that we use some members, which
>> native probably doesn't do in SetTexture. You couldn't call
>> GetTexture in wine with uninitialized memory as we do currently,
>> that's why I think using NULL is the way to go.
> Given the circumstances, the patch is probably the correct way to go, yes.
>
> Does the game just do the broken SetTexture call once, or does it
> repeatedly try to use the released texture?

Only once, the game does what the attached test tries to do. GetTexture 
wont work and may also crash on win.

>
> What happens when you pass an invalid pointer (e.g. (void
> *)0xdeadbeef)? What happens when you pass a valid pointer that points
> to memory filled with zeros or garbage? If SetTexture does not crash,
> what is the return value?

Using deadbeef will crash immediately, you may have luck when deadbeef 
points into something valid. The same goes for zeros or garbage values, 
but it looks like SetTexture accepts them more likely than the other 
approach and won't crash that often. As our internal structure layout 
may be different (likely it is) it is like trowing dices to crash here 
or there.

Note: The assert as is looks a way too strict, but removing it again 
works only till the dsound patch, which obviously overwrites something 
we depend on. The game may only work on windows by luck, since this only 
happens the first run of the game at all.

>
>> Please have a look at http://bugs.winehq.org/show_bug.cgi?id=33055
>> . Well I think there might be some memory corruption somewhere, but
>> it doesn't have anything to do with the this bug as we use already
>> freed memory in our implementation.
> I don't see anything in the log that would keep the texture around.
> There may be some capability flag issues that trigger a broken
> codepath in the game, but such an issue is really hard to find.
>

-------------- next part --------------
diff --git a/dlls/d3d8/tests/device.c b/dlls/d3d8/tests/device.c
index 2f281d2..1c8992d 100644
--- a/dlls/d3d8/tests/device.c
+++ b/dlls/d3d8/tests/device.c
@@ -4768,6 +4768,105 @@ static void test_rtpatch(void)
     DestroyWindow(window);
 }
 
+static void test_InvalidTexture(void)
+{
+    IDirect3DDevice8 *device;
+    IDirect3D8 *d3d8;
+    UINT refcount = 0;
+    HWND window;
+    HRESULT hr;
+    IDirect3DTexture8 *texture = NULL;
+    IDirect3DTexture8 *texture2 = NULL;
+    DWORD *d;
+
+    const struct {
+        float x, y, z;
+    } quad[] =
+    {
+        {-1.0f, -1.0f, -0.5f},
+        {-1.0f,  1.0f, -0.5f},
+        { 1.0f, -1.0f,  1.5f},
+        { 1.0f,  1.0f,  1.5f},
+    };
+
+    if (!(d3d8 = pDirect3DCreate8(D3D_SDK_VERSION)))
+    {
+        skip("Failed to create d3d8 object, skipping tests.\n");
+        return;
+    }
+
+    window = CreateWindowA("d3d8_test_wc", "d3d8_test", WS_OVERLAPPEDWINDOW,
+            0, 0, 640, 480, 0, 0, 0, 0);
+    if (!(device = create_device(d3d8, window, window, TRUE)))
+    {
+        skip("Failed to create a D3D device, skipping tests.\n");
+        IDirect3D8_Release(d3d8);
+        DestroyWindow(window);
+        return;
+    }
+
+    hr = IDirect3DDevice8_SetVertexShader(device, D3DFVF_XYZ);
+
+    hr = IDirect3DDevice8_CreateTexture(device, 32, 32, 3, 0, D3DFMT_X8R8G8B8, D3DPOOL_DEFAULT, &texture2);
+    hr = IDirect3DDevice8_CreateTexture(device, 32, 32, 3, 0, D3DFMT_X8R8G8B8, D3DPOOL_DEFAULT, &texture);
+    if (texture && texture2)
+    {
+        hr = IDirect3DDevice8_SetTexture(device, 0, (IDirect3DBaseTexture8 *) texture);
+        ok(hr == D3D_OK, "SetTexture returned hr %#x.\n", hr);
+
+        /* HeapFree all memory */
+        hr = IDirect3DDevice8_SetTexture(device, 0, NULL);
+        ok(hr == D3D_OK, "SetTexture returned hr %#x.\n", hr);
+        IDirect3DTexture8_Release(texture);
+
+        /* Test some values - write to released memory / simulate new heapalloc,
+         * but due to lack of getting the the same address, just write there.
+         * By accident dsound seems do overwrite some values in my system, ... it may
+         * be also overwritten by something else, that's why the old behaviour won't
+         * work any more.
+         * Details see bug 33055 */
+        d = texture;
+        d[0] = 0;
+
+        hr = IDirect3DDevice8_SetTexture(device, 0, (IDirect3DBaseTexture8 *) texture);
+        ok(hr == D3D_OK, "SetTexture returned hr %#x.\n", hr);
+
+/*
+        {
+            IDirect3DTexture8 *texture3 = NULL;
+            hr = IDirect3DDevice8_GetTexture(device, 0, (IDirect3DBaseTexture8 **) &texture3);
+            ok(hr == D3D_OK, "SetTexture returned hr %#x.\n", hr);
+        }*/
+
+        /* depending on the value in d[] the crash happens at differen positions below here */
+        hr = IDirect3DDevice8_SetTexture(device, 0, (IDirect3DBaseTexture8 *) texture2);
+        ok(hr == D3D_OK, "SetTexture returned hr %#x.\n", hr);
+
+        /* draw */
+        hr = IDirect3DDevice8_BeginScene(device);
+        ok(SUCCEEDED(hr), "Failed to begin scene, hr %#x.\n", hr);
+        hr = IDirect3DDevice8_DrawPrimitiveUP(device, D3DPT_TRIANGLELIST, 0, quad, sizeof(quad));
+        ok(SUCCEEDED(hr), "Failed to draw, hr %#x.\n", hr);
+        hr = IDirect3DDevice8_EndScene(device);
+        ok(SUCCEEDED(hr), "Failed to end scene, hr %#x.\n", hr);
+
+        hr = IDirect3DDevice8_SetTexture(device, 0, NULL);
+        ok(hr == D3D_OK, "SetTexture returned hr %#x.\n", hr);
+
+#if 0
+        /* may crash on win */
+        IDirect3DTexture8_Release(texture2);
+#endif
+    }
+#if 0
+    /* may crash on wine */
+    refcount = IDirect3DDevice8_Release(device);
+    ok(!refcount, "Device has %u references left.\n", refcount);
+    IDirect3D8_Release(d3d8);
+    DestroyWindow(window);
+#endif
+}
+
 START_TEST(device)
 {
     HMODULE d3d8_handle = LoadLibraryA( "d3d8.dll" );
@@ -4799,7 +4898,7 @@ START_TEST(device)
 
         screen_width = GetSystemMetrics(SM_CXSCREEN);
         screen_height = GetSystemMetrics(SM_CYSCREEN);
-
+/*
         test_fpu_setup();
         test_display_modes();
         test_shader_versions();
@@ -4843,7 +4942,8 @@ START_TEST(device)
         test_surface_lockrect_blocks();
         test_set_palette();
         test_swvp_buffer();
-        test_rtpatch();
+        test_rtpatch();*/
+        test_InvalidTexture();
     }
     UnregisterClassA("d3d8_test_wc", GetModuleHandleA(NULL));
 }