[PATCH v2 0/1] MR4155: tests: Build tests with -Wl,--large-address-aware.
-- v2: tests: Build tests with -Wl,--large-address-aware. https://gitlab.winehq.org/wine/wine/-/merge_requests/4155
From: Zebediah Figura <zfigura(a)codeweavers.com> d3d tests are currently running out of address space in gitlab CI. There are other ways to fix this than using -Wl,--large-address-aware, but this is the easiest, and I do not see any drawbacks to it. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53217 --- dlls/kernel32/tests/heap.c | 7 +++---- dlls/ntdll/tests/info.c | 4 ++-- dlls/ntdll/tests/wow64.c | 3 +-- tools/makedep.c | 5 +++++ 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/dlls/kernel32/tests/heap.c b/dlls/kernel32/tests/heap.c index d05eac33174..897597d1b4a 100644 --- a/dlls/kernel32/tests/heap.c +++ b/dlls/kernel32/tests/heap.c @@ -3629,16 +3629,15 @@ static void test_GlobalMemoryStatus(void) ok( memex.ullAvailExtendedVirtual == 0, "got ullAvailExtendedVirtual %#I64x\n", memex.ullAvailExtendedVirtual ); ok( mem.dwMemoryLoad == memex.dwMemoryLoad, "got dwMemoryLoad %lu\n", mem.dwMemoryLoad ); - ok( mem.dwTotalPhys == min( ~(SIZE_T)0 >> 1, memex.ullTotalPhys ) || - broken( mem.dwTotalPhys == ~(SIZE_T)0 ) /* Win <= 8.1 with RAM size > 4GB */, + ok( mem.dwTotalPhys == min( ~(SIZE_T)0, memex.ullTotalPhys ), "got dwTotalPhys %#Ix\n", mem.dwTotalPhys ); - ok( IS_WITHIN_RANGE( mem.dwAvailPhys, min( ~(SIZE_T)0 >> 1, memex.ullAvailPhys ) ) || - broken( mem.dwAvailPhys == ~(SIZE_T)0 ) /* Win <= 8.1 with RAM size > 4GB */, + ok( IS_WITHIN_RANGE( mem.dwAvailPhys, min( ~(SIZE_T)0, memex.ullAvailPhys ) ), "got dwAvailPhys %#Ix\n", mem.dwAvailPhys ); #ifndef _WIN64 todo_wine_if(memex.ullTotalPageFile > 0xfff7ffff) #endif ok( mem.dwTotalPageFile == min( ~(SIZE_T)0, memex.ullTotalPageFile ), "got dwTotalPageFile %#Ix\n", mem.dwTotalPageFile ); + todo_wine ok( IS_WITHIN_RANGE( mem.dwAvailPageFile, min( ~(SIZE_T)0, memex.ullAvailPageFile ) ), "got dwAvailPageFile %#Ix\n", mem.dwAvailPageFile ); ok( mem.dwTotalVirtual == memex.ullTotalVirtual, "got dwTotalVirtual %#Ix\n", mem.dwTotalVirtual ); ok( mem.dwAvailVirtual == memex.ullAvailVirtual, "got dwAvailVirtual %#Ix\n", mem.dwAvailVirtual ); diff --git a/dlls/ntdll/tests/info.c b/dlls/ntdll/tests/info.c index 3b9bad56ea4..2aeaa79e8d4 100644 --- a/dlls/ntdll/tests/info.c +++ b/dlls/ntdll/tests/info.c @@ -192,8 +192,8 @@ static void test_query_basic(void) ok( sbi.HighestUserAddress == sbi2.HighestUserAddress, "Expected HighestUserAddress %p, got %p.\n", (void *)sbi.HighestUserAddress, (void *)sbi2.HighestUserAddress); #else - ok( sbi.HighestUserAddress == (void *)0x7ffeffff, "wrong limit %p\n", sbi.HighestUserAddress); - todo_wine_if( old_wow64 ) + /* 0x7ffeffff without large-address-aware */ + ok( sbi.HighestUserAddress == (void *)0xfffeffff, "wrong limit %p\n", sbi.HighestUserAddress); ok( sbi2.HighestUserAddress == (is_wow64 ? (void *)0xfffeffff : (void *)0x7ffeffff), "wrong limit %p\n", sbi.HighestUserAddress); #endif diff --git a/dlls/ntdll/tests/wow64.c b/dlls/ntdll/tests/wow64.c index 99f6a6dec65..7408ae9fb95 100644 --- a/dlls/ntdll/tests/wow64.c +++ b/dlls/ntdll/tests/wow64.c @@ -1688,8 +1688,7 @@ static void test_nt_wow64(void) ok( status == STATUS_SUCCESS, "failed %lx\n", status ); ok( len == sizeof(sbi2), "wrong length %ld\n", len ); - ok( sbi.HighestUserAddress == (void *)0x7ffeffff, "wrong limit %p\n", sbi.HighestUserAddress); - todo_wine_if( old_wow64 ) + ok( sbi.HighestUserAddress == (void *)0xfffeffff, "wrong limit %p\n", sbi.HighestUserAddress); ok( sbi2.HighestUserAddress == (is_wow64 ? (void *)0xfffeffff : (void *)0x7ffeffff), "wrong limit %p\n", sbi.HighestUserAddress); diff --git a/tools/makedep.c b/tools/makedep.c index 8495d93d8c6..6ac55a1c745 100644 --- a/tools/makedep.c +++ b/tools/makedep.c @@ -3515,6 +3515,7 @@ static void output_test_module( struct makefile *make, unsigned int arch ) strarray_add( &make->all_targets[arch], testmodule ); strarray_add( &make->clean_files, stripped ); + output( "%s:\n", obj_dir_path( make, testmodule )); output_winegcc_command( make, arch ); output_filenames( make->extradllflags ); @@ -3522,9 +3523,11 @@ static void output_test_module( struct makefile *make, unsigned int arch ) output_filenames_obj_dir( make, make->res_files[arch] ); if ((debug_file = get_debug_file( make, testmodule, arch ))) output_filename( strmake( "-Wl,--debug-file,%s", obj_dir_path( make, debug_file ))); + output_filename( "-Wl,--large-address-aware" ); output_filenames( all_libs ); output_filename( arch_make_variable( "LDFLAGS", arch )); output( "\n" ); + output( "%s:\n", obj_dir_path( make, stripped )); output_winegcc_command( make, arch ); output_filename( "-s" ); @@ -3532,9 +3535,11 @@ static void output_test_module( struct makefile *make, unsigned int arch ) output_filenames( make->extradllflags ); output_filenames_obj_dir( make, make->object_files[arch] ); output_filenames_obj_dir( make, make->res_files[arch] ); + output_filename( "-Wl,--large-address-aware" ); output_filenames( all_libs ); output_filename( arch_make_variable( "LDFLAGS", arch )); output( "\n" ); + output( "%s %s:", obj_dir_path( make, testmodule ), obj_dir_path( make, stripped )); output_filenames_obj_dir( make, make->object_files[arch] ); output_filenames_obj_dir( make, make->res_files[arch] ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4155
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=138952 Your paranoid android. === w864 (32 bit report) === kernel32: heap.c:2397: Test failed: LocalLock succeeded === w1064v1507 (32 bit report) === kernel32: heap.c:2397: Test failed: LocalLock succeeded === w1064v1809 (32 bit report) === kernel32: heap.c:2397: Test failed: LocalLock succeeded === w1064_tsign (32 bit report) === kernel32: heap.c:2397: Test failed: LocalLock succeeded === w10pro64 (32 bit report) === kernel32: heap.c:2397: Test failed: LocalLock succeeded === w10pro64_en_AE_u8 (32 bit report) === kernel32: heap.c:2397: Test failed: LocalLock succeeded heap.c:3314: Test failed: HeapValidate failed heap.c:3323: Test failed: HeapValidate failed === w11pro64 (32 bit report) === kernel32: heap.c:2397: Test failed: LocalLock succeeded === w7u_2qxl (32 bit report) === ntdll: info.c:196: Test failed: wrong limit 7FFEFFFF === w7u_adm (32 bit report) === ntdll: info.c:196: Test failed: wrong limit 7FFEFFFF === w7u_el (32 bit report) === ntdll: info.c:196: Test failed: wrong limit 7FFEFFFF === w8 (32 bit report) === ntdll: info.c:196: Test failed: wrong limit 7FFEFFFF === w8adm (32 bit report) === ntdll: info.c:196: Test failed: wrong limit 7FFEFFFF === debian11 (32 bit report) === ntdll: info.c:197: Test failed: wrong limit FFFEFFFF wow64.c:1692: Test failed: wrong limit FFFEFFFF === debian11 (32 bit ar:MA report) === ntdll: info.c:197: Test failed: wrong limit FFFEFFFF wow64.c:1692: Test failed: wrong limit FFFEFFFF === debian11 (32 bit de report) === ntdll: info.c:197: Test failed: wrong limit FFFEFFFF wow64.c:1692: Test failed: wrong limit FFFEFFFF === debian11 (32 bit fr report) === ntdll: info.c:197: Test failed: wrong limit FFFEFFFF wow64.c:1692: Test failed: wrong limit FFFEFFFF === debian11 (32 bit he:IL report) === ntdll: info.c:197: Test failed: wrong limit FFFEFFFF wow64.c:1692: Test failed: wrong limit FFFEFFFF === debian11 (32 bit hi:IN report) === ntdll: info.c:197: Test failed: wrong limit FFFEFFFF wow64.c:1692: Test failed: wrong limit FFFEFFFF === debian11 (32 bit ja:JP report) === ntdll: info.c:197: Test failed: wrong limit FFFEFFFF wow64.c:1692: Test failed: wrong limit FFFEFFFF === debian11 (32 bit zh:CN report) === ntdll: info.c:197: Test failed: wrong limit FFFEFFFF wow64.c:1692: Test failed: wrong limit FFFEFFFF === debian11b (64 bit WoW report) === kernel32: heap.c:3641: Test succeeded inside todo block: got dwAvailPageFile 0x17a916000
Please correct me if I'm wrong, but I think it's safer to use `EXTRADLLFLAGS` for specific modules instead.
I don't know about safer. If anything I'd argue this is the "safer" solution in a sense, since it turns out that more than just d3d tests are running out of address space, and I don't want to spend ages playing whackamole. By contrast there's only a couple of tests in ntdll / kernel32 that make assumptions about the address space limit. It turns out that this also addresses bug 55725 (amstream:amstream), and I wouldn't be surprised if also addressed bug 55605 (winmm:mci).
Otherwise, could you take a look at the following failures?
Addressed, plus another test failure in kernel32:heap. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4155#note_49555
The bigger question is whether all these tests really have unusual usage patterns, or whether this is simply going to hide memory leaks/excessive memory usage that will impact real apps. And if so, shouldn't the tests instead be fixed to better reflect usage patterns of real apps? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4155#note_49557
Yes, test_cube_maps() has unusual usage patterns; all of our d3d tests do. In a very general sense, realistic usage patterns are great to test, but not all results can be tested the most efficiently that way, and some are kind of impossible to test in a realistic manner. If we were simulating a real application we would be presenting to a swapchain, and we would not be constantly reading back GPU textures. Both of these things contribute to test_cube_maps() running out of memory (presentation submits a command buffer in d3d which triggers garbage collection, readback consumes CPU memory). But adding presentation would slow down the test and distract from it, and we fundamentally *can't* test that things work without readback. Also, a real application would usually not be using a software renderer, which takes up a lot more CPU memory by virtue of (a) spawning CPU threads to do the work that the GPU normally does, (b) storing textures in CPU memory that are normally in GPU memory. It's also worth mentioning that, at least in the case of test_cube_maps(), the unusual usage patterns only end up contributing to a small part of the total address space usage. Much of the rest is out of our control. Some of it can maybe be addressed upstream, if we can convince Mesa to care about 32-bit applications, but it will take forever to propagate to our test VMs, and CI will simply be broken until then. Some of it can't (I do not in a million years expect we will ever be able to do anything about LLVM). Most other solutions will take a lot of effort that just doesn't seem worthwhile for tests. And yes, fundamentally, increasing the amount of available address space means that real leaks are less likely to show up. But that's true no matter how we increase it. Trying to increase it a little bit, so that we have *just enough* address space available, seems like the kind of solution that is not going to last very long, and we're going to be right back here again soon enough. I can only feel like it makes more sense to use a larger hammer. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4155#note_49559
Windows doesn't need --large-address-aware to pass our d3d tests, right? Meanwhile Wine needs --large-address-aware to pass our d3d tests with CPU memory hungry drivers, since Wine reserves almost all of the higher half of address space and this deprives Mesa of memory. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4155#note_49626
Windows doesn't need --large-address-aware to pass our d3d tests, right?
No. Windows also doesn't need it to run plenty of real applications that Wine chokes on. It's worth mentioning that those applications aren't necessarily choking on the same things that d3d10core_test.exe is choking on, though.
Meanwhile Wine needs --large-address-aware to pass our d3d tests with CPU memory hungry drivers, since Wine reserves almost all of the higher half of address space and this deprives Mesa of memory.
Yes. It's more than just Mesa, of course. What makes test_cube_maps() *unusual* is mostly Mesa, in one way or another. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4155#note_49646
This merge request was closed by Zebediah Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4155
This is probably addresesed by d9ad68a1ef1bb9288f5f39199a6042f013e6b464, at least for the time being, so closing. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4155#note_49928
participants (5)
-
Alexandre Julliard (＠julliard) -
Jinoh Kang (＠iamahuman)
-
Marvin -
Zebediah Figura
-
Zebediah Figura (＠zfigura)