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

Re: [PATCH vkd3d] Makefile: Explicitly avoid ANSI stdio modifiers when cross compiling.

by Giovanni Mascellani

Hi, Il 25/01/22 18:32, Zebediah Figura (she/her) ha scritto: > MinGW handles CRT linking implicitly. If using ANSI stdio, it will link > against its own CRT library (libmingwex) which provides ANSI-compatible > versions of printf functions, and then link against msvcrt to resolve > everything else. > > You can see this in practice if you disassemble our crosstests—before > this change, they don't link against "printf" from msvcrt, after this > change they do. Ooof, every time I think I understand something about compilers, some new magic sprinkles out. Well, it seems to me that the current situation is correct and doesn't emit warnings, so I think I am fine with it. But if you want to revert that and fix the warnings in some other way, that's great too. Thanks, Giovanni.

Jan. 26, 2022

Re: [PATCH 12/12] dwrite/tests: Fix misleading indentation warnings

by Nikolay Sivov

Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com>

Jan. 26, 2022

Re: [PATCH vkd3d] build: Disable crosstests as well if tests are disabled.

by Giovanni Mascellani

Hi, does this patch change anything other than what is printed at the end of the configuration step? It seems to me that the only variable that changes with this commit is HAVE_CROSSTEST, and I can't see where that is used other than at the end of configure.ac. Also, configuring with --disable-tests and then running "make crosstest" happily compiled the cross tests for me. Giovanni. Il 26/01/22 01:17, Zebediah Figura ha scritto: > Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com> > --- > configure.ac | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/configure.ac b/configure.ac > index d296dfd4b..f69f1ea4a 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -148,7 +148,7 @@ AC_CONFIG_FILES([Makefile]) > AC_OUTPUT > > dnl Output configuration summary > -AS_IF([test "x$CROSSTARGET32" != "xno" -o "x$CROSSTARGET64" != "xno"], > +AS_IF([test "x$enable_tests" != "xno" -a $ "x$CROSSTARGET32" != "xno" -o "x$CROSSTARGET64" != "xno" $], > [HAVE_CROSSTEST=yes], [HAVE_CROSSTEST=no]) > AS_IF([test $DX_FLAG_doc = 1], [HAVE_DOCS=yes], [HAVE_DOCS=no]) > > @@ -166,6 +166,6 @@ AS_ECHO([" > Building documentation: ${HAVE_DOCS} > > Building crosstests: ${HAVE_CROSSTEST}"]) > -AS_IF([test "x$CROSSTARGET32" != "xno"], [AS_ECHO([" Using 32-bit cross compiler: $CROSSCC32"])]) > -AS_IF([test "x$CROSSTARGET64" != "xno"], [AS_ECHO([" Using 64-bit cross compiler: $CROSSCC64"])]) > +AS_IF([test "$HAVE_CROSSTEST" == "yes" -a "x$CROSSTARGET32" != "xno"], [AS_ECHO([" Using 32-bit cross compiler: $CROSSCC32"])]) > +AS_IF([test "$HAVE_CROSSTEST" == "yes" -a "x$CROSSTARGET64" != "xno"], [AS_ECHO([" Using 64-bit cross compiler: $CROSSCC64"])]) > AS_ECHO([])

Jan. 26, 2022

[PATCH] programs/rpcss: use correct integral type

by Eric Pouech

Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- programs/rpcss/rpcss_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/rpcss/rpcss_main.c b/programs/rpcss/rpcss_main.c index af055e6000a..99b4020882b 100644 --- a/programs/rpcss/rpcss_main.c +++ b/programs/rpcss/rpcss_main.c @@ -56,7 +56,7 @@ HRESULT __cdecl irpcss_server_register(handle_t h, const GUID *clsid, unsigned i PMInterfacePointer object, unsigned int *cookie) { struct registered_class *entry; - static int next_cookie; + static LONG next_cookie; if (!(entry = heap_alloc_zero(sizeof(*entry)))) return E_OUTOFMEMORY;

Jan. 26, 2022

[PATCH] programs/progman: use correct integral type

by Eric Pouech

Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- programs/progman/grpfile.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/progman/grpfile.c b/programs/progman/grpfile.c index f3a084d290a..62cc1172fad 100644 --- a/programs/progman/grpfile.c +++ b/programs/progman/grpfile.c @@ -511,7 +511,7 @@ static BOOL GRPFILE_DoWriteGroupFile(HFILE file, PROGGROUP *group) HLOCAL hProgram; INT NumProg, Title, Progs, Icons, Extension; INT CurrProg, CurrIcon, nCmdShow, ptr, seqnum; - DWORD sizeAnd, sizeXor; + UINT sizeAnd, sizeXor; BOOL need_extension; LPCSTR lpszTitle = LocalLock(group->hName);

Jan. 26, 2022

[PATCH] programs/find: use correct integral type

by Eric Pouech

Signed-off-by: Eric Pouech <eric.pouech(a)gmail.com> --- programs/find/find.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/find/find.c b/programs/find/find.c index 7d178874b4d..64c7f227728 100644 --- a/programs/find/find.c +++ b/programs/find/find.c @@ -29,7 +29,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(find); static BOOL read_char_from_handle(HANDLE handle, char *char_out) { static char buffer[4096]; - static UINT buffer_max = 0; + static DWORD buffer_max = 0; static UINT buffer_pos = 0; /* Read next content into buffer */

Jan. 26, 2022

Re: [PATCH v3 07/13] loader: Don't clobber existing memory mappings when reserving addresses.

by Jinoh Kang

On 1/26/22 11:52, Jinoh Kang wrote: > On 1/26/22 00:48, Alexandre Julliard wrote: >> Jinoh Kang <jinoh.kang.kr(a)gmail.com> writes: >> >>> Today, the preloader makes no attempt to avoid unmapping existing >>> memory mappings except the initial stack. This results in irrevocably >>> unmapping some useful preallocated memory areas, such as vDSO. >>> >>> Fix this by reading /proc/self/maps for existing VMAs, and splitting >>> mmap() calls to avoid erasing existing memory mappings. >> >> That defeats the purpose of using the preloader. > > The intention was to *incrementally* scrape memory areas for the reserved ranges, > relocating any critical areas (vDSO, stack, ...) along the way. To clarify, the purpose of the preloader is to reserve some predefined (fixed) virtual memory ranges so that it's safe for Wine to use. It does so in the following manner: 1. It reserves the memory ranges so that future mmap() operations will not touch them. 2. It removes all existing references (from e.g. auxv) to the memory ranges. Note that if the range is already mapped, we can skip (1) and only perform (2). We can forcibly do (1) too, but there are the following drawbacks: - There's no point in re-reserving already occupied memory range. The OS will not reuse them for other allocations anyway, especially since we also do (2). - It might actually be harmful if the memory range coincides with memory that is being actively used (e.g. preloader code/data, stack, etc.). Since Wine does not distinguish between normally mapped pages (e.g. preloader code/data stack) and explicitly reserved pages (PROT_NONE), we can leave the existing memory mappings as-is. After all, (2) is what is actually important. > > It's also why this change is useless without the subsequent patches, > which calls map_reserve_preload_ranges again to actually fill out all > the gaps previously occupied by vDSO/stack. > >> The whole point is to make sure the specified ranges are available. > > It is. That's why I leave the preload_info ranges for Wine even though > I don't actually munmap() those pages. > >> Note that since you don't update the ranges info, the mappings will >> get erased by Wine later anyway. > > That was exactly what I intended: after jumping to ld.so, we no longer > need the code/data from preloader (except stack etc.), so we let Wine > unmap them. > > I'll make this clear in the next revision. > > (Note: the patch does update the ranges info when needed, particularly > when it fails to remap() vDSO/stack.) > -- Sincerely, Jinoh Kang

Jan. 26, 2022

Re: [PATCH v3 07/13] loader: Don't clobber existing memory mappings when reserving addresses.

by Jinoh Kang

On 1/26/22 11:52, Jinoh Kang wrote: > On 1/26/22 00:48, Alexandre Julliard wrote: >> Jinoh Kang <jinoh.kang.kr(a)gmail.com> writes: >> >>> Today, the preloader makes no attempt to avoid unmapping existing >>> memory mappings except the initial stack. This results in irrevocably >>> unmapping some useful preallocated memory areas, such as vDSO. >>> >>> Fix this by reading /proc/self/maps for existing VMAs, and splitting >>> mmap() calls to avoid erasing existing memory mappings. >> >> That defeats the purpose of using the preloader. > > The intention was to *incrementally* scrape memory areas for the reserved ranges, > relocating any critical areas (vDSO, stack, ...) along the way. > > It's also why this change is useless without the subsequent patches, > which calls map_reserve_preload_ranges again to actually fill out all > the gaps previously occupied by vDSO/stack. > >> The whole point is to make sure the specified ranges are available. > > It is. That's why I leave the preload_info ranges for Wine even though > I don't actually munmap() those pages. > >> Note that since you don't update the ranges info, the mappings will >> get erased by Wine later anyway. > > That was exactly what I intended: after jumping to ld.so, we no longer > need the code/data from preloader (except stack etc.), so we let Wine > unmap them. Also note that munmap()-ping preloader code and data from the preloader itself is unsafe, hence the commit message. This is why I'm deferring the actual unmapping to Wine itself. > > I'll make this clear in the next revision. > > (Note: the patch does update the ranges info when needed, particularly > when it fails to remap() vDSO/stack.) > -- Sincerely, Jinoh Kang

Jan. 26, 2022

Re: [PATCH v3 07/13] loader: Don't clobber existing memory mappings when reserving addresses.

by Jinoh Kang

On 1/26/22 00:48, Alexandre Julliard wrote: > Jinoh Kang <jinoh.kang.kr(a)gmail.com> writes: > >> Today, the preloader makes no attempt to avoid unmapping existing >> memory mappings except the initial stack. This results in irrevocably >> unmapping some useful preallocated memory areas, such as vDSO. >> >> Fix this by reading /proc/self/maps for existing VMAs, and splitting >> mmap() calls to avoid erasing existing memory mappings. > > That defeats the purpose of using the preloader. The intention was to *incrementally* scrape memory areas for the reserved ranges, relocating any critical areas (vDSO, stack, ...) along the way. It's also why this change is useless without the subsequent patches, which calls map_reserve_preload_ranges again to actually fill out all the gaps previously occupied by vDSO/stack. > The whole point is to make sure the specified ranges are available. It is. That's why I leave the preload_info ranges for Wine even though I don't actually munmap() those pages. > Note that since you don't update the ranges info, the mappings will > get erased by Wine later anyway. That was exactly what I intended: after jumping to ld.so, we no longer need the code/data from preloader (except stack etc.), so we let Wine unmap them. I'll make this clear in the next revision. (Note: the patch does update the ranges info when needed, particularly when it fails to remap() vDSO/stack.) -- Sincerely, Jinoh Kang

Jan. 26, 2022

Re: [RFC PATCH v2 05/11] server: Add helpers for synchronous APC delivery via ordinary server call reply.

by Jinoh Kang

On 1/26/22 03:26, Zebediah Figura (she/her) wrote: > On 1/22/22 08:36, Jinoh Kang wrote: >> diff --git a/server/protocol.def b/server/protocol.def >> index db73f0418a9..f21f7187c4d 100644 >> --- a/server/protocol.def >> +++ b/server/protocol.def >> @@ -695,6 +695,13 @@ typedef union >> } break_process; >> } apc_result_t; >> +typedef struct >> +{ >> + apc_call_t call; /* APC call arguments */ >> + obj_handle_t apc_handle; /* handle to next APC */ >> + int __pad; >> +} inline_apc_t; >> + > > Why introduce this separate type? As for "why introduce this type at all": Otherwise we have to pass around two arguments (call and apc_handle) instead of one (inline_apc_t) everywhere. This also adds two parameters to select_wait(), and I was not sure about the idea of adding ", NULL, NULL" to every user except the async one. As for "why introduce a new type instead of using existing equivalent one": There's two substitute candidates: "struct queue_apc_request" and "struct select_reply". I'm not sure reusing them is allowed, though. Perhaps we can insert inline_apc_t inside those other two structs? -- Sincerely, Jinoh Kang

Jan. 26, 2022