On 01/21/16 16:04, Sebastian Lackner wrote:
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?
I looked deeper in your patchset and the one occur that I thought was there was my mistake.
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.
Yes, I tried that. See the attached patch (note that SYSCALL macro has an opposite meaning comparing to your patches) on top of my patchset. I couldn't reproduce the problem with Steam. It still works for me 100% of time.
I tried to compare it with your version, but, as I wrote you, it didn't work for me at all. I assume I'm missing something.
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.
Sure, in that case we won't need the flexibility of JIT, so static thunks should be fine. I still think we could avoid text relocations, but that's a different topic.
Thanks, Jacek