The kernel32.cpp LoadLibrary (hooking) ANSI and WIDECHAR issues.

[Riitaoja]

Lately I have been investigating the recursion issue in Dxwnd kernel32.cpp in more detail.

@GHO: You may remember our lengthly conversation in the HeavyGear topic here:
https://sourceforge.net/p/dxwnd/discussion/general/thread/4fe82c35/?page=2
In it I was talking about a weird crash that started to happen in Direct3D mode of the game starting from Dxwnd "v2_03_95". The conclusion was that there must be something wrong with the game.

Now there was also this old topic here:
https://sourceforge.net/p/dxwnd/discussion/general/thread/5df8452a/?page=0
In it there are discussion of various WindowsXP related issues. I am going to concentrate on just one of them here. That is the ANSI and WIDECHAR issue in "kernel32.cpp".

Here are some quotes regarding the issue from you:

System libraries like USER32.DLL have API names with different ordinals comparing WinXP with Win10.

About the "inject suspended" mode I'm using AOE2 because it is replicating the very same problem of other games and it is a game quick to run and to stop: in Win7/10 setting injected mode is useless but it doesn't hurt, on XP it crashes the game. BTW I'm using it also to check for the "hot patch" problem, where logs tell that there is a sudden death after hooking ChangeDisplaySettingsExA and before ChangeDisplaySettingsW.

The problem is pretty localized: in XP you have problems when you hook both ANSI and WIDECHAR versions of these system calls:
ChangeDisplaySettingsA
ChangeDisplaySettingsW
ChangeDisplaySettingsExA
ChangeDisplaySettingsExW
LoadLibraryA
LoadLibraryW
LoadLibraryExA
LoadLibraryExW
Hooking the ANSI calls only the ANSI games work ok. But this is not an explaination, nor a full solution.

I got the source of the problem: the function wrapper was causing another call to LoadLibrary internally and this generated a deadly recursion.

I think I fixed the problem inserting a simple recursion control that prevent the hooker activities in case of inner call. It is a sound solution apart from multithreaded environment, so maybe I'll have to return on it in case of troubles, but so far it seems to work satisfactorily.

---

So I dug up my ancient WinXP laptop and started to test.
It turned out that v2_03_95 did indeed fix the issue where Age of Empires 2 did not start on WindowsXP if "Hot patch" was enabled.
Turning on the "inject suspended process" did not crash the game with v2_03_94 but did crash it on v2_03_93. In any case this issue was not related to kernel32.cpp changes.

However I then tested AOE2 with the latest Dxwnd version and noticed that it was again not working with "Hot patch" enabled on WinXP.

So it turned out that "v2_04_53" is the last one to work in WinXP with AOE2 and "Hot patch" flag.

Possible reason:
v2.04.54:
fix: reorganized c code for hooking through SysLibsTable array

---

And finally I wonder if it might be best to apply a WinXP only check here:
kernel32.cpp line 557:

/* -------------------------------------------------------------------------------

LoadLibrary (hooking) related APIs

/* ---------------------------------------------------------------------------- */

HMODULE WINAPI LoadLibraryExWrapper(LPVOID lpFileName, BOOL IsWidechar, HANDLE hFile, DWORD dwFlags, char *api)
{
    HMODULE libhandle;
    int idx;
    // recursion control: this is necessary so far only on WinXP while other OS like Win7,8,10 don't get into 
    // recursion problems, but in any case better to leave it here, you never know ....
    static BOOL Recursed = FALSE;

/* Add WinXP check: */
    if(IsWinXP()){
/* ---------------- */

    if(IsWidechar){
        OutDebugDW("%s: file=%ls flags=%x\n", api, lpFileName, dwFlags);
        libhandle=(*pLoadLibraryExW)((LPCWSTR)lpFileName, hFile, dwFlags);
    }
    else{
        OutDebugDW("%s: file=%s flags=%x\n", api, lpFileName, dwFlags);
        libhandle=(*pLoadLibraryExA)((LPCTSTR)lpFileName, hFile, dwFlags);
    }

/* Go here if not WinXP: */
    } else{
        OutDebugDW("%s: file=%s flags=%x\n", api, lpFileName, dwFlags);
        libhandle=(*pLoadLibraryExA)((LPCTSTR)lpFileName, hFile, dwFlags);
    }
/* -------------------- */

    if(Recursed) {
        // v2.03.97.fx2: clear Recursed flag when exiting!
        Recursed = FALSE;
        return libhandle;
    }
    Recursed = TRUE;

So that this "if(IsWidechar)" check would be skipped on systems that are not running WinXP. In my testing I have not seen any ill effects and the positive effect has been that HeavyGear works again in Direct3D mode. Of course this is coming from someone who does not understand what the code is trying to do so I will leave it for your better judgment.

P.S.
I wonder if supporting WinXP is starting to produce more problems like this in the future as Win10 moves further away. Perhaps it would be a good idea to produce an alternate legacy version of Dxwnd for those few that still use XP.

[gho]

Well, though I hate not understanding the reasons why something happens, I must surrend to the evidence: if a a game works better, that is for sure a good thing.
Since testing over XP could be painful I'm going to ask your support to make a better test coverage.
In particular, right now, I'd like to have answers to a few questions (oh, my God, it sounds like an electoral poll!):

1) did you really compile and test the fixes you're proposing in your post? (I suppose 'yes', since you made tests with Heavy Gear, but better be sure ...)
2) If so, on top of what release? No, forget abut it, I saw the new OutDebugDW macro of v2.04.65, that's ok.
3) when you say "HeavyGear works again in Direct3D mode" do you mean on WinXP, Win10 or both?
4) in your tests, did you try to set the "Hook / WIDE vs. ANSI" flag? If you do, what is the result?
5) did you try your fix agains some other game? A WIDECHAR game should be needed (see notes below...)

Looking at your code I should say it's simply wrong: the LoadLibraryExWrapper function is supposed to be called by both the ANSI LoadLibraryA and WIDECHAR LoadLibraryW calls that will receive either an ANSI or WIDECHAR dll pathname, so in the LoadLibraryW case for non-XP OS passing a WIDECHAR path to pLoadLibraryExA is supposed to return an error. Maybe this side effect is what makes "Heavy Gear" works (it doesn't load an offending dll, maybe) but for sure a coding like that will risk damaging the behaviour of any other game using WIDECHAR paths.
A more correct implementation would be, perhaps, doing the same thing but after a WIDECHAR to ANSI path conversion, though I suspect that in this case "Heavy Gear" may get back to the previous situation.
Mumble mumble .... in any case we've got to get to the bottom of this nightmare.

[gho]

One more request: if your patch fixes Heavy Gear, it would be quite interesting to have the game logs in the two cases: unpatched and patched dxwnd.dll.
That would identify which is the "turnpoint" where the game start working in a better way. Since the information is in OutDebugDW traces, you shoud obviously turn on at least "DxWnd hacks" and "Debug", but also "DirectDraw trace" and "Direct3D trace" could help understanding what's going on.

[Riitaoja]

Let's back up a bit here. The main point of my post was to inform you that the changes to fix "Hot patching" on Windows XP no longer work as of Dxwnd "v2_04_54" most likely due to the changes introduced in kernel32.cpp in that release.

You can test this on WinXP by trying to run Age of Empires 2 with the "Hot patch" flag enabled. The game won't start. Or by testing the Settlers 3 demo that requires the "Hot patch" flag to run in a window on WinXP.
https://archive.org/details/S3multidemoeng

From this I draw the perhaps wrong conclusion that the HeavyGear mystery might also get solved. But it probably is not so simple.

---

Now for the HeavyGear part:

...Maybe this side effect is what makes "Heavy Gear" works (it doesn't load an offending dll, maybe) but for sure a coding like that will risk damaging the behaviour of any other game using WIDECHAR paths.

I understood from the old posts and the comments in Kernel32.cpp that only WinXP needs to have the follwing "if - else" statements (to fix the hot patch flag) but that you left it in place just in case for all operating systems:

static BOOL Recursed = FALSE;

if(IsWidechar){
    OutDebugDW("%s: file=%ls flags=%x\n", api, lpFileName, dwFlags);
    libhandle=(*pLoadLibraryExW)((LPCWSTR)lpFileName, hFile, dwFlags);
}
else{
    OutDebugDW("%s: file=%s flags=%x\n", api, lpFileName, dwFlags);
    libhandle=(*pLoadLibraryExA)((LPCTSTR)lpFileName, hFile, dwFlags);
}

if(Recursed) {
    // v2.03.97.fx2: clear Recursed flag when exiting!
    Recursed = FALSE;
    return libhandle;
}
Recursed = TRUE;

Where as before the kernel32.cpp (v2_03_94fx3_src and earlier) only had this in it for the same part of the code:

libhandle=(*pLoadLibraryExA)(lpFileName, hFile, dwFlags);

Hence my conclusion that if it worked before on Win7 - Win10 then it should be OK to have the "if - else" apply only for WinXP. (Please note that the "if(IsWidechar)" statement breaks Direct3D mode in HeavyGear on Windows XP too.)

Comparing the HeavyGear logs I think D3DIM.DLL is not getting loaded when the game works. At least the dxwnd log shows no trace of it. Additional mystery that makes me suspicious is why the game starts in a 320x240 window and by using the old "v2_03_94fx3" release or by deleting the "if(IsWidechar)" statement from the code it starts in 800x600?

320x240 is the resolution of the games intro movie. The actual game menu should be 640x480 but is also downscaled to 320x240.

Of course the game can be made to start in 800x600 window on later Dxwnd version too with the flag "Locked size" but this used to not be necessary. So how could the "LoadLibraryExW" part cause this type of change in the games behaviour?

---

Regarding the "LoadLibraryA" vs. "LoadLibraryW" stuff I also read this:
https://stackoverflow.com/questions/5208415/loadlibrary-taking-a-lpctstr

Maybe there is a better way to implement the LoadLibrary that automatically takes into consideration if it should be "A" or "W"?

In any case it might be that newer Windows versions have built in compatibility that can handle the situation better than WinXP.

[Riitaoja]

Comparing the HeavyGear logs in regards to LoadLibrary entries:

D3D Crashing:

LoadLibraryExW: file=DDRAW.DLL flags=800
LoadLibraryExW: FileName=DDRAW.DLL hFile=0 Flags=800(NULL) hmodule=6d6a0000
LoadLibraryExW: hooking lib="DDRAW.DLL" handle=6d6a0000

LoadLibraryExW: file=DSOUND.DLL flags=800
LoadLibraryExW: FileName=DSOUND.DLL hFile=0 Flags=800(NULL) hmodule=6d330000
LoadLibraryExW: hooking lib="DSOUND.DLL" handle=6d330000

D3D Working:

LoadLibraryExW: file=D flags=800
LoadLibrary: RETRY fullpath="D:\HeavyGear/DDRAW.DLL"
LoadLibraryExW: FileName=DDRAW.DLL hFile=0 Flags=800(NULL) hmodule=0
LoadLibraryExW: ERROR FileName=DDRAW.DLL err=126

LoadLibraryExW: file=D flags=800
LoadLibrary: RETRY fullpath="D:\HeavyGear/DSOUND.DLL"
LoadLibraryExW: FileName=DSOUND.DLL hFile=0 Flags=800(NULL) hmodule=0
LoadLibraryExW: ERROR FileName=DSOUND.DLL err=126

So it looks like when it works in Direct3D mode there is actually something wrong!

Additionally in the Direct3D crashing case we have references to these 3 dll files that do not show when running with my modified dxwnd.dll:

LoadLibraryA: file=avrt.dll flags=0
LoadLibraryA: hooking lib="avrt.dll" handle=6d640000

LoadLibraryA: file=kernel32.dll flags=0
Registered DLL FileName=kernel32.dll
LoadLibraryA: push idx=0 library=kernel32.dll hdl=767d0000

LoadLibraryA: file=D3DIM.DLL flags=0
Registered DLL FileName=D3DIM.DLL
LoadLibraryA: push idx=18 library=D3DIM.DLL hdl=6c440000

Now the "avrt.dll" might explain the situation where the game starts in 320x240 resolution window if the "locked size" flag is not enabled. It is a Microsoft Multimedia Realtime Runtime library that is located in System32 folder.

[gho]

Ah! Now things are starting to get clear. I don't want to draw early conclusions, but things seems to resemble what I feared: in order to run better, Heavy Gear needs some error, a situation that clearly you can't replicate in a generalized way, or many other games will cease to work.
Probably the reason why the problem is not so evident is that the game uses the extended widechar version of LoadLibrary - LoadLibraryExW - a call that very seldom other games are using.
Tomorrow I will examine thoroughfully the regression problem about "Hot patching". Fortunately, the behaviour of AoE2 and S3 is moch more replicable across different computers than the crazy and unpredictable behaviour of HG, so hopefully I should get soon to the point.
Goodnight.

[Riitaoja]

Well this was surprising. With the modified Dxwnd.dll because I broke the hooking it was looking for ddraw.dll in the main game folder.

So I had an idea what would happen if i placed ddraw.dll there? Well the result was a surprise.
There are two different versions of ddraw.dll. One is in "C:\Windows\System32\ddraw.dll" and the other one is in "C:\Windows\SysWOW64\ddraw.dll".

Using the one from SysWOW64 crashes the game. However using the one from System32 allows it to run in Direct3D mode just fine.

So to summarize:
HeavyGear crashing mystery seems to be finally solved. The reason the crash happens is that for some reason when run through dxwnd the"C:\Windows\SysWOW64\ddraw.dll" gets used.

EDIT: We posted at the same time! :-D

---

EDIT2:
Actually I think that the ddraw.dll file in System32 is the 64-bit version of the dll which dxwnd can't hook. So by using that file I am actually again bypassing the Dxwnd hooking of the ddraw.dll file since Dxwnd does not support 64-bit executables or libraries.

P.S.
Would it be possible to manually avoid hooking a library in dxwnd?

[gho]

Look! This could be interesting.: I don't know how we could overlook this one.

Back to these log line:

LoadLibraryExW: file=DDRAW.DLL flags=800
LoadLibraryExW: FileName=DDRAW.DLL hFile=0 Flags=800(NULL) hmodule=6d6a0000
LoadLibraryExW: hooking lib="DDRAW.DLL" handle=6d6a0000

I wondered why this game and ONLY this game was giving us so many troubles. Now look at flags=800, a feature that I bet isn't used by many other games: from MSDN https://msdn.microsoft.com/en-us/library/windows/desktop/ms684179(v=vs.85).aspx

---

LOAD_LIBRARY_SEARCH_SYSTEM32 0x00000800

If this value is used, %windows%\system32 is searched for the DLL and its dependencies. Directories in the standard search path are not searched. This value cannot be combined with LOAD_WITH_ALTERED_SEARCH_PATH.

Windows 7, Windows Server 2008 R2, Windows Vista and Windows Server 2008: This value requires KB2533623 to be installed.

Windows Server 2003 and Windows XP: This value is not supported.

---

Definitely, this 0x800 is related to where the DLL are searched for, and maybe it is the reason why DDRAW.DLL and other essential libraries are searched in the system32 folder that holds 64 bit DLLs that are no good for a 32 bit application.
If this is the reason, it will be possible to make a generalized fix (I like generalized interventions!) where DxWnd fixes/emulates the desired behaviour.
A possible fix (maybe you can have the pleasure of trying it before myself) could be the following line placed before the actual LoadLibrary call:
dwFlags &= ~ LOAD_LIBRARY_SEARCH_SYSTEM32;

P.s. probably in developer's minds there was the desire of avoiding a possible misusage of ddraw.dll proxies placed somewere, but they didn't foresee the MS Wow64 trick!

[gho]

Sorry, on my Visual Studio that macro is not defined. To compile the program you may have to add something like this:

#ifndef LOAD_LIBRARY_SEARCH_SYSTEM32
#define LOAD_LIBRARY_SEARCH_SYSTEM32 0x00000800
#endif

[gho]

I made the fix (no flags required, I see now no reason why we should let this flag pass ...) but I also "broke" my HG installation on the portable. Waiting for some testing on my Win10 home pc, you can play with this patched release.
P.s. it is a leaked next release alpha, where I added the "Sys libraries" logging cathegory, so you will have to add the "Logs / Sys libraries" flag to have LoadLibrary calls in the logs.

[gho]

The comparison between Settlers III demo log files from v2.04.53 and v2.04.54 is quite interesting too. I put the two logs in attach, but I can short the reading by pointing out that the new version omits to hook some important dlls:
MSV4W32.dll
WinMM.dll
and, on the contrary, makes a useless hook to NTDLL.dll.
Or maybe the problem is another one: there is a recursion between the hooking of d3d9.dll and NTDLL.dll .....
There's some work to be done here.

update
On WinXP the demo seems still playable with all DxWnd versions if you set DirectX1~6 version.
Possibly, the new more generalized hooking schema wiped away some trick that was in previous releases that ensured a ddraw.dll hook also when not found, or something like this ...

[Riitaoja]

LOAD_LIBRARY_SEARCH_SYSTEM32 0x00000800

If this value is used, %windows%\system32 is searched for the DLL and its dependencies. Directories in the standard search path are not searched. This value cannot be combined with LOAD_WITH_ALTERED_SEARCH_PATH.

Wow! That is a great find. I just hope this will not cause problems elsewhere.

I will also test that settlers3 DX1-6 hook later.

[gho]

Please, use the attached alpha release for testing: it has several last-minute fixes.
One warning: I had many troubles trying to make HG running before noticing that the game doesn't like the "Libs / GDI / Stretch GDI calls" flag that (unfortunately) was left set in latest exported configurations.

[gho]

Sorry, little mistake: I updated the link above because v2.04.66.alpha1.rar had the wrong GUI. Please, note that the correct archive is named v2.04.66.alpha01.rar (with the 0 digit) to avoid SF file caching.
Here on Win7 the game now works both in Direct3D and DirectX software mode, but the trick of removing the PLUGIN\dx5draw.dll file causes a game crash when entering a mission.

[Riitaoja]

Perfect! All issues are solved here. HeavyGear runs now in Direct3D and WinXP "Hot patch" works in Settlers3 and AOE2.