Origin Overlay does not work in Proton (specifically, with d3d11 and dxvk's dxgi) due to IDXGISwapchain methods being not in dxgi.dll

[gofman]

The Origin's detour patcher expects the methods to be hotpatched from IDXGISwapchain1 interface to live in dxgi.dll. It checks that the methods lie within the loaded dxgi.dll image range and refuses to hotpatch them otherwise.

That's currently the same for Proton's dxgi as well due to the same reasons.
The attached PoC hack fixes the issue (attaching just for illustration of what it wants, I would not consider this a fix).

0001-d3d11-Create-dxgi-swapchain-from-dxgi.dll.txt

[doitsujin]

Well that's a huge pain in the arse. It doesn't look like this is possible to fix without losing compatibility to Wine's DXGI implementation, is it?

[gofman]

As a probably saner workaround, adding a proxy interface to dxgi itself without touching d3d11 helps too (that's what I did for Proton's dxgi, see attached). Please note it might happen (especially with a very short function which just jumps to the underlying proxy method) that it still won't be hotpatchable by Origin due to some opcode it won't understand or the function being just too short. In that case attribute((ms_hook_prologue)) function attribute (DECLSPEC_HOTPATCH in Wine) helps. This is less likely for PE .dll (the Wine's dxgi is currently dll.so), if to go this way we can check that it is actually needed before adding that.

0001-dxgi-Use-proxy-IDXGISwapChain-interface-when-importi.txt

[misyltoad]

I was planning on just writing a simple swapchain wrapper/dispatcher in our DXGI. I imagine that works too?

I'm not super familiar with DECLSPEC_HOTPATCH from your patch there. What does that do to help the situation?
Is that something that's only needed in Unix and not PE dlls?

[gofman]

Yes, that works, the second patch I linked (for Wine's dxgi) is doing exactly that.

DECLSPEC_HOTPATCH unwraps to "attribute(ms_hook_prologue)". That means adding a standard opcode sequence at the start of procedure. That is effectively a few bytes noop for x64 and 'push %ebp; mov %esp,%ebp" on x86 which is a standard stack frame setup. Some hotpatchers did not parse the function opcodes at all and expected exactly this sequence at the beginning of the hotpatched function. That is not the case with Origin, but it still breaks on certain opcodes at the start of the function and will probably break on a function consisting, e. g., from a single jump or so. E. g., it tries to hotpatch dxvk's d3d9 TestCooperativeLevel and fails, can be fixed by adding the mentioned attribute (I didn't bring that to attention as I am not aware if anything is broken due to that). The things which can break hotpatching are more often encountered in Unix dlls but can happen in PE as well (as in the TestCooperativeLevel example).

To make things funner, see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91489. That affects x86 only. The fix got to gcc trunk in November. So on x86 (x64 is not affected) the functions get miscompiled with this attribute if take a fast exit path bypassing prologue / epilgue (in the Wine case, usually happens with the stubs containing FIXME only, that is, conditional call which gets fast exit path for the case when call is not made).

So, I suppose it would be ideal to add the attribute to avoid obscure hotpatching issues (in case of Origin, the failure to properly hotpatch the function sometimes manifests itself by a crash due to the jump into the middle of opcode in the hotpatched function), but maybe for x64 only for now until gcc fixed gets populated to the distros.

[misyltoad]

but maybe for x64 only for now until gcc fixed gets populated to the distros.

I mean, min spec for DXVK is GCC 9.3 anyway so... 🐸

I'll do that dispatcher now I guess.

[misyltoad]

Does #1997 solve the issue?

It'd be nice to be smarter than this and have an actual common DXGI windowing abstraction between dxvk and vkd3d-proton, but we can solve that later potentially.

[gofman]

For some reason it doesn't so far. I will check what is missing now and get back (my modified Proton's dxgi still works, so probably some ms_hook_prologues are needed).

[misyltoad]

I think I have an idea why, I made a typo

[gofman]

I think the first thing is QueryInterface interface returns the wrong interface, yeah.

Then, you are not redefining the AddRef / Release methods in DxgiSwapChainDispatcher, AddRef / Release... should not the refcounting go to the dispatcher?

[misyltoad]

It was returning the address of the dispatch in QueryInterface, I've updated the PR.

[misyltoad]

Then, you are not redefining the AddRef / Release methods in DxgiSwapChainDispatcher, AddRef / Release... should not the refcounting go to the dispatcher?

It shouldn't really matter if we refcount the dispatcher or the swapchain, should it?
AFAIK we don't have anything that references the DXGI swapchain internally.

This way just deals with destroying the object easier.

[misyltoad]

Philip prefers the passthrough of refcounting so I'll just do that.

[gofman]

Seems to work now (without adding any ms_hook_prologue).

[misyltoad]

Cool! I'll finish the few nits Philip added then we can merge it :)

[misyltoad]

Updated the PR, can you make sure that doesn't break it somehow (it really shouldn't) and then we can merge it in.

Thanks!

[gofman]

Still works here.