| Message ID | 20231106185112.2755262-15-alex.bennee@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Maintainer updates for 8.2 (gdbstub, tests, plugins) pre-PR | expand |
One important remark below that Greg can answer; the others are nits. On 11/6/23 19:51, Alex Bennée wrote: > diff --git a/contrib/plugins/win32_linker.c b/contrib/plugins/win32_linker.c > new file mode 100644 > index 0000000000..50797d616e > --- /dev/null > +++ b/contrib/plugins/win32_linker.c > @@ -0,0 +1,34 @@ > +/* > + * Copyright (C) 2023, Greg Manning <gmanning@rapitasystems.com> > + * > + * 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). > + * > + * 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. > + */ > + > +#include <Windows.h> Just a nit, but we generally use lowercase "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) { I think this could also use the notification hook and dliNotePreLoadLibrary. That's a little more tidy but it's okay either way. A bit more important: would it make sense to include the hook *in the QEMU executable itself*, rather than in the DLL? If it works, it would be much preferrable. You still would have to add the .lib file to the compilation, but win32_linker.c could simply be placed in os-win32.c with fewer changes to meson.build and the makefiles. > + if targetos == 'windows' > + t += shared_module(i, files(i + '.c') + '../../contrib/plugins/win32_linker.c', > + include_directories: '../../include/qemu', > + objects: [win32_qemu_plugin_api_lib], > + dependencies: glib) > + > + else > + t += shared_module(i, files(i + '.c'), > + include_directories: '../../include/qemu', > + dependencies: glib) > + endif If the win32_linker.c file can be removed (by moving the hook into the emulator), I'd rather have this where win32_qemu_plugin_api_lib is created: if targetos == 'windows' ... else win32_qemu_plugin_api_lib = [] endif and then here you can just use "objects: [win32_qemu_plugin_api_lib]" unconditionally, saving an "if" and some duplication. Paolo > endforeach > endif > if t.length() > 0
From: Paolo Bonzini <pbonzini@redhat.com> One important remark below that Greg can answer; the others are nits. > ... > I think this could also use the notification hook and dliNotePreLoadLibrary. That's a little more tidy but it's okay either way. I don't really mind. I had in mind that there might someday be a single executable and when that happens the hook would silently get out of the way. On the other hand, doing it this way means if the user /happens/ to have a qemu.exe in an unfortunate place then things will fail with very unhelpful error messages, because the linker would sucessfully load the qemu.exe, then (presumably) fail when looking up symbols. > A bit more important: would it make sense to include the hook *in the > QEMU executable itself*, rather than in the DLL? If it works, it would > be much preferrable. You still would have to add the .lib file to the > compilation, but win32_linker.c could simply be placed in os-win32.c > with fewer changes to meson.build and the makefiles. My initial trials of this didn't work. But having read the docs again, I'm going to have another go at it now... Greg. -- Follow Rapita Systems on LinkedIn<https://www.linkedin.com/company/rapita-systems?utm_source=rs_email_sig>
> > A bit more important: would it make sense to include the hook *in the > > QEMU executable itself*, rather than in the DLL? If it works, it would > > be much preferrable. You still would have to add the .lib file to the > > compilation, but win32_linker.c could simply be placed in os-win32.c > > with fewer changes to meson.build and the makefiles. > My initial trials of this didn't work. But having read the docs again, I'm > going to have another go at it now... Here is a diagram of what's going on. /---dynamic load------v qemu libplugin.dll ^---delay load--------/ First, qemu dynamically loads the plugin, which does no linking. It finds the function qemu_plugin_install, and invokes it. As soon as libplugin.dll calls any qemu_plugin_* function, the delay loader steps in and searches for qemu.exe. It fails to find it, and the delay failure helper steps in and returns the right reference to the top level executable. Everything gets linked OK. Windows will look for __pfnDliFailureHook2 in the module doing the delay loading, which in this case is the plugin DLL. So, the __pfnDliFailureHook2 function pointer needs to be in the plugin DLL. We could have qemu set the value of that pointer before it calls install, but I can't find the way to expose a function pointer in such a way that `g_module_symbol` can find it. Possibly we could pass in a pointer to it in qemu_plugin_install or a new qemu_plugin_set_linker_fn, but that is getting back to the sort of shenanigans we're trying to avoid, and which previous attempts at windows plugins were based on. so: maybe, but I'm not sure it would be any tidier. Greg. -- Follow Rapita Systems on LinkedIn<https://www.linkedin.com/company/rapita-systems?utm_source=rs_email_sig>
On Tue, Nov 7, 2023 at 1:43 PM Greg Manning <gmanning@rapitasystems.com> wrote: > Windows will look for __pfnDliFailureHook2 in the module doing the delay > loading, which in this case is the plugin DLL. Fair enough, this is what was not clear from the Microsoft docs. Paolo
diff --git a/configure b/configure index cd6c521bd8..e50ec99fe2 100755 --- a/configure +++ b/configure @@ -1666,6 +1666,9 @@ fi if test "$targetos" = darwin; then echo "CONFIG_DARWIN=y" >> contrib/plugins/$config_host_mak fi +if test "$targetos" = windows; then + echo "CONFIG_WIN32=y" >> contrib/plugins/$config_host_mak +fi # tests/tcg configuration (config_host_mak=tests/tcg/config-host.mak diff --git a/contrib/plugins/win32_linker.c b/contrib/plugins/win32_linker.c new file mode 100644 index 0000000000..50797d616e --- /dev/null +++ b/contrib/plugins/win32_linker.c @@ -0,0 +1,34 @@ +/* + * Copyright (C) 2023, Greg Manning <gmanning@rapitasystems.com> + * + * 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). + * + * 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. + */ + +#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; +} + diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile index 8ba78c7a32..751fa38619 100644 --- a/contrib/plugins/Makefile +++ b/contrib/plugins/Makefile @@ -22,7 +22,14 @@ NAMES += hwprofile NAMES += cache NAMES += drcov -SONAMES := $(addsuffix .so,$(addprefix lib,$(NAMES))) +ifeq ($(CONFIG_WIN32),y) +SO_SUFFIX := .dll +LDLIBS += $(shell $(PKG_CONFIG) --libs glib-2.0) +else +SO_SUFFIX := .so +endif + +SONAMES := $(addsuffix $(SO_SUFFIX),$(addprefix lib,$(NAMES))) # The main QEMU uses Glib extensively so it's perfectly fine to use it # in plugins (which many example do). @@ -35,15 +42,20 @@ all: $(SONAMES) %.o: %.c $(CC) $(CFLAGS) $(PLUGIN_CFLAGS) -c -o $@ $< -lib%.so: %.o -ifeq ($(CONFIG_DARWIN),y) +ifeq ($(CONFIG_WIN32),y) +lib%$(SO_SUFFIX): %.o win32_linker.o ../../plugins/qemu_plugin_api.lib + $(CC) -shared -o $@ $^ $(LDLIBS) +else ifeq ($(CONFIG_DARWIN),y) +lib%$(SO_SUFFIX): %.o $(CC) -bundle -Wl,-undefined,dynamic_lookup -o $@ $^ $(LDLIBS) else +lib%$(SO_SUFFIX): %.o $(CC) -shared -o $@ $^ $(LDLIBS) endif + clean: - rm -f *.o *.so *.d + rm -f *.o *$(SO_SUFFIX) *.d rm -Rf .libs .PHONY: all clean diff --git a/plugins/meson.build b/plugins/meson.build index 71ed996ed3..40d24529c0 100644 --- a/plugins/meson.build +++ b/plugins/meson.build @@ -14,6 +14,25 @@ if not enable_modules endif if get_option('plugins') + if targetos == 'windows' + dlltool = find_program('dlltool', required: true) + + # Generate a .lib file for plugins to link against. + # First, create a .def file listing all the symbols a plugin should expect to have + # available in qemu + win32_plugin_def = configure_file( + input: files('qemu-plugins.symbols'), + output: 'qemu_plugin_api.def', + capture: true, + command: ['sed', '-e', '0,/^/s//EXPORTS/; s/[{};]//g', '@INPUT@']) + # then use dlltool to assemble a delaylib. + win32_qemu_plugin_api_lib = configure_file( + input: win32_plugin_def, + output: 'qemu_plugin_api.lib', + command: [dlltool, '--input-def', '@INPUT@', + '--output-delaylib', '@OUTPUT@', '--dllname', 'qemu.exe'] + ) + endif specific_ss.add(files( 'loader.c', 'core.c', diff --git a/tests/plugin/meson.build b/tests/plugin/meson.build index 322cafcdf6..528bb9d86c 100644 --- a/tests/plugin/meson.build +++ b/tests/plugin/meson.build @@ -1,9 +1,17 @@ t = [] if get_option('plugins') foreach i : ['bb', 'empty', 'insn', 'mem', 'syscall'] - t += shared_module(i, files(i + '.c'), - include_directories: '../../include/qemu', - dependencies: glib) + if targetos == 'windows' + t += shared_module(i, files(i + '.c') + '../../contrib/plugins/win32_linker.c', + include_directories: '../../include/qemu', + objects: [win32_qemu_plugin_api_lib], + dependencies: glib) + + else + t += shared_module(i, files(i + '.c'), + include_directories: '../../include/qemu', + dependencies: glib) + endif endforeach endif if t.length() > 0