-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added DLL loading capability in windows to the std lib. Ensure that t… #616
Conversation
…he exports defined in a .def file are valid symbols in the source dll
Still need to create tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exciting!
@@ -11,6 +11,11 @@ | |||
#include "codegen.hpp" | |||
#include "analyze.hpp" | |||
|
|||
#ifdef ZIG_OS_WINDOWS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pattern we have so far is to isolate windows API calls to os.cpp. That should be the only file that includes windows.h.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that makes sense, I'll move the verification code to there.
pub fn windowsLoadDll(allocator: &mem.Allocator, dll_path: []const u8) -> %windows.HMODULE { | ||
const padded_buff = %return cstr.addNullByte(allocator, dll_path); | ||
defer allocator.free(padded_buff); | ||
const rc = windows.LoadLibraryA(padded_buff.ptr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return windows.LoadLibraryA(padded_buff.ptr) ?? error.DllNotFound
test/compare_output.zig
Outdated
@@ -444,4 +444,18 @@ pub fn addCases(cases: &tests.CompareOutputContext) { | |||
|
|||
tc | |||
}); | |||
|
|||
cases.addCase("windowsLoadDll failure", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When possible, a behavior test is preferred over a compare_output test, because the behavior tests run much faster. See test/behavior.zig and test/cases/*
@@ -84,6 +84,11 @@ pub extern "kernel32" stdcallcc fn WriteFile(in_hFile: HANDLE, in_lpBuffer: &con | |||
in_nNumberOfBytesToWrite: DWORD, out_lpNumberOfBytesWritten: ?&DWORD, | |||
in_out_lpOverlapped: ?&OVERLAPPED) -> BOOL; | |||
|
|||
//TODO: call unicode versions instead of relying on ANSI code page |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
link to #534
#ifdef ZIG_OS_WINDOWS | ||
Buf* dll_name = buf_create_from_buf(link_lib->name); | ||
buf_append_str(dll_name, ".dll"); | ||
HMODULE hmod = GetModuleHandle(buf_ptr(dll_name)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From MSDN GetModuleHandle:
Retrieves a module handle for the specified module. The module must have been loaded by the calling process.
Based on this, I think we have to use LoadLibrary before GetModuleHandle. And LoadLibrary runs the code in the DLL, which can crash the compiler. So we should do it in a child process, and communicate back to zig compiler the results. If we're going to spawn a child process, we might as well write that child process in zig and build it lazily just like compiler_rt.o and builtin.o.
I'm going to close this PR, think a bit harder about the symbol checking, and submit the STL changes as a separate commit |
Added DLL loading capability in windows to the std lib. Ensure that the exports defined in a .def file are valid symbols in the source dll.