Dependency work and manual module import table resolver#20
Dependency work and manual module import table resolver#20raonygamer wants to merge 9 commits intoMinecraftZenova:masterfrom
Conversation
No need to specify the dependency array anymore, the import resolver will automatically load the dependency mod if it isn't loaded and populate the (Import Address Table) with the right addresses from the GetProcAddress with the names from the (Import Lookup Table) of the mod dll.
Added some error handling, fixed the error of DllMain being called even if there is no DllMain, and changed the C-style casts to be reinterpret_casts
ThePixelGamer
left a comment
ThePixelGamer
left a comment
There was a problem hiding this comment.
I feel like a second review is going to be needed after the changes are made due to how many structural changes I asked for, it's looking good though for the most part
…o read the mod and cause a crash
ThePixelGamer
left a comment
ThePixelGamer
left a comment
There was a problem hiding this comment.
Sorry for requesting so many changes, this is the first one that picks at ResolveModModuleImports
| } | ||
| } | ||
|
|
||
| void* LoadModModuleAndResolveImports(const std::string& module) |
There was a problem hiding this comment.
Can this function be dropped in favor of just using a single function? I'm not sure what other places we would use "ResolveModModuleImports"
|
|
||
| typedef BOOL(APIENTRY* DllMainFunction)(HMODULE, DWORD, LPVOID); | ||
| void ResolveModModuleImports(void* hModule, const std::string& moduleName) | ||
| { |
There was a problem hiding this comment.
The code base uses { on the same line only
| } | ||
|
|
||
| uintptr_t hModuleAddress = reinterpret_cast<uintptr_t>(hModule); | ||
| PIMAGE_DOS_HEADER dosHeader = reinterpret_cast<PIMAGE_DOS_HEADER>(hModuleAddress); |
There was a problem hiding this comment.
could the reinterpret cast be inlined into ntHeader's initialization?
| PIMAGE_DOS_HEADER dosHeader = reinterpret_cast<PIMAGE_DOS_HEADER>(hModuleAddress); | ||
| PIMAGE_NT_HEADERS ntHeader = reinterpret_cast<PIMAGE_NT_HEADERS>(hModuleAddress + dosHeader->e_lfanew); | ||
|
|
||
| IMAGE_DATA_DIRECTORY importDir = ntHeader->OptionalHeader.DataDirectory[1]; |
There was a problem hiding this comment.
could importDir be inlined into imports initialization and 1 be swapped out with IMAGE_DIRECTORY_ENTRY_IMPORT
| void ResolveModModuleImports(void* hModule, const std::string& moduleName) | ||
| { | ||
| if (!hModule) { | ||
| Zenova_Log(Warning, "[{}] Could not resolve the imports for the module because it was nullptr.", __FUNCTION__); |
There was a problem hiding this comment.
FUNCTION should already be part of the Log macro (look at FSIG)
| return hModule; | ||
| } | ||
|
|
||
| typedef BOOL(APIENTRY* DllMainFunction)(HMODULE, DWORD, LPVOID); |
There was a problem hiding this comment.
Switch to using and put it with the if statement
| PIMAGE_THUNK_DATA importAddressTable = reinterpret_cast<PIMAGE_THUNK_DATA>(hModuleAddress + imports->FirstThunk); | ||
|
|
||
| if (imports->OriginalFirstThunk && imports->FirstThunk) { | ||
| LPCSTR moduleName = reinterpret_cast<LPCSTR>(hModuleAddress + imports->Name); |
There was a problem hiding this comment.
Could this just be swapped out with this?
std::string dllName(reinterpret_cast<LPCSTR>(hModuleAddress + imports->Name));
dllName.erase(dllName.length() - 4);| } | ||
|
|
||
| std::map<std::pair<uintptr_t, std::string>, ULONGLONG*> symbolTableAddresses; | ||
| while (imports->Characteristics != 0) { |
There was a problem hiding this comment.
Can the while loops be swapped out with a for loop with an empty initialization?
for (; cond; post)| @@ -44,16 +35,7 @@ namespace Zenova { | |||
|
|
|||
| void* ModInfo::loadModule() | |||
There was a problem hiding this comment.
Could loadModule be inlined into Manager::loadMod
| // todo: verify path | ||
| std::string folder = manager.dataFolder + "\\mods\\" + modName + "\\"; | ||
| ModInfo mod(folder); | ||
| void* modHandle = mod.loadModule(); |
There was a problem hiding this comment.
modHandle should be dropped in favor of using mod.mHandle
2 participants
No need to specify the dependency array anymore, the import resolver will automatically load the dependency mod if it isn't loaded and populate the (Import Address Table) with the right addresses from the GetProcAddress with the names from the (Import Lookup Table) of the mod dll.