June 2014 - Wine-Devel - list.winehq.org

Re: oleaut32: Fix TLB dependencies lookup
by Nikolay Sivov June 4, 2014

June 4, 2014

> -IMPORTS = oleaut32 ole32 rpcrt4 user32 gdi32 advapi32 > +IMPORTS = oleaut32 ole32 rpcrt4 user32 gdi32 advapi32 shlwapi > + StrCatW(pathW, (WCHAR[3]){'\\','4',0}); You don't need this. Just use lstrcatW() and please define constant string in a cleaner way.

1 0

Re: oleaut32: Fix TLB dependencies lookup
by Guillaume Charifi June 4, 2014

June 4, 2014

Hello, Thanks for your remarks, I am sending a corrected patch. Regards

1 0

Re: oleaut32: Fix TLB dependencies lookup
by Andrew Eikum June 4, 2014

June 4, 2014

Looking mostly good, but some problems below. On Tue, Jun 03, 2014 at 08:50:46PM +0200, guillaume.charifi(a)sfr.fr wrote: > + hr = LoadTypeLibEx(testTLBNameW, REGKIND_NONE, &ptLib); > + if(hr == S_OK) { > + ITypeLib_Release(ptLib); > + win_skip("test_tlb.tlb is on the path or registered, skipping typelib dependency tests\n"); > + return; > + } > + This fails when I run the tests in my source tree: [aeikum(a)aeikum tests]$ pwd /home/aeikum/wine/dlls/oleaut32/tests typelib.c:5434: Test failed: test_tlb.tlb is on the path or registered, skipping typelib dependency tests Makefile:345: recipe for target 'typelib.ok' failed make: *** [typelib.ok] Error 1 > + /* Search in resource table */ > + if(1) { Why is this if-statement here? > + static const WCHAR TYPELIBW[] = {'T','Y','P','E','L','I','B',0}; > + struct search_res_tlb_param *param = heap_alloc(sizeof(*param)); Why aren't you allocating this on the stack? > + > + param->guid = (GUID *)TLB_get_guid_null(ref_type->pImpTLInfo->guid); > + > + EnumResourceNamesW(NULL, TYPELIBW, search_res_tlb, (LONG_PTR)param); > + > + pTLib = param->pTLib; > + result = param->result; If search_res_tlb() is never run, I think (result == 0 == S_OK). You should initialize it to some failure condition before calling EnumResourceNames. Andrew

1 0

Re: oleaut32: Fix TLB dependencies lookup
by Marvin June 3, 2014

June 3, 2014

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=7404 Your paranoid android. === w2000pro (32 bit typelib) === typelib.c:5451: Test failed: got: 80029c4a

1 0

Re: [3/3] msvcrt: Implement 4k block flushing behavior.
by Piotr Caban June 3, 2014

June 3, 2014

On 05/31/14 01:00, Grazvydas Ignotas wrote: > + } else if(wrcnt >= MSVCRT_WRITE_BLOCK_SIZE) { > + if(msvcrt_set_write_direction(file) != 0) > + break; > + > + if(file->_bufsiz && file->_ptr != file->_base) > + if(msvcrt_flush_buffer(file) == MSVCRT_EOF) > + break; > + > + if(MSVCRT__write(file->_file, ptr, MSVCRT_WRITE_BLOCK_SIZE) <= 0) { > + file->_flag |= MSVCRT__IOERR; > + break; > + } > + written += MSVCRT_WRITE_BLOCK_SIZE; > + wrcnt -= MSVCRT_WRITE_BLOCK_SIZE; > + ptr = (const char*)ptr + MSVCRT_WRITE_BLOCK_SIZE; This doesn't look right for me. It doesn't make sense to have some special size blocks written in different way. I think this is caused by 2 differences in native and wine's implementation of fwrite: - if you check file->_bufsiz value on windows it's set to 4096 (on wine it's 512), this is probably causing the flash to be done when 4096 bytes are written - probably there's some kind of optimization that writes integral number of buffers directly to file - I was not checking it but maybe native flushes the output when buffer is full, wine will exit fwrite with full buffer in this case I've also seen that _bufsiz is set to 0 on wine if buffer was not yet allocated. On windows it's value is 4096 just after opening the file. Cheers, Piotr

2 3

Re: [PATCH 4/6] d3d8/tests: Add a test for D3DUSAGE_WRITEONLY.
by Marvin June 3, 2014

June 3, 2014

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=7398 Your paranoid android. === w8 (32 bit device) === device.c:2397: Test failed: Expected foreground window 000B00CA, got 0002004C. device.c:3039: Test failed: Expected wndproc 0x412780, got 0x412780.

1 0

Re: Recognize cards that expose GLSL 1.30 as DX10 capable even if they don't support EXT_GPU_SHADER4
by Stefan Dösinger June 3, 2014

June 3, 2014

Am 23.05.2014 um 00:23 schrieb Andrei Slăvoiu <andrei.slavoiu(a)gmail.com>: > Mesa drivers do not expose EXT_GPU_SHADER4 as all functionality it offers is > already part of OpenGL 3.0. This causes all cards that are not explicitly > recognized by wine (for example all GCN cards) to be treated as DX9 cards and > presented to Windows apps as Radeon 9500. The problem is that without GL_EXT_gpu_shader4 or GL_ARB_shader_texture_lod we cannot handle the texldd instruction. Right now we cannot use core contexts, so unless the driver exposes one of those extensions in compatibility contexts we are stuck on a shader model 2 feature level :-( . The level detection your patch changes isn't directly related to this, as the shader model version support is controlled differently. But a Radeon 9500 matches a card with shader model 2 support much better than a Radeon R9. Does this driver / gpu combination support GL_ARB_shader_texture_lod?

3 9

[PATCH 0/4] Improve compilation with musl libc
by Felix Janda June 3, 2014

June 3, 2014

Hello, I'm trying to reduce the number of patches required to build wine on a musl libc[1] based system. The first two patches merely fix warnings because of use of deprecated headers. So they are not strictly necessary. The patches should also update configure but why is configure at all in the git repo? To compile wine without patches two other issues remain: Various tools used during the build expect getopt_long_only() to have GNU semantics. They don't expect that musl's getopt_long_only() stops after the first non-option argument. The cleanest fix seems to be to add a configure check testing whether getopt_long_only() behaves correctly instead of just testing for its existence. Then there are problems with compiling dlls/ws2_32/socket.c related to IPX but I haven't investigated their cause yet. [1]: http://musl-libc.org Felix Janda (4): Use <poll.h> instead of <sys/poll.h> Remove unecessary <sys/signal> includes Add missing includes of <poll.h> (POLLIN,...) dnsapi: Replace NETDB_INTERNAL by -1 configure.ac | 2 -- dlls/dinput/joystick_linux.c | 4 ++-- dlls/dinput/joystick_linuxinput.c | 4 ++-- dlls/dnsapi/query.c | 2 +- dlls/iphlpapi/icmp.c | 4 ++-- dlls/kernel32/console.c | 4 ++-- dlls/ntdll/file.c | 5 +---- dlls/ntdll/serial.c | 4 ++-- dlls/ntdll/signal_arm.c | 3 --- dlls/ntdll/signal_arm64.c | 3 --- dlls/ntdll/signal_i386.c | 3 --- dlls/ntdll/signal_powerpc.c | 4 ---- dlls/ntdll/signal_x86_64.c | 3 --- dlls/ntdll/sync.c | 5 +---- dlls/rpcrt4/rpc_transport.c | 4 ++-- dlls/wineoss.drv/midi.c | 5 +---- dlls/winex11.drv/event.c | 5 +---- dlls/wininet/internet.c | 5 +---- dlls/wininet/netconnection.c | 5 +---- dlls/winmm/time.c | 5 +---- dlls/ws2_32/socket.c | 5 +---- include/config.h.in | 6 ------ programs/winedbg/gdbproxy.c | 4 ++-- server/change.c | 3 +++ server/fd.c | 5 +---- server/file.c | 2 +- server/named_pipe.c | 2 +- server/object.h | 4 ++-- server/process.c | 2 +- server/queue.c | 3 +++ server/request.c | 2 +- server/serial.c | 2 +- server/signal.c | 5 +---- server/sock.c | 3 +++ server/thread.c | 2 +- 35 files changed, 42 insertions(+), 87 deletions(-) -- 1.8.5.5

3 8

Re: ntdll: Use lockfree implementation for get_cached_fd.
by Ken Thomases June 2, 2014

June 2, 2014

On May 31, 2014, at 10:28 AM, Sebastian Lackner wrote: > Explanation: The basic idea of this patch is that we have a global > counter 'fd_cache_epoch' which is incremented, whenever the file > descriptor cache is updated. get_cached_fd() first reads the counter, > then the fd entry, and afterwards the counter again - if the counter has > changed we know that a second thread has maybe modified the content, and > have to read it again. The memory barrier is there to ensure that > compilers don't optimize away the two reads of 'fd_cache_epoch'. Please > note that: > > * the order of the write instructions when updating the content is > important - this way we ensure that threads only see either (0, *) = not > set, or a valid entry. > > * I assume that reading/writing up to the pointer size can be done as an > atomic operation (threads either see the old or new value, but no > partial updates). This assumption is also used in a lot other parts of > wine, for example here: > http://source.winehq.org/source/dlls/ntdll/sync.c#L1295 > [ It would probably be more safe to introduce special > interlocked_get/interlocked_set macros/functions for that, but not sure > what other people think about that... ] Thanks for working on this. Interesting stuff. I think there are still some issues. In order to ensure a consistent view of the ordering of memory operations, memory barriers have to be used in both the writer and the reader. The writer issues a write barrier after it has written and the reader issues a read barrier before it reads. The appropriate barriers are implicit in interlocked_xchg_add() for incrementing fd_cache_epoch, but the first read of it in get_cached_fd() has no read barrier before it. I think this is benign, though, because I think this can only result in the code thinking the epoch changed when it hasn't. It may retry unnecessarily. More problematic is the writing of the fd and its attributes. There are barriers implicit in the interlocked_xchg() which clears the fd, but there's no write barrier after the final write to it. There's no read barrier before the first read of them. You mention the order of writing, attributes first then fd, as being important. But there's no barrier in between the two operations, so either the compiler or the CPU could reorder either the writes or the reads, thwarting your design. The compiler can also reorder the reading of the fd_cache_epoch with respect to the reading of the fd or attributes. I think you can get this as the effective sequence: thread1 / get_cached_fd / read fd, get old fd thread2 / add_fd_to_cache / clear fd thread2 / add_fd_to_cache / increment epoch thread2 / add_fd_to_cache / set fd thread1 / get_cached_fd / read epoch thread1 / get_cached_fd / check if epoch has changed, conclude it hasn't thread1 / get_cached_fd / return old fd I think it's sufficient to have a write barrier after writing the fd and then a read barrier just before reading it. I _think_ that this obviates the need for the barrier at the bottom of the do-while loop. That is, you can just move that barrier up a line. I'm not 100% confident of my analysis of that. (Well, frankly, I'm never 100% certain of my analysis of this sort of code.) In the email thread regarding Daniel Horn's patch, I suggested an approach using interlocked_cmpxchg() to check if the fd had changed after reading the attributes. Was there a reason you rejected that approach? You mentioned an approach you experimented with using interlocked_cmpxchg64(). What was that about? Were you treating the fd and attributes combined as a single 64-bit value and then checking them both? Was that safer than my suggestion for some reason? If the only advantage is a slight performance improvement (especially in light of the huge difference that either approach has to the status quo, judging by your chart), then I'd argue for the simpler approach that's more easily analyzed for correctness. If the above issue is addressed, that leaves one remaining problem: get_cached_fd() can encounter a false negative: thread1 / add_fd_to_cache / set fd to -1 thread1 / add_fd_to_cache / increment epoch thread2 / get_cached_fd / read epoch thread2 / get_cached_fd / read attributes thread2 / get_cached_fd / read fd, gets -1 thread2 / get_cached_fd / check if epoch has changed, conclude it hasn't thread1 / add_fd_to_cache / set attributes thread1 / add_fd_to_cache / set fd thread2 / get_cached_fd / return -1 thread2 / server_get_unix_fd / see get_cached_fd as having failed since it returns -1 thread2 / server_get_unix_fd / do get_handle_fd server request thread2 / server_get_unix_fd / call add_fd_to_cache to cache the new fd I think you want to try get_cached_fd() again, inside the critical section, if the first call fails. -Ken

2 2

Re: [2/2] imagehlp: Implement (parts of) BindImageEx (try 8)
by Marvin June 2, 2014

June 2, 2014

Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at https://testbot.winehq.org/JobDetails.pl?Key=7391 Your paranoid android. === wvistau64 (64 bit image) === image.c:426: Test failed: Expected StatusRoutine to be called once with reason BindImportProcedure, but it was called 0 times === w2008s64 (64 bit image) === image.c:426: Test failed: Expected StatusRoutine to be called once with reason BindImportProcedure, but it was called 0 times

1 0