January 2016 - Wine-Devel - list.winehq.org

Re: [PATCH v2 2/8] wined3d: Add support for WINED3DFMT_R8G8B8A8_UINT textures.
by Henri Verbeet Jan. 25, 2016

Jan. 25, 2016

On 25 January 2016 at 10:57, Józef Kucia <jkucia(a)codeweavers.com> wrote: > + {WINED3DFMT_R8G8B8A8_UINT, GL_RGBA8UI_EXT, GL_RGBA8UI_EXT, 0, > + GL_RGBA_INTEGER_EXT, GL_UNSIGNED_INT_8_8_8_8_REV, 0, > + WINED3DFMT_FLAG_TEXTURE, > + EXT_TEXTURE_INTEGER, NULL}, Actually, you can't use GL_UNSIGNED_INT_8_8_8_8_REV with integer formats unless you also have ARB_texture_rgb10_a2ui. (Although usually you do.) The corresponding test is also still failing for me, although in a different way. It works when replacing GL_UNSIGNED_INT_8_8_8_8_REV with either GL_UNSIGNED_INT_8_8_8_8 or GL_UNSIGNED_BYTE, which seems to hint at a driver issue. I'm looking into that.

1 0

RE: [PATCH 1/2] user32: Use correct values to deselect combo box text with EM_SETSEL
by Hugh McMaster Jan. 25, 2016

Jan. 25, 2016

> On Mon, 25 Jan 2016 19:42:13 +0800, Dmitry Timoshkov wrote: >> Hugh McMaster wrote: >> >>>>> diff --git a/dlls/user32/combo.c b/dlls/user32/combo.c >>>>> index c744ee5..4c57b72 100644 >>>>> --- a/dlls/user32/combo.c >>>>> +++ b/dlls/user32/combo.c >>>>> @@ -1189,6 +1189,8 @@ static void COMBO_KillFocus( LPHEADCOMBO lphc ) >>>>> { >>>>> if( CB_GETTYPE(lphc) == CBS_DROPDOWNLIST ) >>>>> SendMessageW(lphc->hWndLBox, LB_CARETOFF, 0, 0); >>>>> + else >>>>> + SendMessageW(lphc->hWndEdit, EM_SETSEL, 0, 0); >>>> >>>> The message test doesn't confirm that this is a correct solution. >>> >>> I tested the sequence from WM_KILLFOCUS twice in dlls/user32/tests/msg.c. >>> >>> [snip] >>> >>> In both sequences, the expected value of EM_SETSEL is 0, 0. >>> >>> Perhaps I'm not understanding what you mean in this case. >> >> I mean that adding a SendMessage() call should fix the message sequence >> if that's how it's supposed to work, and as a result you would remove TRUE >> as an indicator of todo_wine for fixed sequences. I can't remove TRUE from those message tests yet, as the message sequence fails. It seems some messages are not implemented or perhaps being sent in the wrong order. So, for SetFocusButtonSeq, the output is: msg.c:6165: Test failed: SetFocus on a Button: 3: the msg 0x0202 was expected, but got msg 0x0111 instead Is each message from the relevant sequence implemented in one file, e.g. user32/combo.c, or are they spread into multiple files?

2 1

Re: [PATCH] olepro32: Add typelib resource (resend)
by Huw Davies Jan. 25, 2016

Jan. 25, 2016

On Thu, Jan 07, 2016 at 06:15:52PM +1100, Alistair Leslie-Hughes wrote: > diff --git a/dlls/olepro32/olepro.idl b/dlls/olepro32/olepro.idl > new file mode 100644 > index 0000000..a34822d > --- /dev/null > +++ b/dlls/olepro32/olepro.idl > @@ -0,0 +1,358 @@ > +/* > + * Copyright (C) 2015 Alistair Leslie-Hughes > + * This appears to be a stripped down copy of std_ole_v1.idl, which is fine; what's not fine is stripping out the copyright and replacing it with your own. What should happen here is that this typelib should importlib("stdole32.tlb") and not have the IUnknown or IDispatch definitions. This will mean adding stdole32.tlb to the include directory. Huw.

1 0

Re: [PATCH 1/2] user32: Use correct values to deselect combo box text with EM_SETSEL
by Dmitry Timoshkov Jan. 25, 2016

Jan. 25, 2016

Hugh McMaster <hugh.mcmaster(a)outlook.com> wrote: > Signed-off-by: Hugh McMaster <hugh.mcmaster(a)outlook.com> > --- > dlls/user32/combo.c | 2 ++ > dlls/user32/tests/combo.c | 4 ++-- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/dlls/user32/combo.c b/dlls/user32/combo.c > index c744ee5..4c57b72 100644 > --- a/dlls/user32/combo.c > +++ b/dlls/user32/combo.c > @@ -1189,6 +1189,8 @@ static void COMBO_KillFocus( LPHEADCOMBO lphc ) > { > if( CB_GETTYPE(lphc) == CBS_DROPDOWNLIST ) > SendMessageW(lphc->hWndLBox, LB_CARETOFF, 0, 0); > + else > + SendMessageW(lphc->hWndEdit, EM_SETSEL, 0, 0); The message test doesn't confirm that this is a correct solution. -- Dmitry.

2 2

Re: [PATCH 2/2 v3] user32: Test error returned in out of timers case
by Marvin Jan. 25, 2016

Jan. 25, 2016

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=19942 Your paranoid android. === w1064 (32 bit msg) === msg.c:6290: Test failed: Update region shouldn't be empty msg.c:6524: Test failed: InvalidateErase: 0: the msg sequence is not complete: expected 0085 - actual 0000 msg.c:6526: Test failed: Paint: 0: the msg sequence is not complete: expected 000f - actual 0000 msg.c:6296: Test failed: Regions are different msg.c:6296: Test failed: Regions are different msg.c:6296: Test failed: Regions are different === w1064 (64 bit msg) === msg.c:6903: Test failed: WmParentPaint: 2: the msg sequence is not complete: expected 0000 - actual 001a

1 0

Re: [PATCH 2/2 v2] user32: Test error returned in out of timers case
by Sebastian Lackner Jan. 25, 2016

Jan. 25, 2016

On 22.01.2016 16:10, Piotr Caban wrote: > + if (i == sizeof(ids)/sizeof(ids[0])) > + ok(0, "all timers were created successfully\n"); Why don't you use ok() directly to check the condition?

2 1

Re: advapi32/tests: Trace the SIDs if they are not as expected.
by Marvin Jan. 24, 2016

Jan. 24, 2016

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=19941 Your paranoid android. === w864 (32 bit security) === security.c:3774: Test failed: MACHINE\Software owner SID (S-1-5-18) != Administrators SID (S-1-5-32-544). === w1064 (32 bit security) === security.c:3774: Test failed: MACHINE\Software owner SID (S-1-5-18) != Administrators SID (S-1-5-32-544). security.c:3815: Test failed: Builtin Users ACE has unexpected flags (0x12 != 0xa) security.c:3819: Test failed: Builtin Users ACE has unexpected mask (0x20019 != 0x80000000) security.c:3828: Test failed: Builtin Admins ACE has unexpected flags (0x12 != 0x0)

1 0

Re: [PATCH v4] ntoskrnl.exe: Implement ZwLoadDriver and ZwUnloadDriver
by Sebastian Lackner Jan. 24, 2016

Jan. 24, 2016

On 22.01.2016 19:15, Aric Stewart wrote: > Code is mostly moved from winedevice.exe > winedevice.exe now uses these functions > v3: No longer use advapi32 Registry functions > v4: Fix FALSE return code (thanks to Huw Davies for spotting this one) > Remove WINE_ from debuging macros > > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/ntoskrnl.exe/ntoskrnl.c | 306 ++++++++++++++++++++++++++++++++++++ > dlls/ntoskrnl.exe/ntoskrnl.exe.spec | 4 +- > programs/winedevice/device.c | 243 +--------------------------- > 3 files changed, 313 insertions(+), 240 deletions(-) > I don't think moving the loader code from winedevice to ntoskrnl.exe is a good idea, at least not without further changes, because: * We should not initialize the same driver twice, the second initialization could fail because of namespace collisions. * Drivers should be properly uninitialized, no matter how they were loaded. * In theory, all drivers should be loaded into the same address space. This means, winedevice and the wineserver has to keep track of them, and its not sufficient to maintain a list in ntoskrnl.exe. Besides those design issues, I've marked a couple of other issues in the patch below. > > 0001-ntoskrnl.exe-Implement-ZwLoadDriver-and-ZwUnloadDriver.txt > > > diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c > index 04f0198..032ab21 100644 > --- a/dlls/ntoskrnl.exe/ntoskrnl.c > +++ b/dlls/ntoskrnl.exe/ntoskrnl.c > @@ -72,6 +72,18 @@ static DWORD request_thread; > static DWORD client_tid; > static DWORD client_pid; > > +/* Loaded driver list */ > +typedef struct _loaded_driver { > + struct list entry; > + > + UNICODE_STRING name; > + HMODULE module; > + DRIVER_OBJECT driver_obj; > + DRIVER_EXTENSION driver_extension; > +} loaded_driver; > + > +struct list loaded_drivers = LIST_INIT(loaded_drivers); It wouldn't hurt to add locking for such a global list. > + > #ifdef __i386__ > #define DEFINE_FASTCALL1_ENTRYPOINT( name ) \ > __ASM_STDCALL_FUNC( name, 4, \ > @@ -2407,3 +2419,297 @@ NTSTATUS WINAPI CmUnRegisterCallback(LARGE_INTEGER cookie) > FIXME("(%s): stub\n", wine_dbgstr_longlong(cookie.QuadPart)); > return STATUS_NOT_IMPLEMENTED; > } > + > + > +/* find the LDR_MODULE corresponding to the driver module */ > +static LDR_MODULE *find_ldr_module( HMODULE module ) > +{ > + LIST_ENTRY *entry, *list = &NtCurrentTeb()->Peb->LdrData->InMemoryOrderModuleList; > + > + for (entry = list->Flink; entry != list; entry = entry->Flink) > + { > + LDR_MODULE *ldr = CONTAINING_RECORD(entry, LDR_MODULE, InMemoryOrderModuleList); > + if (ldr->BaseAddress == module) return ldr; > + if (ldr->BaseAddress > (void *)module) break; > + } The loader lock should be acquired while iterating through this list. (Yes, this is an existing issue, but before it was less critical because the code was single-threaded.) > + return NULL; > +} > + > +/* load the driver module file */ > +static HMODULE load_driver_module( const WCHAR *name ) > +{ > + IMAGE_NT_HEADERS *nt; > + const IMAGE_IMPORT_DESCRIPTOR *imports; > + SYSTEM_BASIC_INFORMATION info; > + int i; > + INT_PTR delta; > + ULONG size; > + HMODULE module = LoadLibraryW( name ); > + > + if (!module) return NULL; > + nt = RtlImageNtHeader( module ); > + > + if (!(delta = (char *)module - (char *)nt->OptionalHeader.ImageBase)) return module; > + > + /* the loader does not apply relocations to non page-aligned binaries or executables, > + * we have to do it ourselves */ > + > + NtQuerySystemInformation( SystemBasicInformation, &info, sizeof(info), NULL ); > + if (nt->OptionalHeader.SectionAlignment < info.PageSize || > + !(nt->FileHeader.Characteristics & IMAGE_FILE_DLL)) > + { > + DWORD old; > + IMAGE_BASE_RELOCATION *rel, *end; > + > + if ((rel = RtlImageDirectoryEntryToData( module, TRUE, IMAGE_DIRECTORY_ENTRY_BASERELOC, &size ))) > + { > + TRACE( "%s: relocating from %p to %p\n", > + wine_dbgstr_w(name), (char *)module - delta, module ); > + end = (IMAGE_BASE_RELOCATION *)((char *)rel + size); > + while (rel < end && rel->SizeOfBlock) > + { > + void *page = (char *)module + rel->VirtualAddress; > + VirtualProtect( page, info.PageSize, PAGE_EXECUTE_READWRITE, &old ); > + rel = LdrProcessRelocationBlock( page, (rel->SizeOfBlock - sizeof(*rel)) / sizeof(USHORT), > + (USHORT *)(rel + 1), delta ); > + if (old != PAGE_EXECUTE_READWRITE) VirtualProtect( page, info.PageSize, old, &old ); > + if (!rel) goto error; > + } > + /* make sure we don't try again */ > + size = FIELD_OFFSET( IMAGE_NT_HEADERS, OptionalHeader ) + nt->FileHeader.SizeOfOptionalHeader; > + VirtualProtect( nt, size, PAGE_READWRITE, &old ); > + nt->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_BASERELOC].VirtualAddress = 0; > + VirtualProtect( nt, size, old, &old ); > + } > + } > + > + /* make sure imports are relocated too */ > + > + if ((imports = RtlImageDirectoryEntryToData( module, TRUE, IMAGE_DIRECTORY_ENTRY_IMPORT, &size ))) > + { > + for (i = 0; imports[i].Name && imports[i].FirstThunk; i++) > + { > + char *name = (char *)module + imports[i].Name; > + WCHAR buffer[32], *p = buffer; > + > + while (p < buffer + 32) if (!(*p++ = *name++)) break; > + if (p <= buffer + 32) FreeLibrary( load_driver_module( buffer ) ); > + } > + } > + > + return module; > + > +error: > + FreeLibrary( module ); > + return NULL; > +} > + > +/* call the driver init entry point */ > +static NTSTATUS init_driver( const WCHAR *driver_name, UNICODE_STRING *keyname, loaded_driver *driver ) > +{ > + unsigned int i; > + NTSTATUS status; > + const IMAGE_NT_HEADERS *nt = RtlImageNtHeader( driver->module ); > + > + if (!nt->OptionalHeader.AddressOfEntryPoint) return STATUS_SUCCESS; > + > + driver->driver_obj.Size = sizeof(driver->driver_obj); > + driver->driver_obj.DriverSection = find_ldr_module( driver->module ); > + driver->driver_obj.DriverInit = (PDRIVER_INITIALIZE)((char *)driver->module + nt->OptionalHeader.AddressOfEntryPoint); > + driver->driver_obj.DriverExtension = &driver->driver_extension; > + > + driver->driver_obj.DriverExtension->DriverObject = &driver->driver_obj; > + driver->driver_obj.DriverExtension->ServiceKeyName = *keyname; > + > + if (TRACE_ON(relay)) > + DPRINTF( "%04x:Call driver init %p (obj=%p,str=%s)\n", GetCurrentThreadId(), > + driver->driver_obj.DriverInit, &driver->driver_obj, wine_dbgstr_w(keyname->Buffer) ); > + > + status = driver->driver_obj.DriverInit( &driver->driver_obj, keyname ); > + > + if (TRACE_ON(relay)) > + DPRINTF( "%04x:Ret driver init %p (obj=%p,str=%s) retval=%08x\n", GetCurrentThreadId(), > + driver->driver_obj.DriverInit, &driver->driver_obj, wine_dbgstr_w(keyname->Buffer), status ); > + > + TRACE( "init done for %s obj %p\n", wine_dbgstr_w(driver_name), &driver->driver_obj ); > + TRACE( "- DriverInit = %p\n", driver->driver_obj.DriverInit ); > + TRACE( "- DriverStartIo = %p\n", driver->driver_obj.DriverStartIo ); > + TRACE( "- DriverUnload = %p\n", driver->driver_obj.DriverUnload ); > + for (i = 0; i <= IRP_MJ_MAXIMUM_FUNCTION; i++) > + TRACE( "- MajorFunction[%d] = %p\n", i, driver->driver_obj.MajorFunction[i] ); > + > + return status; > +} > + > +/*********************************************************************** > + * ZwLoadDriver (NTOSKRNL.EXE.@) > + */ > +NTSTATUS WINAPI ZwLoadDriver(const UNICODE_STRING *DriverServiceName) Both the Nt* and Zw* function should do the same, so I would suggest to implement Nt* instead and forward Zw*, like its done in ntdll. > +{ > + static const WCHAR driversW[] = {'\\','d','r','i','v','e','r','s','\\',0}; > + static const WCHAR systemrootW[] = {'\\','S','y','s','t','e','m','R','o','o','t','\\',0}; > + static const WCHAR postfixW[] = {'.','s','y','s',0}; > + static const WCHAR ntprefixW[] = {'\\','?','?','\\',0}; > + static const WCHAR ImagePathW[] = {'I','m','a','g','e','P','a','t','h',0}; > + static const WCHAR servicesW[] = {'\\','R','e','g','i','s','t','r','y', > + '\\','M','a','c','h','i','n','e', > + '\\','S','y','s','t','e','m', > + '\\','C','u','r','r','e','n','t','C','o','n','t','r','o','l','S','e','t', > + '\\','S','e','r','v','i','c','e','s','\\',0}; > + > + UNICODE_STRING keypath, reg_str; > + HMODULE module; > + LPWSTR path = NULL, str; > + DWORD size; > + HANDLE driver_hkey; > + OBJECT_ATTRIBUTES attr; > + NTSTATUS status; > + char buffer[256]; > + KEY_VALUE_PARTIAL_INFORMATION *info = (KEY_VALUE_PARTIAL_INFORMATION *)buffer; The initialization here looks unnecessary. > + static const int info_size = offsetof( KEY_VALUE_PARTIAL_INFORMATION, Data ); > + loaded_driver *driver; > + > + /* Check if driver is already loaded */ > + LIST_FOR_EACH_ENTRY(driver, &loaded_drivers, loaded_driver, entry) > + { > + if (RtlEqualUnicodeString(DriverServiceName, &driver->name, FALSE)) > + return ERROR_SUCCESS; > + } > + > + str = HeapAlloc(GetProcessHeap(), 0, sizeof(servicesW) + DriverServiceName->Length*sizeof(WCHAR)); Length is the length in bytes, so the calculation above is wrong. > + lstrcpyW(str, servicesW); > + lstrcatW(str, DriverServiceName->Buffer); I don't think you can assume a terminating NULL character. > + > + attr.Length = sizeof(attr); > + attr.RootDirectory = 0; > + attr.ObjectName = &reg_str; > + attr.Attributes = 0; > + attr.SecurityDescriptor = NULL; > + attr.SecurityQualityOfService = NULL; > + > + RtlInitUnicodeString(&reg_str, str); > + > + status = NtCreateKey(&driver_hkey, KEY_ALL_ACCESS, &attr, 0, NULL, 0, NULL); > + if (status != STATUS_SUCCESS) > + { > + ERR("cannot open key %s, err=0x%x\n", wine_dbgstr_w(str), status); > + HeapFree(GetProcessHeap(), 0, str); > + return status; > + } > + RtlInitUnicodeString(&keypath, str); > + > + /* read the executable path from memory */ > + size = 0; > + RtlInitUnicodeString(&reg_str, ImagePathW); > + > + status = NtQueryValueKey(driver_hkey, &reg_str, KeyValuePartialInformation, buffer, info_size, &size); > + if (status == STATUS_BUFFER_OVERFLOW) > + { > + str = HeapAlloc(GetProcessHeap(), 0, size); str was already set before, so this might leak memory, at least on some exit paths. > + info = (KEY_VALUE_PARTIAL_INFORMATION *) str; > + status = NtQueryValueKey(driver_hkey, &reg_str, KeyValuePartialInformation, str, size, &size); > + > + if (status == STATUS_SUCCESS) > + { > + size = ExpandEnvironmentStringsW((LPCWSTR)info->Data,NULL,0); > + path = HeapAlloc(GetProcessHeap(),0,size*sizeof(WCHAR)); > + ExpandEnvironmentStringsW((LPCWSTR)info->Data,path,size); > + } > + HeapFree(GetProcessHeap(), 0, str); > + if (!path) > + { > + TRACE("Failed to load driver path(0x%x): %s\n", status, > + wine_dbgstr_w((LPCWSTR)info->Data)); info->Data points into str, which was deallocated just before. > + NtClose(driver_hkey); > + return status; > + } > + > + if (!strncmpiW(path, systemrootW, 12)) > + { > + WCHAR buffer[MAX_PATH]; The variable name is the same as above, which might be misleading. > + > + GetWindowsDirectoryW(buffer, MAX_PATH); > + > + str = HeapAlloc(GetProcessHeap(), 0, (size -11 + strlenW(buffer)) > + * sizeof(WCHAR)); > + lstrcpyW(str, buffer); > + lstrcatW(str, path + 11); > + HeapFree(GetProcessHeap(), 0, path); > + path = str; > + } > + else if (!strncmpW(path, ntprefixW, 4)) > + str = path + 4; > + else > + str = path; > + } > + else > + { > + /* default is to use the driver name + ".sys" */ > + WCHAR buffer[MAX_PATH]; > + GetSystemDirectoryW(buffer, MAX_PATH); > + path = HeapAlloc(GetProcessHeap(),0, > + (strlenW(buffer) + strlenW(driversW) + DriverServiceName->Length + strlenW(postfixW) + 1) > + *sizeof(WCHAR)); Similar to above, ->Length is the length in bytes. > + lstrcpyW(path, buffer); > + lstrcatW(path, driversW); > + lstrcatW(path, DriverServiceName->Buffer); > + lstrcatW(path, postfixW); > + str = path; > + } > + NtClose(driver_hkey); > + > + TRACE("loading driver %s\n", wine_dbgstr_w(str)); > + > + module = load_driver_module(str); > + HeapFree(GetProcessHeap(), 0, path); > + > + if (!module) > + return STATUS_NOT_FOUND; > + > + driver = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, sizeof(*driver)); > + driver->module = module; > + RtlDuplicateUnicodeString(1, DriverServiceName, &driver->name); > + > + init_driver(DriverServiceName->Buffer, &keypath, driver); > + > + list_add_tail(&loaded_drivers, &driver->entry); > + > + return STATUS_SUCCESS; > +} > + > +/*********************************************************************** > + * ZwUnloadDriver (NTOSKRNL.EXE.@) > + */ > +NTSTATUS WINAPI ZwUnloadDriver(const UNICODE_STRING *DriverServiceName) > +{ > + loaded_driver *driver, *ptr; > + > + LIST_FOR_EACH_ENTRY_SAFE(driver, ptr, &loaded_drivers, loaded_driver, entry) > + { > + if (RtlEqualUnicodeString(DriverServiceName, &driver->name, FALSE)) > + { > + list_remove(&driver->entry); > + > + if (driver->driver_obj.DriverUnload) > + { > + if (TRACE_ON(relay)) > + DPRINTF("%04x:Call driver unload %p (obj=%p)\n", GetCurrentThreadId(), > + driver->driver_obj.DriverUnload, &driver->driver_obj ); > + > + driver->driver_obj.DriverUnload( &driver->driver_obj); > + > + if (TRACE_ON(relay)) > + DPRINTF("%04x:Ret driver unload %p (obj=%p)\n", GetCurrentThreadId(), > + driver->driver_obj.DriverUnload, &driver->driver_obj); > + } > + > + RtlFreeUnicodeString(&driver->name); > + RtlFreeUnicodeString(&driver->driver_extension.ServiceKeyName); > + FreeLibrary(driver->module); > + HeapFree(GetProcessHeap(), 0, driver); > + return STATUS_SUCCESS; > + } > + } > + > + return STATUS_NOT_FOUND; > +} > diff --git a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec > index 1319ada..a98a523 100644 > --- a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec > +++ b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec > @@ -1314,7 +1314,7 @@ > @ stdcall ZwFsControlFile(long long long long long long long long long long) ntdll.ZwFsControlFile > @ stdcall ZwInitiatePowerAction(long long long long) ntdll.ZwInitiatePowerAction > @ stdcall ZwIsProcessInJob(long long) ntdll.ZwIsProcessInJob > -@ stdcall ZwLoadDriver(ptr) ntdll.ZwLoadDriver > +@ stdcall ZwLoadDriver(ptr) > @ stdcall ZwLoadKey(ptr ptr) ntdll.ZwLoadKey > @ stdcall ZwMakeTemporaryObject(long) ntdll.ZwMakeTemporaryObject > @ stdcall ZwMapViewOfSection(long long ptr long long ptr ptr long long long) ntdll.ZwMapViewOfSection > @@ -1384,7 +1384,7 @@ > @ stdcall ZwTerminateJobObject(long long) ntdll.ZwTerminateJobObject > @ stdcall ZwTerminateProcess(long long) ntdll.ZwTerminateProcess > @ stub ZwTranslateFilePath > -@ stdcall ZwUnloadDriver(ptr) ntdll.ZwUnloadDriver > +@ stdcall ZwUnloadDriver(ptr) > @ stdcall ZwUnloadKey(long) ntdll.ZwUnloadKey > @ stdcall ZwUnmapViewOfSection(long ptr) ntdll.ZwUnmapViewOfSection > @ stdcall ZwWaitForMultipleObjects(long ptr long long ptr) ntdll.ZwWaitForMultipleObjects > diff --git a/programs/winedevice/device.c b/programs/winedevice/device.c > index ef1e1ef..cb6a6ba 100644 > --- a/programs/winedevice/device.c > +++ b/programs/winedevice/device.c > @@ -25,253 +25,19 @@ > > #include "ntstatus.h" > #define WIN32_NO_STATUS > -#include "windef.h" > -#include "winbase.h" > #include "winternl.h" > -#include "winreg.h" > -#include "winnls.h" > #include "winsvc.h" > #include "ddk/wdm.h" > #include "wine/unicode.h" > #include "wine/debug.h" > > WINE_DEFAULT_DEBUG_CHANNEL(winedevice); > -WINE_DECLARE_DEBUG_CHANNEL(relay); > > extern NTSTATUS CDECL wine_ntoskrnl_main_loop( HANDLE stop_event ); > > static WCHAR *driver_name; > static SERVICE_STATUS_HANDLE service_handle; > -static HKEY driver_hkey; > static HANDLE stop_event; > -static DRIVER_OBJECT driver_obj; > -static DRIVER_EXTENSION driver_extension; > - > -/* find the LDR_MODULE corresponding to the driver module */ > -static LDR_MODULE *find_ldr_module( HMODULE module ) > -{ > - LIST_ENTRY *entry, *list = &NtCurrentTeb()->Peb->LdrData->InMemoryOrderModuleList; > - > - for (entry = list->Flink; entry != list; entry = entry->Flink) > - { > - LDR_MODULE *ldr = CONTAINING_RECORD(entry, LDR_MODULE, InMemoryOrderModuleList); > - if (ldr->BaseAddress == module) return ldr; > - if (ldr->BaseAddress > (void *)module) break; > - } > - return NULL; > -} > - > -/* load the driver module file */ > -static HMODULE load_driver_module( const WCHAR *name ) > -{ > - IMAGE_NT_HEADERS *nt; > - const IMAGE_IMPORT_DESCRIPTOR *imports; > - SYSTEM_BASIC_INFORMATION info; > - int i; > - INT_PTR delta; > - ULONG size; > - HMODULE module = LoadLibraryW( name ); > - > - if (!module) return NULL; > - nt = RtlImageNtHeader( module ); > - > - if (!(delta = (char *)module - (char *)nt->OptionalHeader.ImageBase)) return module; > - > - /* the loader does not apply relocations to non page-aligned binaries or executables, > - * we have to do it ourselves */ > - > - NtQuerySystemInformation( SystemBasicInformation, &info, sizeof(info), NULL ); > - if (nt->OptionalHeader.SectionAlignment < info.PageSize || > - !(nt->FileHeader.Characteristics & IMAGE_FILE_DLL)) > - { > - DWORD old; > - IMAGE_BASE_RELOCATION *rel, *end; > - > - if ((rel = RtlImageDirectoryEntryToData( module, TRUE, IMAGE_DIRECTORY_ENTRY_BASERELOC, &size ))) > - { > - WINE_TRACE( "%s: relocating from %p to %p\n", > - wine_dbgstr_w(name), (char *)module - delta, module ); > - end = (IMAGE_BASE_RELOCATION *)((char *)rel + size); > - while (rel < end && rel->SizeOfBlock) > - { > - void *page = (char *)module + rel->VirtualAddress; > - VirtualProtect( page, info.PageSize, PAGE_EXECUTE_READWRITE, &old ); > - rel = LdrProcessRelocationBlock( page, (rel->SizeOfBlock - sizeof(*rel)) / sizeof(USHORT), > - (USHORT *)(rel + 1), delta ); > - if (old != PAGE_EXECUTE_READWRITE) VirtualProtect( page, info.PageSize, old, &old ); > - if (!rel) goto error; > - } > - /* make sure we don't try again */ > - size = FIELD_OFFSET( IMAGE_NT_HEADERS, OptionalHeader ) + nt->FileHeader.SizeOfOptionalHeader; > - VirtualProtect( nt, size, PAGE_READWRITE, &old ); > - nt->OptionalHeader.DataDirectory[IMAGE_DIRECTORY_ENTRY_BASERELOC].VirtualAddress = 0; > - VirtualProtect( nt, size, old, &old ); > - } > - } > - > - /* make sure imports are relocated too */ > - > - if ((imports = RtlImageDirectoryEntryToData( module, TRUE, IMAGE_DIRECTORY_ENTRY_IMPORT, &size ))) > - { > - for (i = 0; imports[i].Name && imports[i].FirstThunk; i++) > - { > - char *name = (char *)module + imports[i].Name; > - WCHAR buffer[32], *p = buffer; > - > - while (p < buffer + 32) if (!(*p++ = *name++)) break; > - if (p <= buffer + 32) FreeLibrary( load_driver_module( buffer ) ); > - } > - } > - > - return module; > - > -error: > - FreeLibrary( module ); > - return NULL; > -} > - > -/* call the driver init entry point */ > -static NTSTATUS init_driver( HMODULE module, UNICODE_STRING *keyname ) > -{ > - unsigned int i; > - NTSTATUS status; > - const IMAGE_NT_HEADERS *nt = RtlImageNtHeader( module ); > - > - if (!nt->OptionalHeader.AddressOfEntryPoint) return STATUS_SUCCESS; > - > - driver_obj.Size = sizeof(driver_obj); > - driver_obj.DriverSection = find_ldr_module( module ); > - driver_obj.DriverInit = (PDRIVER_INITIALIZE)((char *)module + nt->OptionalHeader.AddressOfEntryPoint); > - driver_obj.DriverExtension = &driver_extension; > - > - driver_extension.DriverObject = &driver_obj; > - driver_extension.ServiceKeyName = *keyname; > - > - if (WINE_TRACE_ON(relay)) > - WINE_DPRINTF( "%04x:Call driver init %p (obj=%p,str=%s)\n", GetCurrentThreadId(), > - driver_obj.DriverInit, &driver_obj, wine_dbgstr_w(keyname->Buffer) ); > - > - status = driver_obj.DriverInit( &driver_obj, keyname ); > - > - if (WINE_TRACE_ON(relay)) > - WINE_DPRINTF( "%04x:Ret driver init %p (obj=%p,str=%s) retval=%08x\n", GetCurrentThreadId(), > - driver_obj.DriverInit, &driver_obj, wine_dbgstr_w(keyname->Buffer), status ); > - > - WINE_TRACE( "init done for %s obj %p\n", wine_dbgstr_w(driver_name), &driver_obj ); > - WINE_TRACE( "- DriverInit = %p\n", driver_obj.DriverInit ); > - WINE_TRACE( "- DriverStartIo = %p\n", driver_obj.DriverStartIo ); > - WINE_TRACE( "- DriverUnload = %p\n", driver_obj.DriverUnload ); > - for (i = 0; i <= IRP_MJ_MAXIMUM_FUNCTION; i++) > - WINE_TRACE( "- MajorFunction[%d] = %p\n", i, driver_obj.MajorFunction[i] ); > - > - return status; > -} > - > -/* call the driver unload function */ > -static void unload_driver( HMODULE module, DRIVER_OBJECT *driver_obj ) > -{ > - if (driver_obj->DriverUnload) > - { > - if (WINE_TRACE_ON(relay)) > - WINE_DPRINTF( "%04x:Call driver unload %p (obj=%p)\n", GetCurrentThreadId(), > - driver_obj->DriverUnload, driver_obj ); > - > - driver_obj->DriverUnload( driver_obj ); > - > - if (WINE_TRACE_ON(relay)) > - WINE_DPRINTF( "%04x:Ret driver unload %p (obj=%p)\n", GetCurrentThreadId(), > - driver_obj->DriverUnload, driver_obj ); > - } > - FreeLibrary( module ); > -} > - > -/* load the .sys module for a device driver */ > -static HMODULE load_driver(void) > -{ > - static const WCHAR driversW[] = {'\\','d','r','i','v','e','r','s','\\',0}; > - static const WCHAR systemrootW[] = {'\\','S','y','s','t','e','m','R','o','o','t','\\',0}; > - static const WCHAR postfixW[] = {'.','s','y','s',0}; > - static const WCHAR ntprefixW[] = {'\\','?','?','\\',0}; > - static const WCHAR ImagePathW[] = {'I','m','a','g','e','P','a','t','h',0}; > - static const WCHAR servicesW[] = {'\\','R','e','g','i','s','t','r','y', > - '\\','M','a','c','h','i','n','e', > - '\\','S','y','s','t','e','m', > - '\\','C','u','r','r','e','n','t','C','o','n','t','r','o','l','S','e','t', > - '\\','S','e','r','v','i','c','e','s','\\',0}; > - > - UNICODE_STRING keypath; > - HMODULE module; > - LPWSTR path = NULL, str; > - DWORD type, size; > - > - str = HeapAlloc( GetProcessHeap(), 0, sizeof(servicesW) + strlenW(driver_name)*sizeof(WCHAR) ); > - lstrcpyW( str, servicesW ); > - lstrcatW( str, driver_name ); > - > - if (RegOpenKeyW( HKEY_LOCAL_MACHINE, str + 18 /* skip \registry\machine */, &driver_hkey )) > - { > - WINE_ERR( "cannot open key %s, err=%u\n", wine_dbgstr_w(str), GetLastError() ); > - HeapFree( GetProcessHeap(), 0, str); > - return FALSE; > - } > - RtlInitUnicodeString( &keypath, str ); > - > - /* read the executable path from memory */ > - size = 0; > - if (!RegQueryValueExW( driver_hkey, ImagePathW, NULL, &type, NULL, &size )) > - { > - str = HeapAlloc( GetProcessHeap(), 0, size ); > - if (!RegQueryValueExW( driver_hkey, ImagePathW, NULL, &type, (LPBYTE)str, &size )) > - { > - size = ExpandEnvironmentStringsW(str,NULL,0); > - path = HeapAlloc(GetProcessHeap(),0,size*sizeof(WCHAR)); > - ExpandEnvironmentStringsW(str,path,size); > - } > - HeapFree( GetProcessHeap(), 0, str ); > - if (!path) return FALSE; > - > - if (!strncmpiW( path, systemrootW, 12 )) > - { > - WCHAR buffer[MAX_PATH]; > - > - GetWindowsDirectoryW(buffer, MAX_PATH); > - > - str = HeapAlloc(GetProcessHeap(), 0, (size -11 + strlenW(buffer)) > - * sizeof(WCHAR)); > - lstrcpyW(str, buffer); > - lstrcatW(str, path + 11); > - HeapFree( GetProcessHeap(), 0, path ); > - path = str; > - } > - else if (!strncmpW( path, ntprefixW, 4 )) > - str = path + 4; > - else > - str = path; > - } > - else > - { > - /* default is to use the driver name + ".sys" */ > - WCHAR buffer[MAX_PATH]; > - GetSystemDirectoryW(buffer, MAX_PATH); > - path = HeapAlloc(GetProcessHeap(),0, > - (strlenW(buffer) + strlenW(driversW) + strlenW(driver_name) + strlenW(postfixW) + 1) > - *sizeof(WCHAR)); > - lstrcpyW(path, buffer); > - lstrcatW(path, driversW); > - lstrcatW(path, driver_name); > - lstrcatW(path, postfixW); > - str = path; > - } > - > - WINE_TRACE( "loading driver %s\n", wine_dbgstr_w(str) ); > - > - module = load_driver_module( str ); > - HeapFree( GetProcessHeap(), 0, path ); > - if (!module) return NULL; > - > - init_driver( module, &keypath ); > - return module; > -} > > static DWORD WINAPI service_handler( DWORD ctrl, DWORD event_type, LPVOID event_data, LPVOID context ) > { > @@ -305,7 +71,7 @@ static DWORD WINAPI service_handler( DWORD ctrl, DWORD event_type, LPVOID event_ > static void WINAPI ServiceMain( DWORD argc, LPWSTR *argv ) > { > SERVICE_STATUS status; > - HMODULE driver_module; > + UNICODE_STRING DriverName; > > WINE_TRACE( "starting service %s\n", wine_dbgstr_w(driver_name) ); > > @@ -324,15 +90,16 @@ static void WINAPI ServiceMain( DWORD argc, LPWSTR *argv ) > status.dwWaitHint = 10000; > SetServiceStatus( service_handle, &status ); > > - driver_module = load_driver(); > - if (driver_module) > + RtlInitUnicodeString( &DriverName, driver_name ); > + > + if ( ZwLoadDriver( &DriverName ) == ERROR_SUCCESS ) > { > status.dwCurrentState = SERVICE_RUNNING; > status.dwControlsAccepted = SERVICE_ACCEPT_STOP | SERVICE_ACCEPT_SHUTDOWN; > SetServiceStatus( service_handle, &status ); > > wine_ntoskrnl_main_loop( stop_event ); > - unload_driver( driver_module, &driver_obj ); > + ZwUnloadDriver( &DriverName ); > } > else WINE_ERR( "driver %s failed to load\n", wine_dbgstr_w(driver_name) ); > > > >

3 7

Re: [PATCH v3] mscoree: Implement VTable fixup for x86_64 architecture.
by Sebastian Lackner Jan. 23, 2016

Jan. 23, 2016

On 22.01.2016 23:24, Paul Gofman wrote: > Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> > --- > dlls/mscoree/corruntimehost.c | 91 +++++++++++++++++++++++++++++++++++++------ > 1 file changed, 79 insertions(+), 12 deletions(-) > > diff --git a/dlls/mscoree/corruntimehost.c b/dlls/mscoree/corruntimehost.c > index e437cc2..05ac9c9 100644 > --- a/dlls/mscoree/corruntimehost.c > +++ b/dlls/mscoree/corruntimehost.c > @@ -919,7 +919,68 @@ static const struct vtable_fixup_thunk thunk_template = { > > #include "poppack.h" > > -#else /* !defined(__i386__) */ > +#elif __x86_64__ /* !__i386__ */ > + > +# define CAN_FIXUP_VTABLE 1 > + > +#include "pshpack1.h" > + > +struct vtable_fixup_thunk > +{ > + /* push %rcx; push %rdx; push %r8; push %r9 */ I think it would be better to allocate a proper stack frame (pushq %rbp, movq %rsp, %rbp, subq ...,%rsp) and then use movq instructions. > + BYTE i1[6]; > + /* sub $0x68, %rsp ; 0x10*4 + 0x20 + 0x8 (align stack for subsequent func call) > + movups %xmm0,0x28(%esp); ...; movups %xmm5,0x58(%esp) Shouldn't it be %rsp ? Same for the precompiled assembler code below. Also, its theoretically safe to use movdqa like done in various other wine functions. > + > + ReallyFixupVTable will crash if we are called with unaligned stack There is no need to add a comment for that, its expected to crash when misaligning the stack. > + */ > + BYTE i2[28]; > + /* mov function,%eax */ > + BYTE i3[2]; > + void (CDECL *function)(struct dll_fixup *); > + /* mov fixup,%rcx */ > + BYTE i4[2]; > + struct dll_fixup *fixup; > + /* call *%eax */ > + BYTE i5[2]; > + /* > + movups 0x28(%esp),xmm0; ...; movups 0x58(%esp),xmm3 > + add $0x68, %rsp > + */ > + BYTE i6[28]; > + /* pop %r9; pop %r8; pop %rdx; pop %rcx */ > + BYTE i7[6]; > + /* mov vtable_entry, %rax */ > + BYTE i8[2]; > + void *vtable_entry; > + /* mov [%rax],%rax > + jmp %rax */ > + BYTE i9[5]; > +}; > + > +static const struct vtable_fixup_thunk thunk_template = { > + {0x51,0x52,0x41,0x50,0x41,0x51}, > + {0x48,0x83,0xEC,0x68, > + 0x67,0x0F,0x11,0x44,0x24,0x28, 0x67,0x0F,0x11,0x4C,0x24,0x38, > + 0x67,0x0F,0x11,0x54,0x24,0x48, 0x67,0x0F,0x11,0x5C,0x24,0x58, > + }, > + {0x48,0xB8}, > + NULL, > + {0x48,0xB9}, > + NULL, > + {0xFF,0xD0}, > + {0x67,0x0F,0x10,0x44,0x24,0x28, 0x67,0x0F,0x10,0x4C,0x24,0x38, > + 0x67,0x0F,0x10,0x54,0x24,0x48, 0x67,0x0F,0x10,0x5C,0x24,0x58, > + 0x48,0x83,0xC4,0x68}, > + {0x41,0x59,0x41,0x58,0x5A,0x59}, > + {0x48,0xB8}, > + NULL, > + {0x48,0x8B,0x00,0xFF,0xE0} > +}; > + > +#include "poppack.h" > + > +#else /* !__i386__ && !__x86_64__ */ > > # define CAN_FIXUP_VTABLE 0 > > @@ -982,15 +1043,19 @@ static void CDECL ReallyFixupVTable(struct dll_fixup *fixup) > /* Mono needs an image that belongs to an assembly. */ > image = mono_assembly_get_image(assembly); > > +#if __x86_64__ > + if (fixup->fixup->type & COR_VTABLE_64BIT) > +#else > if (fixup->fixup->type & COR_VTABLE_32BIT) > +#endif > { > - DWORD *vtable = fixup->vtable; > - DWORD *tokens = fixup->tokens; > + void **vtable = fixup->vtable; > + ULONG_PTR *tokens = fixup->tokens; > for (i=0; i<fixup->fixup->count; i++) > { > - TRACE("%x\n", tokens[i]); > - vtable[i] = PtrToUint(mono_marshal_get_vtfixup_ftnptr( > - image, tokens[i], fixup->fixup->type)); > + TRACE("%#lx\n", tokens[i]); > + vtable[i] = mono_marshal_get_vtfixup_ftnptr( > + image, tokens[i], fixup->fixup->type); I would suggest to move those changes and some of the changes below into a separate patch. They can easily be reviewed separately and using PtrToUint() instead of pointer types wasn't even a good idea on 32-bit. ;) > } > } > > @@ -1029,16 +1094,18 @@ static void FixupVTableEntry(HMODULE hmodule, VTableFixup *vtable_fixup) > fixup->vtable = (BYTE*)hmodule + vtable_fixup->rva; > fixup->done = FALSE; > > + TRACE("vtable_fixup->type=0x%x\n",vtable_fixup->type); > +#if __x86_64__ > + if (vtable_fixup->type & COR_VTABLE_64BIT) > +#else > if (vtable_fixup->type & COR_VTABLE_32BIT) > +#endif > { > - DWORD *vtable = fixup->vtable; > - DWORD *tokens; > + void **vtable = fixup->vtable; > + ULONG_PTR *tokens; > int i; > struct vtable_fixup_thunk *thunks = fixup->thunk_code; > > - if (sizeof(void*) > 4) > - ERR("32-bit fixup in 64-bit mode; broken image?\n"); > - > tokens = fixup->tokens = HeapAlloc(GetProcessHeap(), 0, sizeof(*tokens) * vtable_fixup->count); > memcpy(tokens, vtable, sizeof(*tokens) * vtable_fixup->count); > for (i=0; i<vtable_fixup->count; i++) > @@ -1047,7 +1114,7 @@ static void FixupVTableEntry(HMODULE hmodule, VTableFixup *vtable_fixup) > thunks[i].fixup = fixup; > thunks[i].function = ReallyFixupVTable; > thunks[i].vtable_entry = &vtable[i]; > - vtable[i] = PtrToUint(&thunks[i]); > + vtable[i] = &thunks[i]; > } > } > else >

3 3

Re: [PATCH 3/3] mpr: Implement WNetUseConnectionA(). (try 4)
by Alexandre Julliard Jan. 23, 2016

Jan. 23, 2016

Pierre Schweitzer <pierre(a)reactos.org> writes: > + ret = WNetUseConnectionW(hwndOwner, pRes, passW, userIDW, dwFlags, > + accessNameW, lpBufferSize, lpResult); > + if (lpAccessName && lpBufferSize && *lpBufferSize && accessNameW) > + { > + len = WideCharToMultiByte(CP_ACP, 0, accessNameW, -1, NULL, 0, NULL, NULL); > + if (len && len <= *lpBufferSize) > + WideCharToMultiByte(CP_ACP, 0, accessNameW, -1, lpAccessName, len, NULL, NULL); > + } You need to handle failures, and a possibly different lpBufferSize for the W function. It could also use some test cases, which would probably require the W function to do something useful first. -- Alexandre Julliard julliard(a)winehq.org

3 3