- Wine-GitLab - list.winehq.org

[PATCH v5 0/1] MR1065: ntdll: Add CFI unwind info to __wine_syscall_dispatcher (x86_64)
by Florian Kübler Oct. 22, 2022

Oct. 22, 2022

This change is adding DWARF (CFI) unwind information to the hand-written assembly of the `__wine_syscall_dispatcher` function. This enables unwinding through the dispatcher from the Linux stack into (and through) the Windows stack. The general idea is that the `syscall_frame` struct contains the content of the callee-save registers before the function call (in particular the stack pointer and the return address). At any point of the execution, we have a pointer into the `syscall_frame` in $rcx, $rbp or $rsp. For the CFI codes the general idea is that we are defining the computations of the callee-save registers based on the `syscall_frame` using DWARF’s `breg` instruction, rather than relative to CFA. This change adds a bunch of convenience macros, to (hopefully) improve readability of the CFI instructions. Note: Those change was used with great success for unwinding through the dispatcher using a modified LLDB shown in the [“how-wine-works-101”](https://werat.dev/blog/how-wine-works-101/) blog post as well as for in the [Orbit profiler](https://github.com/google/orbit), that has mixed-callstack unwinding support. Test: Inspect callstacks reported by the Orbit profiler while running some Windows targets using the modified wine, as well as verify debugging reports correct callstacks when stepping with our modified LLDB through the dispatcher itself (so that we are able to unwind through the dispatcher at any instruction). -- v5: ntdll: Add CFI unwind info to __wine_syscall_dispatcher (x86_64) https://gitlab.winehq.org/wine/wine/-/merge_requests/1065

2 7

[PATCH 0/3] MR1088: ntdll: Avoid calling pthread_exit on thread exit.
by Rémi Bernon Oct. 22, 2022

Oct. 22, 2022

Using a dedicated exit jmpbuf and removing the need for assembly routines. When Wine handles an exception in unix code, we return to user mode by jumping to the last syscall frame. This can leave some pthread cancel cleanups registered, in the pthread internal linked list, and at the same time later overwrite the stack frame they were registered for. In the same way, jumping to the exit frame on thread exit or abort, can also leave some cleanup handlers registered for invalid stack frames. Depending on the implementation, calling pthread_exit will cause all the registered pthread cleanup handlers to be called, possibly jumping back to now overwritten stack frames and causing segmentation faults. Exiting a pthread normally, by returning from its procedure, or calling exit(0) for the main thread doesn't run pthread_exit and doesn't call cleanup handlers, avoiding that situation. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52213 ### Additional note: For robustness, we should probably try to execute these cleanup handlers when unwinding the stack frames, as we would otherwise leave pthread objects in a potential problematic state (like a mutex locked, etc). It is however hard to do so when the handlers are registered from some C code: pthread C implementation is done by calling some internal pthread functions to register the handlers, and they aren't registered as standard unwind handlers. Only pthread_cancel and pthread_exit can unwind and call / unregister the C handlers, but interrupting that procedure, for instance calling setjmp / longjmp from withing our own handler isn't supported. >From C++ code, pthread cleanup handlers are registered through C++ class constructors / destructors, and it would then be possible to partially unwind and call them at the same time. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1088

3 6

Re: [PATCH 0/2] MR7: Draft: ntdll: Properly track refcount with forwarded exports.
by Rémi Bernon Oct. 22, 2022

Oct. 22, 2022

Rémi Bernon (@rbernon) commented about dlls/ntdll/loader.c: > > RtlEnterCriticalSection( &loader_section ); > > - /* check if the module itself is invalid to return the proper error */ > - if (!get_modref( module )) ret = STATUS_DLL_NOT_FOUND; > - else if ((exports = RtlImageDirectoryEntryToData( module, TRUE, > - IMAGE_DIRECTORY_ENTRY_EXPORT, &exp_size ))) > + wm = get_modref( module ); > + if (!wm) ret = STATUS_DLL_NOT_FOUND; > + else > { > - void *proc = name ? find_named_export( module, exports, exp_size, name->Buffer, -1, NULL ) > - : find_ordinal_export( module, exports, exp_size, ord - exports->Base, NULL ); > - if (proc) > + prev = current_modref; > + current_modref = wm; I think you can simplify this a bit, by doing `if ((current_modref = get_modref(...)))`, after saving its previous value, and restoring it before leaving the CS. Then I'm not sure what side effects this could have, and maybe it'd be better to have this specific change separate from the rest? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7#note_11120

1 0

Re: [PATCH 0/2] MR7: Draft: ntdll: Properly track refcount with forwarded exports.
by Rémi Bernon Oct. 22, 2022

Oct. 22, 2022

Rémi Bernon (@rbernon) commented about dlls/ntdll/loader.c: > if (!end) return NULL; > if (build_import_name( mod_name, forward, end - forward )) return NULL; > > - if (!(wm = find_basename_module( mod_name ))) > + imp = get_modref( module ); > + TRACE( "delay loading %s for '%s'\n", debugstr_w(mod_name), forward ); > + if (load_dll( load_path, mod_name, 0, &wm, imp->system ) == STATUS_SUCCESS && > + !(wm->ldr.Flags & LDR_DONT_RESOLVE_REFS)) > { > - WINE_MODREF *imp = get_modref( module ); > - TRACE( "delay loading %s for '%s'\n", debugstr_w(mod_name), forward ); > - if (load_dll( load_path, mod_name, 0, &wm, imp->system ) == STATUS_SUCCESS && > - !(wm->ldr.Flags & LDR_DONT_RESOLVE_REFS)) > + if ((imports_fixup_done || !current_modref) && > + process_attach( wm->ldr.DdagNode, NULL ) != STATUS_SUCCESS) Can `current_modref` really still be NULL now if `LdrGetProcedureAddress` also sets it? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7#note_11121

1 0

Re: [PATCH 0/2] MR7: Draft: ntdll: Properly track refcount with forwarded exports.
by Rémi Bernon Oct. 22, 2022

Oct. 22, 2022

Rémi Bernon (@rbernon) commented about dlls/ntdll/loader.c: > " If you are using builtin %s, try using the native one instead.\n", > forward, debugstr_w(get_modref(module)->ldr.FullDllName.Buffer), > debugstr_w(get_modref(module)->ldr.BaseDllName.Buffer) ); > + if (wm) LdrUnloadDll( wm->ldr.DllBase ); > + } > + else if (current_modref) > + { > + add_module_dependency( current_modref->ldr.DdagNode, wm->ldr.DdagNode ); Maybe we can then always add the module dependency after it's been loaded? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7#note_11122

1 0

Re: [PATCH v5 0/5] MR944: win32u: Make desktop resize logic asynchronous.
by Alexandre Julliard (＠julliard) Oct. 22, 2022

Oct. 22, 2022

> The first one seems to be caused by an OOM error. I've seen similar issues in the past when running the tests under llvmpipe, and when multiple D3D devices are initialized. The llvmpipe GL context is memory hungry and it quickly fills the 32-bit address space. > > This doesn't disculp this changes in any way and it could be very well related, if for some reason the messages here are causing GL contexts to be re-created more often or at different times. It's possible that the failures could have been triggered before, though I don't remember ever seeing the d3drm one. But they are happening much more often, almost every test run has one of them now. We need to fix that. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/944#note_11204

1 0

Re: [PATCH v5 0/5] MR944: win32u: Make desktop resize logic asynchronous.
by Rémi Bernon Oct. 22, 2022

Oct. 22, 2022

On Tue Oct 18 21:43:53 2022 +0000, Alexandre Julliard wrote: > > The first one seems to be caused by an OOM error. I've seen similar > issues in the past when running the tests under llvmpipe, and when > multiple D3D devices are initialized. The llvmpipe GL context is memory > hungry and it quickly fills the 32-bit address space. > > > > This doesn't disculp this changes in any way and it could be very well > related, if for some reason the messages here are causing GL contexts to > be re-created more often or at different times. > It's possible that the failures could have been triggered before, though > I don't remember ever seeing the d3drm one. > But they are happening much more often, almost every test run has one of > them now. We need to fix that. Yes, I'm investigating. I can reproduce the failures locally, and they look indeed related. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/944#note_11205

1 0

Re: [PATCH v16 0/6] MR1120: Add support for localized functions in msvcrt dlls
by Bartosz Kosiorek (＠gang65) Oct. 22, 2022

Oct. 22, 2022

On Sat Oct 22 13:16:09 2022 +0000, Bartosz Kosiorek wrote: > changed this line in [version 16 of the diff](/wine/wine/-/merge_requests/1120/diffs?diff_id=15105&start_sha=01947548572fd5e442ee7677a63d2ff3c0f166e2#18ec41aa1588580e1f59f4c1c498c00b98b78b89_1012_1012) true, fixes. Thanks! -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1120#note_11802

1 0

[PATCH v3 0/1] MR1128: msvcrt: Display message box in abort() in debug CRT only.
by Paul Gofman (＠gofman) Oct. 22, 2022

Oct. 22, 2022

As far as my testing goes, the message box is never shown on Windows when abort() is called with retail runtime is used, regardless of error mode and abort behaviour. That is not the case with _assert which still can show a dialog even in release mode. It happens rather often that an app meets the error condition during the process exit and that goes silent on Windows while we display the abort dialog on app's exit. The test program below shows that (and the same behaviour is, e. g., with ucrtbase vs ucrtbased). ``` #include <windows.h> #include <stdio.h> #include <stdlib.h> void CDECL _set_app_type(int app_type); #define D(name) static typeof(name) *p_##name #define L(name) p_##name = (void *)GetProcAddress(hdll, #name); if (!p_##name) printf("NULL %s.\n", #name) D(abort); D(_set_app_type); D(_set_abort_behavior); D(_set_error_mode); int main(int argc, char *argv[]) { HMODULE hdll = LoadLibraryA("msvcrt.dll"); printf("hdll %p.\n", hdll); L(_set_app_type); if (!p__set_app_type) if (!(p__set_app_type = (void *)GetProcAddress(hdll, "__set_app_type"))) printf("NULL p__set_app_type.\n"); L(abort); L(_set_abort_behavior); L(_set_error_mode); p__set_app_type(2); if (p__set_abort_behavior) p__set_abort_behavior(_WRITE_ABORT_MSG, _WRITE_ABORT_MSG); p__set_error_mode(_OUT_TO_MSGBOX); p_abort(); } ``` -- v3: msvcrt: Display message box in abort() for specific CRT versions only. https://gitlab.winehq.org/wine/wine/-/merge_requests/1128

4 4

[PATCH 0/1] MR1144: msvcrt: Fix _wputenv_s invalid argument handling.
by Piotr Caban (＠piotr) Oct. 22, 2022

Oct. 22, 2022

-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1144

3 2