[PATCH 0/1] MR8549: dxgi/tests: Avoid uses of uninitialized variable when swapchain fails to create.
From: Yuxuan Shui yshui@codeweavers.com
--- dlls/dxgi/tests/dxgi.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/dlls/dxgi/tests/dxgi.c b/dlls/dxgi/tests/dxgi.c index 03bccc50a37..fc2b6d36df4 100644 --- a/dlls/dxgi/tests/dxgi.c +++ b/dlls/dxgi/tests/dxgi.c @@ -2733,7 +2733,7 @@ static void test_set_fullscreen(IUnknown *device, BOOL is_d3d12) struct swapchain_fullscreen_state initial_state; DXGI_SWAP_CHAIN_DESC swapchain_desc; IDXGIAdapter *adapter = NULL; - IDXGISwapChain *swapchain; + IDXGISwapChain *swapchain = NULL; IDXGIFactory *factory; IDXGIOutput *output; BOOL fullscreen; @@ -2762,6 +2762,11 @@ static void test_set_fullscreen(IUnknown *device, BOOL is_d3d12) capture_fullscreen_state(&initial_state.fullscreen_state, swapchain_desc.OutputWindow); hr = IDXGIFactory_CreateSwapChain(factory, device, &swapchain_desc, &swapchain); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + if (FAILED(hr)) + { + skip("Could not create swapchain\n"); + goto done; + } hr = IDXGISwapChain_GetContainingOutput(swapchain, &output); ok(hr == S_OK || broken(hr == DXGI_ERROR_UNSUPPORTED), /* Win 7 testbot */ "Got unexpected hr %#lx.\n", hr); @@ -2866,8 +2871,11 @@ static void test_set_fullscreen(IUnknown *device, BOOL is_d3d12) done: if (adapter) IDXGIAdapter_Release(adapter); - refcount = IDXGISwapChain_Release(swapchain); - ok(!refcount, "IDXGISwapChain has %lu references left.\n", refcount); + if (swapchain) + { + refcount = IDXGISwapChain_Release(swapchain); + ok(!refcount, "IDXGISwapChain has %lu references left.\n", refcount); + } check_window_fullscreen_state(swapchain_desc.OutputWindow, &initial_state.fullscreen_state); DestroyWindow(swapchain_desc.OutputWindow);
How many more of these are there going to be? This is a waste of time in tests, either they succeed or they don't. If an earlier test fails it doesn't matter what any of the following code does; it needs to be fixed anyway.
On Thu Jul 17 23:50:25 2025 +0000, Elizabeth Figura wrote:
How many more of these are there going to be? This is a waste of time in tests, either they succeed or they don't. If an earlier test fails it doesn't matter what any of the following code does; it needs to be fixed anyway.
This added check is in line with two other checks just a few lines below. and creating swapchain can fail outside winetest's control (e.g. because of an unsuitable driver, etc.). so a fail here doesn't (necessarily) mean a bug in wine.
unless you are suggesting something else?
This added check is in line with two other checks just a few lines below.
No, those calls can actually fail without failing tests. This one can't.
and creating swapchain can fail outside winetest's control (e.g. because of an unsuitable driver, etc.). so a fail here doesn't (necessarily) mean a bug in wine.
Can it? Do you have evidence of this happening?
If it can, then we need to change the test itself, and at that point of course we would exit early. But at the moment it seems that if it's possible to create a device, then that CreateSwapchain() call should also succeed.
Can it? Do you have evidence of this happening?
Yes, that was how I found these problems, I am not finding these by reading the code.
I think this happens when you run inside Xvfb (so llvmpipe) but while also have a real GPU. If I also point wine to the lavapipe vulkan icd file, then swapchain creation succeed and no memory problem were seen. so maybe something to do with driver/device selection.
On Tue Jul 22 16:46:36 2025 +0000, Yuxuan Shui wrote:
Can it? Do you have evidence of this happening?
Yes, that was how I found these problems, I am not finding these by reading the code. I think this happens when you run inside Xvfb (so llvmpipe) but while also have a real GPU. If I also point wine to the lavapipe vulkan icd file, then swapchain creation succeed and no memory problem were seen. so maybe something to do with driver/device selection.
Okay, given the history I assumed these came from ASan.
If this is Wine-specific it's probable that there's something we need to change in Wine, if not just assuming it's a setup bug.
Either way, if there's a test failure, we should fix the failure, not just work around a crash that happens after it.
On Tue Jul 22 17:20:44 2025 +0000, Elizabeth Figura wrote:
Okay, given the history I assumed these came from ASan. If this is Wine-specific it's probable that there's something we need to change in Wine, if not just assuming it's a setup bug. Either way, if there's a test failure, we should fix the failure, not just work around a crash that happens after it.
hmm, this could be a bug with driver/device selection in wine, i guess. i can look into this later. as this doesn't happen inside gitlab CI environment this is not that important rn.
closing.
This merge request was closed by Yuxuan Shui.
-
Elizabeth Figura (＠zfigura) -
Yuxuan Shui
-
Yuxuan Shui (＠yshui)