| Message ID | 20231109092554.1253-2-gmanning@rapitasystems.com |
|---|---|
| State | New |
| Headers | show |
| Series | plugins: Move the windows linking function to qemu | expand |
On 11/9/23 10:25, Greg Manning wrote: > Previously, a plugin author needed an implementation of the > __pfnDliFailureHook2 or __pfnDliNotifyHook2 hook in the plugin. Now all > they need is a null exported pointer with the right name (as in > win32_linker.c). If QEMU finds this, it will set it to the hook > function, which has now moved into qemu (os-win32.c). > > Signed-off-by: Greg Manning <gmanning@rapitasystems.com> > --- Thanks for trying! :) I'm not sure how big an improvement this is, but I'll let Alex judge. The main issue I had with win32_linker.c is the additional complexity for external plugins, which is now decreased but to some extent remains. I should have made that clearer. One possibility is to put the definition in a macro, and have the plugin sources simply expand the macro. Paolo
> Previously, a plugin author needed an implementation of the > __pfnDliFailureHook2 or __pfnDliNotifyHook2 hook in the plugin. Now all > they need is a null exported pointer with the right name (as in > win32_linker.c). If QEMU finds this, it will set it to the hook > function, which has now moved into qemu (os-win32.c). I have a new idea for this. We've made the qemu_plugin_api.lib file which is a delaylib with all the symbol names of all the api functions, so windows can do the whole delay-linking thing. We could also put into that archive the object win32_linker.o: ar -q qemu_plugin.api.lib ../whatever/win32_linker.o Then hopefully when a plugin links to this, it gets the __pfnDliFailureHook2 symbol defined and set up and everything would work. Except gcc strips out any unreferenced symbols from static libs when linking. So the plugin would have to be linked thusly: gcc -shared -o my_plugin.dll -Wl,-u,__pfnDliFailureHook2 my_plugin.o qemu_plugin_api.lib But no other qemu-fiddling-with-things or extra code in plugins required. Hmm. Feels like half a solution. I wonder if there's a way to mark symbols as "always required despite what dead code analysis says". Greg.
On Fri, Nov 10, 2023 at 6:36 PM Greg Manning <gmanning@rapitasystems.com> wrote: > Then hopefully when a plugin links to this, it gets the __pfnDliFailureHook2 > symbol defined and set up and everything would work. Except gcc strips > out any unreferenced symbols from static libs when linking. So the plugin > would have to be linked thusly: > > gcc -shared -o my_plugin.dll -Wl,-u,__pfnDliFailureHook2 my_plugin.o qemu_plugin_api.lib > > But no other qemu-fiddling-with-things or extra code in plugins required. > > Hmm. Feels like half a solution. I wonder if there's a way to mark symbols as > "always required despite what dead code analysis says". To be clear, I don't dislike at all the simpler solution where you just add a macro like this: #ifdef _WIN32 #define QEMU_PLUGIN_HOOK \ /* contents of win32_linker.c */ #else #define QEMU_PLUGIN_HOOK #endif and add QEMU_PLUGIN_HOOK to a source file of every plugin. But if you would like to use a library, you can pass a linker script on the command line as if it was a library, and the paths within the linker script are resolved relative to the linker script itself. So you can place in tests/plugins/meson.build something like: # uses dlltool like it does now... can also use custom_target delaylib = configure_file(output: 'qemu_plugin_api.lib', ...) delayhook = static_library('qemu_delay_hook', sources: 'qemu_delay_hook.c') plugin_api = configure_file(output : 'libqemu_plugin_api.a', input: 'libqemu_plugin_api.ld', copy: true, install_dir: get_option('libdir')) where the last configure_file creates a file with contents such as INPUT(qemu_plugin_api.lib) # from dlltool INPUT(libqemu_delay_hook.a) # compiled from qemu_delay_hook.c EXTERN(__pfnDliNotifyHook2) # equivalent to -Wl,-u And then it should work to add link_depends: [delayhook, delaylib, plugin_api], link_args: plugin_api as in your previous version. Finally, since the hook will be built by ninja, you also need diff --git a/Makefile b/Makefile index 676a4a54f48..7b42d85f1dc 100644 --- a/Makefile +++ b/Makefile @@ -184,6 +184,7 @@ $(SUBDIR_RULES): ifneq ($(filter contrib/plugins, $(SUBDIRS)),) .PHONY: plugins plugins: contrib/plugins/all +contrib/plugins/all: | run-ninja endif .PHONY: recurse-all recurse-clean to ensure that the plugins are built after libqemu_delay_hook.a (of course feel free to change the file names!). The main disadvantage is that the Microsoft linker does not know linker scripts, so that's a point in favor of the macro solution. Paolo
diff --git a/contrib/plugins/win32_linker.c b/contrib/plugins/win32_linker.c index 7534b2b8bf..619fdd38c8 100644 --- a/contrib/plugins/win32_linker.c +++ b/contrib/plugins/win32_linker.c @@ -4,8 +4,8 @@ * This hook, __pfnDliFailureHook2, is documented in the microsoft documentation here: * https://learn.microsoft.com/en-us/cpp/build/reference/error-handling-and-notification * It gets called when a delay-loaded DLL encounters various errors. - * We handle the specific case of a DLL looking for a "qemu.exe", - * and give it the running executable (regardless of what it is named). + * QEMU will set it to a function which handles the specific case of a DLL looking for + * a "qemu.exe", and give it the running executable (regardless of what it is named). * * This work is licensed under the terms of the GNU LGPL, version 2 or later. * See the COPYING.LIB file in the top-level directory. @@ -14,21 +14,4 @@ #include <windows.h> #include <delayimp.h> -FARPROC WINAPI dll_failure_hook(unsigned dliNotify, PDelayLoadInfo pdli); - - -PfnDliHook __pfnDliFailureHook2 = dll_failure_hook; - -FARPROC WINAPI dll_failure_hook(unsigned dliNotify, PDelayLoadInfo pdli) { - if (dliNotify == dliFailLoadLib) { - /* If the failing request was for qemu.exe, ... */ - if (strcmp(pdli->szDll, "qemu.exe") == 0) { - /* Then pass back a pointer to the top level module. */ - HMODULE top = GetModuleHandle(NULL); - return (FARPROC) top; - } - } - /* Otherwise we can't do anything special. */ - return 0; -} - +__declspec(dllexport) PfnDliHook __pfnDliNotifyHook2 = NULL; diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h index 1047d260cb..0f698554b2 100644 --- a/include/sysemu/os-win32.h +++ b/include/sysemu/os-win32.h @@ -30,6 +30,8 @@ #include <windows.h> #include <ws2tcpip.h> #include "qemu/typedefs.h" +#include <delayimp.h> +#include <gmodule.h> #ifdef HAVE_AFUNIX_H #include <afunix.h> @@ -265,6 +267,29 @@ win32_close_exception_handler(struct _EXCEPTION_RECORD*, void*, void *qemu_win32_map_alloc(size_t size, HANDLE *h, Error **errp); void qemu_win32_map_free(void *ptr, HANDLE h, Error **errp); + +/* dll_delaylink_hook: + * @dliNotify: Type of event that caused this callback. + * @pdli: Extra data about the DLL being loaded + * + * For more info on the arguments and when this gets invoked see + * https://learn.microsoft.com/en-us/cpp/build/reference/understanding-the-helper-function + * + * This is to be used on windows as the target of a __pfnDliNotifyHook2 or __pfnDliFailureHook2 + * hook. If the DLL being loaded is 'qemu.exe', it instead passes back a pointer to the main + * executable This gets set into plugins to assist with the plugins delay-linking back to the main + * executable, if they define an appropriate symbol. */ +FARPROC WINAPI dll_delaylink_hook(unsigned dliNotify, PDelayLoadInfo pdli); + +/* set_dll_delaylink_hook: + * @mod: pointer to the DLL being loaded + * + * takes a pointer to a loaded plugin DLL, and tries to find a __pfnDliNotifyHook2 or + * __pfnDliFailureHook2 hook. If it finds either one, and its value is null, it sets it to the + * address fo dll_delaylink_hook. + */ +void set_dll_delaylink_hook(GModule *mod); + #ifdef __cplusplus } #endif diff --git a/os-win32.c b/os-win32.c index 725ad652e8..4a113d1d10 100644 --- a/os-win32.c +++ b/os-win32.c @@ -60,3 +60,36 @@ void os_set_line_buffering(void) setbuf(stdout, NULL); setbuf(stderr, NULL); } + +FARPROC WINAPI dll_delaylink_hook(unsigned dliNotify, PDelayLoadInfo pdli) { + /* If we just tried, or are about to try to load a DLL ... */ + if (dliNotify == dliFailLoadLib || dliNotify == dliNotePreLoadLibrary) { + /* ... if the failing request was for qemu.exe, ... */ + if (strcmp(pdli->szDll, "qemu.exe") == 0) { + /* ... then pass back a pointer to the top level module. */ + HMODULE top = GetModuleHandle(NULL); + return (FARPROC) top; + } + } + /* Otherwise we can't do anything special. */ + return 0; +} +void set_dll_delaylink_hook(GModule *mod) { + /* find the __pfnDliFailureHook2 symbol in the DLL. + * if found, set it to our handler. + */ + gpointer sym; + PfnDliHook *hook; + if (g_module_symbol(mod, "__pfnDliFailureHook2", &sym)) { + hook = (PfnDliHook*) sym; + if (hook != NULL && *hook == NULL) { + *hook = &dll_delaylink_hook; + } + } + if (g_module_symbol(mod, "__pfnDliNotifyHook2", &sym)) { + hook = (PfnDliHook*) sym; + if (hook != NULL && *hook == NULL) { + *hook = &dll_delaylink_hook; + } + } +} diff --git a/plugins/loader.c b/plugins/loader.c index 734c11cae0..7ead9b26e4 100644 --- a/plugins/loader.c +++ b/plugins/loader.c @@ -241,6 +241,9 @@ static int plugin_load(struct qemu_plugin_desc *desc, const qemu_info_t *info, E } QTAILQ_INSERT_TAIL(&plugin.ctxs, ctx, entry); ctx->installing = true; + #ifdef CONFIG_WIN32 + set_dll_delaylink_hook(ctx->handle); + #endif rc = install(ctx->id, info, desc->argc, desc->argv); ctx->installing = false; if (rc) {
Previously, a plugin author needed an implementation of the __pfnDliFailureHook2 or __pfnDliNotifyHook2 hook in the plugin. Now all they need is a null exported pointer with the right name (as in win32_linker.c). If QEMU finds this, it will set it to the hook function, which has now moved into qemu (os-win32.c). Signed-off-by: Greg Manning <gmanning@rapitasystems.com> --- contrib/plugins/win32_linker.c | 23 +++-------------------- include/sysemu/os-win32.h | 25 +++++++++++++++++++++++++ os-win32.c | 33 +++++++++++++++++++++++++++++++++ plugins/loader.c | 3 +++ 4 files changed, 64 insertions(+), 20 deletions(-)