April 2016 - Wine-Devel - list.winehq.org

Re: [PATCH 2/2] ucrtbase: Implement _register_onexit_function()

by Nikolay Sivov

On 24.04.2016 21:28, Sebastian Lackner wrote: > On 24.04.2016 20:05, Nikolay Sivov wrote: >> + table->_first = MSVCRT_calloc(sizeof(void *), 32); > > Not really an issue, but shouldn't the first argument be the counter, and the second one the size? > It should, I copied from existing function, worth fixing, thanks.

April 24, 2016

Re: WineHQ upgrade breaking dinput8:dinput?

by Francois Gouget

I have rechecked this to make sure the test does not have a subtle bug that causes it to depend on the stack layout. In the process I have reduced the test to the minimum set necessary to reproduce the bug (see attachment). But in the end I did not find anything wrong so it does look like a compiler bug. I don't know how to proceed from there. * I think it would be better if someone more knowlegeable than me reported this bug to the MinGW guys. Maybe someone who could undertand what happens in the assembly code? * On the Wine side it would really be nice to get this test (and the three others that are impacted) to stop failing. But removing all the static directives really does not feel right. Would it be better to arrange for these tests to be compiled with -O0? Any other cleaner workaround. So I'm really hoping someone can step up and move this forward. Here are the relevant bugs: * dinput8:dinput regression caused by new compiler https://bugs.winehq.org/show_bug.cgi?id=40384 * usp10:usp10 regression caused by new cross-compiler https://bugs.winehq.org/show_bug.cgi?id=40385 * wininet:url regression caused by new cross-compiler https://bugs.winehq.org/show_bug.cgi?id=40386 * Unexplained new random comctl32:header failure (cross-compiler issue?) https://bugs.winehq.org/show_bug.cgi?id=40442 -- Francois Gouget <fgouget(a)free.fr> http://fgouget.free.fr/ 1 + e ^ ( i * pi ) = 0

April 24, 2016

Re: [PATCH 2/2] ucrtbase: Implement _register_onexit_function()

by Sebastian Lackner

On 24.04.2016 20:05, Nikolay Sivov wrote: > + table->_first = MSVCRT_calloc(sizeof(void *), 32); Not really an issue, but shouldn't the first argument be the counter, and the second one the size?

April 24, 2016

Re: [PATCH 1/2] ucrtbase: Implement _initialize_onexit_table()

by Sebastian Lackner

On 24.04.2016 20:05, Nikolay Sivov wrote: > diff --git a/dlls/ucrtbase/ucrtbase.spec b/dlls/ucrtbase/ucrtbase.spec > index a54a71a..96e0c3b 100644 > --- a/dlls/ucrtbase/ucrtbase.spec > +++ b/dlls/ucrtbase/ucrtbase.spec > @@ -419,7 +419,7 @@ > @ cdecl _i64tow(int64 ptr long) ntdll._i64tow > @ cdecl _i64tow_s(int64 ptr long long) MSVCRT__i64tow_s > @ cdecl _initialize_narrow_environment() > -@ stub _initialize_onexit_table > +@ cdecl _initialize_onexit_table() MSVCRT__initialize_onexit_table Shouldn't there be one "ptr" argument? Same for the other spec files. > @ cdecl _initialize_wide_environment() > @ cdecl _initterm(ptr ptr) > @ cdecl _initterm_e(ptr ptr) >

April 24, 2016

Re: [PATCH 1/4] d3drm: Use sizeof instead of strlen in d3drm_texture3_GetClassName.

by Aaryaman Vasishta

Agreed, a seperate patch could be written for that. Cheers, Aaryaman On Sun, Apr 24, 2016 at 9:44 PM, Nikolay Sivov <bunglehead(a)gmail.com> wrote: > On 24.04.2016 18:50, Aaryaman Vasishta wrote: > > This size of class name turns out to be equal to sizeof("Texture") which > is strlen("Texture") + 1. This was recently discovered while testing the > size returned by GetClassName. > > > > Signed-off-by: Aaryaman Vasishta <jem456.vasishta(a)gmail.com> > > --- > > dlls/d3drm/texture.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/dlls/d3drm/texture.c b/dlls/d3drm/texture.c > > index f81fc03..60374d3 100644 > > --- a/dlls/d3drm/texture.c > > +++ b/dlls/d3drm/texture.c > > @@ -796,7 +796,7 @@ static HRESULT WINAPI > d3drm_texture3_GetClassName(IDirect3DRMTexture3 *iface, DW > > { > > TRACE("iface %p, size %p, name %p.\n", iface, size, name); > > > > - if (!size || *size < strlen("Texture") || !name) > > + if (!size || *size < sizeof("Texture") || !name) > > return E_INVALIDARG; > > > > strcpy(name, "Texture"); > > > > GetClassName() is duplicated for every object, it makes sense to > introduce a helper function eventually. > > >

April 24, 2016

Re: [PATCH 1/4] d3drm: Use sizeof instead of strlen in d3drm_texture3_GetClassName.

by Nikolay Sivov

On 24.04.2016 18:50, Aaryaman Vasishta wrote: > This size of class name turns out to be equal to sizeof("Texture") which is strlen("Texture") + 1. This was recently discovered while testing the size returned by GetClassName. > > Signed-off-by: Aaryaman Vasishta <jem456.vasishta(a)gmail.com> > --- > dlls/d3drm/texture.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/dlls/d3drm/texture.c b/dlls/d3drm/texture.c > index f81fc03..60374d3 100644 > --- a/dlls/d3drm/texture.c > +++ b/dlls/d3drm/texture.c > @@ -796,7 +796,7 @@ static HRESULT WINAPI d3drm_texture3_GetClassName(IDirect3DRMTexture3 *iface, DW > { > TRACE("iface %p, size %p, name %p.\n", iface, size, name); > > - if (!size || *size < strlen("Texture") || !name) > + if (!size || *size < sizeof("Texture") || !name) > return E_INVALIDARG; > > strcpy(name, "Texture"); > GetClassName() is duplicated for every object, it makes sense to introduce a helper function eventually.

April 24, 2016

Re: [PATCH] kernel32/tests: Add initial tests for GetConsoleFontInfo (v2)

by Sebastian Lackner

On 22.04.2016 15:21, Hugh McMaster wrote: > Changes since v1: Use CONSOLE_FONT_INFO instead of undocumented CONSOLE_FONT. > The structure is identical. > > Signed-off-by: Hugh McMaster <hugh.mcmaster(a)outlook.com> > --- > dlls/kernel32/tests/console.c | 57 +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 57 insertions(+) > You'll have to add a stub first, otherwise it breaks the tests: $ (cd dlls/kernel32/tests/; make console.ok) ../../../tools/runtest -q -P wine -T ../../.. -M kernel32.dll -p kernel32_test.exe.so console && touch console.ok fixme:console:GetConsoleProcessList ((nil),0): stub fixme:console:GetConsoleProcessList ((nil),1): stub fixme:console:GetConsoleProcessList (0x118ca0,0): stub fixme:console:GetConsoleProcessList (0x118ca0,1): stub fixme:console:GetConsoleProcessList (0x118ca0,0): stub fixme:console:SetConsoleFont (0x63, 0): stub! fixme:console:SetConsoleFont (0x63, 0): stub! wine: Call from 0x7b43b8a2 to unimplemented function KERNEL32.dll.GetConsoleFontInfo, aborting wine: Unimplemented function KERNEL32.dll.GetConsoleFontInfo called at address 0x7b43b8a2 (thread 0009), starting debugger...

April 24, 2016

Re: [PATCH 2/3] d3drm/tests: Replace test_Texture.

by Stefan Dösinger

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256 Am 2016-04-24 um 11:38 schrieb Aaryaman Vasishta: > By exactly matching size, did you mean sizeof("Texture") itself? > Isn't that already there? I might have missed something here. What you have is size = <huge>; GetClassName(size, buffer); ok(HR == OK); ok(size == sizeof("Texture")); what I mean is size = sizeof("Texture") - 1; // or strlen("Texture"); GetClassname(size, buffer); ok(hr == ERROR); ok(size == ???); what I mean is size = sizeof("Texture"); GetClassname(size, buffer); ok(hr == OK); ok(size == sizeof("Texture")); -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJXHMX7AAoJEN0/YqbEcdMwY0EP/2+s7ipkmnTXBJIonvAD5aWn 5tvySdprGsvhcCJWwG1oOxdBJP+50D7eixS952E7JBLKYK6gby6g7bYOf2N3vEJ3 mqArdePbCBSo+aQg5jgT9zSLhiBzPgphYJuh5b/8dj9dfSQaqKQj2+wC8EY1xQd5 1t5vx2flOqiQ1fcQS1ywVQm8jhfSHqSJTL3xN0m4CmxCpF+FfFcEf30vAlVI3W0o Z1Hdl8LLgth8Zg+VOSVUxaxbEUgM/QS7ganT3+ICYVUNZ65UiSE3tXqYgYRzmdgo 27OCx0u7apodUV0rRApwhrEJqujl2/OYCCVD6Gj2ejA/9AoaJCGBwlGsCaarCEWG OPpiVUoIJfLk2u8AblgyXr12ux1abgRp31SBPH4ToAR9iy3FSvr8iKBTRifpNzrw tXeyFoDdtBsR9Br+D+WRJ7WXllmU6Y3LgDcbi56T/N0oumWlWZO+pWvaVIP9Cf0u SvtHgWy0j6K0lK4PEr5ok/yJAwNV7WMYDcwQgG5Vr7/aTj6fmj3fZFffZ9IrPqHW ANaU7Wm5z+4KLAeE7cbYv8miLcrMXWlBFc8MiZxjzdaI4VSoPsEOfR0R2Tb77rSh tC9wUg/bTlhw8rZWPI3MNXtFOl1JZO7xSTWloqiYw4GN8tft24QMTbxNZZuTCCKX gayjF4/rKP2SQQBjf+ob =+f75 -----END PGP SIGNATURE-----

April 24, 2016

Re: Fix a heap corruption when printing specific REG_MULTI_SZ values.

by Sebastian Lackner

On 24.04.2016 14:53, Hugh McMaster wrote: > On Saturday, 23 April 2016 10:24 PM, Sebastian Lackner wrote: > >> On 23.04.2016 11:56, Hugh McMaster wrote: >>> On Saturday, 23 April 2016 3:35 AM, Sebastian Lackner wrote: >>> >>>> The current calculation does not take the terminating \0 into account. >>> >>>> -buffer = HeapAlloc(GetProcessHeap(), 0, tmp_size * 2); >>>> +buffer = HeapAlloc(GetProcessHeap(), 0, tmp_size * 2 + sizeof(WCHAR)); >>> >>> Which particular cases cause the heap corruption? >> >> It happens for example with a string like L"\0\0\0\0". After stripping off >> the last two "\0" the allocated memory will have a size of 4 characters, >> but the code below writes 5 characters. You can also test with warn+heap >> after creating such a registry key. > > Such a string is invalid on Windows. You can't add it using regedit (the > string is trimmed to a single \0 after closing the dialog editor). It also causes > an error with 'reg add'. > > That said, users could potentially modify Wine's user.reg, causing a heap > corruption, so this patch is fine. I'll send a Sign-Off. > I've reproduced it by using regedit, so its not just a theoretical issue.

April 24, 2016

Re: Fix a heap corruption when printing specific REG_MULTI_SZ values.

by Hugh McMaster

On Saturday, 23 April 2016 10:24 PM, Sebastian Lackner wrote: >On 23.04.2016 11:56, Hugh McMaster wrote: >> On Saturday, 23 April 2016 3:35 AM, Sebastian Lackner wrote: >> >>> The current calculation does not take the terminating \0 into account. >> >>> -buffer = HeapAlloc(GetProcessHeap(), 0, tmp_size * 2); >>> +buffer = HeapAlloc(GetProcessHeap(), 0, tmp_size * 2 + sizeof(WCHAR)); >> >> Which particular cases cause the heap corruption? > >It happens for example with a string like L"\0\0\0\0". After stripping off >the last two "\0" the allocated memory will have a size of 4 characters, >but the code below writes 5 characters. You can also test with warn+heap >after creating such a registry key. Such a string is invalid on Windows. You can't add it using regedit (the string is trimmed to a single \0 after closing the dialog editor). It also causes an error with 'reg add'. That said, users could potentially modify Wine's user.reg, causing a heap corruption, so this patch is fine. I'll send a Sign-Off.

April 24, 2016