On 21.01.2016 15:43, Jacek Caban wrote:
On 01/19/16 17:06, Alexandre Julliard wrote:
Jacek Caban jacek@codeweavers.com writes:
All such solutions require changes in both Chrome and Wine, which makes it tricky to decide on a solution. What's your opinion?
My concern is not just about hooking the system calls, it's what happens once they are hooked. Sebastian said that hooks inside the Ldr* functions don't work right. I'm worried that supporting this properly would require that our internal code paths follow the Windows ones closely, which is neither feasible nor desirable.
This could of course also be changed in Chrome so that it could deal with the way Wine's ntdll is implemented, but that would probably be more intrusive.
It's hard to discuss what Sebastian reports without understanding the problem. For what I know, his variant of patches exposes way more hookable calls than it should. It's not a secret that on Windows Nt* functions will not call any other exported functions. This is an implementation detail that I don't consider too internal to follow. The fact that those hooks may be called recursively with Sebastian's patches makes me think that it's the most likely reason of the problems.
Could you please point out where you think my patchset doesn't match Windows behavior / leads to recursive calls which should not occur? Or, have you already tried improving your patchset, to implement forwarding of Ldr* and Rtl* functions through the thunks, like it is supposed to work on Windows? I would guess you encounter exactly the same problem.
For other internal behaviour, we already have cases where they do matter. My recent example is Office 2013 online installer (which has sandbox-like code that among others merges some special dirs, sometimes containing DLLs, into file system). It's obvious that we need NtOpenFile-alike in loader to open a file in loader and all changes that it required was to pass SYNCHRONIZE flag. For all I know native may use different APIs there, but all is fine using this call as long as we pass an argument that is sane and makes sense anyway.
That said, I fully agree that we don't want to follow Windows too closely, but I don't think we're at this point yet. Also, my variant of the patch (others could be reworked to follow that as well) exposes even less internal behaviour than existing code, because we need to explicitly mark parts that would go through exported thunks.
Here is my proposal for fixing the situation:
Short term we use one of variants of patches for syscall thunks. This will give an immediate fix for at lest some (most common?) Wine configs.
If the issues with Ldr* functions can be solved I'm fine with that. However, I wouldn't necessarily recommend the JIT approach if we have only one supported config. Lets assume we would use winebuild statically generated thunks (like my patch based on Erichs work), the additional logic for JIT could be implemented at any time later.
Long term (as in we do it now, but we will benefit way later due to latency of Chrome release and the time needed for embedders to pick it), we do changes in Chrome. My thinking is that we could make runtime matching of all known thunks in Chrome instead of allowing only one variant in given config (this would fix Wine with all Windows versions set as well as wow64). For 64-bit we could make the check less strict (allowing jump in place of mov eax,52h) or invent other, Wine-specific, solution.
Thanks, Jacek