- Wine-Devel - list.winehq.org

[PATCH] user32: Implement WindowFromPhysicalPoint
by Jinoh Kang Oct. 3, 2021

Oct. 3, 2021

physical<->logical is an identity mapping in other functions and also >win8, so make it consistent with those. Signed-off-by: Jinoh Kang <jinoh.kang.kr(a)gmail.com> --- .../ext-ms-win-ntuser-uicontext-ext-l1-1-0.spec | 2 +- .../ext-ms-win-ntuser-window-l1-1-1.spec | 2 +- dlls/user32/sysparams.c | 8 ++++++++ dlls/user32/user32.spec | 1 + 4 files changed, 11 insertions(+), 2 deletions(-) diff --git a/dlls/ext-ms-win-ntuser-uicontext-ext-l1-1-0/ext-ms-win-ntuser-uicontext-ext-l1-1-0.spec b/dlls/ext-ms-win-ntuser-uicontext-ext-l1-1-0/ext-ms-win-ntuser-uicontext-ext-l1-1-0.spec index ea155c098d9..acc03f665de 100644 --- a/dlls/ext-ms-win-ntuser-uicontext-ext-l1-1-0/ext-ms-win-ntuser-uicontext-ext-l1-1-0.spec +++ b/dlls/ext-ms-win-ntuser-uicontext-ext-l1-1-0/ext-ms-win-ntuser-uicontext-ext-l1-1-0.spec @@ -51,5 +51,5 @@ @ stub SoundSentry @ stdcall UnhookWindowsHookEx(long) user32.UnhookWindowsHookEx @ stdcall UpdateLayeredWindow(long long ptr ptr long ptr long ptr long) user32.UpdateLayeredWindow -@ stub WindowFromPhysicalPoint +@ stdcall WindowFromPhysicalPoint(int64) user32.WindowFromPhysicalPoint @ stdcall WindowFromPoint(int64) user32.WindowFromPoint diff --git a/dlls/ext-ms-win-ntuser-window-l1-1-1/ext-ms-win-ntuser-window-l1-1-1.spec b/dlls/ext-ms-win-ntuser-window-l1-1-1/ext-ms-win-ntuser-window-l1-1-1.spec index ea155c098d9..acc03f665de 100644 --- a/dlls/ext-ms-win-ntuser-window-l1-1-1/ext-ms-win-ntuser-window-l1-1-1.spec +++ b/dlls/ext-ms-win-ntuser-window-l1-1-1/ext-ms-win-ntuser-window-l1-1-1.spec @@ -51,5 +51,5 @@ @ stub SoundSentry @ stdcall UnhookWindowsHookEx(long) user32.UnhookWindowsHookEx @ stdcall UpdateLayeredWindow(long long ptr ptr long ptr long ptr long) user32.UpdateLayeredWindow -@ stub WindowFromPhysicalPoint +@ stdcall WindowFromPhysicalPoint(int64) user32.WindowFromPhysicalPoint @ stdcall WindowFromPoint(int64) user32.WindowFromPoint diff --git a/dlls/user32/sysparams.c b/dlls/user32/sysparams.c index d6f08654ab0..733acd3d3c6 100644 --- a/dlls/user32/sysparams.c +++ b/dlls/user32/sysparams.c @@ -4506,6 +4506,14 @@ BOOL WINAPI SetPhysicalCursorPos( INT x, INT y ) return SetCursorPos( x, y ); } +/*********************************************************************** + * WindowFromPhysicalPoint (USER32.@) + */ +HWND WINAPI WindowFromPhysicalPoint( POINT pt ) +{ + return WindowFromPoint( pt ); +} + /*********************************************************************** * LogicalToPhysicalPoint (USER32.@) */ diff --git a/dlls/user32/user32.spec b/dlls/user32/user32.spec index 8d3ac70210c..8d5f1bc4145 100644 --- a/dlls/user32/user32.spec +++ b/dlls/user32/user32.spec @@ -820,6 +820,7 @@ # @ stub WinOldAppHackoMatic @ stdcall WindowFromDC(long) @ stdcall WindowFromPoint(int64) +@ stdcall WindowFromPhysicalPoint(int64) # @ stub YieldTask # @ stub _SetProcessDefaultLayout @ stdcall keybd_event(long long long long) -- 2.31.1

1 0

GCC 11
by Eric Pouech Oct. 3, 2021

Oct. 3, 2021

For sharing : - newest gcc version (11) sets by default of bunch of new tests for code correctness - it spits out around ten thousand lines of warnings (*), making it quite impossible to look into compiler output there are a couple of genuine issues reported by gcc11 - Rémi and I already sent a serie of patches for addressing those - a strange indentation waiting for a volunteer (in dlls/wininet/internet.c:2878, the line lpwhh->ErrorMask = *(ULONG*)lpBuffer; at the end of the if/else chain is misindented) unfortunately, most of the generated warnings are false positives, that need to be fixed (or removerd) before GCC11 becomes mainstream - Fedora 35 with GCC11 is in beta right now, and GA is mid of October - I haven't checked other distros though, but I expect similar evolution in coming weeks/months - in Fedora both gcc and mingw-gcc have been upgraded to version 11 The false positives we need to tackle: A) misleading indentation =================== GCC11 complains about code like: foo; todo_wine ok(tst, ...); bar; (gcc11 warns about 'bar ' not being properly indented wrt todo_wine) there are (as of today) 817 occurrences of those in wine tree (spread across 127 files) and counts for ~90% (*) of the line of warnings generated by GCC only 2 true positives are generated by this GCC warning Remarks: - constructs on a single line don't generate the warning todo_wine ok(tst, ...); // ok no warning, whatever the indentation of the line wrt the rest of the code - having the ok(tst, ...) inside a block after the todo_wine doesn't generate the warning possible solutions: - disable the warning in configure cons: will not trigger on true positive pros: minor impact on code base (either committed and under progress in dev:s trees) - merge the todo_wine and ok() line into a single one pros: keep current indentation, coherent with todo_wine + block cons: harder to read, esp. for todo_wine_if cons: for todo_wine_if, could generate too long lines cons: large impact on code base pros/cons: git blame on ok() line will tell who removed the todo_wine, not who wrote the test - reindent only problematic todo_wine foo; todo_wine ok(tst, ...); bar; ^ without indenting the line ok(tst, ...); ^ without indenting the "todo_wine {\n<...>\n}" nor the "todo_wine ok()" on a single line constructs as they don't generate warnings pros/cons: indentation isn't used for adding comments to the code (that's what comments are for) cons: large impact on code base - reindent all todo_wine same as above, but also reindenting the "todo_wine {\n<...>\n}" and the "todo_wine ok()" on a single line constructs pros/cons: indentation isn't used for adding comments to the code (that's what comments are for) cons: large impact on code base B) uninitialized objects =============== GCC11 warns on code like: void foo(const TYPE* s); ..... TYPE s; foo(&s); because, s isn't initialized, and is passed as const ptr, hence could be read by function foo without being initialized proposal: - depending on context, different fixes are at hand: either initialize s, or when applicable, play with aliasing C) partial structures ============ In a bunch of places, we use a pointer to a structure, where underlying allocated object is smaller than the actual size of the structure. (roughly 400 lines of warnings (*)) It's used: - when dealing with two structures sharing their first fields (like in BITMAP for example) - in tests, to test behavior will various sizes of objects as input... proposal: disable this warning in configure D) sizeof/sizeof ========= GCC11 warns when sizeof(foo)/sizeof(bar) is used and cannot be reduced to computing the number of elements of an array proposal: rewrite as sizeof(foo)/(sizeof(bar)) (note the parenthesis around sizeof(bar), as suggested by GCC) A+ (*) those values are rough estimates, only consider the order of magnitude

3 8

[PATCH v2] gdi32: Create the registry keys recursively, if needed.
by Gabriel Ivăncescu Oct. 3, 2021

Oct. 3, 2021

Fixes a regression introduced by 1cdc74b2d62d1c94005c46f9c8f4b566aa8bdcbd when creating new prefixes, as NtCreateKey does not recursively create keys. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- Most of the code is taken from kernelbase's create_key, but simplified to gdi32's cases only. dlls/gdi32/font.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/dlls/gdi32/font.c b/dlls/gdi32/font.c index 98c70a6..d54f9a5 100644 --- a/dlls/gdi32/font.c +++ b/dlls/gdi32/font.c @@ -29,6 +29,8 @@ #include <stdlib.h> #include <string.h> #include <assert.h> +#include "ntstatus.h" +#define WIN32_NO_STATUS #include "winerror.h" #include "windef.h" #include "winbase.h" @@ -569,11 +571,13 @@ static HKEY reg_open_key( HKEY root, const WCHAR *name, ULONG name_len ) return ret; } +/* wrapper for NtCreateKey that creates the key recursively if necessary */ static HKEY reg_create_key( HKEY root, const WCHAR *name, ULONG name_len, DWORD options, DWORD *disposition ) { UNICODE_STRING nameW = { name_len, name_len, (WCHAR *)name }; OBJECT_ATTRIBUTES attr; + NTSTATUS status; HANDLE ret; attr.Length = sizeof(attr); @@ -583,7 +587,28 @@ static HKEY reg_create_key( HKEY root, const WCHAR *name, ULONG name_len, attr.SecurityDescriptor = NULL; attr.SecurityQualityOfService = NULL; - if (NtCreateKey( &ret, MAXIMUM_ALLOWED, &attr, 0, NULL, options, disposition )) return 0; + status = NtCreateKey( &ret, MAXIMUM_ALLOWED, &attr, 0, NULL, options, disposition ); + if(status == STATUS_OBJECT_NAME_NOT_FOUND) + { + DWORD pos = 0, i = 0, len = name_len / sizeof(WCHAR); + + while (i < len && name[i] != '\\') i++; + if (i == len) return 0; + for (;;) + { + nameW.Buffer = (WCHAR *)name + pos; + nameW.Length = (i - pos) * sizeof(WCHAR); + status = NtCreateKey( &ret, MAXIMUM_ALLOWED, &attr, 0, NULL, options, disposition ); + + if (attr.RootDirectory != root) NtClose( attr.RootDirectory ); + if (!NT_SUCCESS(status)) return 0; + if (i == len) break; + attr.RootDirectory = ret; + while (i < len && name[i] == '\\') i++; + pos = i; + while (i < len && name[i] != '\\') i++; + } + } return ret; } -- 2.31.1

2 1

[PATCH] gdi32: Create the Software\Wine\Fonts keys one at a time.
by Gabriel Ivăncescu Oct. 3, 2021

Oct. 3, 2021

Fixes a regression introduced by 1cdc74b2d62d1c94005c46f9c8f4b566aa8bdcbd when creating new prefixes, as NtCreateKey does not recursively create keys. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/gdi32/font.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/dlls/gdi32/font.c b/dlls/gdi32/font.c index 98c70a6..31310b1 100644 --- a/dlls/gdi32/font.c +++ b/dlls/gdi32/font.c @@ -6209,19 +6209,31 @@ UINT font_init(void) OBJECT_ATTRIBUTES attr = { sizeof(attr) }; UNICODE_STRING name; HANDLE mutex, hkcu; + HKEY key, wine_key; DWORD disposition; UINT dpi = 0; static WCHAR wine_font_mutexW[] = {'\\','B','a','s','e','N','a','m','e','d','O','b','j','e','c','t','s', '\\','_','_','W','I','N','E','_','F','O','N','T','_','M','U','T','E','X','_','_'}; - static const WCHAR wine_fonts_keyW[] = - {'S','o','f','t','w','a','r','e','\\','W','i','n','e','\\','F','o','n','t','s'}; + static const WCHAR softwareW[] = {'S','o','f','t','w','a','r','e'}; + static const WCHAR wineW[] = {'W','i','n','e'}; + static const WCHAR fontsW[] = {'F','o','n','t','s'}; static const WCHAR cacheW[] = {'C','a','c','h','e'}; if (RtlOpenCurrentUser( MAXIMUM_ALLOWED, &hkcu )) return 0; - wine_fonts_key = reg_create_key( hkcu, wine_fonts_keyW, sizeof(wine_fonts_keyW), 0, NULL ); - if (wine_fonts_key) dpi = init_font_options( hkcu ); + key = reg_create_key( hkcu, softwareW, sizeof(softwareW), 0, NULL ); + if (key) + { + wine_key = reg_create_key( key, wineW, sizeof(wineW), 0, NULL ); + NtClose( key ); + if (wine_key) + { + wine_fonts_key = reg_create_key( wine_key, fontsW, sizeof(fontsW), 0, NULL ); + NtClose( wine_key ); + if (wine_fonts_key) dpi = init_font_options( hkcu ); + } + } NtClose( hkcu ); if (!dpi) return 96; update_codepage( dpi ); -- 2.31.1

4 4

RFC: Reparse Point/NT Symlink Support [3]
by Erich E. Hoover Oct. 2, 2021

Oct. 2, 2021

I would love any and all feedback on updated versions of my "Reparse Point" patches, which are now available in staging: https://github.com/wine-staging/wine-staging/tree/master/patches/ntdll-Junc… Major changes since the last RFC version are: 1) Deletion of symlinks is now atomic (where supported). 2) Regular Unix symlinks are now reported as WSL Unix symlinks. 3) FILE_OPEN_REPARSE_POINT is now properly supported for applications that wish to access reparse points directly (symlink operations by file descriptor instead of by path). 4) Wine's cmd "dir" command now shows reparse point types and targets. 5) Wine's cmd "mklink" command now supports creating junction points and NT symlinks. 6) Absolute reparse points that are outside of the prefix no longer contain the path to drive Z (or appropriate drive letter). 7) Absolute reparse points now contain a string identifying the end of the Wine prefix that is used to modify the target path on file access if a prefix has been relocated (if the target is within the prefix). 8) Many, many bug fixes. Thank you in advance to those who look these patches over, and previous iterations, your feedback is greatly appreciated! I know that these patches have been a long time coming, but I believe that we're finally getting to a place where we've figured out all the gotchas. Hopefully other folks agree ;) === Context: For those of you that wonder "why do these patches exist?", I have been putting these together over the past few years for several reasons: 1) More modern Windows programs use NT symlinks to avoid duplicating files and taking up extra disk space (particularly .NET). 2) Wine currently has no mechanism to make symlinks on the "PE" side of the dividing line, these patches allow this for Windows file paths and a small additional patch (creating WSL Unix symlinks) allows the same behavior for Unix paths. 3) Wine currently takes up a lot of space with system DLLs that are duplicates of the system-wide files, a logical extension to these patches is a Wine-specific reparse tag that allows these system DLLs to be "copy-on-write" so that Wine prefixes don't take up unnecessary disk space. Please note that this feature is not a focus of the patchset (not included), as I would like to get the infrastructure in place before tackling that problem. Not currently supported: 1) Reparse points of type other than junction points, NT symlinks, and WSL Unix symlinks. 2) Creating WSL Unix symlinks. 3) Changing ownership permissions (lrwxrwxrwx) on the symlink instead of the target (currently a Linux limitation). Implementation overview: • Reparse points are implemented as Unix symlinks where relative symlinks always start with "./" and absolute symlinks start with "/". • Following the relative/absolute flag the 32-bit reparse point tag (junction point or NT symlink) is encoded as a relative path where each bit is encoded as "/" (0) or "./" (1). When creating WSL Unix symlinks (future feature) this tag encoding will be skipped. • For NT symlinks this is followed by a file/directory flag ("./" for directories, "/" for files) so that dangling symlinks can still be deleted properly with RemoveFile or RemoveDirectory (as appropriate). • After the reparse tag (and file/directory flag) the Unix path is then appended, relative paths are included unmodified and absolute paths have some minor modifications. • If the path is an absolute path and points outside the prefix then the portion of the path to drive Z (or appropriate drive letter) is removed. • If the path is an absolute path that points inside the prefix then after the prefix path an 8-bit 'P' is encoded the same way that the reparse point tag is ("/"=0, "./"=1). • If an absolute symlink is opened by the wineserver then the prefix path is checked (if appropriate) and if it doesn't match the path of the current Wine prefix then the symlink is modified to replace the old prefix path with the current prefix path. • Because reparse points involve a modification to an existing empty file/directory, symlink creation and "removal" is implemented by creating the symlink at a temporary location and then replacing the original with an atomic renameat2 call (on supported systems), equivalent call (BSD/Mac), or by unlinking the original and moving the symlink into place (for legacy systems that do not support atomic replacement). Best, Erich

4 17

[PATCH] d3dx10_34/tests: Enable tests.
by Nikolay Sivov Oct. 2, 2021

Oct. 2, 2021

Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- configure | 1 + configure.ac | 1 + dlls/d3dx10_34/Makefile.in | 1 + dlls/d3dx10_34/tests/Makefile.in | 6 ++++++ 4 files changed, 9 insertions(+) create mode 100644 dlls/d3dx10_34/tests/Makefile.in diff --git a/configure b/configure index db592f0868d..d4d14333b24 100755 --- a/configure +++ b/configure @@ -20339,6 +20339,7 @@ wine_fn_config_makefile dlls/d3drm enable_d3drm wine_fn_config_makefile dlls/d3drm/tests enable_tests wine_fn_config_makefile dlls/d3dx10_33 enable_d3dx10_33 wine_fn_config_makefile dlls/d3dx10_34 enable_d3dx10_34 +wine_fn_config_makefile dlls/d3dx10_34/tests enable_tests wine_fn_config_makefile dlls/d3dx10_35 enable_d3dx10_35 wine_fn_config_makefile dlls/d3dx10_35/tests enable_tests wine_fn_config_makefile dlls/d3dx10_36 enable_d3dx10_36 diff --git a/configure.ac b/configure.ac index 0bc46a1427b..0d5eba21d1c 100644 --- a/configure.ac +++ b/configure.ac @@ -3063,6 +3063,7 @@ WINE_CONFIG_MAKEFILE(dlls/d3drm) WINE_CONFIG_MAKEFILE(dlls/d3drm/tests) WINE_CONFIG_MAKEFILE(dlls/d3dx10_33) WINE_CONFIG_MAKEFILE(dlls/d3dx10_34) +WINE_CONFIG_MAKEFILE(dlls/d3dx10_34/tests) WINE_CONFIG_MAKEFILE(dlls/d3dx10_35) WINE_CONFIG_MAKEFILE(dlls/d3dx10_35/tests) WINE_CONFIG_MAKEFILE(dlls/d3dx10_36) diff --git a/dlls/d3dx10_34/Makefile.in b/dlls/d3dx10_34/Makefile.in index ffe8d90dbaf..6a7aa8446d9 100644 --- a/dlls/d3dx10_34/Makefile.in +++ b/dlls/d3dx10_34/Makefile.in @@ -1,4 +1,5 @@ MODULE = d3dx10_34.dll +IMPORTLIB = d3dx10_34 EXTRADLLFLAGS = -Wb,--prefer-native diff --git a/dlls/d3dx10_34/tests/Makefile.in b/dlls/d3dx10_34/tests/Makefile.in new file mode 100644 index 00000000000..9d07ee26b9a --- /dev/null +++ b/dlls/d3dx10_34/tests/Makefile.in @@ -0,0 +1,6 @@ +TESTDLL = d3dx10_34.dll +IMPORTS = d3dx10_34 ole32 gdi32 +PARENTSRC = ../../d3dx10_43/tests + +C_SRCS = \ + d3dx10.c -- 2.33.0

3 2

[PATCH v2 1/7] ntdll: Protect module list access in LdrFindEntryForAddress() with SRW lock.
by Paul Gofman Oct. 1, 2021

Oct. 1, 2021

Fixes racy access to LDR lists in lookup_function_info(). Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- Supersedes 215952-215974. dlls/ntdll/actctx.c | 6 ------ dlls/ntdll/loader.c | 21 ++++++++++++++++++--- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/dlls/ntdll/actctx.c b/dlls/ntdll/actctx.c index f9afb655885..bc222dadf27 100644 --- a/dlls/ntdll/actctx.c +++ b/dlls/ntdll/actctx.c @@ -705,10 +705,8 @@ static inline const char* debugstr_version(const struct assembly_version *ver) static NTSTATUS get_module_filename( HMODULE module, UNICODE_STRING *str, unsigned int extra_len ) { NTSTATUS status; - ULONG_PTR magic; LDR_DATA_TABLE_ENTRY *pldr; - LdrLockLoaderLock(0, NULL, &magic); status = LdrFindEntryForAddress( module, &pldr ); if (status == STATUS_SUCCESS) { @@ -721,7 +719,6 @@ static NTSTATUS get_module_filename( HMODULE module, UNICODE_STRING *str, unsign } else status = STATUS_NO_MEMORY; } - LdrUnlockLoaderLock(0, magic); return status; } @@ -3301,12 +3298,10 @@ static NTSTATUS find_query_actctx( HANDLE *handle, DWORD flags, ULONG class ) } else if (flags & (QUERY_ACTCTX_FLAG_ACTCTX_IS_ADDRESS|QUERY_ACTCTX_FLAG_ACTCTX_IS_HMODULE)) { - ULONG_PTR magic; LDR_DATA_TABLE_ENTRY *pldr; if (!*handle) return STATUS_INVALID_PARAMETER; - LdrLockLoaderLock( 0, NULL, &magic ); if (!LdrFindEntryForAddress( *handle, &pldr )) { if ((flags & QUERY_ACTCTX_FLAG_ACTCTX_IS_HMODULE) && *handle != pldr->DllBase) @@ -3315,7 +3310,6 @@ static NTSTATUS find_query_actctx( HANDLE *handle, DWORD flags, ULONG class ) *handle = pldr->ActivationContext; } else status = STATUS_DLL_NOT_FOUND; - LdrUnlockLoaderLock( 0, magic ); } else if (!*handle && (class != ActivationContextBasicInformation)) *handle = process_actctx; diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index e00c1e0c31d..55a6213d906 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -176,6 +176,10 @@ static PEB_LDR_DATA ldr = { &ldr.InMemoryOrderModuleList, &ldr.InMemoryOrderModuleList }, { &ldr.InInitializationOrderModuleList, &ldr.InInitializationOrderModuleList } }; +/* Ldr data is modified with loader locked and exclusive lock held. + * Taking shared lock to access the data is required outside of loader lock only. + */ +static RTL_SRWLOCK ldr_data_srw_lock = RTL_SRWLOCK_INIT; static RTL_BITMAP tls_bitmap; static RTL_BITMAP tls_expansion_bitmap; @@ -1245,10 +1249,12 @@ static WINE_MODREF *alloc_module( HMODULE hModule, const UNICODE_STRING *nt_name wm->ldr.EntryPoint = (char *)hModule + nt->OptionalHeader.AddressOfEntryPoint; } + RtlAcquireSRWLockExclusive( &ldr_data_srw_lock ); InsertTailList(&NtCurrentTeb()->Peb->LdrData->InLoadOrderModuleList, &wm->ldr.InLoadOrderLinks); InsertTailList(&NtCurrentTeb()->Peb->LdrData->InMemoryOrderModuleList, &wm->ldr.InMemoryOrderLinks); + RtlReleaseSRWLockExclusive( &ldr_data_srw_lock ); /* wait until init is called for inserting into InInitializationOrderModuleList */ if (!(nt->OptionalHeader.DllCharacteristics & IMAGE_DLLCHARACTERISTICS_NX_COMPAT)) @@ -1454,8 +1460,12 @@ static NTSTATUS process_attach( WINE_MODREF *wm, LPVOID lpReserved ) } if (!wm->ldr.InInitializationOrderLinks.Flink) + { + RtlAcquireSRWLockExclusive( &ldr_data_srw_lock ); InsertTailList(&NtCurrentTeb()->Peb->LdrData->InInitializationOrderModuleList, &wm->ldr.InInitializationOrderLinks); + RtlReleaseSRWLockExclusive( &ldr_data_srw_lock ); + } /* Call DLL entry point */ if (status == STATUS_SUCCESS) @@ -1578,13 +1588,14 @@ NTSTATUS WINAPI LdrDisableThreadCalloutsForDll(HMODULE hModule) /****************************************************************** * LdrFindEntryForAddress (NTDLL.@) * - * The loader_section must be locked while calling this function */ NTSTATUS WINAPI LdrFindEntryForAddress( const void *addr, PLDR_DATA_TABLE_ENTRY *pmod ) { + NTSTATUS ret = STATUS_NO_MORE_ENTRIES; PLIST_ENTRY mark, entry; PLDR_DATA_TABLE_ENTRY mod; + RtlAcquireSRWLockShared( &ldr_data_srw_lock ); mark = &NtCurrentTeb()->Peb->LdrData->InMemoryOrderModuleList; for (entry = mark->Flink; entry != mark; entry = entry->Flink) { @@ -1593,10 +1604,12 @@ NTSTATUS WINAPI LdrFindEntryForAddress( const void *addr, PLDR_DATA_TABLE_ENTRY (const char *)addr < (char*)mod->DllBase + mod->SizeOfImage) { *pmod = mod; - return STATUS_SUCCESS; + ret = STATUS_SUCCESS; + break; } } - return STATUS_NO_MORE_ENTRIES; + RtlReleaseSRWLockShared( &ldr_data_srw_lock ); + return ret; } /****************************************************************** @@ -3510,10 +3523,12 @@ void WINAPI LdrShutdownThread(void) */ static void free_modref( WINE_MODREF *wm ) { + RtlAcquireSRWLockExclusive( &ldr_data_srw_lock ); RemoveEntryList(&wm->ldr.InLoadOrderLinks); RemoveEntryList(&wm->ldr.InMemoryOrderLinks); if (wm->ldr.InInitializationOrderLinks.Flink) RemoveEntryList(&wm->ldr.InInitializationOrderLinks); + RtlReleaseSRWLockExclusive( &ldr_data_srw_lock ); TRACE(" unloading %s\n", debugstr_w(wm->ldr.FullDllName.Buffer)); if (!TRACE_ON(module)) -- 2.31.1

2 8

[PATCH 1/5] winegstreamer: Move the GstAutoplugSelectResult definition to wg_parser.c.
by Zebediah Figura Oct. 1, 2021

Oct. 1, 2021

Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/winegstreamer/gst_private.h | 7 ------- dlls/winegstreamer/wg_parser.c | 7 +++++++ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/dlls/winegstreamer/gst_private.h b/dlls/winegstreamer/gst_private.h index 49e06b31369..923bba25d38 100644 --- a/dlls/winegstreamer/gst_private.h +++ b/dlls/winegstreamer/gst_private.h @@ -36,13 +36,6 @@ #include "wine/debug.h" #include "wine/strmbase.h" -typedef enum -{ - GST_AUTOPLUG_SELECT_TRY, - GST_AUTOPLUG_SELECT_EXPOSE, - GST_AUTOPLUG_SELECT_SKIP, -} GstAutoplugSelectResult; - static inline const char *debugstr_time(REFERENCE_TIME time) { ULONGLONG abstime = time >= 0 ? time : -time; diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c index f0815e37689..dde137ef186 100644 --- a/dlls/winegstreamer/wg_parser.c +++ b/dlls/winegstreamer/wg_parser.c @@ -34,6 +34,13 @@ #include <gst/video/video.h> #include <gst/audio/audio.h> +typedef enum +{ + GST_AUTOPLUG_SELECT_TRY, + GST_AUTOPLUG_SELECT_EXPOSE, + GST_AUTOPLUG_SELECT_SKIP, +} GstAutoplugSelectResult; + /* GStreamer callbacks may be called on threads not created by Wine, and * therefore cannot access the Wine TEB. This means that we must use GStreamer * debug logging instead of Wine debug logging. In order to be safe we forbid -- 2.33.0

1 4

[PATCH 22/22] kernel32/tests: Add tests for loader locking.
by Paul Gofman Oct. 1, 2021

Oct. 1, 2021

Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> --- dlls/kernel32/tests/Makefile.in | 2 + dlls/kernel32/tests/actctx.c | 2 +- dlls/kernel32/tests/loader.c | 579 +++++++++++++++++++++++++++ dlls/kernel32/tests/locking_dll.c | 50 +++ dlls/kernel32/tests/locking_dll.spec | 1 + 5 files changed, 633 insertions(+), 1 deletion(-) create mode 100644 dlls/kernel32/tests/locking_dll.c create mode 100644 dlls/kernel32/tests/locking_dll.spec diff --git a/dlls/kernel32/tests/Makefile.in b/dlls/kernel32/tests/Makefile.in index e9516603ce9..be3a8fc8dfa 100644 --- a/dlls/kernel32/tests/Makefile.in +++ b/dlls/kernel32/tests/Makefile.in @@ -21,6 +21,8 @@ SOURCES = \ heap.c \ loader.c \ locale.c \ + locking_dll.c \ + locking_dll.spec \ mailslot.c \ module.c \ path.c \ diff --git a/dlls/kernel32/tests/actctx.c b/dlls/kernel32/tests/actctx.c index 163ea405222..f37796a6aba 100644 --- a/dlls/kernel32/tests/actctx.c +++ b/dlls/kernel32/tests/actctx.c @@ -2584,7 +2584,7 @@ static void delete_manifest_file(const char *filename) DeleteFileA(path); } -static void extract_resource(const char *name, const char *type, const char *path) +void extract_resource(const char *name, const char *type, const char *path) { DWORD written; HANDLE file; diff --git a/dlls/kernel32/tests/loader.c b/dlls/kernel32/tests/loader.c index 4f1b11338a6..df530055db4 100644 --- a/dlls/kernel32/tests/loader.c +++ b/dlls/kernel32/tests/loader.c @@ -70,6 +70,10 @@ static NTSTATUS (WINAPI *pLdrLockLoaderLock)(ULONG, ULONG *, ULONG_PTR *); static NTSTATUS (WINAPI *pLdrUnlockLoaderLock)(ULONG, ULONG_PTR); static NTSTATUS (WINAPI *pLdrLoadDll)(LPCWSTR,DWORD,const UNICODE_STRING *,HMODULE*); static NTSTATUS (WINAPI *pLdrUnloadDll)(HMODULE); + +typedef void (CALLBACK *LDRENUMPROC)(LDR_DATA_TABLE_ENTRY *, void *, BOOLEAN *); +static NTSTATUS (WINAPI *pLdrEnumerateLoadedModules)( void *unknown, LDRENUMPROC callback, void *context ); + static void (WINAPI *pRtlInitUnicodeString)(PUNICODE_STRING,LPCWSTR); static void (WINAPI *pRtlAcquirePebLock)(void); static void (WINAPI *pRtlReleasePebLock)(void); @@ -88,6 +92,8 @@ static BOOL (WINAPI *pWow64DisableWow64FsRedirection)(void **); static BOOL (WINAPI *pWow64RevertWow64FsRedirection)(void *); static HMODULE (WINAPI *pLoadPackagedLibrary)(LPCWSTR lpwLibFileName, DWORD Reserved); +void extract_resource(const char *name, const char *type, const char *path); + static PVOID RVAToAddr(DWORD_PTR rva, HMODULE module) { if (rva == 0) @@ -4036,6 +4042,574 @@ static void test_Wow64Transition(void) debugstr_wn(name->SectionFileName.Buffer, name->SectionFileName.Length / sizeof(WCHAR))); } +static BOOL test_loader_lock_repeat_lock; +static DWORD test_loader_lock_expected_wait_result; +static unsigned int test_loader_notification_count; +static unsigned int test_loader_expected_notification_count; +static const WCHAR *ldr_notify_track_dll; +static HMODULE lock_dll_handle; +static HANDLE lock_ready_event, next_test_event; +static BOOL test_loader_lock_test_failed; +static volatile LONG test_loader_lock_timeout_count; + +#define BLOCKING_TESTS_ENABLED 1 + +static void CALLBACK test_loader_lock_module_enum_locking_callback(LDR_DATA_TABLE_ENTRY *mod, + void *context, BOOLEAN *stop) +{ + DWORD *result = context; + + SetEvent(lock_ready_event); + *result = WaitForSingleObject(next_test_event, 3000); + *stop = TRUE; +} + +static void CALLBACK test_loader_lock_dummy_callback(LDR_DATA_TABLE_ENTRY *mod, + void *context, BOOLEAN *stop) +{ + *stop = TRUE; +} + +static void CALLBACK test_loader_lock_ldr_notify_locking(ULONG reason, LDR_DLL_NOTIFICATION_DATA *data, void *context) +{ + DWORD *result = context; + NTSTATUS status; + HMODULE hmodule; + void *cookie; + BOOL bret; + + if (reason != LDR_DLL_NOTIFICATION_REASON_UNLOADED) return; + + status = LdrRegisterDllNotification(0, test_loader_lock_ldr_notify_locking, NULL, &cookie); + ok(!status, "Got unexpected status %#x.\n", status); + status = LdrUnregisterDllNotification( cookie ); + ok(!status, "Got unexpected status %#x.\n", status); + + hmodule = LoadLibraryW(L"winmm.dll"); + ok(!!hmodule, "GetModuleHandleW failed, err %u.\n", GetLastError()); + bret = FreeLibrary(hmodule); + ok(bret, "FreeLibrary failed, err %u.\n", GetLastError()); + + bret = GetModuleHandleExW(0, L"lock.dll", &hmodule); + ok(bret, "GetModuleHandleExW failed, err %u.\n", GetLastError()); + ok(!!hmodule, "GetModuleHandleW failed, err %u.\n", GetLastError()); + bret = FreeLibrary(hmodule); + ok(bret, "FreeLibrary failed, err %u.\n", GetLastError()); + + hmodule = LoadLibraryW(L"authz.dll"); + ok(!hmodule, "LoadLibraryW succeeded.\n"); + ok(GetLastError() == ERROR_NOT_FOUND, "Got unexpected error %u.\n", GetLastError()); + + status = pLdrEnumerateLoadedModules(NULL, test_loader_lock_dummy_callback, NULL); + ok(!status, "Got unexpected status %#x.\n", status); + + SetEvent(lock_ready_event); + *result = WaitForSingleObject(next_test_event, 3000); +} + +static DWORD WINAPI test_loader_lock_thread(void *param) +{ + static const WCHAR env_var[] = L"test_wait_reason_value"; + void (WINAPI *p_set_lock_result_addr)( DWORD *addr ); + unsigned int test, iter; + ULONG_PTR magic; + NTSTATUS status; + HMODULE hmodule; + void *cookie; + DWORD result; + WCHAR str[8]; + BOOL bret; + + WaitForSingleObject(next_test_event, INFINITE); + + test = 1; + /* 1. Test with loader lock held. */ + iter = 0; + do + { + status = pLdrLockLoaderLock(0, NULL, &magic); + ok(!status, "Got unexpected status %#x.\n", status); + SetEvent(lock_ready_event); + + result = WaitForSingleObject(next_test_event, 3000); + + if (!iter && result == WAIT_TIMEOUT) + { + /* First test timed out, likely the old loader. */ + test_loader_lock_test_failed = TRUE; + status = pLdrUnlockLoaderLock(0, magic); + ok(!status, "Got unexpected status %#x.\n", status); + SetEvent(lock_ready_event); + return 0; + } + + ok(result == test_loader_lock_expected_wait_result, + "Got unexpected wait result %#x, expected %#x, test %u.%u.\n", + result, test_loader_lock_expected_wait_result, test, iter); + + todo_wine_if(test && iter == 1) + ok(test_loader_notification_count == test_loader_expected_notification_count, + "Got unexpected notification count %u, expected %u, test %u.%u.\n", + test_loader_notification_count, test_loader_expected_notification_count, test, iter); + + status = pLdrUnlockLoaderLock(0, magic); + ok(!status, "Got unexpected status %#x.\n", status); + + if (result == WAIT_TIMEOUT) + WaitForSingleObject(next_test_event, INFINITE); + ++iter; + } while (test_loader_lock_repeat_lock); + + SetEvent(lock_ready_event); + WaitForSingleObject(next_test_event, INFINITE); + /* 2. Test with the thread blocked in DLL entry point during process attach. */ + swprintf(str, ARRAY_SIZE(str), L"%u", DLL_PROCESS_ATTACH); + bret = SetEnvironmentVariableW(env_var, str); + ok(bret, "SetEnvironmentVariableW failed.\n"); + ++test; + iter = 0; + do + { + hmodule = LoadLibraryA("lock.dll"); + ok(!!hmodule, "LoadLibrary failed, err %u.\n", GetLastError()); + p_set_lock_result_addr = (void *)GetProcAddress(hmodule, "set_lock_result_addr"); + p_set_lock_result_addr(&result); + + if (result == WAIT_TIMEOUT) + InterlockedIncrement(&test_loader_lock_timeout_count); + + bret = FreeLibrary(hmodule); + ok(bret, "FreeLibrary failed, err %u.\n", GetLastError()); + + ok(result == test_loader_lock_expected_wait_result, + "Got unexpected wait result %#x, expected %#x, test %u.%u.\n", + result, test_loader_lock_expected_wait_result, test, iter); + + if (result == WAIT_TIMEOUT) + WaitForSingleObject(next_test_event, INFINITE); + ++iter; + } while (test_loader_lock_repeat_lock); + + SetEvent(lock_ready_event); + WaitForSingleObject(next_test_event, INFINITE); + + /* 3. Test with the thread blocked in DLL entry point during process detach. */ + swprintf(str, ARRAY_SIZE(str), L"%u", DLL_PROCESS_DETACH); + bret = SetEnvironmentVariableW(env_var, str); + ok(bret, "SetEnvironmentVariableW failed.\n"); + ++test; + iter = 0; + do + { + lock_dll_handle = hmodule = LoadLibraryA("lock.dll"); + ok(!!hmodule, "LoadLibrary failed, err %u.\n", GetLastError()); + p_set_lock_result_addr = (void *)GetProcAddress(hmodule, "set_lock_result_addr"); + ok(!!p_set_lock_result_addr, "Got NULL p_set_lock_result_addr.\n"); + p_set_lock_result_addr(&result); + bret = FreeLibrary(hmodule); + ok(bret, "FreeLibrary failed, err %u.\n", GetLastError()); + + lock_dll_handle = NULL; + + if (result == WAIT_TIMEOUT) + InterlockedIncrement(&test_loader_lock_timeout_count); + + ok(result == test_loader_lock_expected_wait_result, + "Got unexpected wait result %#x, expected %#x, test %u.%u.\n", + result, test_loader_lock_expected_wait_result, test, iter); + + if (result == WAIT_TIMEOUT) + WaitForSingleObject(next_test_event, INFINITE); + ++iter; + } while (test_loader_lock_repeat_lock); + + SetEvent(lock_ready_event); + WaitForSingleObject(next_test_event, INFINITE); + + /* 4. Test with the thread blocked in LdrEnumerateLoadedModules callback. */ + ++test; + iter = 0; + do + { + pLdrEnumerateLoadedModules(NULL, test_loader_lock_module_enum_locking_callback, &result); + + if (result == WAIT_TIMEOUT) + InterlockedIncrement(&test_loader_lock_timeout_count); + + ok(result == test_loader_lock_expected_wait_result, + "Got unexpected wait result %#x, expected %#x, test %u.%u.\n", + result, test_loader_lock_expected_wait_result, test, iter); + ok(test_loader_notification_count == test_loader_expected_notification_count, + "Got unexpected notification count %u, expected %u, test %u.%u.\n", + test_loader_notification_count, test_loader_expected_notification_count, test, iter); + + if (result == WAIT_TIMEOUT) + WaitForSingleObject(next_test_event, INFINITE); + ++iter; + } while (test_loader_lock_repeat_lock); + + SetEvent(lock_ready_event); + WaitForSingleObject(next_test_event, INFINITE); + + /* 5. Test with the thread blocked in LDR notification callback. */ + LdrRegisterDllNotification(0, test_loader_lock_ldr_notify_locking, &result, &cookie); + /* lock.dll will return FALSE from the DLL entry point. */ + bret = SetEnvironmentVariableW(env_var, L"-1"); + ok(bret, "SetEnvironmentVariableW failed.\n"); + + ++test; + iter = 0; + do + { + hmodule = LoadLibraryA("lock.dll"); + ok(!!hmodule, "LoadLibrary failed, err %u.\n", GetLastError()); + bret = FreeLibrary(hmodule); + ok(bret, "FreeLibrary failed, err %u.\n", GetLastError()); + + if (result == WAIT_TIMEOUT) + InterlockedIncrement(&test_loader_lock_timeout_count); + + ok(result == test_loader_lock_expected_wait_result, + "Got unexpected wait result %#x, expected %#x, test %u.%u.\n", + result, test_loader_lock_expected_wait_result, test, iter); + ok(test_loader_notification_count == test_loader_expected_notification_count, + "Got unexpected notification count %u, expected %u, test %u.%u.\n", + test_loader_notification_count, test_loader_expected_notification_count, test, iter); + + if (result == WAIT_TIMEOUT) + WaitForSingleObject(next_test_event, INFINITE); + ++iter; + } while (test_loader_lock_repeat_lock); + LdrUnregisterDllNotification( cookie ); + SetEvent(lock_ready_event); + return 0; +} + +static void CALLBACK test_loader_lock_ldr_notify(ULONG reason, LDR_DLL_NOTIFICATION_DATA *data, void *context) +{ + if (!lstrcmpW(data->Loaded.BaseDllName->Buffer, ldr_notify_track_dll)) + ++test_loader_notification_count; +} + +static void CALLBACK test_loader_lock_enum_callback(LDR_DATA_TABLE_ENTRY *mod, void *context, BOOLEAN *stop) +{ + *stop = TRUE; +} + +static void test_loader_lock(void) +{ +#define CHECK ok(!test_loader_lock_timeout_count, "Got timeout count %d.\n", test_loader_lock_timeout_count) + static const WCHAR not_loaded_dll_name[] = L"authz.dll"; + static const WCHAR preloaded_dll_name[] = L"winmm.dll"; + HMODULE hmodule_preloaded, hmodule; + ULONG_PTR magic; + NTSTATUS status; + HANDLE thread; + void *cookie; + void *proc; + BOOL bret; + + extract_resource("locking_dll.dll", "TESTDLL", "lock.dll"); + + lock_ready_event = CreateEventA(NULL, FALSE, FALSE, "test_lock_ready_event"); + next_test_event = CreateEventA(NULL, FALSE, FALSE, "test_next_test_event"); + + thread = CreateThread(NULL, 0, test_loader_lock_thread, NULL, 0, NULL); + + hmodule_preloaded = LoadLibraryW(preloaded_dll_name); + ok(!!hmodule_preloaded, "LoadLibrary failed, err %u.\n", GetLastError()); + hmodule = GetModuleHandleW(not_loaded_dll_name); + ok(!hmodule, "%s is already loaded.\n", not_loaded_dll_name); + + test_loader_notification_count = 0; + + /* 1. Test with loader lock held. */ + trace("Test 1.\n"); + SetEvent(next_test_event); + WaitForSingleObject(lock_ready_event, INFINITE); + test_loader_lock_expected_wait_result = WAIT_OBJECT_0; + + status = LdrRegisterDllNotification(0, test_loader_lock_ldr_notify, NULL, &cookie); + ok(!status, "Got unexpected status %#x.\n", status); + CHECK; + ldr_notify_track_dll = not_loaded_dll_name; + + bret = GetModuleHandleExW(0, preloaded_dll_name, &hmodule); + ok(bret, "GetModuleHandleEx failed, err %u.\n", GetLastError()); + ok(hmodule == hmodule_preloaded, "Got unexpected hmodule %p, expected %p.\n", hmodule, hmodule_preloaded); + CHECK; + + bret = FreeLibrary(hmodule); + ok(bret, "FreeLibrary failed, err %u.\n", GetLastError()); + CHECK; + + ok(!test_loader_notification_count, "Got unexpected test_loader_notification_count %u.\n", test_loader_notification_count); + + if (BLOCKING_TESTS_ENABLED) + { + CHECK; + test_loader_lock_repeat_lock = TRUE; + SetEvent(next_test_event); + WaitForSingleObject(lock_ready_event, INFINITE); + + if (test_loader_lock_test_failed) + { + win_skip("Old loader, tests skipped.\n"); + goto done; + } + + /* With loader lock held notification callback is called which should mean that: + * - LDR notifications themselves do not wait on loader lock; + * - The library load goes far enough to call the LDR notification until it blocks on the loader lock. + */ + test_loader_expected_notification_count = 1; + test_loader_lock_expected_wait_result = WAIT_TIMEOUT; + hmodule = LoadLibraryW(not_loaded_dll_name); + ok(!!hmodule, "LoadLibrary failed, err %u.\n", GetLastError()); + + SetEvent(next_test_event); + WaitForSingleObject(lock_ready_event, INFINITE); + + test_loader_notification_count = 0; + test_loader_expected_notification_count = 0; + bret = FreeLibrary(hmodule); + ok(bret, "FreeLibrary failed, err %u.\n", GetLastError()); + + ok(test_loader_notification_count == 1, "Got unexpected notification count %u, expected %u.\n", + test_loader_notification_count, 1); + test_loader_notification_count = 0; + test_loader_expected_notification_count = 0; + + SetEvent(next_test_event); + WaitForSingleObject(lock_ready_event, INFINITE); + pLdrEnumerateLoadedModules(NULL, test_loader_lock_enum_callback, NULL); + + test_loader_lock_repeat_lock = FALSE; + } + + LdrUnregisterDllNotification( cookie ); + + SetEvent(next_test_event); + WaitForSingleObject(lock_ready_event, INFINITE); + + if (test_loader_lock_test_failed) + { + win_skip("Old loader, tests skipped.\n"); + goto done; + } + + /* 2. Test with the thread blocked in DLL entry point during process attach. */ + trace("Test 2.\n"); + SetEvent(next_test_event); + + WaitForSingleObject(lock_ready_event, INFINITE); + test_loader_lock_expected_wait_result = WAIT_OBJECT_0; + + status = LdrRegisterDllNotification(0, test_loader_lock_ldr_notify, NULL, &cookie); + ok(!status, "Got unexpected status %#x.\n", status); + CHECK; + + bret = GetModuleHandleExW(0, preloaded_dll_name, &hmodule); + CHECK; + ok(bret, "GetModuleHandleEx failed, err %u.\n", GetLastError()); + ok(hmodule == hmodule_preloaded, "Got unexpected hmodule %p, expected %p.\n", hmodule, hmodule_preloaded); + + proc = GetProcAddress(hmodule, "timeGetTime"); + CHECK; + ok(!!proc, "GetProcAddress failed.\n"); + + bret = FreeLibrary(hmodule); + CHECK; + ok(bret, "FreeLibrary failed, err %u.\n", GetLastError()); + + hmodule = GetModuleHandleA("lock.dll"); + CHECK; + ok(!!hmodule, "GetModuleHandleA failed, err %u.\n", GetLastError()); + + bret = GetModuleHandleExW(0, L"lock.dll", &hmodule); + CHECK; + ok(bret, "GetModuleHandleEx failed, err %u.\n", GetLastError()); + + lock_dll_handle = hmodule; + + status = LdrAddRefDll(0, hmodule); + CHECK; + ok(!status, "Got unexpected status %#x.\n", status); + + bret = FreeLibrary(hmodule); + CHECK; + ok(bret, "FreeLibrary failed, err %u.\n", GetLastError()); + + bret = FreeLibrary(hmodule); + CHECK; + ok(bret, "FreeLibrary failed, err %u.\n", GetLastError()); + + hmodule = LoadLibraryW(preloaded_dll_name); + CHECK; + ok(!!hmodule, "LoadLibrary failed, err %u.\n", GetLastError()); + + bret = FreeLibrary(hmodule); + CHECK; + ok(bret, "FreeLibrary failed, err %u.\n", GetLastError()); + + if (BLOCKING_TESTS_ENABLED) + { + test_loader_lock_repeat_lock = TRUE; + SetEvent(next_test_event); + WaitForSingleObject(lock_ready_event, INFINITE); + + test_loader_lock_expected_wait_result = WAIT_TIMEOUT; + + ok(!!lock_dll_handle, "Got NULL lock_dll_handle.\n"); + proc = GetProcAddress(lock_dll_handle, "unknown"); + ok(!proc, "GetProcAddress succeeded.\n"); + + test_loader_lock_repeat_lock = FALSE; + } + + LdrUnregisterDllNotification( cookie ); + SetEvent(next_test_event); + WaitForSingleObject(lock_ready_event, INFINITE); + /* 3. Test with the thread blocked in DLL entry point during process detach. */ + trace("Test 3.\n"); + + SetEvent(next_test_event); + WaitForSingleObject(lock_ready_event, INFINITE); + test_loader_lock_timeout_count = 0; + test_loader_lock_expected_wait_result = WAIT_OBJECT_0; + + status = LdrRegisterDllNotification(0, test_loader_lock_ldr_notify, NULL, &cookie); + ok(!status, "Got unexpected status %#x.\n", status); + CHECK; + + bret = GetModuleHandleExW(0, preloaded_dll_name, &hmodule); + ok(bret, "GetModuleHandleEx failed, err %u.\n", GetLastError()); + ok(hmodule == hmodule_preloaded, "Got unexpected hmodule %p, expected %p.\n", hmodule, hmodule_preloaded); + CHECK; + + proc = GetProcAddress(hmodule, "timeGetTime"); + ok(!!proc, "GetProcAddress failed.\n"); + CHECK; + + bret = FreeLibrary(hmodule); + ok(bret, "FreeLibrary failed, err %u.\n", GetLastError()); + CHECK; + + ok(!!lock_dll_handle, "Got NULL lock_dll_handle.\n"); + + proc = GetProcAddress(lock_dll_handle, "set_lock_result_addr"); + CHECK; + + ok(!proc, "GetProcAddress failed, err %u.\n", GetLastError()); + ok(GetLastError() == ERROR_INVALID_PARAMETER, "Got unexpected error %u.\n", GetLastError()); + + status = LdrAddRefDll(0, lock_dll_handle); + CHECK; + ok(status == STATUS_DLL_NOT_FOUND, "Got unexpected status %#x.\n", status); + + if (BLOCKING_TESTS_ENABLED) + { + test_loader_lock_repeat_lock = TRUE; + SetEvent(next_test_event); + WaitForSingleObject(lock_ready_event, INFINITE); + + test_loader_lock_expected_wait_result = WAIT_TIMEOUT; + hmodule = GetModuleHandleW(L"lock.dll"); + ok(!hmodule, "GetModuleHandleW succeeded.\n", GetLastError()); + ok(GetLastError() == ERROR_MOD_NOT_FOUND, "Got unexpected error %u.\n", GetLastError()); + test_loader_lock_repeat_lock = FALSE; + } + + LdrUnregisterDllNotification( cookie ); + SetEvent(next_test_event); + WaitForSingleObject(lock_ready_event, INFINITE); + + /* 4. Test with the thread blocked in LdrEnumerateLoadedModules callback. */ + trace("Test 4.\n"); + + SetEvent(next_test_event); + WaitForSingleObject(lock_ready_event, INFINITE); + test_loader_lock_timeout_count = 0; + test_loader_lock_expected_wait_result = WAIT_OBJECT_0; + + status = LdrRegisterDllNotification(0, test_loader_lock_ldr_notify, NULL, &cookie); + ok(!status, "Got unexpected status %#x.\n", status); + CHECK; + + hmodule = GetModuleHandleW(preloaded_dll_name); + ok(!!hmodule, "LoadLibrary failed, err %u.\n", GetLastError()); + CHECK; + + if (BLOCKING_TESTS_ENABLED) + { + test_loader_lock_repeat_lock = TRUE; + SetEvent(next_test_event); + WaitForSingleObject(lock_ready_event, INFINITE); + + test_loader_lock_expected_wait_result = WAIT_TIMEOUT; + + status = pLdrLockLoaderLock(0, NULL, &magic); + ok(!status, "Got unexpected status %#x.\n", status); + status = pLdrUnlockLoaderLock(0, magic); + ok(!status, "Got unexpected status %#x.\n", status); + + test_loader_lock_repeat_lock = FALSE; + } + + LdrUnregisterDllNotification( cookie ); + + SetEvent(next_test_event); + WaitForSingleObject(lock_ready_event, INFINITE); + + /* 5. Test with the thread blocked in LDR notification callback. */ + trace("Test 5.\n"); + SetEvent(next_test_event); + WaitForSingleObject(lock_ready_event, INFINITE); + + test_loader_lock_timeout_count = 0; + test_loader_lock_expected_wait_result = WAIT_OBJECT_0; + + hmodule = GetModuleHandleW(preloaded_dll_name); + ok(!!hmodule, "LoadLibrary failed, err %u.\n", GetLastError()); + CHECK; + + if (BLOCKING_TESTS_ENABLED) + { + test_loader_lock_repeat_lock = TRUE; + SetEvent(next_test_event); + WaitForSingleObject(lock_ready_event, INFINITE); + test_loader_lock_expected_wait_result = WAIT_TIMEOUT; + + status = LdrRegisterDllNotification(0, test_loader_lock_ldr_notify, NULL, &cookie); + ok(!status, "Got unexpected status %#x.\n", status); + LdrUnregisterDllNotification( cookie ); + + SetEvent(next_test_event); + WaitForSingleObject(lock_ready_event, INFINITE); + + /* This doesn't block in load notifications on Windows, only unload. */ + status = pLdrLockLoaderLock(0, NULL, &magic); + ok(!status, "Got unexpected status %#x.\n", status); + status = pLdrUnlockLoaderLock(0, magic); + ok(!status, "Got unexpected status %#x.\n", status); + + test_loader_lock_repeat_lock = FALSE; + } + + SetEvent(next_test_event); + WaitForSingleObject(lock_ready_event, INFINITE); + + + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); + +done: + DeleteFileA("lock.dll"); + FreeLibrary(hmodule_preloaded); + CloseHandle(lock_ready_event); + CloseHandle(next_test_event); +} + START_TEST(loader) { int argc; @@ -4060,6 +4634,7 @@ START_TEST(loader) pLdrUnlockLoaderLock = (void *)GetProcAddress(ntdll, "LdrUnlockLoaderLock"); pLdrLoadDll = (void *)GetProcAddress(ntdll, "LdrLoadDll"); pLdrUnloadDll = (void *)GetProcAddress(ntdll, "LdrUnloadDll"); + pLdrEnumerateLoadedModules = (void *)GetProcAddress(ntdll, "LdrEnumerateLoadedModules"); pRtlInitUnicodeString = (void *)GetProcAddress(ntdll, "RtlInitUnicodeString"); pRtlAcquirePebLock = (void *)GetProcAddress(ntdll, "RtlAcquirePebLock"); pRtlReleasePebLock = (void *)GetProcAddress(ntdll, "RtlReleasePebLock"); @@ -4099,6 +4674,7 @@ START_TEST(loader) return; } +if(0){ test_filenames(); test_ResolveDelayLoadedAPI(); test_ImportDescriptors(); @@ -4113,6 +4689,9 @@ START_TEST(loader) test_dll_file( "advapi32.dll" ); test_dll_file( "user32.dll" ); test_Wow64Transition(); +} + test_loader_lock(); +return; /* loader test must be last, it can corrupt the internal loader state on Windows */ test_Loader(); } diff --git a/dlls/kernel32/tests/locking_dll.c b/dlls/kernel32/tests/locking_dll.c new file mode 100644 index 00000000000..31aff05fe84 --- /dev/null +++ b/dlls/kernel32/tests/locking_dll.c @@ -0,0 +1,50 @@ +#include <stdarg.h> +#include <stdlib.h> +#include "ntstatus.h" +#define WIN32_NO_STATUS +#include "windef.h" +#include "winbase.h" +#include "winternl.h" + +static DWORD last_lock_result, *lock_result_addr; + +BOOL WINAPI DllMain( HINSTANCE instance, DWORD reason, LPVOID reserved ) +{ + HANDLE lock_ready_event, next_test_event; + DWORD wait_reason; + NTSTATUS status; + char str[8]; + + GetEnvironmentVariableA("test_wait_reason_value", str, sizeof(str)); + wait_reason = atoi(str); + + if (reason != wait_reason) return TRUE; + + lock_ready_event = OpenEventA(EVENT_ALL_ACCESS, FALSE, "test_lock_ready_event"); + next_test_event = OpenEventA(EVENT_ALL_ACCESS, FALSE, "test_next_test_event"); + + SetEvent( lock_ready_event ); + last_lock_result = WaitForSingleObject(next_test_event, 2000); + + if (0) + { + if (reason == DLL_PROCESS_DETACH) + { + status = LdrAddRefDll(0, instance); + if (status) last_lock_result = ~0u; + status = LdrAddRefDll(LDR_ADDREF_DLL_PIN, instance); + if (status != STATUS_UNSUCCESSFUL) last_lock_result = ~0u - 1; + } + } + if (lock_result_addr) *lock_result_addr = last_lock_result; + + CloseHandle(lock_ready_event); + CloseHandle(next_test_event); + return TRUE; +} + +void WINAPI set_lock_result_addr( DWORD *addr ) +{ + lock_result_addr = addr; + *addr = last_lock_result; +} diff --git a/dlls/kernel32/tests/locking_dll.spec b/dlls/kernel32/tests/locking_dll.spec new file mode 100644 index 00000000000..327515590c3 --- /dev/null +++ b/dlls/kernel32/tests/locking_dll.spec @@ -0,0 +1 @@ +@ stdcall set_lock_result_addr(ptr) -- 2.31.1

2 1

[PATCH 00/22] ntdll: Relax the loader locking to match newer Windows behaviour.
by Paul Gofman Oct. 1, 2021

Oct. 1, 2021

I actually mean to send first 7 patches from this series for review and not 22 at once but it feels like the changes introduced by these first patches make sense in this exact form only if the general idea of locking rework is accepted. So I am sending the whole thing in its current state in a hope it would be possible to see what I am suggesting and make a decision on the general idea at least. I did some cleanup on all the functional patches, although would look once again at those below patch 7 at least before sending that for review. The test in the last patch obviously needs more work, it works well for me on the recent enough Windows 10 but not that well yet on earlier versions, also needs cleaning up and splitting. Some facts about the current Windows loader behaviour: - It seems that loader lock (the one that can be managed externally through LdrLockLoaderLock) is held only when calling the app's functions from the loader: DLL entry point, TLS callbacks, LDR notifications and LdrEnumerateLoadedModules callback. There is a quirk (at least on the newer Win10) with LDR notifications: the loader lock is held during the unload event but not during the load event, at least the first one. I am guessing that maybe it is unheld only until the first dependency DLL entry point is called (so they take it once there and then keep until process attach ends, leaving first load notification behind), but I didn't test that with the DLL with dependencies yet. In any case it looks to me this quirk may be left for later. - There is apparently other lock mechanics involved as Windows does not allow loading process of different DLLs to go in parallel; - It looks like any function which does not end up requiring actual loading or unloading of the DLL and does not query anything from the module currently being loaded or unloaded can be executed in parallel with the current load process. E. g., LoadLibrary("b.dll") won't ever block if the DLL is already loaded, LdrAddRefDll never blocks at all even if called for the module currently being loaded (hanging in dll entry point process attach) or unloaded (hanging in detach), LdrUnloadDll will only block if it decrements load count to zero. - The behaviour of the functions operating on the module being currently loaded or unloaded looks quirky. Here is what I recovered from testing on the 19043.1237 (that is mostly covered in the test in the last patch): - LdrUnloadDll blocks until the DLL load sequence is complete and succeeds without waiting if unload sequence is in progress; - LdrAddRefDll never blocks but behaviour depends on whether it is called from the app callback (tested with entry point and LDR notification) or outside of that (patch 0014). - LdrGetDllHandleEx blocks during module unload but not load; - LdrGetProcedureAddress waits until the module is being loaded and immediately returns an error if module is being unloaded; - Looks like calling functions from LDR notifications is the special case. Those that wouldn't block work, but those that would block if called from outside return error STATUS_NOT_FOUND. That is apparently not how that behaves from DLL entry points and TLS callbacks. I think the main patch in the series is patch 0008 which introduces exclusive locks downgrade / upgrade functions. The idea is that we can take shared or exclusive lock from loader function. The exclusive lock can be downgraded to shared. However, the thread still holds exlusiveness in this state in a sense that other exclusive lock cannot be taken until the initial exclusive lock is released. Shared lock waiters can joing when the exlusive lock is downgraded. This works recursively so loader functions can call each other and call outside callbacks with the lock held, with the exception that exclusive lock cannot be taken recursively if shared is currently held. So, the way of using it is: - Never call the callbacks with read lock held: that will assert once the application calls some loader function which takes the exclusive lock. - The way to call the callbacks is to have an exclusive lock and downgrade it to shared, then the recursive taking of exclusive lock is ok; - If it is unclear at function start whether exclusive lock is needed or not, the way to go is to start from shared lock, try to perform the operation (in case of LdrAddRefDll / LdrUnloadLibrary I used an extra short lived SRW lock), and if that is not possible release the shared lock and restart from scratch with exclusive. I think maybe it would be technically possible not to introduce the limitation and allow upgrading shared locks to exclusive but it seems it would make things much more complicated than they are: it would require some neater and finer grained structure access sync to avoid invalidating all the data the shared locker depends on at the moment it wants to upgrade to exclusive and yields to exclusive library load / unload sequence. Signed-off-by: Paul Gofman <pgofman(a)codeweavers.com> -- 2.31.1

2 39