Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=46089 Signed-off-by: Andr� Hentschel nerv@dawncrow.de --- dlls/kernel32/tests/loader.c | 39 +++++++++++++++++++++++++++++++++--- dlls/ntdll/loader.c | 23 +++++++++++++++++++-- include/delayloadhandler.h | 1 + 3 files changed, 58 insertions(+), 5 deletions(-)
diff --git a/dlls/kernel32/tests/loader.c b/dlls/kernel32/tests/loader.c index bc88c5f6cf..f5204fec0d 100644 --- a/dlls/kernel32/tests/loader.c +++ b/dlls/kernel32/tests/loader.c @@ -48,7 +48,7 @@ struct PROCESS_BASIC_INFORMATION_PRIVATE };
static LONG *child_failures; -static WORD cb_count; +static WORD cb_count, cb_count_sys; static DWORD page_size; static BOOL is_win64 = sizeof(void *) > sizeof(int); static BOOL is_wow64; @@ -70,7 +70,8 @@ static NTSTATUS (WINAPI *pLdrUnlockLoaderLock)(ULONG, ULONG_PTR); static void (WINAPI *pRtlAcquirePebLock)(void); static void (WINAPI *pRtlReleasePebLock)(void); static PVOID (WINAPI *pResolveDelayLoadedAPI)(PVOID, PCIMAGE_DELAYLOAD_DESCRIPTOR, - PDELAYLOAD_FAILURE_DLL_CALLBACK, PVOID, + PDELAYLOAD_FAILURE_DLL_CALLBACK, + PDELAYLOAD_FAILURE_SYSTEM_ROUTINE, PIMAGE_THUNK_DATA ThunkAddress,ULONG); static PVOID (WINAPI *pRtlImageDirectoryEntryToData)(HMODULE,BOOL,WORD,ULONG *); static DWORD (WINAPI *pFlsAlloc)(PFLS_CALLBACK_FUNCTION); @@ -3339,6 +3340,14 @@ static PVOID WINAPI failuredllhook(ULONG ul, DELAYLOAD_INFO* pd) return (void*)0xdeadbeef; }
+static PVOID WINAPI failuresyshook(const char *dll, const char *function) +{ + ok(!strcmp(dll, "secur32.dll"), "wrong dll: %s\n", dll); + ok(!((ULONG_PTR)function >> 16), "expected ordinal, got %p\n", function); + cb_count_sys++; + return NULL; +} + static void test_ResolveDelayLoadedAPI(void) { static const char test_dll[] = "secur32.dll"; @@ -3573,8 +3582,30 @@ static void test_ResolveDelayLoadedAPI(void) load = (void *)GetProcAddress(htarget, (char*)iibn->Name); }
+ /* test without failure dll callback */ + cb_count = 0; + cb_count_sys = 0; + ret = pResolveDelayLoadedAPI(hlib, delaydir, NULL, failuresyshook, &itda[i], 0); + if (td[i].succeeds) + { + ok(ret != NULL, "Test %u: ResolveDelayLoadedAPI failed\n", i); + ok(ret == load, "Test %u: expected %p, got %p\n", i, load, ret); + ok(ret == (void*)itda[i].u1.AddressOfData, "Test %u: expected %p, got %p\n", + i, ret, (void*)itda[i].u1.AddressOfData); + ok(!cb_count, "Test %u: Wrong callback count: %d\n", i, cb_count); + ok(!cb_count_sys, "Test %u: Wrong sys callback count: %d\n", i, cb_count_sys); + } + else + { + ok(!ret, "Test %u: ResolveDelayLoadedAPI succeeded with %p\n", i, ret); + ok(!cb_count, "Test %u: Wrong callback count: %d\n", i, cb_count); + ok(cb_count_sys, "Test %u: Wrong sys callback count: %d\n", i, cb_count_sys); + } + + /* test with failure dll callback */ cb_count = 0; - ret = pResolveDelayLoadedAPI(hlib, delaydir, failuredllhook, NULL, &itda[i], 0); + cb_count_sys = 0; + ret = pResolveDelayLoadedAPI(hlib, delaydir, failuredllhook, failuresyshook, &itda[i], 0); if (td[i].succeeds) { ok(ret != NULL, "Test %u: ResolveDelayLoadedAPI failed\n", i); @@ -3582,11 +3613,13 @@ static void test_ResolveDelayLoadedAPI(void) ok(ret == (void*)itda[i].u1.AddressOfData, "Test %u: expected %p, got %p\n", i, ret, (void*)itda[i].u1.AddressOfData); ok(!cb_count, "Test %u: Wrong callback count: %d\n", i, cb_count); + ok(!cb_count_sys, "Test %u: Wrong sys callback count: %d\n", i, cb_count_sys); } else { ok(ret == (void*)0xdeadbeef, "Test %u: ResolveDelayLoadedAPI succeeded with %p\n", i, ret); ok(cb_count, "Test %u: Wrong callback count: %d\n", i, cb_count); + ok(!cb_count_sys, "Test %u: Wrong sys callback count: %d\n", i, cb_count_sys); } } delaydir++; diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c index c5e8d0d7c1..838be31c11 100644 --- a/dlls/ntdll/loader.c +++ b/dlls/ntdll/loader.c @@ -2934,7 +2934,8 @@ BOOLEAN WINAPI RtlDllShutdownInProgress(void) * LdrResolveDelayLoadedAPI (NTDLL.@) */ void* WINAPI LdrResolveDelayLoadedAPI( void* base, const IMAGE_DELAYLOAD_DESCRIPTOR* desc, - PDELAYLOAD_FAILURE_DLL_CALLBACK dllhook, void* syshook, + PDELAYLOAD_FAILURE_DLL_CALLBACK dllhook, + PDELAYLOAD_FAILURE_SYSTEM_ROUTINE syshook, IMAGE_THUNK_DATA* addr, ULONG flags ) { IMAGE_THUNK_DATA *pIAT, *pINT; @@ -2992,7 +2993,25 @@ fail: delayinfo.TargetModuleBase = *phmod; delayinfo.Unused = NULL; delayinfo.LastError = nts; - return dllhook(4, &delayinfo); + + if (!dllhook && syshook) + { + if (IMAGE_SNAP_BY_ORDINAL(pINT[id].u1.Ordinal)) + { + DWORD_PTR ord = LOWORD(pINT[id].u1.Ordinal); + dllhook = syshook(name, (char*)ord); + } + else + { + const IMAGE_IMPORT_BY_NAME* iibn = get_rva(base, pINT[id].u1.AddressOfData); + dllhook = syshook(name, (char*)iibn->Name); + } + } + + if (dllhook) + return dllhook(4, &delayinfo); + else + return NULL; }
/****************************************************************** diff --git a/include/delayloadhandler.h b/include/delayloadhandler.h index 06b658990c..e8336111ff 100644 --- a/include/delayloadhandler.h +++ b/include/delayloadhandler.h @@ -47,6 +47,7 @@ typedef struct _DELAYLOAD_INFO } DELAYLOAD_INFO, *PDELAYLOAD_INFO;
typedef PVOID (WINAPI *PDELAYLOAD_FAILURE_DLL_CALLBACK)(ULONG, PDELAYLOAD_INFO); +typedef PVOID (WINAPI *PDELAYLOAD_FAILURE_SYSTEM_ROUTINE)(LPCSTR, LPCSTR);
#ifdef __cplusplus }
Hi,
While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=44007
Your paranoid android.
=== debian9 (64 bit Wow Wine report) ===
kernel32: loader.c:531: Test failed: 1273: loading failed err 193 loader.c:531: Test failed: 1277: loading failed err 193 loader.c:531: Test failed: 1285: loading failed err 193
=== debian9 (build log) ===
Am 06.11.18 um 22:00 schrieb Marvin:
Hi,
While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=44007
Your paranoid android.
=== debian9 (64 bit Wow Wine report) ===
kernel32: loader.c:531: Test failed: 1273: loading failed err 193 loader.c:531: Test failed: 1277: loading failed err 193 loader.c:531: Test failed: 1285: loading failed err 193
=== debian9 (build log) ===
those are pre-existing
On Wed, 7 Nov 2018, André Hentschel wrote: [...]
kernel32: loader.c:531: Test failed: 1273: loading failed err 193 loader.c:531: Test failed: 1277: loading failed err 193 loader.c:531: Test failed: 1285: loading failed err 193
those are pre-existing
The reason why the TestBot does not notice is that the error message contains the line number. I'm not talking about the initial ':531' (that one is ignored) but the ': 1273:' that follows which identifies the map_image_section() call site. That changed so it thinks it's a new error.
That should only be an issue when the test is modified so that it's not a big deal.
Otherwise a solution may be to use ok_() in map_image_section() so it reports the line of the call site. This means losing the exact map_image_section() line where the error occurred but that's probably ok since each error message is easily identifiable.
André Hentschel nerv@dawncrow.de writes:
@@ -2992,7 +2993,25 @@ fail: delayinfo.TargetModuleBase = *phmod; delayinfo.Unused = NULL; delayinfo.LastError = nts;
- return dllhook(4, &delayinfo);
- if (!dllhook && syshook)
- {
if (IMAGE_SNAP_BY_ORDINAL(pINT[id].u1.Ordinal)){DWORD_PTR ord = LOWORD(pINT[id].u1.Ordinal);dllhook = syshook(name, (char*)ord);}else{const IMAGE_IMPORT_BY_NAME* iibn = get_rva(base, pINT[id].u1.AddressOfData);dllhook = syshook(name, (char*)iibn->Name);}- }
- if (dllhook)
return dllhook(4, &delayinfo);- else
return NULL;
Is the syshook really supposed to return another hook? This would need test cases.
-
Alexandre Julliard -
André Hentschel -
Francois Gouget -
Marvin