May 2020 - Wine-devel - list.winehq.org

[RFC PATCH 00/11] Thread-local heap implementation.
by Rémi Bernon 06 May '20

06 May '20

Hi all! This is a heap implementation based on thread-local structures, that I have been keeping locally for quite some time. The goal was to improve Wine's heap performance in multithreaded scenarios and see if it could help performance in some games. The good news is that this implementation is performing well, according to third-party heap micro benchmarks. The bad news is that it doesn't change performance much in general, as allocations are usually scarse during gameplay. I could still see improvements for loading times, and less stalling as well. As I've been tweaking it lately and now that it passes the heap tests I believe it could still be somehow useful for others, or interesting for anyone eager to try. It could also possibly be used for comparison, to determine if the heap is the bottleneck for some use case. The implementation details are in patch #9 for anyone interested. The thread-local heaps are enabled by HeapSetInformation calls, when the call requests LFH heap, but I also added a WINELFH environment variable to guard it so that it can also be disabled globally. There are multiple issues that I am aware of, but for the applications that I tried they didn't seem to cause much trouble: * Block headers are different from the default ones, but then FWIW the default block headers don't seem to match with recent Windows either, which also vary depending on the Windows version. * HeapWalk will not show the blocks allocated from the thread-local heaps. It could be possible to also walk the current thread-local heap but then because of implementation detail, some blocks would stlil not show up (fully used arenas are detached from their heap until a block is freed). * HeapDestroy doesn't automatically free the blocks that were allocated from the thread-local heaps and they can possibly be lost forever for the same reason as above. Although it could be possible to keep track of fully used arenas, the same thread-local heaps are shared between heaps, so there's currently no way to tell which block should be freed on destroy. * The freed blocks are deferred to the thread that allocated them, but in general they are quickly reused. There's some validation being done but it could still cause some issues in case of use after free. It should be easily fixed by adding a buffer, but as I didn't see any game needing it, didn't seem to worth the trouble for now. Cheers, Rémi Bernon (11): kernel32: Catch page faults in GlobalSize. kernel32/tests: Add HeapSetInformation and LFH tests. ntdll: Split standard heap functions. ntdll: Add thread destroy notification function. ntdll: Add extended heap type and LFH stubs. ntdll: Implement RtlSetHeapInformation for LFH. ntdll: Move undocumented flags to ntdll_misc.h. HACK: ntdll: Conditionally enable LFH. ntdll: Implement Low Fragmentation Heap. ntdll: Enable LFH for process heap. msvcrt: Enable LFH for internal heaps. dlls/kernel32/heap.c | 78 +-- dlls/kernel32/tests/heap.c | 66 +++ dlls/msvcrt/heap.c | 4 + dlls/ntdll/Makefile.in | 1 + dlls/ntdll/heap.c | 220 ++++++-- dlls/ntdll/heap_lfh.c | 1057 ++++++++++++++++++++++++++++++++++++ dlls/ntdll/loader.c | 3 + dlls/ntdll/ntdll_misc.h | 28 + dlls/ntdll/thread.c | 1 + 9 files changed, 1379 insertions(+), 79 deletions(-) create mode 100644 dlls/ntdll/heap_lfh.c -- 2.26.1

3 20

[PATCH 2/2] winegcc: Use --safeseh winebuild option on msvc target.
by Jacek Caban 06 May '20

06 May '20

lld-link requires all x86 object files to be SEH compatible unless it's explicitly disabled. Signed-off-by: Jacek Caban <jacek(a)codeweavers.com> --- tools/winegcc/winegcc.c | 2 ++ 1 file changed, 2 insertions(+)

1 0

[PATCH 1/2] winebuild: Introduce --safeseh option that allows marking assembly files as SEH compatible.
by Jacek Caban 06 May '20

06 May '20

Signed-off-by: Jacek Caban <jacek(a)codeweavers.com> --- tools/winebuild/build.h | 1 + tools/winebuild/main.c | 7 +++++++ tools/winebuild/utils.c | 6 ++++++ tools/winebuild/winebuild.man.in | 3 +++ 4 files changed, 17 insertions(+)

1 0

[PATCH] mf: Fix timer instance leak on error path in SetTimer().
by Nikolay Sivov 06 May '20

06 May '20

Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/mf/session.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/dlls/mf/session.c b/dlls/mf/session.c index d6b881218b..25832afdbb 100644 --- a/dlls/mf/session.c +++ b/dlls/mf/session.c @@ -4093,15 +4093,19 @@ static HRESULT WINAPI present_clock_timer_SetTimer(IMFTimer *iface, DWORD flags, hr = MF_S_CLOCK_STOPPED; if (SUCCEEDED(hr)) + { list_add_tail(&clock->timers, &clock_timer->entry); + if (cancel_key) + { + *cancel_key = &clock_timer->IUnknown_iface; + IUnknown_AddRef(*cancel_key); + } + } LeaveCriticalSection(&clock->cs); - if (SUCCEEDED(hr) && cancel_key) - { - *cancel_key = &clock_timer->IUnknown_iface; - IUnknown_AddRef(*cancel_key); - } + if (FAILED(hr)) + IUnknown_Release(&clock_timer->IUnknown_iface); return hr; } -- 2.26.2

1 0

[PATCH 1/2] dwrite: Update DWRITE_SHAPING_TEXT_PROPERTIES definition.
by Nikolay Sivov 06 May '20

06 May '20

Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/dwrite/analyzer.c | 7 +++---- include/dwrite.idl | 4 +++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/dlls/dwrite/analyzer.c b/dlls/dwrite/analyzer.c index c2fd5c54d3..2cbac7700c 100644 --- a/dlls/dwrite/analyzer.c +++ b/dlls/dwrite/analyzer.c @@ -1178,6 +1178,9 @@ static HRESULT WINAPI dwritetextanalyzer_GetGlyphs(IDWriteTextAnalyzer2 *iface, method = get_number_substitutes(substitution, digits); + /* FIXME: have the shaping engine set this */ + memset(text_props, 0, length * sizeof(*text_props)); + for (i = 0; i < length; i++) { /* FIXME: set to better values */ glyph_props[i].justification = text[i] == ' ' ? SCRIPT_JUSTIFY_BLANK : SCRIPT_JUSTIFY_CHARACTER; @@ -1186,10 +1189,6 @@ static HRESULT WINAPI dwritetextanalyzer_GetGlyphs(IDWriteTextAnalyzer2 *iface, glyph_props[i].isZeroWidthSpace = 0; glyph_props[i].reserved = 0; - /* FIXME: have the shaping engine set this */ - text_props[i].isShapedAlone = 0; - text_props[i].reserved = 0; - clustermap[i] = i; string[i] = text[i]; diff --git a/include/dwrite.idl b/include/dwrite.idl index b92b1544e0..a8c523edda 100644 --- a/include/dwrite.idl +++ b/include/dwrite.idl @@ -546,7 +546,9 @@ typedef struct DWRITE_TYPOGRAPHIC_FEATURES typedef struct DWRITE_SHAPING_TEXT_PROPERTIES { UINT16 isShapedAlone : 1; - UINT16 reserved : 15; + UINT16 reserved1 : 1; + UINT16 canBreakShapingAfter : 1; + UINT16 reserved : 13; } DWRITE_SHAPING_TEXT_PROPERTIES; typedef struct DWRITE_SHAPING_GLYPH_PROPERTIES -- 2.26.2

1 1

[PATCH] msi: Return void from init_automation_object.
by Hans Leidekker 06 May '20

06 May '20

Signed-off-by: Hans Leidekker <hans(a)codeweavers.com> --- dlls/msi/automation.c | 72 +++++++++---------------------------------- 1 file changed, 14 insertions(+), 58 deletions(-) diff --git a/dlls/msi/automation.c b/dlls/msi/automation.c index ba3a068a74..87c7815094 100644 --- a/dlls/msi/automation.c +++ b/dlls/msi/automation.c @@ -543,18 +543,15 @@ static const IProvideMultipleClassInfoVtbl ProvideMultipleClassInfoVtbl = ProvideMultipleClassInfo_GetInfoOfIndex }; -static HRESULT init_automation_object(AutomationObject *This, MSIHANDLE msiHandle, tid_t tid) +static void init_automation_object(AutomationObject *This, MSIHANDLE msiHandle, tid_t tid) { TRACE("(%p, %d, %s)\n", This, msiHandle, debugstr_guid(get_riid_from_tid(tid))); This->IDispatch_iface.lpVtbl = &AutomationObjectVtbl; This->IProvideMultipleClassInfo_iface.lpVtbl = &ProvideMultipleClassInfoVtbl; This->ref = 1; - This->msiHandle = msiHandle; This->tid = tid; - - return S_OK; } /* @@ -1002,21 +999,15 @@ static HRESULT record_invoke( static HRESULT create_record(MSIHANDLE msiHandle, IDispatch **disp) { AutomationObject *record; - HRESULT hr; record = msi_alloc(sizeof(*record)); if (!record) return E_OUTOFMEMORY; - hr = init_automation_object(record, msiHandle, Record_tid); - if (hr != S_OK) - { - msi_free(record); - return hr; - } + init_automation_object(record, msiHandle, Record_tid); *disp = &record->IDispatch_iface; - return hr; + return S_OK; } static HRESULT list_invoke( @@ -1123,12 +1114,7 @@ static HRESULT create_list(const WCHAR *product, IDispatch **dispatch) list = msi_alloc_zero(sizeof(ListObject)); if (!list) return E_OUTOFMEMORY; - hr = init_automation_object(&list->autoobj, 0, StringList_tid); - if (hr != S_OK) - { - msi_free(list); - return hr; - } + init_automation_object(&list->autoobj, 0, StringList_tid); *dispatch = &list->autoobj.IDispatch_iface; @@ -2442,7 +2428,6 @@ static HRESULT installer_invoke( HRESULT create_msiserver(IUnknown *outer, void **ppObj) { AutomationObject *installer; - HRESULT hr; TRACE("(%p %p)\n", outer, ppObj); @@ -2452,99 +2437,70 @@ HRESULT create_msiserver(IUnknown *outer, void **ppObj) installer = msi_alloc(sizeof(AutomationObject)); if (!installer) return E_OUTOFMEMORY; - hr = init_automation_object(installer, 0, Installer_tid); - if (hr != S_OK) - { - msi_free(installer); - return hr; - } + init_automation_object(installer, 0, Installer_tid); *ppObj = &installer->IDispatch_iface; - return hr; + return S_OK; } HRESULT create_session(MSIHANDLE msiHandle, IDispatch *installer, IDispatch **disp) { SessionObject *session; - HRESULT hr; session = msi_alloc(sizeof(SessionObject)); if (!session) return E_OUTOFMEMORY; - hr = init_automation_object(&session->autoobj, msiHandle, Session_tid); - if (hr != S_OK) - { - msi_free(session); - return hr; - } + init_automation_object(&session->autoobj, msiHandle, Session_tid); session->installer = installer; *disp = &session->autoobj.IDispatch_iface; - return hr; + return S_OK; } static HRESULT create_database(MSIHANDLE msiHandle, IDispatch **dispatch) { AutomationObject *database; - HRESULT hr; TRACE("(%d %p)\n", msiHandle, dispatch); database = msi_alloc(sizeof(AutomationObject)); if (!database) return E_OUTOFMEMORY; - hr = init_automation_object(database, msiHandle, Database_tid); - if (hr != S_OK) - { - msi_free(database); - return hr; - } + init_automation_object(database, msiHandle, Database_tid); *dispatch = &database->IDispatch_iface; - return hr; + return S_OK; } static HRESULT create_view(MSIHANDLE msiHandle, IDispatch **dispatch) { AutomationObject *view; - HRESULT hr; TRACE("(%d %p)\n", msiHandle, dispatch); view = msi_alloc(sizeof(AutomationObject)); if (!view) return E_OUTOFMEMORY; - hr = init_automation_object(view, msiHandle, View_tid); - if (hr != S_OK) - { - msi_free(view); - return hr; - } + init_automation_object(view, msiHandle, View_tid); *dispatch = &view->IDispatch_iface; - return hr; + return S_OK; } static HRESULT create_summaryinfo(MSIHANDLE msiHandle, IDispatch **disp) { AutomationObject *info; - HRESULT hr; info = msi_alloc(sizeof(*info)); if (!info) return E_OUTOFMEMORY; - hr = init_automation_object(info, msiHandle, SummaryInfo_tid); - if (hr != S_OK) - { - msi_free(info); - return hr; - } + init_automation_object(info, msiHandle, SummaryInfo_tid); *disp = &info->IDispatch_iface; - return hr; + return S_OK; } -- 2.20.1

1 0

[PATCH 2/4] user32/tests: Test button styles.
by Zhiyi Zhang 06 May '20

06 May '20

Signed-off-by: Zhiyi Zhang <zzhang(a)codeweavers.com> --- dlls/user32/tests/msg.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/dlls/user32/tests/msg.c b/dlls/user32/tests/msg.c index 93ab7b4a27d..f84525a0bf9 100644 --- a/dlls/user32/tests/msg.c +++ b/dlls/user32/tests/msg.c @@ -31,6 +31,7 @@ #include "winuser.h" #include "winnls.h" #include "dbt.h" +#include "commctrl.h" #include "wine/test.h" @@ -17881,6 +17882,37 @@ static void test_invalid_window(void) ok(GetLastError() == ERROR_INVALID_WINDOW_HANDLE, "wrong error %u\n", GetLastError()); } +static void test_button_style(void) +{ + DWORD type, expected_type; + HWND button; + LRESULT ret; + DWORD i, j; + + for (i = BS_PUSHBUTTON; i <= BS_DEFCOMMANDLINK; ++i) + { + button = CreateWindowA(WC_BUTTONA, "test", i, 0, 0, 50, 50, NULL, 0, 0, NULL); + ok(button != NULL, "Expected button not null.\n"); + + type = GetWindowLongW(button, GWL_STYLE) & BS_TYPEMASK; + expected_type = (i == BS_USERBUTTON ? BS_PUSHBUTTON : i); + ok(type == expected_type, "Expected type %#x, got %#x.\n", expected_type, type); + + for (j = BS_PUSHBUTTON; j <= BS_DEFCOMMANDLINK; ++j) + { + ret = SendMessageA(button, BM_SETSTYLE, j, FALSE); + ok(ret == 0, "Expected %#x, got %#lx.\n", 0, ret); + + type = GetWindowLongW(button, GWL_STYLE) & BS_TYPEMASK; + expected_type = j; + + ok(type == expected_type, "Original type %#x, expected new type %#x, got %#x.\n", i, + expected_type, type); + } + DestroyWindow(button); + } +} + START_TEST(msg) { char **test_argv; @@ -17964,6 +17996,7 @@ START_TEST(msg) test_mdi_messages(); test_button_messages(); test_button_bm_get_set_image(); + test_button_style(); test_autoradio_BM_CLICK(); test_autoradio_kbd_move(); test_static_messages(); -- 2.25.1

2 1

[PATCH 4/4] comctl32/button: Do not set BS_PUSHBUTTON and BS_DEFPUSHBUTTON style directly to Split Buttons and Command Links.
by Zhiyi Zhang 06 May '20

06 May '20

For Split Buttons and Command Links of common control version 6, setting BS_PUSHBUTTON and BS_DEFPUSHBUTTON style toggles their default bit. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47621 Signed-off-by: Zhiyi Zhang <zzhang(a)codeweavers.com> --- dlls/comctl32/button.c | 12 +++++++++--- dlls/comctl32/tests/button.c | 1 - 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/dlls/comctl32/button.c b/dlls/comctl32/button.c index b615df4a165..4f7a3052b10 100644 --- a/dlls/comctl32/button.c +++ b/dlls/comctl32/button.c @@ -847,8 +847,14 @@ static LRESULT CALLBACK BUTTON_WindowProc(HWND hWnd, UINT uMsg, WPARAM wParam, L break; case BM_SETSTYLE: - btn_type = wParam & BS_TYPEMASK; - style = (style & ~BS_TYPEMASK) | btn_type; + { + DWORD new_btn_type; + + new_btn_type= wParam & BS_TYPEMASK; + if (btn_type >= BS_SPLITBUTTON && new_btn_type <= BS_DEFPUSHBUTTON) + new_btn_type = (btn_type & ~BS_DEFPUSHBUTTON) | new_btn_type; + + style = (style & ~BS_TYPEMASK) | new_btn_type; SetWindowLongW( hWnd, GWL_STYLE, style ); /* Only redraw if lParam flag is set.*/ @@ -856,7 +862,7 @@ static LRESULT CALLBACK BUTTON_WindowProc(HWND hWnd, UINT uMsg, WPARAM wParam, L InvalidateRect( hWnd, NULL, TRUE ); break; - + } case BM_CLICK: SendMessageW( hWnd, WM_LBUTTONDOWN, 0, 0 ); SendMessageW( hWnd, WM_LBUTTONUP, 0, 0 ); diff --git a/dlls/comctl32/tests/button.c b/dlls/comctl32/tests/button.c index 933db6e9825..716850073ff 100644 --- a/dlls/comctl32/tests/button.c +++ b/dlls/comctl32/tests/button.c @@ -2303,7 +2303,6 @@ static void test_style(void) else expected_type = j; - todo_wine_if(i >= BS_SPLITBUTTON && j <= BS_DEFPUSHBUTTON) ok(type == expected_type || broken(type == j), /* XP */ "Original type %#x, expected new type %#x, got %#x.\n", i, expected_type, type); } -- 2.25.1

2 1

[PATCH 3/4] comctl32/tests: Test button styles.
by Zhiyi Zhang 06 May '20

06 May '20

Signed-off-by: Zhiyi Zhang <zzhang(a)codeweavers.com> --- dlls/comctl32/tests/button.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/dlls/comctl32/tests/button.c b/dlls/comctl32/tests/button.c index 14071b2ecd0..933db6e9825 100644 --- a/dlls/comctl32/tests/button.c +++ b/dlls/comctl32/tests/button.c @@ -2276,6 +2276,41 @@ static void test_bcm_get_ideal_size(void) DeleteObject(hfont); } +static void test_style(void) +{ + DWORD type, expected_type; + HWND button; + LRESULT ret; + DWORD i, j; + + for (i = BS_PUSHBUTTON; i <= BS_DEFCOMMANDLINK; ++i) + { + button = CreateWindowA(WC_BUTTONA, "test", i, 0, 0, 50, 50, NULL, 0, 0, NULL); + ok(button != NULL, "Expected button not null.\n"); + + type = GetWindowLongW(button, GWL_STYLE) & BS_TYPEMASK; + expected_type = (i == BS_USERBUTTON ? BS_PUSHBUTTON : i); + ok(type == expected_type, "Expected type %#x, got %#x.\n", expected_type, type); + + for (j = BS_PUSHBUTTON; j <= BS_DEFCOMMANDLINK; ++j) + { + ret = SendMessageA(button, BM_SETSTYLE, j, FALSE); + ok(ret == 0, "Expected %#x, got %#lx.\n", 0, ret); + + type = GetWindowLongW(button, GWL_STYLE) & BS_TYPEMASK; + if (i >= BS_SPLITBUTTON && j <= BS_DEFPUSHBUTTON) + expected_type = (i & ~BS_DEFPUSHBUTTON) | j; + else + expected_type = j; + + todo_wine_if(i >= BS_SPLITBUTTON && j <= BS_DEFPUSHBUTTON) + ok(type == expected_type || broken(type == j), /* XP */ + "Original type %#x, expected new type %#x, got %#x.\n", i, expected_type, type); + } + DestroyWindow(button); + } +} + START_TEST(button) { ULONG_PTR ctx_cookie; @@ -2298,6 +2333,7 @@ START_TEST(button) test_get_set_textmargin(); test_state(); test_bcm_get_ideal_size(); + test_style(); unload_v6_module(ctx_cookie, hCtx); } -- 2.25.1

2 1

[PATCH 1/4] user32/tests: Add a trailing '\n' to some ok() calls.
by Zhiyi Zhang 06 May '20

06 May '20

Signed-off-by: Zhiyi Zhang <zzhang(a)codeweavers.com> --- dlls/user32/tests/dialog.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dlls/user32/tests/dialog.c b/dlls/user32/tests/dialog.c index 5a6a0bf5fb7..3f6fe6cfca1 100644 --- a/dlls/user32/tests/dialog.c +++ b/dlls/user32/tests/dialog.c @@ -652,8 +652,8 @@ static void test_WM_NEXTDLGCTL(void) * Check whether the style of the button which got the focus, changed to BS_DEFPUSHBUTTON and * the style of default button changed to BS_PUSHBUTTON. */ - ok(get_button_style(g_hwndTestDlgBut1) == BS_DEFPUSHBUTTON, "Button1's style not set to BS_DEFPUSHBUTTON"); - ok(get_button_style(g_hwndTestDlgBut2) == BS_PUSHBUTTON, "Button2's style not set to BS_PUSHBUTTON"); + ok(get_button_style(g_hwndTestDlgBut1) == BS_DEFPUSHBUTTON, "Button1's style not set to BS_DEFPUSHBUTTON\n"); + ok(get_button_style(g_hwndTestDlgBut2) == BS_PUSHBUTTON, "Button2's style not set to BS_PUSHBUTTON\n"); /* Move focus to Button2 using "WM_NEXTDLGCTL" */ DefDlgProcA(g_hwndTestDlg, WM_NEXTDLGCTL, 0, 0); @@ -667,8 +667,8 @@ static void test_WM_NEXTDLGCTL(void) * Check whether the style of the button which got the focus, changed to BS_DEFPUSHBUTTON and * the style of button which lost the focus changed to BS_PUSHBUTTON. */ - ok(get_button_style(g_hwndTestDlgBut1) == BS_PUSHBUTTON, "Button1's style not set to BS_PUSHBUTTON"); - ok(get_button_style(g_hwndTestDlgBut2) == BS_DEFPUSHBUTTON, "Button2's style not set to BS_DEFPUSHBUTTON"); + ok(get_button_style(g_hwndTestDlgBut1) == BS_PUSHBUTTON, "Button1's style not set to BS_PUSHBUTTON\n"); + ok(get_button_style(g_hwndTestDlgBut2) == BS_DEFPUSHBUTTON, "Button2's style not set to BS_DEFPUSHBUTTON\n"); /* Move focus to Edit control using "WM_NEXTDLGCTL" */ DefDlgProcA(g_hwndTestDlg, WM_NEXTDLGCTL, 0, 0); -- 2.25.1

1 0