- Wine-gitlab - list.winehq.org

Re: [PATCH v2 0/3] MR449: wined3d: Fix client_map_count refcounting errors. - approved
by Alexandre Julliard (＠julliard) 14 Jul '22

14 Jul '22

This merge request was approved by Alexandre Julliard. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/449

1 0

[PATCH 0/1] MR451: ntdll: Only check casefold flag when filesystem supports it.
by Tuupertunut (＠Tuupertunut) 14 Jul '22

14 Jul '22

Currently the wine function `get_dir_case_sensitivity_stat` calls `ioctl` syscall with command `EXT2_IOC_GETFLAGS` and tries to read a flag `EXT4_CASEFOLD_FL` from the result. It does this on all filesystems without checking if the filesystem supports that flag. This patch adds a check: only do the call on filesystems that are known to support `EXT4_CASEFOLD_FL`. To my knowledge these are EXT4 and F2FS. Advantages: - The `EXT4_CASEFOLD_FL` flag is nonstandard and other filesystems may use the same binary value to represent another flag. This patch prevents flag compatibility issues with other filesystems. - This fixes (or works around) an issue on FUSE filesystems using libfuse 3.10 or older and kernel 5.13 or newer, where unsupported `ioctl` calls always succeed and return random (uninitialized) data (see https://github.com/libfuse/libfuse/issues/640) On such setups Wine randomly fails to read files because it thinks that the `EXT4_CASEFOLD_FL` is set when really it is just random data. Ubuntu 22.04 LTS has settled on libfuse 3.10 and kernel 5.15 so this issue will be around for years to come. Disadvantages: - The flag will not be read on possible other current and future filesystems that support it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/451

3 6

[PATCH 0/6] MR464: Implement _StructuredTaskCollection::_Schedule{,_loc}
by Torge Matthies (＠tmatthies) 14 Jul '22

14 Jul '22

- Based on !463 - _RunAndWait and exception handling implemented in later commits @piotr -- https://gitlab.winehq.org/wine/wine/-/merge_requests/464

2 6

Re: [PATCH 0/4] MR384: winsock: Support fallback to ICMP over SOCK_DGRAM on Linux
by Paul Gofman (＠gofman) 14 Jul '22

14 Jul '22

> GitLab > > gitlab-to-mail <https://gitlab.winehq.org/project_2_bot> started a new > discussion > <https://gitlab.winehq.org/wine/wine/-/merge_requests/384#note_4039>: > > Zebediah Figura replied on the mailing list: > > |On 7/5/22 14:53, Paul Gofman wrote: @@ -602,9 +623,68 @@ static > NTSTATUS try_recv( int fd, struct async_recv_ioctl *async, ULONG_PTR > *siz } status = (hdr.msg_flags & MSG_TRUNC) ? STATUS_BUFFER_OVERFLOW : > STATUS_SUCCESS; + if (async->fixup_type == > SOCK_PROT_FIXUP_ICMP_OVER_DGRAM) + { + unsigned int tot_len = > sizeof(struct ip_hdr) + ret; + size_t len = hdr.msg_iov[0].iov_len; + > char *buf = hdr.msg_iov[0].iov_base; + struct cmsghdr *cmsg; + struct > ip_hdr ip_h; + + if (ret + sizeof(ip_h) > len) + status = > STATUS_BUFFER_OVERFLOW; + + if (len < sizeof(ip_h)) + { + ret = len; + > } + else + { + ret = min( ret, len - sizeof(ip_h) ); + memmove( buf + > sizeof(ip_h), buf, ret ); + ret += sizeof(ip_h); + } + memset( &ip_h, > 0, sizeof(ip_h) ); + ip_h.v_hl = (4 << 4) | (sizeof(ip_h) >> 2); + > ip_h.tot_len = htons( tot_len ); + ip_h.protocol = 1; + ip_h.saddr = > unix_addr.in.sin_addr.s_addr; + + for (cmsg = CMSG_FIRSTHDR( &hdr ); > cmsg; cmsg = CMSG_NXTHDR( &hdr, cmsg )) + { + if (cmsg->cmsg_level != > IPPROTO_IP) continue; + switch (cmsg->cmsg_type) + { +#if > defined(IP_TTL) + case IP_TTL: + ip_h.ttl = *(BYTE *)CMSG_DATA( cmsg > ); + break; +#endif +#if defined(IP_TOS) + case IP_TOS: + ip_h.tos = > *(BYTE *)CMSG_DATA( cmsg ); + break; +#endif +#if defined(IP_PKTINFO) > + case IP_PKTINFO: + { + struct in_pktinfo *info; + + info = (struct > in_pktinfo *)CMSG_DATA( cmsg ); + ip_h.daddr = info->ipi_addr.s_addr; > + break; + } +#endif + } + } + memcpy( buf, &ip_h, min( sizeof(ip_h), > len )); + } Would you mind making this a helper function? try_recv() > is already large and this seems to be pretty self-contained logic.| |Sure.| > |> if (async->control) > { > + if (async->fixup_type == > SOCK_PROT_FIXUP_ICMP_OVER_DGRAM) > + FIXME( "May return extra control > headers.\n" ); > + What is this for?| When initializing the fixup on server I am setting IP_RECVTTL, IP_RECVTOS and IP_PKTINFO socket options. If the app is calling recvmsg() it may get the headers it didn't request. My reasoning that such a situation doesn't deserve further code complication to avoid but deserves a fixme. > |> @@ -737,17 +818,26 @@ static NTSTATUS sock_recv( HANDLE handle, > HANDLE event, PIO_APC_ROUTINE apc, voi > } > } > > - SERVER_START_REQ( > recv_socket ) > + do > { > - req->force_async = force_async; > - > req->async = server_async( handle, &async->io, event, apc, apc_user, > iosb_client_ptr(io) ); > - req->oob = !!(unix_flags & MSG_OOB); > - > status = wine_server_call( req ); > - wait_handle = > wine_server_ptr_handle( reply->wait ); > - options = reply->options; > > - nonblocking = reply->nonblocking; > - } > - SERVER_END_REQ; > + > SERVER_START_REQ( recv_socket ) > + { > + req->force_async = > force_async; > + req->async = server_async( handle, &async->io, event, > apc, apc_user, iosb_client_ptr(io) ); > + req->oob = !!(unix_flags & > MSG_OOB); > + req->fixup_type = async->fixup_type; > + status = > wine_server_call( req ); > + if (status == > STATUS_MORE_PROCESSING_REQUIRED) > + { > + async->fixup_type = > reply->fixup_type; > + continue; > + } > + wait_handle = > wine_server_ptr_handle( reply->wait ); > + options = reply->options; > > + nonblocking = reply->nonblocking; > + } > + SERVER_END_REQ; > + } > while (status == STATUS_MORE_PROCESSING_REQUIRED); > > alerted = > status == STATUS_ALERTED; > if (alerted) I'm missing something; why do > we need to call this in a loop? Why does the client needs to > communicate the fixup type to the server in the first place?| The async may be initiated on the server and delivered to the process (through SIGUSR1)�� before we set async->fixup_type. It might be not likely in practice, but I double verified that it is possible by injecting Sleep(...) before setting fixup type to async structure from server. So I need to make sure that fixup_type in async structure is correctly set before try_recv() can be reached. So idea is, fixup_type is "no fixup" by default, no extra server call is involved in normal case. But if fixup is required and client does not yet know about it server doesn't�� do anything and just returns fixup type with STATUS_MORE_PROCESSING_REQUIRED. > |> diff --git a/server/protocol.def b/server/protocol.def > index > a8beccaf468..e1e5f3d9faa 100644 > --- a/server/protocol.def > +++ > b/server/protocol.def > @@ -1446,17 +1446,23 @@ enum server_fd_type > > file_pos_t count; /* count of bytes to unlock */ > @END > > - > /* > Perform a recv on a socket */ > @REQ(recv_socket) > int oob; /* are we > receiving OOB data? */ > async_data_t async; /* async I/O parameters > */ > int force_async; /* Force asynchronous mode? */ > + int > fixup_type; /* protocol fixup type returned previously */ > @REPLY > > obj_handle_t wait; /* handle to wait on for blocking recv */ > > unsigned int options; /* device open options */ > int nonblocking; /* > is socket non-blocking? */ > + int fixup_type; /* protocol fixup type > (see below) */ > @END > +enum sock_prot_fixup_type > +{ > + > SOCK_PROT_FIXUP_NONE, > + SOCK_PROT_FIXUP_ICMP_OVER_DGRAM, > +}; > Making this an enum strikes me as odd in general. A bitmask might make > more sense, but frankly I'd just leave it as a dedicated field until > we have something else that needs fixups. Do you have any plans for > introducing other types of fixups?| Well, originally I did something between the lines of this (had icmp_over_dgram field in request / reply). But then I looked at that and it felt odd that general recv_socket() has some special icmp related fields, it felt like naming that "fixup_type" tells what it is without going into very specific ICMP fixup details looking weird at this level. If you feel like it is better to name it, e. g., "icmp_over_dgram" I can change it back. I don't currently have any other types of fixups in mind. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/384#note_4045

3 6

[PATCH v2 0/1] MR427: ntdll: Preserve rbx register when calling DLL entry point on x64.
by Paul Gofman (＠gofman) 14 Jul '22

14 Jul '22

Assassin's Creed Origins clobbers rbx in its main module (second of three) TLS callbacks. That is apparent right in the beginning of the TLS callback disassembly when rbx is set to '4' unconditionally without any prior save. That leads to a fault in call_tls_callbacks() which is still in __TRY block and gets handled. However the third TLS callback is not executed and that leads to intermittent hangs later on. It is rather involved to make a TLS callback test in Wine testsuite as there is no portable way to generate a custom TLS callback. I've made a test program (based on the example here: https://lallouslab.net/2017/05/30/using-cc-tls-callbacks-in-visual-studio-w…) and compiled it with MSVC. The source code is here: https://gist.github.com/gofman/3287a953bcab3a5c888a8d494461cb8a. The program calls all the callbacks on Windows 10 here. There is also a similar wrapper already there for i386 on another occasion. -- v2: ntdll: Preserve rbx register when calling DLL entry point on x64. https://gitlab.winehq.org/wine/wine/-/merge_requests/427

2 2

[PATCH 0/1] MR462: secur32: Fix GnuTLS system priority file override.
by Hans Leidekker (＠hans) 14 Jul '22

14 Jul '22

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53344 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/462

2 1

Re: [PATCH 0/1] MR363: user32/tests: Test for window exposure behaviours.
by Jinoh Kang (＠iamahuman) 14 Jul '22

14 Jul '22

Jinoh Kang (@iamahuman) commented about dlls/user32/tests/msg.c: > DeleteObject( hrgn2 ); > } > > +static void visualize_region_differences( HWND hwnd, HWND hother, HRGN hrgn_expect, HRGN hrgn_actual ) I included this function since it was really helpful for me while testing debugging the implementation--it eliminates the need to interpret the rectangle list manually. I can remove this if it clutters the test, though. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/363#note_4317

1 0

Re: [PATCH 0/2] MR314: ntdll/tests: Preserve EFLAGS across syscall on x64. - closed
by Jinoh Kang (＠iamahuman) 14 Jul '22

14 Jul '22

This merge request was closed by Jinoh Kang. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/314

1 0

Re: [PATCH 0/2] MR314: ntdll/tests: Preserve EFLAGS across syscall on x64.
by Jinoh Kang (＠iamahuman) 14 Jul '22

14 Jul '22

Superseded by MR !282 (probably). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/314#note_4315

1 0

Re: [PATCH v2 0/4] MR47: kernel32/tests: Add tests for CancelSynchronousIo.
by Jinoh Kang (＠iamahuman) 14 Jul '22

14 Jul '22

Jinoh Kang (@iamahuman) commented about server/async.c: > return woken; > } > > +static int cancel_blocking( struct process *process, struct thread *thread, client_ptr_t iosb ) > +{ > + struct async *async; > + int woken = 0; > + > +restart: > + LIST_FOR_EACH_ENTRY( async, &process->asyncs, struct async, process_entry ) > + { > + if (async->terminated || async->canceled) continue; There is a pre-existing bug where cancellation requests are ignored for alerted asyncs (i.e. async instances that are not actually completed and can go back from the "terminated" state to the pending state). Perhaps it's worth addressing that bug first? https://www.winehq.org/pipermail/wine-devel/2022-March/210053.html -- https://gitlab.winehq.org/wine/wine/-/merge_requests/47#note_3836

2 1