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

Re: [v6 PATCH 00/21] x86: Enable User-Mode Instruction Prevention

by Stas Sergeev

31.03.2017 17:11, Alexandre Julliard пишет: > In fact it would be nice to be able to make sidt/sgdt/etc. segfault > too. I know a new syscall is a pain, Maybe arch_prctl() then?

March 31, 2017

Re: [PATCH] wmvcore: Add WMReader stubs.

by Andrew Eikum

On Thu, Mar 30, 2017 at 05:30:09PM -0500, Jefferson Carpenter wrote: > Do FIXME statements need to print out the passed-in arguments? Only > outputting the method name, you can still tell what needs to be > implemented. Also, to output each argument, there are typedefs to > consider, and you have to choose whether to dereference any pointer > arguments, which makes it rather difficult to automate the process. > It's nice if you can put in the effort, but for just stubbing interfaces I think you can get away with FIXME("%p\n", This). Actual function implementations should trace the arguments correctly. Andrew

March 31, 2017

Re: [v6 PATCH 00/21] x86: Enable User-Mode Instruction Prevention

by Alexandre Julliard

Ricardo Neri <ricardo.neri-calderon(a)linux.intel.com> writes: > On Thu, 2017-03-30 at 13:10 +0300, Stas Sergeev wrote: >> 30.03.2017 08:14, Ricardo Neri пишет: >> >>>> But at least dosemu implements it, so probably it is needed. >> >>> Right. >> >>> >> >>>> Of course if it is used by one of 100 DOS progs, then there >> >>>> is an option to just add its support to dosemu2 and pretend >> >>>> the compatibility problems did not exist. :) >> >>> Do you mean relaying the GP fault to dosemu instead of trapping it and >> >>> emulating it in the kernel? >> >> Yes, that would be optimal if this does not severely break >> >> the current setups. If we can find out that smsw is not in >> >> the real use, we can probably do exactly that. >> >> But other >> >> instructions are not in real use in v86 for sure, so I >> >> wouldn't be adding the explicit test-cases to the kernel >> >> that will make you depend on some particular behaviour >> >> that no one may need. >> >> My objection was that we shouldn't >> >> write tests before we know exactly how we want this to work. >> > OK, if only SMSW is used then I'll keep the emulation for SMSW only. >> In fact, smsw has an interesting property, which is that >> no one will ever want to disable its in-kernel emulation >> to provide its own. >> So while I'll try to estimate its usage, emulating it in kernel >> will not be that problematic in either case. > > Ah good to know! > >> As for protected mode, if wine only needs sgdt/sidt, then >> again, no one will want to disable its emulation. Not the >> case with sldt, but AFAICS wine doesn't need sldt, and so >> we can leave sldt without a fixups. Is my understanding >> correct? > > This is my understanding as well. I could not find any use of sldt in > wine. Alexandre, would you mind confirming? Some versions of the Themida software protection are known to use sldt as part of the virtual machine detection code [1]. The check currently fails because it expects the LDT to be zero, so the app is already broken, but sldt segfaulting would still cause a crash where there wasn't one before. However, I'm only aware of one application using this, and being able to catch and emulate sldt ourselves would actually give us a chance to fix this app in newer Wine versions, so I'm not opposed to having it segfault. In fact it would be nice to be able to make sidt/sgdt/etc. segfault too. I know a new syscall is a pain, but as far as Wine is concerned, being able to opt out from any emulation would be potentially useful. [1] https://www.winehq.org/pipermail/wine-bugs/2008-February/094470.html -- Alexandre Julliard julliard(a)winehq.org

March 31, 2017

Re: [PATCH 2/2] regedit: Handle comment lines with leading spaces and tabs

by Hugh McMaster

On Friday, 31 March 2017 11:07 PM, Alexandre Julliard wrote: > You can also ignore white space before comments, but still return it for > other lines, if there is a case where the callers would want that. > Otherwise getting rid of whitespace right away is fine. A few test cases > with leading whitespace may be a good idea. Somewhat strangely, we actually call isspaceW() in processSetValue() before we get the registry value name. But it is already too late. We need to check for whitespace in processRegEntry() or earlier; otherwise, the import fails. So I think handling whitespace in get_lineA/W() will be far more efficient and easier to maintain. But to be certain, I'll add some tests with whitespace variations first. Thanks for the advice. -- Hugh McMaster

March 31, 2017

Re: [PATCH 2/2] regedit: Handle comment lines with leading spaces and tabs

by Alexandre Julliard

Hugh McMaster <hugh.mcmaster(a)outlook.com> writes: > I guess it depends on how you think we should handle such whitespace. parse_file_header() checks for whitespace > before processing the file header. We don't bother checking in processRegEntry() or processRegEntry31(). > > On Windows, all lines in registry files can have leading spaces and tabs. The exception is Windows 3.1, which > doesn't allow leading whitespace at all. > > In Wine, we ignore any registry line with leading whitespace. So importing a file like the following will fail. > (In case it's not clear, there are two spaces before the opening square bracket.) > > REGEDIT4 > [HKEY_CURRENT_USER\Software\Wine\regedit_test] > "Value"="Data" > > One option is to move the check for ';' and '#' to processRegEntry(), but that would require A->W line conversion first. > We would then add an appropriate while loop in that function, handling all line cases. > > Otherwise, we could add the while loops into get_lineA/W() and remove the whitespace check from parse_file_header(), > as it would be redundant. You can also ignore white space before comments, but still return it for other lines, if there is a case where the callers would want that. Otherwise getting rid of whitespace right away is fine. A few test cases with leading whitespace may be a good idea. -- Alexandre Julliard julliard(a)winehq.org

March 31, 2017

Re: [PATCH 04/13] msvcrt: Add CurrentScheduler class stub

by Piotr Caban

Hi, On 03/31/17 11:42, Gijs Vermeulen wrote: > But shouldn't this also change: > > ?_Get(a)_CurrentScheduler@details(a)Concurrency@@SA?AV_Scheduler(a)23@XZ > ?_GetNumberOfVirtualProcessors(a)_CurrentScheduler@details(a)Concurrency@@SAIXZ > ?_Id(a)_CurrentScheduler@details(a)Concurrency@@SAIXZ > -arch=win32 > ?_ScheduleTask(a)_CurrentScheduler@details(a)Concurrency@@SAXP6AXPAX(a)Z0@Z > -arch=win64 > ?_ScheduleTask(a)_CurrentScheduler@details(a)Concurrency@@SAXP6AXPEAX(a)Z0@Z > > Or do these not do the same? It's a different class (_CurrentScheduler vs CurrentScheduler). Thanks, Piotr

March 31, 2017

Re: [PATCH 04/13] msvcrt: Add CurrentScheduler class stub

by Gijs Vermeulen

Hi, I'm sorry if this is a stupid question. But shouldn't this also change: ?_Get(a)_CurrentScheduler@details(a)Concurrency@@SA?AV_Scheduler(a)23@XZ ?_GetNumberOfVirtualProcessors(a)_CurrentScheduler@details(a)Concurrency@@SAIXZ ?_Id(a)_CurrentScheduler@details(a)Concurrency@@SAIXZ -arch=win32 ?_ScheduleTask(a)_CurrentScheduler@details(a)Concurrency@ @SAXP6AXPAX(a)Z0@Z -arch=win64 ?_ScheduleTask(a)_CurrentScheduler@details(a)Concurrency@ @SAXP6AXPEAX(a)Z0@Z Or do these not do the same? Sorry if I'm wasting your time. Sincerely, Gijs 2017-03-30 22:47 GMT+02:00 Piotr Caban <piotr(a)codeweavers.com>: > Signed-off-by: Piotr Caban <piotr(a)codeweavers.com> > --- > dlls/concrt140/concrt140.spec | 40 ++++++++--------- > dlls/msvcr100/msvcr100.spec | 28 ++++++------ > dlls/msvcr110/msvcr110.spec | 40 ++++++++--------- > dlls/msvcr120/msvcr120.spec | 40 ++++++++--------- > dlls/msvcr120_app/msvcr120_app.spec | 40 ++++++++--------- > dlls/msvcrt/scheduler.c | 88 ++++++++++++++++++++++++++++++ > +++++++ > 6 files changed, 182 insertions(+), 94 deletions(-) > > > > > >

March 31, 2017

Re: [PATCH 2/2] regedit: Handle comment lines with leading spaces and tabs

by Hugh McMaster

On Friday, 31 March 2017 4:43 AM, Alexandre Julliard wrote: > Hugh McMaster writes: >> --- a/programs/regedit/regproc.c >> +++ b/programs/regedit/regproc.c >> @@ -749,6 +749,7 @@ static char *get_lineA(FILE *fp) >> next = line; >> continue; >> } >> + while (*line == ' ' || *line == '\t') line++; >> if (*line == ';' || *line == '#') >> { >> line = next; > > This is going to affect non-comment lines too, is that what you want? > If so, the commit message is a bit misleading. I guess it depends on how you think we should handle such whitespace. parse_file_header() checks for whitespace before processing the file header. We don't bother checking in processRegEntry() or processRegEntry31(). On Windows, all lines in registry files can have leading spaces and tabs. The exception is Windows 3.1, which doesn't allow leading whitespace at all. In Wine, we ignore any registry line with leading whitespace. So importing a file like the following will fail. (In case it's not clear, there are two spaces before the opening square bracket.) REGEDIT4 [HKEY_CURRENT_USER\Software\Wine\regedit_test] "Value"="Data" One option is to move the check for ';' and '#' to processRegEntry(), but that would require A->W line conversion first. We would then add an appropriate while loop in that function, handling all line cases. Otherwise, we could add the while loops into get_lineA/W() and remove the whitespace check from parse_file_header(), as it would be redundant. I'm open to other ideas as well. Thanks for raising this. -- Hugh McMaster

March 31, 2017

Re: [v6 PATCH 00/21] x86: Enable User-Mode Instruction Prevention

by Ricardo Neri

On Thu, 2017-03-30 at 13:10 +0300, Stas Sergeev wrote: > 30.03.2017 08:14, Ricardo Neri пишет: > >>>> But at least dosemu implements it, so probably it is needed. > >>> Right. > >>> > >>>> Of course if it is used by one of 100 DOS progs, then there > >>>> is an option to just add its support to dosemu2 and pretend > >>>> the compatibility problems did not exist. :) > >>> Do you mean relaying the GP fault to dosemu instead of trapping it and > >>> emulating it in the kernel? > >> Yes, that would be optimal if this does not severely break > >> the current setups. If we can find out that smsw is not in > >> the real use, we can probably do exactly that. > >> But other > >> instructions are not in real use in v86 for sure, so I > >> wouldn't be adding the explicit test-cases to the kernel > >> that will make you depend on some particular behaviour > >> that no one may need. > >> My objection was that we shouldn't > >> write tests before we know exactly how we want this to work. > > OK, if only SMSW is used then I'll keep the emulation for SMSW only. > In fact, smsw has an interesting property, which is that > no one will ever want to disable its in-kernel emulation > to provide its own. > So while I'll try to estimate its usage, emulating it in kernel > will not be that problematic in either case. Ah good to know! > As for protected mode, if wine only needs sgdt/sidt, then > again, no one will want to disable its emulation. Not the > case with sldt, but AFAICS wine doesn't need sldt, and so > we can leave sldt without a fixups. Is my understanding > correct? This is my understanding as well. I could not find any use of sldt in wine. Alexandre, would you mind confirming? > In this case, I suppose, we are very well on a way to avoid > the extra syscalls to toggle the emulation features. Great! Then I will keep the emulation for sgdt, sidt, and smsw but not for str and sldt; for both vm86 and protected mode. This seems to be the agreement. Thanks and BR, Ricardo

March 31, 2017

Re: [PATCH] wmvcore: Add WMReader stubs.

by Jefferson Carpenter

Do FIXME statements need to print out the passed-in arguments? Only outputting the method name, you can still tell what needs to be implemented. Also, to output each argument, there are typedefs to consider, and you have to choose whether to dereference any pointer arguments, which makes it rather difficult to automate the process. I could submit just the IDL changes, without the stubs added to wmvcore_main.c. Sorry Alistair, they seem to include your additions as well. On Thu, Mar 30, 2017 at 8:20 AM, Andrew Eikum <aeikum(a)codeweavers.com> wrote: > On Thu, Mar 30, 2017 at 08:25:03AM +0000, Alistair Leslie-Hughes wrote: > > I have a basic patch for adding tests in the works, but I was waiting > > for my IDL changes to be accepted first. > > > > So, If someone wants to review the IDL changes, Since Sebastian has been > > busy with other stuff. > > > > https://source.winehq.org/patches/data/131403 > > > > Okay :) I just noticed one thing: > > > + HRESULT GetAttributeCountEx( > > + [in] WORD stream_num, > > + [out] WORD attributes ); > > If attributes is an out-param, it should be a pointer, right? > > Andrew >

March 30, 2017