February 2017 - Wine-Devel - list.winehq.org

Re: [PATCH 2/2] wineinstall - Add support for parallel builds using 'nproc' to detect cpu count

by Austin English

On Tue, Feb 28, 2017 at 2:16 AM, Austin English <austinenglish(a)gmail.com> wrote: > On Mon, Feb 27, 2017 at 8:20 PM, Rosanne DiMesio <dimesio(a)earthlink.net> wrote: >> On Mon, 27 Feb 2017 18:53:21 -0500 >> Chris Morgan <chmorgan(a)gmail.com> wrote: >> >>> >>> wineinstall still serves a pretty useful purpose of making it very easy for >>> people to get started building wine. >> >> Not in my experience. It does not warn users about missing dependencies, and they show up on the forum wondering why the Wine they built doesn't work. I advise users against using it, and would like nothing better than for it to go away. >> >> >> -- >> Rosanne DiMesio >> dimesio(a)earthlink.net > > https://source.winehq.org/patches/data/130850 > > -- > -Austin > GPG: 14FB D7EA A041 937B It's dead Jim: https://source.winehq.org/git/wine.git/commitdiff/a33f4646cd8f3ac3b12087126… -- -Austin GPG: 14FB D7EA A041 937B

Feb. 28, 2017

Re: [PATCH] oledb32/tests: Fix a use-after-free in test_converttobyrefwstr.

by Alistair Leslie-Hughes

Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com>

Feb. 28, 2017

Re: [v2] Use CP 1200 (Unicode) for Win32/Win64 version info.

by Alexandre Julliard

Akihiro Sagawa <sagawa.aki(a)gmail.com> writes: > [v2] Fix test failures in mshtml.dll caused by npmshtml.dll. > According to Gecko Plugin API Reference[1], plugin's version stamp must > be set to English US and Windows Multilingual, i.e. "040904E4". > [1] https://developer.mozilla.org/en-US/docs/Plugins/Guide/Plug-in_Development_… > > Signed-off-by: Akihiro Sagawa <sagawa.aki(a)gmail.com> > --- > dlls/npmshtml/rsrc.rc | 1 + > dlls/version/tests/info.c | 2 -- > include/wine/wine_common_ver.rc | 24 ++++++++++++++++++++++-- > 3 files changed, 23 insertions(+), 4 deletions(-) It's still failing here: ../../../tools/runtest -q -P wine -T ../../.. -M mshtml.dll -p mshtml_test.exe.so htmldoc && touch htmldoc.ok htmldoc.c:6287: Test failed: open failed: 80040152 wine: Unhandled page fault on read access to 0xdeadbeef at address 0x7ed051dd (thread 0154), starting debugger... Unhandled exception: page fault on read access to 0xdeadbeef in 32-bit code (0x7ed051dd). Register dump: CS:0023 SS:002b DS:002b ES:002b FS:0063 GS:006b EIP:7ed051dd ESP:0033fc20 EBP:0033fc78 EFLAGS:00010202( R- -- I - - - ) EAX:deadbeef EBX:7ed6a000 ECX:00000001 EDX:7ed80f00 ESI:0000188d EDI:7ed4388a Stack dump: 0x0033fc20: 00000001 7ed449d1 80040152 7ed050ad 0x0033fc30: 005e0000 00689650 005e8de8 00000000 0x0033fc40: 7ed39473 7ed7f1a0 80040152 7ed4388a 0x0033fc50: 005b4d80 7ed4388a 005e8de8 deadbeef 0x0033fc60: 7ed4521a 00000000 7ed04f0e 7ed6a000 0x0033fc70: 005b4d80 7ed4388a 0033fcd8 7ed0ddd9 Backtrace: =>0 0x7ed051dd test_open_window+0x2dd(doc=<is not available>, do_block=0) [/home/julliard/wine/wine/dlls/mshtml/tests/../../../include/mshtml.h:51848] in mshtml_test (0x0033fc78) 1 0x7ed0ddd9 test_HTMLDocument_http+0x678(with_wbapp=<is not available>) [/home/julliard/wine/wine/dlls/mshtml/tests/htmldoc.c:7877] in mshtml_test (0x0033fcd8) 2 0x7ed128d9 func_htmldoc+0xda8() [/home/julliard/wine/wine/dlls/mshtml/tests/htmldoc.c:8742] in mshtml_test (0x0033fd58) 3 0x7ecc305d main+0x25c(argc=<is not available>, argv=<is not available>) [/home/julliard/wine/wine/dlls/mshtml/tests/../../../include/wine/test.h:589] in mshtml_test (0x0033fe08) 4 0x7ed38d8b __wine_spec_exe_entry+0x4a(peb=<couldn't compute location>) [/home/julliard/wine/wine/dlls/winecrt0/exe_entry.c:36] in mshtml_test (0x0033fe48) 5 0x7b45e56c call_process_entry+0xb() in kernel32 (0x0033fe68) 6 0x7b45f4a9 start_process+0x68(entry=<couldn't compute location>) [/home/julliard/wine/wine/dlls/kernel32/process.c:1116] in kernel32 (0x0033fea8) 7 0x7bc8071c call_thread_func_wrapper+0xb() in ntdll (0x0033fec8) 8 0x7bc834d9 call_thread_func+0xa8(entry=0x7b45f440, arg=0x7ed38d40, frame=0x33ffc8) [/home/julliard/wine/wine/dlls/ntdll/signal_i386.c:2759] in ntdll (0x0033ffa8) 9 0x7bc806fa call_thread_entry_point+0x11() in ntdll (0x0033ffc8) 10 0x7bc53f73 start_process+0x12(arg=0xffcb6418) [/home/julliard/wine/wine/dlls/ntdll/loader.c:3054] in ntdll (0x0033ffe8) 11 0xf756d8dd __x86.get_pc_thunk.di+0x2a() in libwine.so.1 (0x00000000) 12 0xf756da40 wine_switch_to_stack+0x1f(func=0x7bc53f60, arg=0xffcb6418, stack=0x340000) [/home/julliard/wine/wine/libs/wine/port.c:77] in libwine.so.1 (0xffcb63e8) 13 0x7bc59b26 LdrInitializeThunk+0x235(kernel_start=<couldn't compute location>, unknown2=<couldn't compute location>, unknown3=<couldn't compute location>, unknown4=<couldn't compute location>) [/home/julliard/wine/wine/dlls/ntdll/loader.c:3115] in ntdll (0xffcb6438) 14 0x7b4653af __wine_kernel_init+0xa4e() [/home/julliard/wine/wine/dlls/kernel32/process.c:1310] in kernel32 (0xffcb7328) 15 0x7bc5a983 __wine_process_init+0x152() [/home/julliard/wine/wine/dlls/ntdll/loader.c:3324] in ntdll (0xffcb7398) 16 0xf756bb43 wine_init+0x2a2(argc=0x3, argv=0xffcb78e4, error="", error_size=0x400) [/home/julliard/wine/wine/libs/wine/loader.c:956] in libwine.so.1 (0xffcb73e8) 17 0x7c000b58 main+0x97(argc=<is not available>, argv=<is not available>) [/home/julliard/wine/wine/loader/main.c:254] in <wine-loader> (0xffcb7838) 18 0xf7375276 __libc_start_main+0xf5() in libc.so.6 (0x00000000) 0x7ed051dd test_open_window+0x2dd [/home/julliard/wine/wine/dlls/mshtml/tests/../../../include/mshtml.h:51848] in mshtml_test: movl 0x0(%eax),%edx 51848 return This->lpVtbl->close(This); -- Alexandre Julliard julliard(a)winehq.org

Feb. 28, 2017

Re: [PATCH 4/5] server: Added server side mamed pipe read and write implementation and use it for message mode pipes.

by Sebastian Lackner

On 28.02.2017 18:10, Jacek Caban wrote: > On 28.02.2017 16:44, Sebastian Lackner wrote: >> Well, there are also situations where it would be relatively easy to avoid >> the roundtrip. We could return immediately at least when no events, APCs or >> completions are involved, right? Since this is the most common situation it >> might still be worth the effort. > > If event is not passed, we still need to signal object itself. Thinking > about it some more, I guess we could just signal object it before > returning for non-overlapped operations returning immediately. It > wouldn't be exactly right, but it can't be used for synchronization > reliably in this case anyway. > > Another thing is that we currently don't know if there is user APC until > APC_ASYNC_IO returns, but that can be fixed. > > Since it's just an optimization, can we process with the patch itself > first? I'm happy to optimize that, but I find it hard to agree that an > optimization should block inclusion of patches targeted at correctness. I'm not saying that it should block anything, I will leave the design decision up to Alexandre. Nevertheless, I think it certainly makes sense to discuss about such aspects in advance. Depending on the solution used it might also require refactoring of this patch, which is easier to do before it was accepted. >> For flush we are currently not using any callback, so marking it in the handler >> would be too late. Thats also something we could fix of course - Nevertheless, >> as you can see it already gets pretty complicated. ;) If its possible to >> simplify this somehow, that would certainly be an improvement. > I think you misunderstood me somehow. I can only guess what you mean, > since current flush() implementation queue async or return no wait > handle - they never call async_terminate() from what I can tell. Even if > they did, it would work with what I proposed: Sorry if the previous message wasn't clear. Yes, I was thinking about the hypothetical situation that we have to call async_terminate from a flush handler. > flush handler creates async - it's marked as returning immediately > flush calls async_terminate() - due to above, async_terminate is aware > that it's an immediate result > flush op returns to request handler, which finds that there was an > immediate return, so does not mark async as returning asynchronously. Yes, something like that would be necessary. > > I'm not sure what you expect me to improve in this regards. This is > about hypothetical solution to a problem that's not strictly related to > this patch. And I don't think it's that complicated ;) Well, my main suggestion is that it would be better to handle async returns different than sync returns. Especially, we probably shouldn't use APC_ASYNC_IO as a way to delay async_set_result for synchronous returns. Also, we should use the reply buffer for read calls, or replace the wine_server_set_reply with a length argument if this is not feasible. Best regards, Sebastian

Feb. 28, 2017

Re: [PATCH 1/2] server: Added server side named pipe read and write implementation and use it for message mode pipes.

by Sebastian Lackner

On 28.02.2017 14:14, Jacek Caban wrote: > + LIST_FOR_EACH_ENTRY( message, &pipe_end->message_queue, struct pipe_message, entry ) > + avail += message->iosb->in_size - message->read_pos; > + reply_size = min( reply_size, avail ); > + > + if (avail) > + { > + message = LIST_ENTRY( list_head(&pipe_end->message_queue), struct pipe_message, entry ); > + reply_size = min( reply_size, message->iosb->in_size ); > + } Isn't there a problem with this size check? Lets assume we have two messages with 10 byte, for the first one the read_pos = 5. Then avail == 15, and reply_size is clamped to 10 - but it will attempt to copy 10 bytes starting position 5? > + > + if (!(buffer = set_reply_data_size(FIELD_OFFSET( FILE_PIPE_PEEK_BUFFER, Data[reply_size] )))) return; > + buffer->NamedPipeState = 0; /* FIXME */ > + buffer->ReadDataAvailable = avail; > + buffer->NumberOfMessages = 0; /* FIXME */ > + buffer->MessageLength = 0; /* FIXME */ > + if (avail) memcpy( buffer->Data, (const char *)message->iosb->in_data + message->read_pos, reply_size ); > +} > +

Feb. 28, 2017

Re: [PATCH 4/5] server: Added server side mamed pipe read and write implementation and use it for message mode pipes.

by Alexandre Julliard

Jacek Caban <jacek(a)codeweavers.com> writes: > Back to the current patch, it just uses currently available mechanisms > to achieve the task and it seems to work well. As I described earlier, I > could implement that without APCs for immediate result, but we would > still need some notification to server that data has been completely > transferred. If we want to do it that way, the cleanest solution IMO > would be a new server call done by clients in server_*_file() functions > called for immediate results after setting iosb fields and receiving > data. Would you prefer that? That would certainly be better than write_process_memory, yes. But we can go with APCs for now. -- Alexandre Julliard julliard(a)winehq.org

Feb. 28, 2017

Re: [PATCH 4/5] server: Added server side mamed pipe read and write implementation and use it for message mode pipes.

by Jacek Caban

On 28.02.2017 16:44, Sebastian Lackner wrote: > On 28.02.2017 15:14, Jacek Caban wrote: >> On 28.02.2017 14:40, Sebastian Lackner wrote: >>> On 28.02.2017 14:03, Jacek Caban wrote: >>>> As long as we transfer result over server socket, we don't have control >>>> in server over when iosb and result buffer are filled, so we need a >>>> signal from client when we can signal async completion (signal >>>> event/object, queue completion). I sent a patch that was more inline >>>> with how we do that for calls where I/O is implemented on client side >>>> [1], but Alexandre preferred that to be in server [2]. Having that in >>>> server is not only cleaner, but also gives us more control, like in >>>> which case to fill iosb or signal event. It will also make it possible >>>> to implement other solution (bellow). >>> I assume Alexandre was mainly referring to the event / APC stuff. I assume >>> he also would be fine with a code path to return the result immediately >>> when available. This could for example be done by marking the async object >>> as "in progress" to avoid queing unnecessary APCs. If a real result is >>> available at the end of the call, it could be transferred right away. This >>> would also help to fix the problem with "immediate returns" below. >> What about the problem of client having to signal server when it >> received and processed the data? When would you set the event in that >> case? We can transfer data immediately, but we still need to know when >> we can set event / queue APC. Currently we know that we can do that when >> APC_ASYNC_IO returns. > Well, there are also situations where it would be relatively easy to avoid > the roundtrip. We could return immediately at least when no events, APCs or > completions are involved, right? Since this is the most common situation it > might still be worth the effort. If event is not passed, we still need to signal object itself. Thinking about it some more, I guess we could just signal object it before returning for non-overlapped operations returning immediately. It wouldn't be exactly right, but it can't be used for synchronization reliably in this case anyway. Another thing is that we currently don't know if there is user APC until APC_ASYNC_IO returns, but that can be fixed. Since it's just an optimization, can we process with the patch itself first? I'm happy to optimize that, but I find it hard to agree that an optimization should block inclusion of patches targeted at correctness. >>>>> Also please note that we need to distinguish "immediate returns" and >>>>> "async returns" to properly implement FileIoCompletionNotificationInformation. >>>>> This especially means that waiting internally is not an option. >>>> It's easy to distinguish and still have internal waits. I can't point >>>> the right solution without looking deeper at >>>> FileIoCompletionNotificationInformation, but just as an example, you >>>> could do: >>>> >>>> if (iosb->status != STATUS_PENDING) mark_async_as_immediate_result(); >>>> >>>> inside request handlers just after calling fd ops. Then you'd have all >>>> the info you need inside async object. Am I missing something? >>> With the currently proposed solution this would not work. The fd ops >>> already call async_terminate -> async_set_result -> add_async_completion -> >>> add_completion, but depending on the completion flags this should not be >>> done if it is an immediate result. There is no way to know if its supposed >>> to be an "immediate return" inside of the async_*() helpers, and checking >>> afterwards is too late (unless we introduce a flag to block async objects >>> from doing actions right away). Did you have a different idea how it should >>> be implemented? >> In cases covered by my patch, async_terminate doesn't call >> async_set_result, but queues APC instead, so that's not a problem. >> >> More generally speaking, if we had a code path like you described, the >> solution I proposed would still work, just reverse the logic. Mark all >> created asyncs as immediate returns, and change it to async where >> appropriate, something like: >> >> if (iosb->status == STATUS_PENDING) mark_async_as_async_result(); >> >> in read/write/ioctl/flush should do. register_async request could do that unconditionally. > For flush we are currently not using any callback, so marking it in the handler > would be too late. Thats also something we could fix of course - Nevertheless, > as you can see it already gets pretty complicated. ;) If its possible to > simplify this somehow, that would certainly be an improvement. I think you misunderstood me somehow. I can only guess what you mean, since current flush() implementation queue async or return no wait handle - they never call async_terminate() from what I can tell. Even if they did, it would work with what I proposed: flush handler creates async - it's marked as returning immediately flush calls async_terminate() - due to above, async_terminate is aware that it's an immediate result flush op returns to request handler, which finds that there was an immediate return, so does not mark async as returning asynchronously. I'm not sure what you expect me to improve in this regards. This is about hypothetical solution to a problem that's not strictly related to this patch. And I don't think it's that complicated ;) Cheers, Jacek

Feb. 28, 2017

Re: [PATCH 3/6] wined3d: Handle raw thread group shared memory declarations.

by Józef Kucia

On Tue, Feb 28, 2017 at 5:11 PM, Henri Verbeet <hverbeet(a)gmail.com> wrote: > On 28 February 2017 at 14:42, Józef Kucia <jkucia(a)codeweavers.com> wrote: >> @@ -1047,6 +1047,31 @@ static HRESULT shader_get_registers_used(struct wined3d_shader *shader, const st >> { >> reg_maps->temporary_count = ins.declaration.count; >> } >> + else if (ins.handler_idx == WINED3DSIH_DCL_TGSM_RAW) >> + { >> + unsigned int reg_idx = ins.declaration.tgsm_raw.reg.reg.idx[0].offset; >> + if (reg_idx >= MAX_TGSM_REGISTERS) >> + { >> + ERR("Invalid TGSM register index %u.\n", reg_idx); >> + break; >> + } >> + if (shader_version.type == WINED3D_SHADER_TYPE_COMPUTE) >> + { >> + struct wined3d_shader_tgsm *tgsm; >> + >> + if (!wined3d_array_reserve((void **)&reg_maps->tgsm, &reg_maps->tgsm_capacity, >> + reg_idx + 1, sizeof(*reg_maps->tgsm))) > I'm not sure about this. I guess the expectation is that these will > get allocated consecutively and starting from 0, while even starting > at MAX_TGSM_REGISTERS-1 and going down wouldn't be terrible. Still, > I'm wondering if it wouldn't be worth it to only allocate space for > the number of declarations, and use e.g. bsearch(). Yes, the shader compile seems to always allocate TGSM registers consecutively. Additionally, the typical shader uses up to a few TGSM registers (1-5). Anyway, I can change it to use bsearch(), if this is preferred.

Feb. 28, 2017

Re: [PATCH 3/6] wined3d: Handle raw thread group shared memory declarations.

by Henri Verbeet

On 28 February 2017 at 14:42, Józef Kucia <jkucia(a)codeweavers.com> wrote: > @@ -1047,6 +1047,31 @@ static HRESULT shader_get_registers_used(struct wined3d_shader *shader, const st > { > reg_maps->temporary_count = ins.declaration.count; > } > + else if (ins.handler_idx == WINED3DSIH_DCL_TGSM_RAW) > + { > + unsigned int reg_idx = ins.declaration.tgsm_raw.reg.reg.idx[0].offset; > + if (reg_idx >= MAX_TGSM_REGISTERS) > + { > + ERR("Invalid TGSM register index %u.\n", reg_idx); > + break; > + } > + if (shader_version.type == WINED3D_SHADER_TYPE_COMPUTE) > + { > + struct wined3d_shader_tgsm *tgsm; > + > + if (!wined3d_array_reserve((void **)&reg_maps->tgsm, &reg_maps->tgsm_capacity, > + reg_idx + 1, sizeof(*reg_maps->tgsm))) I'm not sure about this. I guess the expectation is that these will get allocated consecutively and starting from 0, while even starting at MAX_TGSM_REGISTERS-1 and going down wouldn't be terrible. Still, I'm wondering if it wouldn't be worth it to only allocate space for the number of declarations, and use e.g. bsearch().

Feb. 28, 2017

Re: [PATCH 4/5] server: Added server side mamed pipe read and write implementation and use it for message mode pipes.

by Sebastian Lackner

On 28.02.2017 15:14, Jacek Caban wrote: > On 28.02.2017 14:40, Sebastian Lackner wrote: >> On 28.02.2017 14:03, Jacek Caban wrote: >>> As long as we transfer result over server socket, we don't have control >>> in server over when iosb and result buffer are filled, so we need a >>> signal from client when we can signal async completion (signal >>> event/object, queue completion). I sent a patch that was more inline >>> with how we do that for calls where I/O is implemented on client side >>> [1], but Alexandre preferred that to be in server [2]. Having that in >>> server is not only cleaner, but also gives us more control, like in >>> which case to fill iosb or signal event. It will also make it possible >>> to implement other solution (bellow). >> I assume Alexandre was mainly referring to the event / APC stuff. I assume >> he also would be fine with a code path to return the result immediately >> when available. This could for example be done by marking the async object >> as "in progress" to avoid queing unnecessary APCs. If a real result is >> available at the end of the call, it could be transferred right away. This >> would also help to fix the problem with "immediate returns" below. > What about the problem of client having to signal server when it > received and processed the data? When would you set the event in that > case? We can transfer data immediately, but we still need to know when > we can set event / queue APC. Currently we know that we can do that when > APC_ASYNC_IO returns. Well, there are also situations where it would be relatively easy to avoid the roundtrip. We could return immediately at least when no events, APCs or completions are involved, right? Since this is the most common situation it might still be worth the effort. > >>>> Also please note that we need to distinguish "immediate returns" and >>>> "async returns" to properly implement FileIoCompletionNotificationInformation. >>>> This especially means that waiting internally is not an option. >>> It's easy to distinguish and still have internal waits. I can't point >>> the right solution without looking deeper at >>> FileIoCompletionNotificationInformation, but just as an example, you >>> could do: >>> >>> if (iosb->status != STATUS_PENDING) mark_async_as_immediate_result(); >>> >>> inside request handlers just after calling fd ops. Then you'd have all >>> the info you need inside async object. Am I missing something? >> With the currently proposed solution this would not work. The fd ops >> already call async_terminate -> async_set_result -> add_async_completion -> >> add_completion, but depending on the completion flags this should not be >> done if it is an immediate result. There is no way to know if its supposed >> to be an "immediate return" inside of the async_*() helpers, and checking >> afterwards is too late (unless we introduce a flag to block async objects >> from doing actions right away). Did you have a different idea how it should >> be implemented? > In cases covered by my patch, async_terminate doesn't call > async_set_result, but queues APC instead, so that's not a problem. > > More generally speaking, if we had a code path like you described, the > solution I proposed would still work, just reverse the logic. Mark all > created asyncs as immediate returns, and change it to async where > appropriate, something like: > > if (iosb->status == STATUS_PENDING) mark_async_as_async_result(); > > in read/write/ioctl/flush should do. register_async request could do that unconditionally. For flush we are currently not using any callback, so marking it in the handler would be too late. Thats also something we could fix of course - Nevertheless, as you can see it already gets pretty complicated. ;) If its possible to simplify this somehow, that would certainly be an improvement. Best regards, Sebastian

Feb. 28, 2017