dll: add dll_LoadLibrary for single library loading #370
Reference in New Issue
Block a user
Delete Branch "hrigar/kolibrios:feature/dll-LoadLibrary"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Resolves #281
This PR adds a new
dll_LoadLibraryfunction todll.objthat loads a single library by name or full path, auto-callslib_init, and returns the export table pointer.dll_loadis refactored on top ofdll_LoadLibrary.Add similar functionality to the /programs/dll.inc repository file. It should have the same functionality as the library.
Should dll.Load be rewritten to call dll.LoadLibrary in a loop? It looks like an unnecessary code duplication at the moment.
Currently dll.Load follows Load -> Link -> Init order sequence, but using LoadLibrary internally would change this to Load -> Init -> Link. We'd need to verify both orders produce identical effects.
Can we then have two commits? The first one as it is now, and the second one on top that changes the logic to call dll.LoadLibrary from dll.Load. This way, if anything breaks, we will just revert the second commit and that's it.
@@ -122,0 +99,4 @@; if first function in export table begins with 'lib_', call it as DLL initialization; return eax = pointer to export table, or 0 on failureproc dll.LoadLibrary, lib_path:dwordmov esi, [lib_path]Error in the implementation of the calling convention for this function. There is no saving of ESI, EDI, EBX registers
Fixed, preserving the registers now.
@@ -173,2 +173,2 @@dll.GetProcAddress, 'dll_sym'dll.GetProcAddress, 'dll_sym', \dll.LoadLibrary, 'dll_LoadLibrary'It is better to follow the existing name style: dll_load, dll_link, dll_sym. Hence, not dll_LoadLibrary but somethink like dll_loadlib, or dll_load_lib, dll_load_library
Yes, renamed export to
dll_loadlib@@ -90,0 +90,4 @@or eax, eaxjz .failpush eaxIs this push (and the corresponding pop) needed?
Isn't it a calling convention for a caller to preserve eax, even though dll.Init currently doesn't modify eax?
dll.Init is an internal function that preserves all the registers itself. At the moment, these push/pop eax are redundant. In the future, dll.Init might return some value in eax, in which case push/pop eax will have to be removed anyway.
@@ -90,0 +100,4 @@pop edi esi ebxret.fail:xor eax, eaxIs this xor needed?
No, its redundant. will remove this.
@@ -64,12 +42,10 @@ proc dll.Load, import_table:dwordret.fail_load:add esp, 4Maybe this line and the 'ret' below can be removed.
@@ -122,0 +126,4 @@.done:pop eaxpop edi esi ebxretI believe this 'ret' and the above line can be safely removed.
@@ -31,2 +16,4 @@pop edxtest eax, eaxjz .failstdcall dll.Link, eax, edxPlease add a check for the success of the function, similar to the one in the dll.inc file for the library
added
the patch adds a check with a transition to a non-existent label
I can see a crucial problem in current implementation:
Success path is treated as failure indll.LoadIn
programs/dll.inc(proc dll.Load, right afterstdcall dll.Link), the newtest eax, eax+jnz .faillogic is inverted.In this implementation,
dll.Linkreturns non-zero on success and0on unresolved-symbol failure, so this check fails valid loads.Thus, you also need to change
dll.Linkand sync it's behaviour with library.bdd9489279toeb022fef9e