test_index_buffer_offset() occasionally fails (on some machines and/or Mesa versions) due to a bug in radeonsi. Although the bug has since been fixed upstream, I ended up writing these patches for it, and since they reduce GL calls I'm inclined to believe they're a good idea regardless.
The relevant Mesa bug is <https://gitlab.freedesktop.org/mesa/mesa/-/issues/6138>.
--
v2: wined3d: Do not pause transform feedback after every draw call.
wined3d: Pause transform feedback in wined3d_context_gl_draw_textured_quad().
wined3d: Pause transform feedback in wined3d_context_gl_draw_shaded_quad().
https://gitlab.winehq.org/wine/wine/-/merge_requests/450
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
> 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
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
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