[PATCH v2 0/4] MR2436: d3dcompiler: Always initialize output shader blob pointer in D3DCompile2().
-- v2: d3dcompiler: Fix bytecode buffer leaks in D3DCompile2(). d3dcompiler: Always free messages string in D3DCompile2(). d3dcompiler: Allow D3DCompile2() to succeed with null output shader blob pointer. d3dcompiler: Always initialize output shader blob pointer in D3DCompile2(). https://gitlab.winehq.org/wine/wine/-/merge_requests/2436
From: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/d3dcompiler_43/compiler.c | 2 ++ dlls/d3dcompiler_43/tests/hlsl_d3d9.c | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/dlls/d3dcompiler_43/compiler.c b/dlls/d3dcompiler_43/compiler.c index 8fb38cf592c..d3c7b81df12 100644 --- a/dlls/d3dcompiler_43/compiler.c +++ b/dlls/d3dcompiler_43/compiler.c @@ -462,6 +462,8 @@ HRESULT WINAPI D3DCompile2(const void *data, SIZE_T data_size, const char *filen if (secondary_flags) FIXME("Ignoring secondary flags %#x.\n", secondary_flags); + if (shader_blob) + *shader_blob = NULL; if (messages_blob) *messages_blob = NULL; diff --git a/dlls/d3dcompiler_43/tests/hlsl_d3d9.c b/dlls/d3dcompiler_43/tests/hlsl_d3d9.c index a24cb9692e9..15af598c90d 100644 --- a/dlls/d3dcompiler_43/tests/hlsl_d3d9.c +++ b/dlls/d3dcompiler_43/tests/hlsl_d3d9.c @@ -1412,7 +1412,8 @@ static void test_fail(void) { for (i = 0; i < ARRAY_SIZE(tests); ++i) { - compiled = errors = NULL; + errors = NULL; + compiled = (void *)0xdeadbeef; hr = D3DCompile(tests[i], strlen(tests[i]), NULL, NULL, NULL, "test", targets[j], 0, 0, &compiled, &errors); ok(hr == E_FAIL, "Test %u, target %s: Got unexpected hr %#lx.\n", i, targets[j], hr); ok(!!errors, "Test %u, target %s, expected non-NULL error blob.\n", i, targets[j]); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2436
From: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/d3dcompiler_43/compiler.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/d3dcompiler_43/compiler.c b/dlls/d3dcompiler_43/compiler.c index a3c7b22581f..3f302f23503 100644 --- a/dlls/d3dcompiler_43/compiler.c +++ b/dlls/d3dcompiler_43/compiler.c @@ -552,8 +552,8 @@ HRESULT WINAPI D3DCompile2(const void *data, SIZE_T data_size, const char *filen } memcpy(ID3D10Blob_GetBufferPointer(*messages_blob), messages, size); } - else - vkd3d_shader_free_messages(messages); + + vkd3d_shader_free_messages(messages); } if (!ret && shader_blob) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2436
From: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/d3dcompiler_43/compiler.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/dlls/d3dcompiler_43/compiler.c b/dlls/d3dcompiler_43/compiler.c index 3f302f23503..a7926785184 100644 --- a/dlls/d3dcompiler_43/compiler.c +++ b/dlls/d3dcompiler_43/compiler.c @@ -556,17 +556,21 @@ HRESULT WINAPI D3DCompile2(const void *data, SIZE_T data_size, const char *filen vkd3d_shader_free_messages(messages); } - if (!ret && shader_blob) + if (ret) + return hresult_from_vkd3d_result(ret); + + if (!shader_blob) { - if (FAILED(hr = D3DCreateBlob(byte_code.size, shader_blob))) - { - vkd3d_shader_free_shader_code(&byte_code); - return hr; - } - memcpy(ID3D10Blob_GetBufferPointer(*shader_blob), byte_code.code, byte_code.size); + vkd3d_shader_free_shader_code(&byte_code); + return S_OK; } - return hresult_from_vkd3d_result(ret); + if (SUCCEEDED(hr = D3DCreateBlob(byte_code.size, shader_blob))) + memcpy(ID3D10Blob_GetBufferPointer(*shader_blob), byte_code.code, byte_code.size); + + vkd3d_shader_free_shader_code(&byte_code); + + return hr; } HRESULT WINAPI D3DCompile(const void *data, SIZE_T data_size, const char *filename, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2436
From: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/d3dcompiler_43/compiler.c | 2 +- dlls/d3dcompiler_43/tests/hlsl_d3d9.c | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/dlls/d3dcompiler_43/compiler.c b/dlls/d3dcompiler_43/compiler.c index d3c7b81df12..a3c7b22581f 100644 --- a/dlls/d3dcompiler_43/compiler.c +++ b/dlls/d3dcompiler_43/compiler.c @@ -556,7 +556,7 @@ HRESULT WINAPI D3DCompile2(const void *data, SIZE_T data_size, const char *filen vkd3d_shader_free_messages(messages); } - if (!ret) + if (!ret && shader_blob) { if (FAILED(hr = D3DCreateBlob(byte_code.size, shader_blob))) { diff --git a/dlls/d3dcompiler_43/tests/hlsl_d3d9.c b/dlls/d3dcompiler_43/tests/hlsl_d3d9.c index 15af598c90d..b52b840f3bf 100644 --- a/dlls/d3dcompiler_43/tests/hlsl_d3d9.c +++ b/dlls/d3dcompiler_43/tests/hlsl_d3d9.c @@ -1742,6 +1742,22 @@ static void test_include(void) delete_directory(L"include"); } +static void test_no_output_blob(void) +{ + static const char vs_source[] = + "float4 main(float4 pos : POSITION, inout float2 texcoord : TEXCOORD0) : POSITION\n" + "{\n" + " return pos;\n" + "}"; + ID3D10Blob *errors; + HRESULT hr; + + errors = (void *)0xdeadbeef; + hr = D3DCompile(vs_source, strlen(vs_source), NULL, NULL, NULL, "main", "vs_2_0", 0, 0, NULL, &errors); + ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); + ok(!errors, "Unexpected errors blob.\n"); +} + START_TEST(hlsl_d3d9) { HMODULE mod; @@ -1770,4 +1786,5 @@ START_TEST(hlsl_d3d9) test_constant_table(); test_fail(); test_include(); + test_no_output_blob(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2436
On Mon Mar 20 14:42:47 2023 +0000, Matteo Bruni wrote:
This can leak byte_code. Pushed something for that. That was pre-existing issue, right?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2436#note_27433
On Tue Mar 21 19:02:20 2023 +0000, Nikolay Sivov wrote:
Pushed something for that. That was pre-existing issue, right? Indeed, I had missed that.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2436#note_27466
Thanks for the additional leak fixes :) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2436#note_27467
This merge request was approved by Matteo Bruni. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2436
participants (3)
-
Matteo Bruni (＠Mystral) -
Nikolay Sivov
-
Nikolay Sivov (＠nsivov)