[PATCH 0/3] MR758: d3d10: Test D3D10CreateEffectFromMemory.
The test results show that data passed to D3D10CreateEffectFromMemory won't be modified. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/758
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/d3d10/tests/effect.c | 58 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/dlls/d3d10/tests/effect.c b/dlls/d3d10/tests/effect.c index 3bd6d6d0ecc..6cbe57ac62a 100644 --- a/dlls/d3d10/tests/effect.c +++ b/dlls/d3d10/tests/effect.c @@ -8244,6 +8244,63 @@ static void test_effect_fx_4_1(void) ok(!refcount, "Device has %lu references left.\n", refcount); } +static void test_effect_data(void) +{ + ID3D10EffectPool *pool; + ID3D10Effect *effect; + ID3D10Device *device; + DWORD buffer[1024]; + ULONG refcount; + HRESULT hr; + + if (!(device = create_device())) + { + skip("Failed to create device, skipping tests.\n"); + return; + } + + memcpy(buffer, fx_test_ecbt, sizeof(fx_test_ecbt)); + + if (strcmp(winetest_platform, "wine")) /* Crash on wine. */ + { + hr = create_effect(buffer, 0, NULL, NULL, &effect); + ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#lx.\n", hr); + ok(!memcmp(buffer, fx_test_ecbt, sizeof(fx_test_ecbt)), "Data is not modified.\n"); + } + + hr = create_effect(buffer, 0, device, NULL, &effect); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + ok(!memcmp(buffer, fx_test_ecbt, sizeof(fx_test_ecbt)), "Data is not modified.\n"); + effect->lpVtbl->Release(effect); + + hr = create_effect(buffer, D3D10_EFFECT_COMPILE_ALLOW_SLOW_OPS, device, NULL, &effect); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + ok(!memcmp(buffer, fx_test_ecbt, sizeof(fx_test_ecbt)), "Data is not modified.\n"); + effect->lpVtbl->Release(effect); + + hr = create_effect(buffer, D3D10_EFFECT_SINGLE_THREADED, device, NULL, &effect); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + ok(!memcmp(buffer, fx_test_ecbt, sizeof(fx_test_ecbt)), "Data is not modified.\n"); + effect->lpVtbl->Release(effect); + + memcpy(buffer, fx_test_pool_child, sizeof(fx_test_pool_child)); + + hr = create_effect(buffer, D3D10_EFFECT_COMPILE_CHILD_EFFECT, device, NULL, &effect); + ok(hr == E_INVALIDARG, "Got unexpected hr %#lx.\n", hr); + ok(!memcmp(buffer, fx_test_pool_child, sizeof(fx_test_pool_child)), "Data is not modified.\n"); + + hr = create_effect_pool(fx_test_pool, device, &pool); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + hr = create_effect(buffer, D3D10_EFFECT_COMPILE_CHILD_EFFECT, device, pool, &effect); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + ok(!memcmp(buffer, fx_test_pool_child, sizeof(fx_test_pool_child)), "Data is not modified.\n"); + effect->lpVtbl->Release(effect); + pool->lpVtbl->Release(pool); + + refcount = ID3D10Device_Release(device); + ok(!refcount, "Device has %lu references left.\n", refcount); +} + START_TEST(effect) { test_effect_constant_buffer_type(); @@ -8271,4 +8328,5 @@ START_TEST(effect) test_effect_index_expression(); test_effect_value_expression(); test_effect_fx_4_1(); + test_effect_data(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/758
From: Ziqing Hui <zhui(a)codeweavers.com> --- dlls/d3d10/tests/effect.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dlls/d3d10/tests/effect.c b/dlls/d3d10/tests/effect.c index 6cbe57ac62a..c25d3288b52 100644 --- a/dlls/d3d10/tests/effect.c +++ b/dlls/d3d10/tests/effect.c @@ -7117,6 +7117,12 @@ static void test_effect_pool(void) todo_wine ok(hr == E_INVALIDARG, "Got unexpected hr %#lx.\n", hr); + if (strcmp(winetest_platform, "wine")) /* Crash on wine. */ + { + hr = create_effect_pool(fx_test_pool, NULL, &pool); + ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#lx.\n", hr); + } + hr = create_effect_pool(fx_test_pool, device, &pool); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/758
From: Ziqing Hui <zhui(a)codeweavers.com> Passing NULL data to D3D10CreateEffectFromMemory crashes. Passing NULL data to D3D10CreateEffectPoolFromMemory returns E_INVALIDARG. --- dlls/d3d10/effect.c | 6 ++++++ dlls/d3d10/tests/effect.c | 7 ------- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/dlls/d3d10/effect.c b/dlls/d3d10/effect.c index 84ecfdcbbab..a9c8eaed5e8 100644 --- a/dlls/d3d10/effect.c +++ b/dlls/d3d10/effect.c @@ -9537,6 +9537,9 @@ static HRESULT d3d10_create_effect(void *data, SIZE_T data_size, ID3D10Device *d struct d3d10_effect *object; HRESULT hr; + if (!device) + return D3DERR_INVALIDCALL; + if (!(object = calloc(1, sizeof(*object)))) return E_OUTOFMEMORY; @@ -9636,6 +9639,9 @@ HRESULT WINAPI D3D10CreateEffectPoolFromMemory(void *data, SIZE_T data_size, UIN TRACE("data %p, data_size %Iu, fx_flags %#x, device %p, effect_pool %p.\n", data, data_size, fx_flags, device, effect_pool); + if (!data) + return E_INVALIDARG; + if (FAILED(hr = d3d10_create_effect(data, data_size, device, NULL, D3D10_EFFECT_IS_POOL, &object))) { diff --git a/dlls/d3d10/tests/effect.c b/dlls/d3d10/tests/effect.c index c25d3288b52..a13ef2bfb91 100644 --- a/dlls/d3d10/tests/effect.c +++ b/dlls/d3d10/tests/effect.c @@ -7114,14 +7114,10 @@ static void test_effect_pool(void) ok(!!device2, "Failed to create d3d device.\n"); hr = D3D10CreateEffectPoolFromMemory(NULL, 0, 0, device, &pool); - todo_wine ok(hr == E_INVALIDARG, "Got unexpected hr %#lx.\n", hr); - if (strcmp(winetest_platform, "wine")) /* Crash on wine. */ - { hr = create_effect_pool(fx_test_pool, NULL, &pool); ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#lx.\n", hr); - } hr = create_effect_pool(fx_test_pool, device, &pool); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); @@ -8267,12 +8263,9 @@ static void test_effect_data(void) memcpy(buffer, fx_test_ecbt, sizeof(fx_test_ecbt)); - if (strcmp(winetest_platform, "wine")) /* Crash on wine. */ - { hr = create_effect(buffer, 0, NULL, NULL, &effect); ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#lx.\n", hr); ok(!memcmp(buffer, fx_test_ecbt, sizeof(fx_test_ecbt)), "Data is not modified.\n"); - } hr = create_effect(buffer, 0, device, NULL, &effect); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/758
Nikolay Sivov (@nsivov) commented about dlls/d3d10/tests/effect.c:
+ if (strcmp(winetest_platform, "wine")) /* Crash on wine. */ + { + hr = create_effect(buffer, 0, NULL, NULL, &effect); + ok(hr == D3DERR_INVALIDCALL, "Got unexpected hr %#lx.\n", hr); + ok(!memcmp(buffer, fx_test_ecbt, sizeof(fx_test_ecbt)), "Data is not modified.\n"); + } + + hr = create_effect(buffer, 0, device, NULL, &effect); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + ok(!memcmp(buffer, fx_test_ecbt, sizeof(fx_test_ecbt)), "Data is not modified.\n"); + effect->lpVtbl->Release(effect); + + hr = create_effect(buffer, D3D10_EFFECT_COMPILE_ALLOW_SLOW_OPS, device, NULL, &effect); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + ok(!memcmp(buffer, fx_test_ecbt, sizeof(fx_test_ecbt)), "Data is not modified.\n"); + effect->lpVtbl->Release(effect); Could you explain, where does this come from? Why would you think that input blob would ever change?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/758#note_7439
On Thu Sep 1 09:49:55 2022 +0000, Nikolay Sivov wrote:
Could you explain, where does this come from? Why would you think that input blob would ever change? This is my fault, see https://gitlab.winehq.org/wine/wine/-/merge_requests/697#note_7369 / https://gitlab.winehq.org/wine/wine/-/merge_requests/332#note_3151.
I should have mentioned that I didn't expect a Wine test to check for that, just a manual look on the side (in fact I was picturing this as VirtualAlloc(), copy the source there, VirtualProtect(PAGE_READONLY), call D3D10CreateEffectFromMemory(), run on Windows and verify that it doesn't crash). I will probably have a go at it myself. I don't think that at this point we have a good reason to have the tests from this patch in Wine. Sorry for the extra work that I accidentally had you do. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/758#note_7453
Rest of the MR seems fine to me -- https://gitlab.winehq.org/wine/wine/-/merge_requests/758#note_7456
On Thu Sep 1 12:07:39 2022 +0000, Matteo Bruni wrote:
This is my fault, see https://gitlab.winehq.org/wine/wine/-/merge_requests/697#note_7369 / https://gitlab.winehq.org/wine/wine/-/merge_requests/332#note_3151. I should have mentioned that I didn't expect a Wine test to check for that, just a manual look on the side (in fact I was picturing this as VirtualAlloc(), copy the source there, VirtualProtect(PAGE_READONLY), call D3D10CreateEffectFromMemory(), run on Windows and verify that it doesn't crash). I will probably have a go at it myself. I don't think that at this point we have a good reason to have the tests from this patch in Wine. Sorry for the extra work that I accidentally had you do. FWIW I did the test I mentioned earlier and no surprises, everything works just fine.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/758#note_7461
participants (4)
-
Matteo Bruni (＠Mystral) -
Nikolay Sivov (＠nsivov)
-
Ziqing Hui
-
Ziqing Hui (＠zhui)