[Bug 48641] New: skse64 is broken with the ntdll-ForceBottomUpAlloc patchset

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=48641

Bug ID: 48641
           Summary: skse64 is broken with the ntdll-ForceBottomUpAlloc
                    patchset
           Product: Wine-staging
           Version: 5.0-rc3
          Hardware: x86
                OS: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: -unknown
          Assignee: [e-mail]
          Reporter: [e-mail]
                CC: [e-mail], [e-mail]
      Distribution: ---

While SKSE64 now works properly under wine, it hangs (then fails to launch)
when the ntdll-ForceBottomUpAlloc patchset is enabled. Building wine-staging
without that specific patchset works fine.

This is similar to how it was in earlier wine versions, before a better version
of the F4SE patches (https://github.com/hdmap/wine-hackery/tree/master/f4se)
were mainlined.

[Bug 48641] skse64 is broken with the ntdll-ForceBottomUpAlloc patchset

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=48641

--- Comment #1 from Alistair Leslie-Hughes  ---
There was an update to this patchset in the wine-staging v5.0 release.

Please try with the newer version.

[Bug 48641] skse64 is broken with the ntdll-ForceBottomUpAlloc patchset

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=48641

--- Comment #2 from qsniyg  ---
Yes, I tried with wine-staging 5.0 and 5.1, which had the issue. Recompiling
5.2 and removing that patchset worked fine though.

[Bug 48641] skse64 is broken with the ntdll-ForceBottomUpAlloc patchset

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=48641

Zebediah Figura  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

[Bug 48641] skse64 is broken with the ntdll-ForceBottomUpAlloc patchset

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=48641

qsniyg  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|5.0-rc3                     |5.2

[Bug 48641] skse64 is broken with the ntdll-ForceBottomUpAlloc patchset

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=48641

--- Comment #3 from Paul Gofman  ---
The original bug is here: https://bugs.winehq.org/show_bug.cgi?id=44893

More info needed to approach this:

1. Description of referenced bug is different. If the problem in Staging is now
similar to the earlier Wine versions then presumably it should report
""couldn't allocate trampoline...". Is this the case? Or is the behaviour
reported here in this bug is actually different from the earlier problem?

2. Could you please bisect which patch fixed
https://bugs.winehq.org/show_bug.cgi?id=44893 in Wine for you if possible?

3. Could you please attach WINEDEBUG=+virtual,+loader,+loaddll,+module,+process
logs (from start, without any cutting; they should be not insanely big and are
compressed very well)? The two versions: with all Staging patchset there and
the problem, and for successful start with the blamed patch removed.

[Bug 48641] skse64 is broken with the ntdll-ForceBottomUpAlloc patchset

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=48641

--- Comment #4 from qsniyg  ---
1:

It does report "couldn't allocate trampoline..." (as you'll see in the bad log,
and it isn't present in the good log)

2:

I can't recompile wine at the moment, but I would guess (from looking at
https://github.com/hdmap/wine-hackery/blob/master/f4se) that it would be these
two patches:

https://github.com/wine-mirror/wine/commit/1aa963efd7c7c7f91423f5edb9811f6ff...

https://github.com/wine-mirror/wine/commit/12be24af8cab0e5f78795b164ec8847ba...

3:

Here's the bad log with vanilla wine-staging 5.2 (i.e. the ForceBottomUpAlloc
patchset is enabled):
https://drive.google.com/file/d/1nAveaa3lv5RPkiLK2QTy042i6IpXo3fc/view

Here's the good log with wine-staging git
(https://github.com/wine-staging/wine-staging/commit/469cbe7ed827636c48007bad...),
and the ForceBottomUpAlloc patchset disabled:
https://drive.google.com/file/d/1791GZXkG8flJYzbtXDL2gg5oP25F9D-1/view

[Bug 48641] skse64 is broken with the ntdll-ForceBottomUpAlloc patchset

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=48641

--- Comment #5 from Paul Gofman  ---
(In reply to qsniyg from comment #4)
> 2:
>
> I can't recompile wine at the moment, but I would guess (from looking at
> https://github.com/hdmap/wine-hackery/blob/master/f4se) that it would be
> these two patches:
>
> https://github.com/wine-mirror/wine/commit/
> 1aa963efd7c7c7f91423f5edb9811f6ff95c06c0
>
> https://github.com/wine-mirror/wine/commit/
> 12be24af8cab0e5f78795b164ec8847bafc30852

If I did not miss something important, the first patch in "wine-hackery" is
different from that one committed, and the committed one is not doing that
tweak which could potentially make this broken hotpatcher happy. That is, the
hackery patch was making the executable image allocations top down, the
committed patch is not doing that. It adds support for top down allocation for
NtMapViewOfSection(), but that doesn't affect how Wine loads executable.

Looking at comment https://bugs.winehq.org/show_bug.cgi?id=44893#c5 and
https://bugs.winehq.org/show_bug.cgi?id=44893#c9 I suspect that the problem was
not really fixed in Wine but something along the ongoing changes has tweaked
the memory layout a bit. What I see in the logs is that with BottomUpAlloc
patchset it seems to take the lower free memory for some previous allocations
(as it is supposed to), which is later not available for the hooker's
trampolines. So I am not sure yet if anything to be fixed related to the
BottomUpAlloc patchset itself, or rather the original issue(s) were not really
fixed in Wine and it started to work by chance, with some different memory
layout / reserving handling in Wine hindered by different VirtualAlloc
behaviour allocating the memory top down.

So maybe it could give some idea what we can do if you could bisect which
exactly change in mainstream Wine tweaked this to work.

[Bug 48641] skse64 is broken with the ntdll-ForceBottomUpAlloc patchset

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=48641

--- Comment #6 from Paul Gofman  ---
Created attachment 66510
  --> https://bugs.winehq.org/attachment.cgi?id=66510
Test hack 1

Can you please try with this patch on top of Staging (with all patches there)
if it changes anything? And attach the same logs as before with this patch.
This is just a test tweak in attempt to get some ideas what is going on there.

I also suspect the version of SKSE has been changed between
https://bugs.winehq.org/show_bug.cgi?id=44893 was analysed and when you spotted
SKSE to work which could fix on of the issues not fixed in Wine.

[Bug 48641] skse64 is broken with the ntdll-ForceBottomUpAlloc patchset

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=48641

Freso  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

[Bug 48641] skse64 is broken with the ntdll-ForceBottomUpAlloc patchset

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=48641

--- Comment #7 from qsniyg  ---
Sorry for the late response (never got around to compiling staging again...).
The bug now happens with wine vanilla as well, I haven't yet bisected the
cause.

After applying a rebased version of your test hack, it still doesn't work.
However, after doing further research, it's not the cause of the issue.

Before the patch (with a err trace added to display the output):

0398:trace:virtual:NtQueryVirtualMemory (0xffffffffffffffff, 0x13fffffff,
info_class=0, 0x21f760, 48, 0x21f728)
0398:err:virtual:get_basic_memory_info .State=4096, .RegionSize=0x1000
...
06dc:trace:virtual:NtQueryVirtualMemory (0xffffffffffffffff, 0x13fff0fff,
info_class=0, 0x21f760, 48, 0x21f728)
06dc:err:virtual:get_basic_memory_info .State=65536, .RegionSize=0x10000
, .BaseAddress=0x13fff0000
06dc:trace:virtual:NtAllocateVirtualMemory 0xffffffffffffffff 0x13fff0000
00010000 3000 00000040

After the patch:

...
0398:trace:virtual:NtQueryVirtualMemory (0xffffffffffffffff, 0x13ffb0fff,
info_class=0, 0x21f760, 48, 0x21f728)
0398:err:virtual:get_basic_memory_info .State=65536, .RegionSize=0x10000
, .BaseAddress=0x13ffb0000
0398:trace:virtual:NtAllocateVirtualMemory 0xffffffffffffffff 0x13ffb0000
00010000 3000 00000040

This works fine, and corresponds to the following in
src/skse64/skse64/skse64.cpp
(https://github.com/JimBeamBeagle/SimplyKnock/blob/e0e64412201c9eac1881377e35...
):

if(!g_branchTrampoline.Create(1024 * 64))
        {
            _ERROR("couldn't create branch trampoline. this is fatal. skipping
remainder of init process.");
            return;
        }

However, what fails is the line right after in the log:

0398:trace:virtual:NtQueryVirtualMemory (0xffffffffffffffff, 0x2236ffff,
info_class=0, 0x21f760, 48, 0x21f728)
0398:err:virtual:get_basic_memory_info .State=65536, .RegionSize=0x1000
, .BaseAddress=0x2236f000

Source code (the difference here is g_moduleHandle, which is the handle of
skse_1_5_97.dll, which loads at 0x22370000):

if(!g_localTrampoline.Create(1024 * 64, g_moduleHandle))
        {
            _ERROR("couldn't create codegen buffer. this is fatal. skipping
remainder of init process.");
            return;
        }

The reason this fails is because of the following in BranchTrampoline::Create
(src/skse64/skse64_common/BranchTrampoline.cpp, link:
https://github.com/JimBeamBeagle/SimplyKnock/blob/e0e64412201c9eac1881377e35...
):

uintptr_t lowestOKAddress = moduleBase - 0x80000000 + (1024 * 1024 * 128); 
  // largest 32-bit displacement with 128MB scratch space
    ...
        if (addr < lowestOKAddress)
        {
            _ERROR("couldn't allocate trampoline, no free space before image");
            break;
        }

lowestOKAddress = 0x22370000 - 0x80000000 + (1024 * 1024 * 128), which is a
negative value (and therefore overflows).

Applying an updated version of the SKSE hack (sending MEM_TOP_DOWN to
NtMapViewOfSection in open_dll_file) allows it to work fine (loads it at
7FFFFED40000), but this is probably not a proper solution.

[Bug 48641] skse64 is broken with the ntdll-ForceBottomUpAlloc patchset

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=48641

--- Comment #8 from qsniyg  ---
I think this bug is mostly just a duplicate of
https://bugs.winehq.org/show_bug.cgi?id=44893 by this point, since the behavior
can be replicated both in vanilla wine and wine-staging (5.19).

By the way, forgot to mention in the last comment, thank you very much for
looking into this!

[Bug 48641] skse64 is broken with the ntdll-ForceBottomUpAlloc patchset

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=48641

Anya  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

[Bug 48641] skse64 is broken with the ntdll-ForceBottomUpAlloc patchset

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=48641

soredake  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|broaden_acid002@simplelogin |
                   |.com                        |

[Bug 48641] skse64 is broken with the ntdll-ForceBottomUpAlloc patchset

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=48641

Neko-san  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]