[PATCH 00/12] Enable building plugins on Windows with Clang

[PATCH 00/12] Enable building plugins on Windows with Clang

[Pierrick Bouvier]

For now, it was only possible to build plugins using GCC on Windows. However,
windows-aarch64 only supports Clang.

The first patches (already posted and normally pulled) are fixes and
prerequisite to build plugins with meson. They are integrated here because it's
not possible to have two based-on in the cover letter.

Then, we fix compilation warnings on Windows.
After that, we can enable linking with Clang on Windows.

Finally, we enhanced the plugin symbols export list (automatically generated,
and not a static file), and we report some of the information we discovered on
the path in the documentation.

Built and tested on Windows (all msys env)/Linux/MacOS for x86_64 and aarch64
hosts.

Pierrick Bouvier (12):
  scripts: remove erroneous file that breaks git clone on Windows
  contrib/plugins/cflow: fix warning
  meson: build contrib/plugins with meson
  contrib/plugins: remove Makefile for contrib/plugins
  qga: fix -Wsometimes-uninitialized windows warning
  qga: fix missing static and prototypes windows warnings
  win32: use compiler option instead of attribute gcc_struct
  plugins: enable linking with clang/lld
  plugins: add missing export for qemu_plugin_num_vcpus
  plugins: detect qemu plugin API symbols from header
  plugins: eradicate qemu-plugins.symbols static file
  docs: add information on how to setup build environments

 MAINTAINERS                               |   1 +
 docs/about/build-platforms.rst            |   4 +-
 docs/devel/build-system.rst               | 100 ++++++++++++++++++++++
 configure                                 |  18 ----
 Makefile                                  |  10 ---
 meson.build                               |  14 ++-
 include/qemu/compiler.h                   |   7 +-
 include/qemu/qemu-plugin.h                |   1 +
 subprojects/libvhost-user/libvhost-user.h |   6 +-
 contrib/plugins/cflow.c                   |   6 +-
 qga/commands-windows-ssh.c                |   2 +-
 contrib/plugins/Makefile                  |  87 -------------------
 contrib/plugins/meson.build               |  28 ++++++
 plugins/meson.build                       |  36 ++++++--
 plugins/qemu-plugins.symbols              |  59 -------------
 qga/vss-win32/install.cpp                 |   6 +-
 qga/vss-win32/provider.cpp                |   5 +-
 qga/vss-win32/requester.cpp               |   8 +-
 scripts/meson-buildoptions.               |   0
 scripts/qemu-plugin-symbols.py            |  45 ++++++++++
 tests/tcg/plugins/meson.build             |   3 +-
 21 files changed, 238 insertions(+), 208 deletions(-)
 delete mode 100644 contrib/plugins/Makefile
 create mode 100644 contrib/plugins/meson.build
 delete mode 100644 plugins/qemu-plugins.symbols
 delete mode 100644 scripts/meson-buildoptions.
 create mode 100755 scripts/qemu-plugin-symbols.py

-- 
2.39.5

[PATCH 01/12] scripts: remove erroneous file that breaks git clone on Windows

[Pierrick Bouvier]

This file was created by mistake in recent ed7667188 (9p: remove
'proxy' filesystem backend driver).

When cloning the repository using native git for windows, we see this:
Error: error: invalid path 'scripts/meson-buildoptions.'
Error: The process 'C:\Program Files\Git\bin\git.exe' failed with exit code 128

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 scripts/meson-buildoptions. | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 delete mode 100644 scripts/meson-buildoptions.

diff --git a/scripts/meson-buildoptions. b/scripts/meson-buildoptions.
deleted file mode 100644
index e69de29bb2d..00000000000
-- 
2.39.5

[PATCH 02/12] contrib/plugins/cflow: fix warning

[Pierrick Bouvier]

contrib/plugins/cflow.c: In function ‘plugin_exit’:
contrib/plugins/cflow.c:167:19: error: declaration of ‘n’ shadows a previous local [-Werror=shadow=local]
  167 |         NodeData *n = l->data;
      |                   ^
contrib/plugins/cflow.c:139:9: note: shadowed declaration is here
  139 |     int n = 0;
      |         ^

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 contrib/plugins/cflow.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/contrib/plugins/cflow.c b/contrib/plugins/cflow.c
index 6faa55d10d1..b39974d1cf3 100644
--- a/contrib/plugins/cflow.c
+++ b/contrib/plugins/cflow.c
@@ -136,7 +136,7 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
     g_autoptr(GString) result = g_string_new("collected ");
     GList *data;
     GCompareFunc sort = &hottest;
-    int n = 0;
+    int i = 0;
 
     g_mutex_lock(&node_lock);
     g_string_append_printf(result, "%d control flow nodes in the hash table\n",
@@ -162,8 +162,8 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
     data = g_list_sort(data, sort);
 
     for (GList *l = data;
-         l != NULL && n < topn;
-         l = l->next, n++) {
+         l != NULL && i < topn;
+         l = l->next, i++) {
         NodeData *n = l->data;
         const char *type = n->mid_count ? "sync fault" : "branch";
         g_string_append_printf(result, "  addr: 0x%"PRIx64 " %s: %s (%s)\n",
-- 
2.39.5

[PATCH 03/12] meson: build contrib/plugins with meson

[Pierrick Bouvier]

Tried to unify this meson.build with tests/tcg/plugins/meson.build but
the resulting modules are not output in the right directory.

Originally proposed by Anton Kochkov, thank you!

Solves: https://gitlab.com/qemu-project/qemu/-/issues/1710
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 meson.build                 |  4 ++++
 contrib/plugins/meson.build | 28 ++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+)
 create mode 100644 contrib/plugins/meson.build

diff --git a/meson.build b/meson.build
index f7d45175212..d8af08299e0 100644
--- a/meson.build
+++ b/meson.build
@@ -3684,6 +3684,10 @@ subdir('accel')
 subdir('plugins')
 subdir('ebpf')
 
+if 'CONFIG_TCG' in config_all_accel
+  subdir('contrib/plugins')
+endif
+
 common_user_inc = []
 
 subdir('common-user')
diff --git a/contrib/plugins/meson.build b/contrib/plugins/meson.build
new file mode 100644
index 00000000000..63a32c2b4f0
--- /dev/null
+++ b/contrib/plugins/meson.build
@@ -0,0 +1,28 @@
+contrib_plugins = ['bbv', 'cache', 'cflow', 'drcov', 'execlog', 'hotblocks',
+                   'hotpages', 'howvec', 'hwprofile', 'ips', 'stoptrigger']
+if host_os != 'windows'
+  # lockstep uses socket.h
+  contrib_plugins += 'lockstep'
+endif
+
+t = []
+if get_option('plugins')
+  foreach i : contrib_plugins
+    if host_os == 'windows'
+      t += shared_module(i, files(i + '.c') + 'win32_linker.c',
+                        include_directories: '../../include/qemu',
+                        link_depends: [win32_qemu_plugin_api_lib],
+                        link_args: ['-Lplugins', '-lqemu_plugin_api'],
+                        dependencies: glib)
+    else
+      t += shared_module(i, files(i + '.c'),
+                        include_directories: '../../include/qemu',
+                        dependencies: glib)
+    endif
+  endforeach
+endif
+if t.length() > 0
+  alias_target('contrib-plugins', t)
+else
+  run_target('contrib-plugins', command: find_program('true'))
+endif
-- 
2.39.5

[PATCH 04/12] contrib/plugins: remove Makefile for contrib/plugins

[Pierrick Bouvier]

Now replaced by meson build.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 configure                | 18 ---------
 Makefile                 | 10 -----
 contrib/plugins/Makefile | 87 ----------------------------------------
 3 files changed, 115 deletions(-)
 delete mode 100644 contrib/plugins/Makefile

diff --git a/configure b/configure
index 4a0159e1832..8f0ba505187 100755
--- a/configure
+++ b/configure
@@ -1073,7 +1073,6 @@ if test "$plugins" != "no" && test $host_bits -eq 64; then
         plugins="no"
     else
         plugins=yes
-        subdirs="$subdirs contrib/plugins"
     fi
 fi
 
@@ -1704,7 +1703,6 @@ LINKS="$LINKS .gdbinit scripts" # scripts needed by relative path in .gdbinit
 LINKS="$LINKS tests/avocado tests/data"
 LINKS="$LINKS tests/qemu-iotests/check tests/qemu-iotests/Makefile"
 LINKS="$LINKS python"
-LINKS="$LINKS contrib/plugins/Makefile "
 for f in $LINKS ; do
     if [ -e "$source_path/$f" ]; then
         symlink "$source_path/$f" "$f"
@@ -1790,22 +1788,6 @@ if test "$default_targets" = "yes"; then
   echo "CONFIG_DEFAULT_TARGETS=y" >> $config_host_mak
 fi
 
-# contrib/plugins configuration
-echo "# Automatically generated by configure - do not modify" > contrib/plugins/$config_host_mak
-echo "SRC_PATH=$source_path/contrib/plugins" >> contrib/plugins/$config_host_mak
-echo "PKG_CONFIG=${pkg_config}" >> contrib/plugins/$config_host_mak
-echo "CC=$cc $CPU_CFLAGS" >> contrib/plugins/$config_host_mak
-echo "CFLAGS=${CFLAGS-$default_cflags} $EXTRA_CFLAGS" >> contrib/plugins/$config_host_mak
-if test "$host_os" = windows; then
-  echo "DLLTOOL=$dlltool" >> contrib/plugins/$config_host_mak
-fi
-if test "$host_os" = darwin; then
-  echo "CONFIG_DARWIN=y" >> contrib/plugins/$config_host_mak
-fi
-if test "$host_os" = windows; then
-  echo "CONFIG_WIN32=y" >> contrib/plugins/$config_host_mak
-fi
-
 # tests/tcg configuration
 mkdir -p tests/tcg
 echo "# Automatically generated by configure - do not modify" > tests/tcg/$config_host_mak
diff --git a/Makefile b/Makefile
index 917c9a34d1c..b65b0bd41a8 100644
--- a/Makefile
+++ b/Makefile
@@ -187,11 +187,6 @@ SUBDIR_RULES=$(foreach t, all clean distclean, $(addsuffix /$(t), $(SUBDIRS)))
 $(SUBDIR_RULES):
 	$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $(dir $@) V="$(V)" TARGET_DIR="$(dir $@)" $(notdir $@),)
 
-ifneq ($(filter contrib/plugins, $(SUBDIRS)),)
-.PHONY: plugins
-plugins: contrib/plugins/all
-endif
-
 .PHONY: recurse-all recurse-clean
 recurse-all: $(addsuffix /all, $(SUBDIRS))
 recurse-clean: $(addsuffix /clean, $(SUBDIRS))
@@ -307,11 +302,6 @@ help:
 	$(call print-help,cscope,Generate cscope index)
 	$(call print-help,sparse,Run sparse on the QEMU source)
 	@echo  ''
-ifneq ($(filter contrib/plugins, $(SUBDIRS)),)
-	@echo  'Plugin targets:'
-	$(call print-help,plugins,Build the example TCG plugins)
-	@echo  ''
-endif
 	@echo  'Cleaning targets:'
 	$(call print-help,clean,Remove most generated files but keep the config)
 	$(call print-help,distclean,Remove all generated files)
diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile
deleted file mode 100644
index bbddd4800ff..00000000000
--- a/contrib/plugins/Makefile
+++ /dev/null
@@ -1,87 +0,0 @@
-# -*- Mode: makefile -*-
-#
-# This Makefile example is fairly independent from the main makefile
-# so users can take and adapt it for their build. We only really
-# include config-host.mak so we don't have to repeat probing for
-# programs that the main configure has already done for us.
-#
-
-include config-host.mak
-
-TOP_SRC_PATH = $(SRC_PATH)/../..
-
-VPATH += $(SRC_PATH)
-
-NAMES :=
-NAMES += bbv
-NAMES += execlog
-NAMES += hotblocks
-NAMES += hotpages
-NAMES += howvec
-
-# The lockstep example communicates using unix sockets,
-# and can't be easily made to work on windows.
-ifneq ($(CONFIG_WIN32),y)
-NAMES += lockstep
-endif
-
-NAMES += hwprofile
-NAMES += cache
-NAMES += drcov
-NAMES += ips
-NAMES += stoptrigger
-NAMES += cflow
-
-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 is perfectly fine to use it
-# in plugins (which many example do).
-PLUGIN_CFLAGS := $(shell $(PKG_CONFIG) --cflags glib-2.0)
-PLUGIN_CFLAGS += -fPIC -Wall
-PLUGIN_CFLAGS += -I$(TOP_SRC_PATH)/include/qemu
-
-# Helper that honours V=1 so we get some output when compiling
-quiet-@ = $(if $(V),,@$(if $1,printf "  %-7s %s\n" "$(strip $1)" "$(strip $2)" && ))
-quiet-command = $(call quiet-@,$2,$3)$1
-
-# for including , in command strings
-COMMA := ,
-
-all: $(SONAMES)
-
-%.o: %.c
-	$(call quiet-command, \
-		$(CC) $(CFLAGS) $(PLUGIN_CFLAGS) -c -o $@ $<, \
-	        BUILD, plugin $@)
-
-ifeq ($(CONFIG_WIN32),y)
-lib%$(SO_SUFFIX): %.o win32_linker.o ../../plugins/libqemu_plugin_api.a
-	$(call quiet-command, \
-		$(CC) -shared -o $@ $^ $(LDLIBS), \
-		LINK, plugin $@)
-else ifeq ($(CONFIG_DARWIN),y)
-lib%$(SO_SUFFIX): %.o
-	$(call quiet-command, \
-		$(CC) -bundle -Wl$(COMMA)-undefined$(COMMA)dynamic_lookup -o $@ $^ $(LDLIBS), \
-		LINK, plugin $@)
-else
-lib%$(SO_SUFFIX): %.o
-	$(call quiet-command, \
-		$(CC) -shared -o $@ $^ $(LDLIBS), \
-		LINK, plugin $@)
-endif
-
-
-clean distclean:
-	rm -f *.o *$(SO_SUFFIX) *.d
-	rm -Rf .libs
-
-.PHONY: all clean
-.SECONDARY:
-- 
2.39.5

[PATCH 05/12] qga: fix -Wsometimes-uninitialized windows warning

[Pierrick Bouvier]

Reported by clang, but not gcc.

[5123/5678] Compiling C object qga/qemu-ga.exe.p/commands-windows-ssh.c.obj
FAILED: qga/qemu-ga.exe.p/commands-windows-ssh.c.obj
"cc" "-Iqga/qemu-ga.exe.p" "-Iqga" "-I../qga" "-I." "-Iqapi" "-Itrace" "-Iui" "-Iui/shader" "-IC:/msys64/clangarm64/include/glib-2.0" "-IC:/msys64/clangarm64/lib/glib-2.0/include" "-fdiagnostics-color=auto" "-Wall" "-Winvalid-pch" "-Werror" "-std=gnu11" "-O2" "-g" "-fstack-protector-strong" "-Wempty-body" "-Wendif-labels" "-Wexpansion-to-defined" "-Wformat-security" "-Wformat-y2k" "-Wignored-qualifiers" "-Winit-self" "-Wmissing-format-attribute" "-Wmissing-prototypes" "-Wnested-externs" "-Wold-style-definition" "-Wredundant-decls" "-Wstrict-prototypes" "-Wtype-limits" "-Wundef" "-Wvla" "-Wwrite-strings" "-Wno-gnu-variable-sized-type-not-at-end" "-Wno-initializer-overrides" "-Wno-missing-include-dirs" "-Wno-psabi" "-Wno-shift-negative-value" "-Wno-string-plus-int" "-Wno-tautological-type-limit-compare" "-Wno-typedef-redefinition" "-Wthread-safety" "-iquote" "." "-iquote" "C:/w/qemu" "-iquote" "C:/w/qemu/include" "-iquote" "C:/w/qemu/host/include/aarch64" "-iquote" "C:/w/qemu/host/include/generic" "-iquote" "C:/w/qemu/tcg/aarch64" "-D_GNU_SOURCE" "-D_FILE_OFFSET_BITS=64" "-D_LARGEFILE_SOURCE" "-fno-strict-aliasing" "-fno-common" "-fwrapv" "-fno-pie" "-ftrivial-auto-var-init=zero" "-fzero-call-used-regs=used-gpr" -MD -MQ qga/qemu-ga.exe.p/commands-windows-ssh.c.obj -MF "qga/qemu-ga.exe.p/commands-windows-ssh.c.obj.d" -o qga/qemu-ga.exe.p/commands-windows-ssh.c.obj "-c" ../qga/commands-windows-ssh.c
../qga/commands-windows-ssh.c:383:9: error: variable 'userPSID' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
  383 |     if (!create_acl(userInfo, &pACL, errp)) {
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../qga/commands-windows-ssh.c:415:15: note: uninitialized use occurs here
  415 |     LocalFree(userPSID);
      |               ^~~~~~~~
../qga/commands-windows-ssh.c:383:5: note: remove the 'if' if its condition is always false
  383 |     if (!create_acl(userInfo, &pACL, errp)) {
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  384 |         goto error;
      |         ~~~~~~~~~~~
  385 |     }
      |     ~
../qga/commands-windows-ssh.c:380:18: note: initialize the variable 'userPSID' to silence this warning
  380 |     PSID userPSID;
      |                  ^
      |                   = NULL
1 error generated.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 qga/commands-windows-ssh.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qga/commands-windows-ssh.c b/qga/commands-windows-ssh.c
index 6a642e3ba81..df45c17b757 100644
--- a/qga/commands-windows-ssh.c
+++ b/qga/commands-windows-ssh.c
@@ -377,7 +377,7 @@ error:
 static bool set_file_permissions(PWindowsUserInfo userInfo, Error **errp)
 {
     PACL pACL = NULL;
-    PSID userPSID;
+    PSID userPSID = NULL;
 
     /* Creates the access control structure */
     if (!create_acl(userInfo, &pACL, errp)) {
-- 
2.39.5

Re: [PATCH 05/12] qga: fix -Wsometimes-uninitialized windows warning

[Konstantin Kostiuk]

[-- Attachment #1: Type: text/plain, Size: 3437 bytes --]

Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>

On Thu, Oct 31, 2024 at 6:05 AM Pierrick Bouvier <
pierrick.bouvier@linaro.org> wrote:

> Reported by clang, but not gcc.
>
> [5123/5678] Compiling C object qga/qemu-ga.exe.p/commands-windows-ssh.c.obj
> FAILED: qga/qemu-ga.exe.p/commands-windows-ssh.c.obj
> "cc" "-Iqga/qemu-ga.exe.p" "-Iqga" "-I../qga" "-I." "-Iqapi" "-Itrace"
> "-Iui" "-Iui/shader" "-IC:/msys64/clangarm64/include/glib-2.0"
> "-IC:/msys64/clangarm64/lib/glib-2.0/include" "-fdiagnostics-color=auto"
> "-Wall" "-Winvalid-pch" "-Werror" "-std=gnu11" "-O2" "-g"
> "-fstack-protector-strong" "-Wempty-body" "-Wendif-labels"
> "-Wexpansion-to-defined" "-Wformat-security" "-Wformat-y2k"
> "-Wignored-qualifiers" "-Winit-self" "-Wmissing-format-attribute"
> "-Wmissing-prototypes" "-Wnested-externs" "-Wold-style-definition"
> "-Wredundant-decls" "-Wstrict-prototypes" "-Wtype-limits" "-Wundef" "-Wvla"
> "-Wwrite-strings" "-Wno-gnu-variable-sized-type-not-at-end"
> "-Wno-initializer-overrides" "-Wno-missing-include-dirs" "-Wno-psabi"
> "-Wno-shift-negative-value" "-Wno-string-plus-int"
> "-Wno-tautological-type-limit-compare" "-Wno-typedef-redefinition"
> "-Wthread-safety" "-iquote" "." "-iquote" "C:/w/qemu" "-iquote"
> "C:/w/qemu/include" "-iquote" "C:/w/qemu/host/include/aarch64" "-iquote"
> "C:/w/qemu/host/include/generic" "-iquote" "C:/w/qemu/tcg/aarch64"
> "-D_GNU_SOURCE" "-D_FILE_OFFSET_BITS=64" "-D_LARGEFILE_SOURCE"
> "-fno-strict-aliasing" "-fno-common" "-fwrapv" "-fno-pie"
> "-ftrivial-auto-var-init=zero" "-fzero-call-used-regs=used-gpr" -MD -MQ
> qga/qemu-ga.exe.p/commands-windows-ssh.c.obj -MF
> "qga/qemu-ga.exe.p/commands-windows-ssh.c.obj.d" -o
> qga/qemu-ga.exe.p/commands-windows-ssh.c.obj "-c"
> ../qga/commands-windows-ssh.c
> ../qga/commands-windows-ssh.c:383:9: error: variable 'userPSID' is used
> uninitialized whenever 'if' condition is true
> [-Werror,-Wsometimes-uninitialized]
>   383 |     if (!create_acl(userInfo, &pACL, errp)) {
>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../qga/commands-windows-ssh.c:415:15: note: uninitialized use occurs here
>   415 |     LocalFree(userPSID);
>       |               ^~~~~~~~
> ../qga/commands-windows-ssh.c:383:5: note: remove the 'if' if its
> condition is always false
>   383 |     if (!create_acl(userInfo, &pACL, errp)) {
>       |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   384 |         goto error;
>       |         ~~~~~~~~~~~
>   385 |     }
>       |     ~
> ../qga/commands-windows-ssh.c:380:18: note: initialize the variable
> 'userPSID' to silence this warning
>   380 |     PSID userPSID;
>       |                  ^
>       |                   = NULL
> 1 error generated.
>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>  qga/commands-windows-ssh.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/qga/commands-windows-ssh.c b/qga/commands-windows-ssh.c
> index 6a642e3ba81..df45c17b757 100644
> --- a/qga/commands-windows-ssh.c
> +++ b/qga/commands-windows-ssh.c
> @@ -377,7 +377,7 @@ error:
>  static bool set_file_permissions(PWindowsUserInfo userInfo, Error **errp)
>  {
>      PACL pACL = NULL;
> -    PSID userPSID;
> +    PSID userPSID = NULL;
>
>      /* Creates the access control structure */
>      if (!create_acl(userInfo, &pACL, errp)) {
> --
> 2.39.5
>
>

[-- Attachment #2: Type: text/html, Size: 4698 bytes --]

Re: [PATCH 05/12] qga: fix -Wsometimes-uninitialized windows warning

[Konstantin Kostiuk]

[-- Attachment #1: Type: text/plain, Size: 3710 bytes --]

Hi Pierrick,

I got this patch in my PULL with QGA patches.

Best Regards,
Konstantin Kostiuk.

On Thu, Oct 31, 2024 at 3:32 PM Konstantin Kostiuk <kkostiuk@redhat.com>
wrote:

> Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
>
> On Thu, Oct 31, 2024 at 6:05 AM Pierrick Bouvier <
> pierrick.bouvier@linaro.org> wrote:
>
>> Reported by clang, but not gcc.
>>
>> [5123/5678] Compiling C object
>> qga/qemu-ga.exe.p/commands-windows-ssh.c.obj
>> FAILED: qga/qemu-ga.exe.p/commands-windows-ssh.c.obj
>> "cc" "-Iqga/qemu-ga.exe.p" "-Iqga" "-I../qga" "-I." "-Iqapi" "-Itrace"
>> "-Iui" "-Iui/shader" "-IC:/msys64/clangarm64/include/glib-2.0"
>> "-IC:/msys64/clangarm64/lib/glib-2.0/include" "-fdiagnostics-color=auto"
>> "-Wall" "-Winvalid-pch" "-Werror" "-std=gnu11" "-O2" "-g"
>> "-fstack-protector-strong" "-Wempty-body" "-Wendif-labels"
>> "-Wexpansion-to-defined" "-Wformat-security" "-Wformat-y2k"
>> "-Wignored-qualifiers" "-Winit-self" "-Wmissing-format-attribute"
>> "-Wmissing-prototypes" "-Wnested-externs" "-Wold-style-definition"
>> "-Wredundant-decls" "-Wstrict-prototypes" "-Wtype-limits" "-Wundef" "-Wvla"
>> "-Wwrite-strings" "-Wno-gnu-variable-sized-type-not-at-end"
>> "-Wno-initializer-overrides" "-Wno-missing-include-dirs" "-Wno-psabi"
>> "-Wno-shift-negative-value" "-Wno-string-plus-int"
>> "-Wno-tautological-type-limit-compare" "-Wno-typedef-redefinition"
>> "-Wthread-safety" "-iquote" "." "-iquote" "C:/w/qemu" "-iquote"
>> "C:/w/qemu/include" "-iquote" "C:/w/qemu/host/include/aarch64" "-iquote"
>> "C:/w/qemu/host/include/generic" "-iquote" "C:/w/qemu/tcg/aarch64"
>> "-D_GNU_SOURCE" "-D_FILE_OFFSET_BITS=64" "-D_LARGEFILE_SOURCE"
>> "-fno-strict-aliasing" "-fno-common" "-fwrapv" "-fno-pie"
>> "-ftrivial-auto-var-init=zero" "-fzero-call-used-regs=used-gpr" -MD -MQ
>> qga/qemu-ga.exe.p/commands-windows-ssh.c.obj -MF
>> "qga/qemu-ga.exe.p/commands-windows-ssh.c.obj.d" -o
>> qga/qemu-ga.exe.p/commands-windows-ssh.c.obj "-c"
>> ../qga/commands-windows-ssh.c
>> ../qga/commands-windows-ssh.c:383:9: error: variable 'userPSID' is used
>> uninitialized whenever 'if' condition is true
>> [-Werror,-Wsometimes-uninitialized]
>>   383 |     if (!create_acl(userInfo, &pACL, errp)) {
>>       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> ../qga/commands-windows-ssh.c:415:15: note: uninitialized use occurs here
>>   415 |     LocalFree(userPSID);
>>       |               ^~~~~~~~
>> ../qga/commands-windows-ssh.c:383:5: note: remove the 'if' if its
>> condition is always false
>>   383 |     if (!create_acl(userInfo, &pACL, errp)) {
>>       |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>   384 |         goto error;
>>       |         ~~~~~~~~~~~
>>   385 |     }
>>       |     ~
>> ../qga/commands-windows-ssh.c:380:18: note: initialize the variable
>> 'userPSID' to silence this warning
>>   380 |     PSID userPSID;
>>       |                  ^
>>       |                   = NULL
>> 1 error generated.
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>  qga/commands-windows-ssh.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/qga/commands-windows-ssh.c b/qga/commands-windows-ssh.c
>> index 6a642e3ba81..df45c17b757 100644
>> --- a/qga/commands-windows-ssh.c
>> +++ b/qga/commands-windows-ssh.c
>> @@ -377,7 +377,7 @@ error:
>>  static bool set_file_permissions(PWindowsUserInfo userInfo, Error **errp)
>>  {
>>      PACL pACL = NULL;
>> -    PSID userPSID;
>> +    PSID userPSID = NULL;
>>
>>      /* Creates the access control structure */
>>      if (!create_acl(userInfo, &pACL, errp)) {
>> --
>> 2.39.5
>>
>>

[-- Attachment #2: Type: text/html, Size: 5333 bytes --]

[PATCH 06/12] qga: fix missing static and prototypes windows warnings

[Pierrick Bouvier]

Reported by clang++, but not by g++.

../qga/vss-win32/provider.cpp:48:6: error: no previous prototype for function 'LockModule' [-Werror,-Wmissing-prototypes]
   48 | void LockModule(BOOL lock)
      |      ^
../qga/vss-win32/provider.cpp:48:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   48 | void LockModule(BOOL lock)
      | ^
      | static
../qga/vss-win32/provider.cpp:531:13: error: no previous prototype for function 'DllMain' [-Werror,-Wmissing-prototypes]
  531 | BOOL WINAPI DllMain(HINSTANCE hinstDll, DWORD dwReason, LPVOID lpReserved)
      |             ^
../qga/vss-win32/provider.cpp:531:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
  531 | BOOL WINAPI DllMain(HINSTANCE hinstDll, DWORD dwReason, LPVOID lpReserved)
      | ^
      | static

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 qga/vss-win32/install.cpp   | 6 +++++-
 qga/vss-win32/provider.cpp  | 5 ++++-
 qga/vss-win32/requester.cpp | 8 ++++----
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/qga/vss-win32/install.cpp b/qga/vss-win32/install.cpp
index 84944133f79..5cea5bcf747 100644
--- a/qga/vss-win32/install.cpp
+++ b/qga/vss-win32/install.cpp
@@ -39,7 +39,7 @@ const GUID CLSID_WbemLocator = { 0x4590f811, 0x1d3a, 0x11d0,
 const GUID IID_IWbemLocator = { 0xdc12a687, 0x737f, 0x11cf,
     {0x88, 0x4d, 0x00, 0xaa, 0x00, 0x4b, 0x2e, 0x24} };
 
-void errmsg(DWORD err, const char *text)
+static void errmsg(DWORD err, const char *text)
 {
     /*
      * `text' contains function call statement when errmsg is called via chk().
@@ -242,6 +242,7 @@ out:
 }
 
 /* Unregister this module from COM+ Applications Catalog */
+STDAPI COMUnregister(void);
 STDAPI COMUnregister(void)
 {
     qga_debug_begin;
@@ -256,6 +257,7 @@ out:
 }
 
 /* Register this module to COM+ Applications Catalog */
+STDAPI COMRegister(void);
 STDAPI COMRegister(void)
 {
     qga_debug_begin;
@@ -380,11 +382,13 @@ out:
     return hr;
 }
 
+STDAPI_(void) CALLBACK DLLCOMRegister(HWND, HINSTANCE, LPSTR, int);
 STDAPI_(void) CALLBACK DLLCOMRegister(HWND, HINSTANCE, LPSTR, int)
 {
     COMRegister();
 }
 
+STDAPI_(void) CALLBACK DLLCOMUnregister(HWND, HINSTANCE, LPSTR, int);
 STDAPI_(void) CALLBACK DLLCOMUnregister(HWND, HINSTANCE, LPSTR, int)
 {
     COMUnregister();
diff --git a/qga/vss-win32/provider.cpp b/qga/vss-win32/provider.cpp
index cc72e5ef1b9..a102a23fbf1 100644
--- a/qga/vss-win32/provider.cpp
+++ b/qga/vss-win32/provider.cpp
@@ -45,7 +45,7 @@ const IID IID_IVssEnumObject = { 0xAE1C7110, 0x2F60, 0x11d3,
     {0x8A, 0x39, 0x00, 0xC0, 0x4F, 0x72, 0xD8, 0xE3} };
 
 
-void LockModule(BOOL lock)
+static void LockModule(BOOL lock)
 {
     if (lock) {
         InterlockedIncrement(&g_nComObjsInUse);
@@ -527,6 +527,9 @@ STDAPI DllCanUnloadNow()
     return g_nComObjsInUse == 0 ? S_OK : S_FALSE;
 }
 
+EXTERN_C
+BOOL WINAPI DllMain(HINSTANCE hinstDll, DWORD dwReason, LPVOID lpReserved);
+
 EXTERN_C
 BOOL WINAPI DllMain(HINSTANCE hinstDll, DWORD dwReason, LPVOID lpReserved)
 {
diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
index 9884c65e707..4401d55e3a4 100644
--- a/qga/vss-win32/requester.cpp
+++ b/qga/vss-win32/requester.cpp
@@ -254,8 +254,8 @@ out:
     qga_debug_end;
 }
 
-DWORD get_reg_dword_value(HKEY baseKey, LPCSTR subKey, LPCSTR valueName,
-                          DWORD defaultData)
+static DWORD get_reg_dword_value(HKEY baseKey, LPCSTR subKey, LPCSTR valueName,
+                                 DWORD defaultData)
 {
     qga_debug_begin;
 
@@ -272,12 +272,12 @@ DWORD get_reg_dword_value(HKEY baseKey, LPCSTR subKey, LPCSTR valueName,
     return dwordData;
 }
 
-bool is_valid_vss_backup_type(VSS_BACKUP_TYPE vssBT)
+static bool is_valid_vss_backup_type(VSS_BACKUP_TYPE vssBT)
 {
     return (vssBT > VSS_BT_UNDEFINED && vssBT < VSS_BT_OTHER);
 }
 
-VSS_BACKUP_TYPE get_vss_backup_type(
+static VSS_BACKUP_TYPE get_vss_backup_type(
     VSS_BACKUP_TYPE defaultVssBT = DEFAULT_VSS_BACKUP_TYPE)
 {
     qga_debug_begin;
-- 
2.39.5

Re: [PATCH 06/12] qga: fix missing static and prototypes windows warnings

[Philippe Mathieu-Daudé]

On 31/10/24 01:04, Pierrick Bouvier wrote:
> Reported by clang++, but not by g++.
> 
> ../qga/vss-win32/provider.cpp:48:6: error: no previous prototype for function 'LockModule' [-Werror,-Wmissing-prototypes]
>     48 | void LockModule(BOOL lock)
>        |      ^
> ../qga/vss-win32/provider.cpp:48:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
>     48 | void LockModule(BOOL lock)
>        | ^
>        | static
> ../qga/vss-win32/provider.cpp:531:13: error: no previous prototype for function 'DllMain' [-Werror,-Wmissing-prototypes]
>    531 | BOOL WINAPI DllMain(HINSTANCE hinstDll, DWORD dwReason, LPVOID lpReserved)
>        |             ^
> ../qga/vss-win32/provider.cpp:531:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
>    531 | BOOL WINAPI DllMain(HINSTANCE hinstDll, DWORD dwReason, LPVOID lpReserved)
>        | ^
>        | static
> 
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   qga/vss-win32/install.cpp   | 6 +++++-
>   qga/vss-win32/provider.cpp  | 5 ++++-
>   qga/vss-win32/requester.cpp | 8 ++++----
>   3 files changed, 13 insertions(+), 6 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 06/12] qga: fix missing static and prototypes windows warnings

[Konstantin Kostiuk]

[-- Attachment #1: Type: text/plain, Size: 4635 bytes --]

Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>

On Thu, Oct 31, 2024 at 6:05 AM Pierrick Bouvier <
pierrick.bouvier@linaro.org> wrote:

> Reported by clang++, but not by g++.
>
> ../qga/vss-win32/provider.cpp:48:6: error: no previous prototype for
> function 'LockModule' [-Werror,-Wmissing-prototypes]
>    48 | void LockModule(BOOL lock)
>       |      ^
> ../qga/vss-win32/provider.cpp:48:1: note: declare 'static' if the function
> is not intended to be used outside of this translation unit
>    48 | void LockModule(BOOL lock)
>       | ^
>       | static
> ../qga/vss-win32/provider.cpp:531:13: error: no previous prototype for
> function 'DllMain' [-Werror,-Wmissing-prototypes]
>   531 | BOOL WINAPI DllMain(HINSTANCE hinstDll, DWORD dwReason, LPVOID
> lpReserved)
>       |             ^
> ../qga/vss-win32/provider.cpp:531:1: note: declare 'static' if the
> function is not intended to be used outside of this translation unit
>   531 | BOOL WINAPI DllMain(HINSTANCE hinstDll, DWORD dwReason, LPVOID
> lpReserved)
>       | ^
>       | static
>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>  qga/vss-win32/install.cpp   | 6 +++++-
>  qga/vss-win32/provider.cpp  | 5 ++++-
>  qga/vss-win32/requester.cpp | 8 ++++----
>  3 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/qga/vss-win32/install.cpp b/qga/vss-win32/install.cpp
> index 84944133f79..5cea5bcf747 100644
> --- a/qga/vss-win32/install.cpp
> +++ b/qga/vss-win32/install.cpp
> @@ -39,7 +39,7 @@ const GUID CLSID_WbemLocator = { 0x4590f811, 0x1d3a,
> 0x11d0,
>  const GUID IID_IWbemLocator = { 0xdc12a687, 0x737f, 0x11cf,
>      {0x88, 0x4d, 0x00, 0xaa, 0x00, 0x4b, 0x2e, 0x24} };
>
> -void errmsg(DWORD err, const char *text)
> +static void errmsg(DWORD err, const char *text)
>  {
>      /*
>       * `text' contains function call statement when errmsg is called via
> chk().
> @@ -242,6 +242,7 @@ out:
>  }
>
>  /* Unregister this module from COM+ Applications Catalog */
> +STDAPI COMUnregister(void);
>  STDAPI COMUnregister(void)
>  {
>      qga_debug_begin;
> @@ -256,6 +257,7 @@ out:
>  }
>
>  /* Register this module to COM+ Applications Catalog */
> +STDAPI COMRegister(void);
>  STDAPI COMRegister(void)
>  {
>      qga_debug_begin;
> @@ -380,11 +382,13 @@ out:
>      return hr;
>  }
>
> +STDAPI_(void) CALLBACK DLLCOMRegister(HWND, HINSTANCE, LPSTR, int);
>  STDAPI_(void) CALLBACK DLLCOMRegister(HWND, HINSTANCE, LPSTR, int)
>  {
>      COMRegister();
>  }
>
> +STDAPI_(void) CALLBACK DLLCOMUnregister(HWND, HINSTANCE, LPSTR, int);
>  STDAPI_(void) CALLBACK DLLCOMUnregister(HWND, HINSTANCE, LPSTR, int)
>  {
>      COMUnregister();
> diff --git a/qga/vss-win32/provider.cpp b/qga/vss-win32/provider.cpp
> index cc72e5ef1b9..a102a23fbf1 100644
> --- a/qga/vss-win32/provider.cpp
> +++ b/qga/vss-win32/provider.cpp
> @@ -45,7 +45,7 @@ const IID IID_IVssEnumObject = { 0xAE1C7110, 0x2F60,
> 0x11d3,
>      {0x8A, 0x39, 0x00, 0xC0, 0x4F, 0x72, 0xD8, 0xE3} };
>
>
> -void LockModule(BOOL lock)
> +static void LockModule(BOOL lock)
>  {
>      if (lock) {
>          InterlockedIncrement(&g_nComObjsInUse);
> @@ -527,6 +527,9 @@ STDAPI DllCanUnloadNow()
>      return g_nComObjsInUse == 0 ? S_OK : S_FALSE;
>  }
>
> +EXTERN_C
> +BOOL WINAPI DllMain(HINSTANCE hinstDll, DWORD dwReason, LPVOID
> lpReserved);
> +
>  EXTERN_C
>  BOOL WINAPI DllMain(HINSTANCE hinstDll, DWORD dwReason, LPVOID lpReserved)
>  {
> diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
> index 9884c65e707..4401d55e3a4 100644
> --- a/qga/vss-win32/requester.cpp
> +++ b/qga/vss-win32/requester.cpp
> @@ -254,8 +254,8 @@ out:
>      qga_debug_end;
>  }
>
> -DWORD get_reg_dword_value(HKEY baseKey, LPCSTR subKey, LPCSTR valueName,
> -                          DWORD defaultData)
> +static DWORD get_reg_dword_value(HKEY baseKey, LPCSTR subKey, LPCSTR
> valueName,
> +                                 DWORD defaultData)
>  {
>      qga_debug_begin;
>
> @@ -272,12 +272,12 @@ DWORD get_reg_dword_value(HKEY baseKey, LPCSTR
> subKey, LPCSTR valueName,
>      return dwordData;
>  }
>
> -bool is_valid_vss_backup_type(VSS_BACKUP_TYPE vssBT)
> +static bool is_valid_vss_backup_type(VSS_BACKUP_TYPE vssBT)
>  {
>      return (vssBT > VSS_BT_UNDEFINED && vssBT < VSS_BT_OTHER);
>  }
>
> -VSS_BACKUP_TYPE get_vss_backup_type(
> +static VSS_BACKUP_TYPE get_vss_backup_type(
>      VSS_BACKUP_TYPE defaultVssBT = DEFAULT_VSS_BACKUP_TYPE)
>  {
>      qga_debug_begin;
> --
> 2.39.5
>
>

[-- Attachment #2: Type: text/html, Size: 5482 bytes --]

Re: [PATCH 06/12] qga: fix missing static and prototypes windows warnings

[Konstantin Kostiuk]

[-- Attachment #1: Type: text/plain, Size: 4974 bytes --]

Hi Pierrick,

I got this patch in my PULL with QGA patches.

Best Regards,
Konstantin Kostiuk.


On Thu, Oct 31, 2024 at 3:32 PM Konstantin Kostiuk <kkostiuk@redhat.com>
wrote:

> Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
>
> On Thu, Oct 31, 2024 at 6:05 AM Pierrick Bouvier <
> pierrick.bouvier@linaro.org> wrote:
>
>> Reported by clang++, but not by g++.
>>
>> ../qga/vss-win32/provider.cpp:48:6: error: no previous prototype for
>> function 'LockModule' [-Werror,-Wmissing-prototypes]
>>    48 | void LockModule(BOOL lock)
>>       |      ^
>> ../qga/vss-win32/provider.cpp:48:1: note: declare 'static' if the
>> function is not intended to be used outside of this translation unit
>>    48 | void LockModule(BOOL lock)
>>       | ^
>>       | static
>> ../qga/vss-win32/provider.cpp:531:13: error: no previous prototype for
>> function 'DllMain' [-Werror,-Wmissing-prototypes]
>>   531 | BOOL WINAPI DllMain(HINSTANCE hinstDll, DWORD dwReason, LPVOID
>> lpReserved)
>>       |             ^
>> ../qga/vss-win32/provider.cpp:531:1: note: declare 'static' if the
>> function is not intended to be used outside of this translation unit
>>   531 | BOOL WINAPI DllMain(HINSTANCE hinstDll, DWORD dwReason, LPVOID
>> lpReserved)
>>       | ^
>>       | static
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>  qga/vss-win32/install.cpp   | 6 +++++-
>>  qga/vss-win32/provider.cpp  | 5 ++++-
>>  qga/vss-win32/requester.cpp | 8 ++++----
>>  3 files changed, 13 insertions(+), 6 deletions(-)
>>
>> diff --git a/qga/vss-win32/install.cpp b/qga/vss-win32/install.cpp
>> index 84944133f79..5cea5bcf747 100644
>> --- a/qga/vss-win32/install.cpp
>> +++ b/qga/vss-win32/install.cpp
>> @@ -39,7 +39,7 @@ const GUID CLSID_WbemLocator = { 0x4590f811, 0x1d3a,
>> 0x11d0,
>>  const GUID IID_IWbemLocator = { 0xdc12a687, 0x737f, 0x11cf,
>>      {0x88, 0x4d, 0x00, 0xaa, 0x00, 0x4b, 0x2e, 0x24} };
>>
>> -void errmsg(DWORD err, const char *text)
>> +static void errmsg(DWORD err, const char *text)
>>  {
>>      /*
>>       * `text' contains function call statement when errmsg is called via
>> chk().
>> @@ -242,6 +242,7 @@ out:
>>  }
>>
>>  /* Unregister this module from COM+ Applications Catalog */
>> +STDAPI COMUnregister(void);
>>  STDAPI COMUnregister(void)
>>  {
>>      qga_debug_begin;
>> @@ -256,6 +257,7 @@ out:
>>  }
>>
>>  /* Register this module to COM+ Applications Catalog */
>> +STDAPI COMRegister(void);
>>  STDAPI COMRegister(void)
>>  {
>>      qga_debug_begin;
>> @@ -380,11 +382,13 @@ out:
>>      return hr;
>>  }
>>
>> +STDAPI_(void) CALLBACK DLLCOMRegister(HWND, HINSTANCE, LPSTR, int);
>>  STDAPI_(void) CALLBACK DLLCOMRegister(HWND, HINSTANCE, LPSTR, int)
>>  {
>>      COMRegister();
>>  }
>>
>> +STDAPI_(void) CALLBACK DLLCOMUnregister(HWND, HINSTANCE, LPSTR, int);
>>  STDAPI_(void) CALLBACK DLLCOMUnregister(HWND, HINSTANCE, LPSTR, int)
>>  {
>>      COMUnregister();
>> diff --git a/qga/vss-win32/provider.cpp b/qga/vss-win32/provider.cpp
>> index cc72e5ef1b9..a102a23fbf1 100644
>> --- a/qga/vss-win32/provider.cpp
>> +++ b/qga/vss-win32/provider.cpp
>> @@ -45,7 +45,7 @@ const IID IID_IVssEnumObject = { 0xAE1C7110, 0x2F60,
>> 0x11d3,
>>      {0x8A, 0x39, 0x00, 0xC0, 0x4F, 0x72, 0xD8, 0xE3} };
>>
>>
>> -void LockModule(BOOL lock)
>> +static void LockModule(BOOL lock)
>>  {
>>      if (lock) {
>>          InterlockedIncrement(&g_nComObjsInUse);
>> @@ -527,6 +527,9 @@ STDAPI DllCanUnloadNow()
>>      return g_nComObjsInUse == 0 ? S_OK : S_FALSE;
>>  }
>>
>> +EXTERN_C
>> +BOOL WINAPI DllMain(HINSTANCE hinstDll, DWORD dwReason, LPVOID
>> lpReserved);
>> +
>>  EXTERN_C
>>  BOOL WINAPI DllMain(HINSTANCE hinstDll, DWORD dwReason, LPVOID
>> lpReserved)
>>  {
>> diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
>> index 9884c65e707..4401d55e3a4 100644
>> --- a/qga/vss-win32/requester.cpp
>> +++ b/qga/vss-win32/requester.cpp
>> @@ -254,8 +254,8 @@ out:
>>      qga_debug_end;
>>  }
>>
>> -DWORD get_reg_dword_value(HKEY baseKey, LPCSTR subKey, LPCSTR valueName,
>> -                          DWORD defaultData)
>> +static DWORD get_reg_dword_value(HKEY baseKey, LPCSTR subKey, LPCSTR
>> valueName,
>> +                                 DWORD defaultData)
>>  {
>>      qga_debug_begin;
>>
>> @@ -272,12 +272,12 @@ DWORD get_reg_dword_value(HKEY baseKey, LPCSTR
>> subKey, LPCSTR valueName,
>>      return dwordData;
>>  }
>>
>> -bool is_valid_vss_backup_type(VSS_BACKUP_TYPE vssBT)
>> +static bool is_valid_vss_backup_type(VSS_BACKUP_TYPE vssBT)
>>  {
>>      return (vssBT > VSS_BT_UNDEFINED && vssBT < VSS_BT_OTHER);
>>  }
>>
>> -VSS_BACKUP_TYPE get_vss_backup_type(
>> +static VSS_BACKUP_TYPE get_vss_backup_type(
>>      VSS_BACKUP_TYPE defaultVssBT = DEFAULT_VSS_BACKUP_TYPE)
>>  {
>>      qga_debug_begin;
>> --
>> 2.39.5
>>
>>

[-- Attachment #2: Type: text/html, Size: 6154 bytes --]

Re: [PATCH 06/12] qga: fix missing static and prototypes windows warnings

[Pierrick Bouvier]

Hi Konstantin,

On 11/4/24 05:43, Konstantin Kostiuk wrote:
> Hi Pierrick,
> 
> I got this patch in my PULL with QGA patches.
> 
> Best Regards,
> Konstantin Kostiuk.
> 

Thanks for pulling those two patches before the upcoming release.

Regards,
Pierrick

[PATCH 07/12] win32: use compiler option instead of attribute gcc_struct

[Pierrick Bouvier]

This attribute is not recognized by clang, but the associated option is.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 meson.build                               | 8 ++++----
 include/qemu/compiler.h                   | 7 +------
 subprojects/libvhost-user/libvhost-user.h | 6 +-----
 3 files changed, 6 insertions(+), 15 deletions(-)

diff --git a/meson.build b/meson.build
index d8af08299e0..d0d5dbe1479 100644
--- a/meson.build
+++ b/meson.build
@@ -330,10 +330,10 @@ elif host_os == 'sunos'
 elif host_os == 'haiku'
   qemu_common_flags += ['-DB_USE_POSITIVE_POSIX_ERRORS', '-D_BSD_SOURCE', '-fPIC']
 elif host_os == 'windows'
-  if not compiler.compiles('struct x { int y; } __attribute__((gcc_struct));',
-                           args: '-Werror')
-    error('Your compiler does not support __attribute__((gcc_struct)) - please use GCC instead of Clang')
-  endif
+  # https://gcc.gnu.org/onlinedocs/gcc/x86-Type-Attributes.html
+  # We use this compilation option instead of relying on gcc_struct attribute
+  # because clang does not support it (but supports the option).
+  qemu_common_flags += ['-mno-ms-bitfields']
 endif
 
 # Choose instruction set (currently x86-only)
diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h
index c06954ccb41..d904408e5ed 100644
--- a/include/qemu/compiler.h
+++ b/include/qemu/compiler.h
@@ -22,12 +22,7 @@
 #define QEMU_EXTERN_C extern
 #endif
 
-#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
-# define QEMU_PACKED __attribute__((gcc_struct, packed))
-#else
-# define QEMU_PACKED __attribute__((packed))
-#endif
-
+#define QEMU_PACKED __attribute__((packed))
 #define QEMU_ALIGNED(X) __attribute__((aligned(X)))
 
 #ifndef glue
diff --git a/subprojects/libvhost-user/libvhost-user.h b/subprojects/libvhost-user/libvhost-user.h
index deb40e77b3f..2ffc58c11b1 100644
--- a/subprojects/libvhost-user/libvhost-user.h
+++ b/subprojects/libvhost-user/libvhost-user.h
@@ -186,11 +186,7 @@ typedef struct VhostUserShared {
     unsigned char uuid[UUID_LEN];
 } VhostUserShared;
 
-#if defined(_WIN32) && (defined(__x86_64__) || defined(__i386__))
-# define VU_PACKED __attribute__((gcc_struct, packed))
-#else
-# define VU_PACKED __attribute__((packed))
-#endif
+#define VU_PACKED __attribute__((packed))
 
 typedef struct VhostUserMsg {
     int request;
-- 
2.39.5

Re: [PATCH 07/12] win32: use compiler option instead of attribute gcc_struct

[Daniel P. Berrangé]

On Wed, Oct 30, 2024 at 09:04:21PM -0700, Pierrick Bouvier wrote:
> This attribute is not recognized by clang, but the associated option is.
> 
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>  meson.build                               | 8 ++++----
>  include/qemu/compiler.h                   | 7 +------
>  subprojects/libvhost-user/libvhost-user.h | 6 +-----
>  3 files changed, 6 insertions(+), 15 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index d8af08299e0..d0d5dbe1479 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -330,10 +330,10 @@ elif host_os == 'sunos'
>  elif host_os == 'haiku'
>    qemu_common_flags += ['-DB_USE_POSITIVE_POSIX_ERRORS', '-D_BSD_SOURCE', '-fPIC']
>  elif host_os == 'windows'
> -  if not compiler.compiles('struct x { int y; } __attribute__((gcc_struct));',
> -                           args: '-Werror')
> -    error('Your compiler does not support __attribute__((gcc_struct)) - please use GCC instead of Clang')
> -  endif
> +  # https://gcc.gnu.org/onlinedocs/gcc/x86-Type-Attributes.html
> +  # We use this compilation option instead of relying on gcc_struct attribute
> +  # because clang does not support it (but supports the option).
> +  qemu_common_flags += ['-mno-ms-bitfields']
>  endif

Is this really safe for us to use ?   The current gcc_struct
attribute affects only structs marked as QEMU_PACKED. This
flag will affect all code.

If we call from QEMU code into Windows native APIs, and pass
or receive structs, then those structs' layouts would be
affected by this flag. I don't have a specific example, but
this feels unsafe to me, otherwise we would have done this
originally rather than only targetting internal packed structs
with the gcc_struct attribute.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 07/12] win32: use compiler option instead of attribute gcc_struct

[Thomas Huth]

On 31/10/2024 10.28, Daniel P. Berrangé wrote:
> On Wed, Oct 30, 2024 at 09:04:21PM -0700, Pierrick Bouvier wrote:
>> This attribute is not recognized by clang, but the associated option is.
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>   meson.build                               | 8 ++++----
>>   include/qemu/compiler.h                   | 7 +------
>>   subprojects/libvhost-user/libvhost-user.h | 6 +-----
>>   3 files changed, 6 insertions(+), 15 deletions(-)
>>
>> diff --git a/meson.build b/meson.build
>> index d8af08299e0..d0d5dbe1479 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -330,10 +330,10 @@ elif host_os == 'sunos'
>>   elif host_os == 'haiku'
>>     qemu_common_flags += ['-DB_USE_POSITIVE_POSIX_ERRORS', '-D_BSD_SOURCE', '-fPIC']
>>   elif host_os == 'windows'
>> -  if not compiler.compiles('struct x { int y; } __attribute__((gcc_struct));',
>> -                           args: '-Werror')
>> -    error('Your compiler does not support __attribute__((gcc_struct)) - please use GCC instead of Clang')
>> -  endif
>> +  # https://gcc.gnu.org/onlinedocs/gcc/x86-Type-Attributes.html
>> +  # We use this compilation option instead of relying on gcc_struct attribute
>> +  # because clang does not support it (but supports the option).
>> +  qemu_common_flags += ['-mno-ms-bitfields']
>>   endif
> 
> Is this really safe for us to use ?   The current gcc_struct
> attribute affects only structs marked as QEMU_PACKED. This
> flag will affect all code.
> 
> If we call from QEMU code into Windows native APIs, and pass
> or receive structs, then those structs' layouts would be
> affected by this flag. I don't have a specific example, but
> this feels unsafe to me, otherwise we would have done this
> originally rather than only targetting internal packed structs
> with the gcc_struct attribute.

I agree with Daniel, we likely cannot use this switch globally.

But seems like Clang folks are trying to include support for the attribute, 
see: https://gitlab.com/qemu-project/qemu/-/issues/2476#note_2159643081
so I'd rather recommend to wait for that for proper Clang support here.

  Thomas

Re: [PATCH 07/12] win32: use compiler option instead of attribute gcc_struct

[Pierrick Bouvier]

On 10/31/24 03:44, Thomas Huth wrote:
> On 31/10/2024 10.28, Daniel P. Berrangé wrote:
>> On Wed, Oct 30, 2024 at 09:04:21PM -0700, Pierrick Bouvier wrote:
>>> This attribute is not recognized by clang, but the associated option is.
>>>
>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>> ---
>>>    meson.build                               | 8 ++++----
>>>    include/qemu/compiler.h                   | 7 +------
>>>    subprojects/libvhost-user/libvhost-user.h | 6 +-----
>>>    3 files changed, 6 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/meson.build b/meson.build
>>> index d8af08299e0..d0d5dbe1479 100644
>>> --- a/meson.build
>>> +++ b/meson.build
>>> @@ -330,10 +330,10 @@ elif host_os == 'sunos'
>>>    elif host_os == 'haiku'
>>>      qemu_common_flags += ['-DB_USE_POSITIVE_POSIX_ERRORS', '-D_BSD_SOURCE', '-fPIC']
>>>    elif host_os == 'windows'
>>> -  if not compiler.compiles('struct x { int y; } __attribute__((gcc_struct));',
>>> -                           args: '-Werror')
>>> -    error('Your compiler does not support __attribute__((gcc_struct)) - please use GCC instead of Clang')
>>> -  endif
>>> +  # https://gcc.gnu.org/onlinedocs/gcc/x86-Type-Attributes.html
>>> +  # We use this compilation option instead of relying on gcc_struct attribute
>>> +  # because clang does not support it (but supports the option).
>>> +  qemu_common_flags += ['-mno-ms-bitfields']
>>>    endif
>>
>> Is this really safe for us to use ?   The current gcc_struct
>> attribute affects only structs marked as QEMU_PACKED. This
>> flag will affect all code.
>>
>> If we call from QEMU code into Windows native APIs, and pass
>> or receive structs, then those structs' layouts would be
>> affected by this flag. I don't have a specific example, but
>> this feels unsafe to me, otherwise we would have done this
>> originally rather than only targetting internal packed structs
>> with the gcc_struct attribute.
> 
> I agree with Daniel, we likely cannot use this switch globally.
> 
> But seems like Clang folks are trying to include support for the attribute,
> see: https://gitlab.com/qemu-project/qemu/-/issues/2476#note_2159643081
> so I'd rather recommend to wait for that for proper Clang support here.
> 
>    Thomas
> 

Thanks for your reviews.
(adding Martin to the conversation, who worked on llvm-mingw toolchain, 
and commented on GitLab issue).

As mentioned in gcc documentation, this option applies only to packed 
structures, or when bitfield are used. So this is this second case that 
may be a problem for us indeed.
Looking at mingw windows headers, I could find a few of them, but I 
didn't check if they were used directly or indirectly by our code.

After reading the gitlab issue attached, and links associated, it seems 
that QEMU is one of the only big projects using this. And clang support 
is only blocked by this.
The upstream llvm support for this might take more time than expected 
(the PR was opened more than 1 year ago... and the original report for 
missing attribute support was in 2015), so I'm not very confident this 
will appear "soon".

I noticed that Daniel conducted a small investigation using pahole, and 
the report was that there were not so many difference (one found, but 
only compile x86_64-softmmu target).

I would be willing to perform a full build (all dependencies, and all 
targets), and compare what we obtain.
If we can fix the corner cases, would you be open to accept removing 
gcc_struct from the codebase?
I can understand if it's a big NO, but I think it's unfortunate that we 
are blocked today just because of this.

[PATCH 08/12] plugins: enable linking with clang/lld

[Pierrick Bouvier]

Windows uses a special mechanism to enable plugins to work (DLL delay
loading). Option for lld is different than ld.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 meson.build                   |  4 ++++
 contrib/plugins/meson.build   |  2 +-
 plugins/meson.build           | 24 ++++++++++++++++++++----
 tests/tcg/plugins/meson.build |  3 +--
 4 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/meson.build b/meson.build
index d0d5dbe1479..fe11570b147 100644
--- a/meson.build
+++ b/meson.build
@@ -334,6 +334,10 @@ elif host_os == 'windows'
   # We use this compilation option instead of relying on gcc_struct attribute
   # because clang does not support it (but supports the option).
   qemu_common_flags += ['-mno-ms-bitfields']
+
+  if compiler.get_id() == 'clang' and compiler.get_linker_id() != 'ld.lld'
+    error('On windows, you need to use lld with clang - use msys2 clang64/clangarm64 env')
+  endif
 endif
 
 # Choose instruction set (currently x86-only)
diff --git a/contrib/plugins/meson.build b/contrib/plugins/meson.build
index 63a32c2b4f0..484b9a808c8 100644
--- a/contrib/plugins/meson.build
+++ b/contrib/plugins/meson.build
@@ -12,7 +12,7 @@ if get_option('plugins')
       t += shared_module(i, files(i + '.c') + 'win32_linker.c',
                         include_directories: '../../include/qemu',
                         link_depends: [win32_qemu_plugin_api_lib],
-                        link_args: ['-Lplugins', '-lqemu_plugin_api'],
+                        link_args: win32_qemu_plugin_api_link_flags,
                         dependencies: glib)
     else
       t += shared_module(i, files(i + '.c'),
diff --git a/plugins/meson.build b/plugins/meson.build
index 1cc039d29b2..51297582f93 100644
--- a/plugins/meson.build
+++ b/plugins/meson.build
@@ -11,14 +11,15 @@ if not enable_modules
       capture: true,
       command: ['sed', '-ne', 's/^[[:space:]]*\\(qemu_.*\\);/_\\1/p', '@INPUT@'])
     emulator_link_args += ['-Wl,-exported_symbols_list,plugins/qemu-plugins-ld64.symbols']
+  elif host_os == 'windows' and meson.get_compiler('c').get_id() == 'clang'
+    # LLVM/lld does not support exporting specific symbols. However, it works
+    # out of the box with dllexport/dllimport attribute we set in the code.
   else
     emulator_link_args += ['-Xlinker', '--dynamic-list=' + (meson.project_source_root() / 'plugins/qemu-plugins.symbols')]
   endif
 endif
 
 if host_os == '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
@@ -27,12 +28,27 @@ if host_os == 'windows'
     output: 'qemu_plugin_api.def',
     capture: true,
     command: ['sed', '-e', '0,/^/s//EXPORTS/; s/[{};]//g', '@INPUT@'])
+
   # then use dlltool to assemble a delaylib.
+  # The delaylib will have an "imaginary" name (qemu.exe), that is used by the
+  # linker file we add with plugins (win32_linker.c) to identify that we want
+  # to find missing symbols in current program.
+  win32_qemu_plugin_api_link_flags = ['-Lplugins', '-lqemu_plugin_api']
+  if meson.get_compiler('c').get_id() == 'clang'
+    # With LLVM/lld, delaylib is specified at link time (-delayload)
+    dlltool = find_program('llvm-dlltool', required: true)
+    dlltool_cmd = [dlltool, '-d', '@INPUT@', '-l', '@OUTPUT@', '-D', 'qemu.exe']
+    win32_qemu_plugin_api_link_flags += ['-Wl,-delayload=qemu.exe']
+  else
+    # With gcc/ld, delay lib is built with a specific delay parameter.
+    dlltool = find_program('dlltool', required: true)
+    dlltool_cmd = [dlltool, '--input-def', '@INPUT@',
+                   '--output-delaylib', '@OUTPUT@', '--dllname', 'qemu.exe']
+  endif
   win32_qemu_plugin_api_lib = configure_file(
     input: win32_plugin_def,
     output: 'libqemu_plugin_api.a',
-    command: [dlltool, '--input-def', '@INPUT@',
-              '--output-delaylib', '@OUTPUT@', '--dllname', 'qemu.exe']
+    command: dlltool_cmd
   )
 endif
 specific_ss.add(files(
diff --git a/tests/tcg/plugins/meson.build b/tests/tcg/plugins/meson.build
index f847849b1b7..87a17d67bd4 100644
--- a/tests/tcg/plugins/meson.build
+++ b/tests/tcg/plugins/meson.build
@@ -5,9 +5,8 @@ if get_option('plugins')
       t += shared_module(i, files(i + '.c') + '../../../contrib/plugins/win32_linker.c',
                         include_directories: '../../../include/qemu',
                         link_depends: [win32_qemu_plugin_api_lib],
-                        link_args: ['-Lplugins', '-lqemu_plugin_api'],
+                        link_args: win32_qemu_plugin_api_link_flags,
                         dependencies: glib)
-
     else
       t += shared_module(i, files(i + '.c'),
                         include_directories: '../../../include/qemu',
-- 
2.39.5

[PATCH 09/12] plugins: add missing export for qemu_plugin_num_vcpus

[Pierrick Bouvier]

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 include/qemu/qemu-plugin.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 622c9a02327..0fba36ae028 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -803,6 +803,7 @@ void qemu_plugin_register_atexit_cb(qemu_plugin_id_t id,
                                     qemu_plugin_udata_cb_t cb, void *userdata);
 
 /* returns how many vcpus were started at this point */
+QEMU_PLUGIN_API
 int qemu_plugin_num_vcpus(void);
 
 /**
-- 
2.39.5

Re: [PATCH 09/12] plugins: add missing export for qemu_plugin_num_vcpus

[Philippe Mathieu-Daudé]

On 31/10/24 01:04, Pierrick Bouvier wrote:

Fixes: 4a448b148ca ("plugins: add qemu_plugin_num_vcpus function")

> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   include/qemu/qemu-plugin.h | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index 622c9a02327..0fba36ae028 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -803,6 +803,7 @@ void qemu_plugin_register_atexit_cb(qemu_plugin_id_t id,
>                                       qemu_plugin_udata_cb_t cb, void *userdata);
>   
>   /* returns how many vcpus were started at this point */
> +QEMU_PLUGIN_API
>   int qemu_plugin_num_vcpus(void);
>   
>   /**

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 10/12] plugins: detect qemu plugin API symbols from header

[Pierrick Bouvier]

Instead of using a static file (error prone and hard to keep in sync),
we generate it using a script.

Note: if a symbol is not exported, we'll now notice it when linking for
Windows/MacOS platforms.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 MAINTAINERS                    |  1 +
 plugins/meson.build            | 12 ++++++---
 scripts/qemu-plugin-symbols.py | 45 ++++++++++++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 3 deletions(-)
 create mode 100755 scripts/qemu-plugin-symbols.py

diff --git a/MAINTAINERS b/MAINTAINERS
index f48d9142b8a..d34b6606e95 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3719,6 +3719,7 @@ F: plugins/
 F: tests/tcg/plugins/
 F: tests/avocado/tcg_plugins.py
 F: contrib/plugins/
+F: scripts/qemu-plugin-symbols.py
 
 AArch64 TCG target
 M: Richard Henderson <richard.henderson@linaro.org>
diff --git a/plugins/meson.build b/plugins/meson.build
index 51297582f93..d60be2a4d6d 100644
--- a/plugins/meson.build
+++ b/plugins/meson.build
@@ -2,11 +2,17 @@ if not get_option('plugins')
   subdir_done()
 endif
 
+qemu_plugin_symbols = configure_file(
+  input: files('../include/qemu/qemu-plugin.h'),
+  output: 'qemu-plugin.symbols',
+  capture: true,
+  command: [files('../scripts/qemu-plugin-symbols.py'), '@INPUT@'])
+
 # Modules need more symbols than just those in plugins/qemu-plugins.symbols
 if not enable_modules
   if host_os == 'darwin'
     configure_file(
-      input: files('qemu-plugins.symbols'),
+      input: qemu_plugin_symbols,
       output: 'qemu-plugins-ld64.symbols',
       capture: true,
       command: ['sed', '-ne', 's/^[[:space:]]*\\(qemu_.*\\);/_\\1/p', '@INPUT@'])
@@ -15,7 +21,7 @@ if not enable_modules
     # LLVM/lld does not support exporting specific symbols. However, it works
     # out of the box with dllexport/dllimport attribute we set in the code.
   else
-    emulator_link_args += ['-Xlinker', '--dynamic-list=' + (meson.project_source_root() / 'plugins/qemu-plugins.symbols')]
+    emulator_link_args += ['-Xlinker', '--dynamic-list=' + qemu_plugin_symbols.full_path()]
   endif
 endif
 
@@ -24,7 +30,7 @@ if host_os == 'windows'
   # 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'),
+    input: qemu_plugin_symbols,
     output: 'qemu_plugin_api.def',
     capture: true,
     command: ['sed', '-e', '0,/^/s//EXPORTS/; s/[{};]//g', '@INPUT@'])
diff --git a/scripts/qemu-plugin-symbols.py b/scripts/qemu-plugin-symbols.py
new file mode 100755
index 00000000000..e285ebb8f9e
--- /dev/null
+++ b/scripts/qemu-plugin-symbols.py
@@ -0,0 +1,45 @@
+#!/usr/bin/env python3
+# -*- coding: utf-8 -*-
+#
+# Extract QEMU Plugin API symbols from a header file
+#
+# Copyright 2024 Linaro Ltd
+#
+# Author: Pierrick Bouvier <pierrick.bouvier@linaro.org>
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or later.
+# See the COPYING file in the top-level directory.
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+import argparse
+import re
+
+def extract_symbols(plugin_header):
+    with open(plugin_header) as file:
+        content = file.read()
+    # Remove QEMU_PLUGIN_API macro definition.
+    content = content.replace('#define QEMU_PLUGIN_API', '')
+    expected = content.count('QEMU_PLUGIN_API')
+    # Find last word between QEMU_PLUGIN_API and (, matching on several lines.
+    # We use *? non-greedy quantifier.
+    syms = re.findall(r'QEMU_PLUGIN_API.*?(\w+)\s*\(', content, re.DOTALL)
+    syms.sort()
+    # Ensure we found as many symbols as API markers.
+    assert len(syms) == expected
+    return syms
+
+def main() -> None:
+    parser = argparse.ArgumentParser(description='Extract QEMU plugin symbols')
+    parser.add_argument('plugin_header', help='Path to QEMU plugin header.')
+    args = parser.parse_args()
+
+    syms = extract_symbols(args.plugin_header)
+
+    print('{')
+    for s in syms:
+        print("  {};".format(s))
+    print('};')
+
+if __name__ == '__main__':
+    main()
-- 
2.39.5

[PATCH 11/12] plugins: eradicate qemu-plugins.symbols static file

[Pierrick Bouvier]

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 plugins/qemu-plugins.symbols | 59 ------------------------------------
 1 file changed, 59 deletions(-)
 delete mode 100644 plugins/qemu-plugins.symbols

diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols
deleted file mode 100644
index 032661f9ea7..00000000000
--- a/plugins/qemu-plugins.symbols
+++ /dev/null
@@ -1,59 +0,0 @@
-{
-  qemu_plugin_bool_parse;
-  qemu_plugin_end_code;
-  qemu_plugin_entry_code;
-  qemu_plugin_get_hwaddr;
-  qemu_plugin_get_registers;
-  qemu_plugin_hwaddr_device_name;
-  qemu_plugin_hwaddr_is_io;
-  qemu_plugin_hwaddr_phys_addr;
-  qemu_plugin_insn_data;
-  qemu_plugin_insn_disas;
-  qemu_plugin_insn_haddr;
-  qemu_plugin_insn_size;
-  qemu_plugin_insn_symbol;
-  qemu_plugin_insn_vaddr;
-  qemu_plugin_mem_get_value;
-  qemu_plugin_mem_is_big_endian;
-  qemu_plugin_mem_is_sign_extended;
-  qemu_plugin_mem_is_store;
-  qemu_plugin_mem_size_shift;
-  qemu_plugin_num_vcpus;
-  qemu_plugin_outs;
-  qemu_plugin_path_to_binary;
-  qemu_plugin_read_memory_vaddr;
-  qemu_plugin_read_register;
-  qemu_plugin_register_atexit_cb;
-  qemu_plugin_register_flush_cb;
-  qemu_plugin_register_vcpu_exit_cb;
-  qemu_plugin_register_vcpu_idle_cb;
-  qemu_plugin_register_vcpu_init_cb;
-  qemu_plugin_register_vcpu_insn_exec_cb;
-  qemu_plugin_register_vcpu_insn_exec_cond_cb;
-  qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu;
-  qemu_plugin_register_vcpu_mem_cb;
-  qemu_plugin_register_vcpu_mem_inline_per_vcpu;
-  qemu_plugin_register_vcpu_resume_cb;
-  qemu_plugin_register_vcpu_syscall_cb;
-  qemu_plugin_register_vcpu_syscall_ret_cb;
-  qemu_plugin_register_vcpu_tb_exec_cb;
-  qemu_plugin_register_vcpu_tb_exec_cond_cb;
-  qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu;
-  qemu_plugin_register_vcpu_tb_trans_cb;
-  qemu_plugin_request_time_control;
-  qemu_plugin_reset;
-  qemu_plugin_scoreboard_free;
-  qemu_plugin_scoreboard_find;
-  qemu_plugin_scoreboard_new;
-  qemu_plugin_start_code;
-  qemu_plugin_tb_get_insn;
-  qemu_plugin_tb_n_insns;
-  qemu_plugin_tb_vaddr;
-  qemu_plugin_u64_add;
-  qemu_plugin_u64_get;
-  qemu_plugin_u64_set;
-  qemu_plugin_u64_sum;
-  qemu_plugin_uninstall;
-  qemu_plugin_update_ns;
-  qemu_plugin_vcpu_for_each;
-};
-- 
2.39.5

[PATCH 12/12] docs: add information on how to setup build environments

[Pierrick Bouvier]

MacOS and Linux are straightforward, but Windows needs a bit more
details.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 docs/about/build-platforms.rst |   4 +-
 docs/devel/build-system.rst    | 100 +++++++++++++++++++++++++++++++++
 2 files changed, 103 insertions(+), 1 deletion(-)

diff --git a/docs/about/build-platforms.rst b/docs/about/build-platforms.rst
index 8fd7da140a3..fd1a8cabe36 100644
--- a/docs/about/build-platforms.rst
+++ b/docs/about/build-platforms.rst
@@ -29,6 +29,9 @@ The `Repology`_ site is a useful resource to identify
 currently shipped versions of software in various operating systems,
 though it does not cover all distros listed below.
 
+You can find how to install build dependencies for different systems on the
+:ref:`setup-build-env` page.
+
 Supported host architectures
 ----------------------------
 
@@ -118,7 +121,6 @@ Optional build dependencies
   cross compilation using ``docker`` or ``podman``, or to use pre-built
   binaries distributed with QEMU.
 
-
 Windows
 -------
 
diff --git a/docs/devel/build-system.rst b/docs/devel/build-system.rst
index d42045a2325..fa5601fed82 100644
--- a/docs/devel/build-system.rst
+++ b/docs/devel/build-system.rst
@@ -595,3 +595,103 @@ Built by Makefile:
   meson.build.  The rules are produced from Meson's JSON description of
   tests (obtained with "meson introspect --tests") through the script
   scripts/mtest2make.py.
+
+.. _setup-build-env:
+
+Setup build environment
+=======================
+
+Linux
+-----
+
+Fedora
+++++++
+
+::
+
+    sudo dnf update && sudo dnf builddep qemu
+
+Debian/Ubuntu
++++++++++++++
+
+You first need to enable `Sources List <https://wiki.debian.org/SourcesList>`_.
+Then, use apt to install dependencies:
+
+::
+
+    sudo apt update && sudo apt build-dep qemu
+
+MacOS
+-----
+
+You first need to install `Homebrew <https://brew.sh/>`_. Then, use it to
+install dependencies:
+
+::
+
+    brew update && brew install $(brew deps --include-build qemu)
+
+Windows
+-------
+
+You first need to install `MSYS2 <https://www.msys2.org/>`_.
+MSYS2 offers `different environments <https://www.msys2.org/docs/environments/>`_.
+x86_64 environments are based on GCC, while aarch64 is based on Clang.
+
+We recommend to use UCRT64 for windows-x86_64 and CLANGARM64 for windows-aarch64
+(only available on windows-aarch64 hosts).
+
+Then, you can open a windows shell, and enter msys2 env using:
+
+::
+
+    c:/msys64/msys2_shell.cmd -defterm -here -no-start -ucrt64
+    # Replace -ucrt64 by -clangarm64 or -mingw64 for other environments.
+
+MSYS2 package manager does not offer a built-in way to install build
+dependencies. You can start with this list of packages using pacman:
+
+Note: Dependencies need to be installed again if you use a different MSYS2
+environment.
+
+::
+
+    # update MSYS2 itself, you need to reopen your shell at the end.
+    pacman -Syu
+    pacman -S \
+        base-devel binutils bison diffutils flex git grep make sed \
+        ${MINGW_PACKAGE_PREFIX}-toolchain \
+        ${MINGW_PACKAGE_PREFIX}-glib2 \
+        ${MINGW_PACKAGE_PREFIX}-gtk3 \
+        ${MINGW_PACKAGE_PREFIX}-libnfs \
+        ${MINGW_PACKAGE_PREFIX}-libssh \
+        ${MINGW_PACKAGE_PREFIX}-ninja \
+        ${MINGW_PACKAGE_PREFIX}-pixman \
+        ${MINGW_PACKAGE_PREFIX}-pkgconf \
+        ${MINGW_PACKAGE_PREFIX}-python \
+        ${MINGW_PACKAGE_PREFIX}-SDL2 \
+        ${MINGW_PACKAGE_PREFIX}-zstd
+
+If you want to install all dependencies, it's possible to use recipe used to
+build QEMU in MSYS2 itself.
+
+::
+
+    pacman -S wget
+    wget https://raw.githubusercontent.com/msys2/MINGW-packages/refs/heads/master/mingw-w64-qemu/PKGBUILD
+    # Some packages may be missing for your environment, installation will still
+    # be done though.
+    makepkg -s PKGBUILD || true
+
+Build on windows-aarch64
+++++++++++++++++++++++++
+
+When trying to cross compile meson for x86_64 using UCRT64 or MINGW64 env,
+configure will run into an error because the cpu detected is not correct.
+
+Meson detects x86_64 processes emulated, so you need to manually set the cpu,
+and force a cross compilation (with empty prefix).
+
+::
+
+    ./configure --cpu=x86_64 --cross-prefix=
-- 
2.39.5

Re: [PATCH 12/12] docs: add information on how to setup build environments

[Daniel P. Berrangé]

On Wed, Oct 30, 2024 at 09:04:26PM -0700, Pierrick Bouvier wrote:
> MacOS and Linux are straightforward, but Windows needs a bit more
> details.
> 
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>  docs/about/build-platforms.rst |   4 +-
>  docs/devel/build-system.rst    | 100 +++++++++++++++++++++++++++++++++
>  2 files changed, 103 insertions(+), 1 deletion(-)

> +Windows
> +-------
> +
> +You first need to install `MSYS2 <https://www.msys2.org/>`_.
> +MSYS2 offers `different environments <https://www.msys2.org/docs/environments/>`_.
> +x86_64 environments are based on GCC, while aarch64 is based on Clang.
> +
> +We recommend to use UCRT64 for windows-x86_64 and CLANGARM64 for windows-aarch64
> +(only available on windows-aarch64 hosts).

I don't believe we should be recommending ucrt64 for windows. We have two
testing jobs, one cross compiled from Mingw Fedora, and one native under
Msys2, both of which are validating mingw64. We have no ucrt64 testing.
IIUC, Stefan's QEMU installer binaries are also mingw32/mingw64 based,
not ucrt64.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 12/12] docs: add information on how to setup build environments

[Pierrick Bouvier]

On 10/31/24 02:24, Daniel P. Berrangé wrote:
> On Wed, Oct 30, 2024 at 09:04:26PM -0700, Pierrick Bouvier wrote:
>> MacOS and Linux are straightforward, but Windows needs a bit more
>> details.
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>   docs/about/build-platforms.rst |   4 +-
>>   docs/devel/build-system.rst    | 100 +++++++++++++++++++++++++++++++++
>>   2 files changed, 103 insertions(+), 1 deletion(-)
> 
>> +Windows
>> +-------
>> +
>> +You first need to install `MSYS2 <https://www.msys2.org/>`_.
>> +MSYS2 offers `different environments <https://www.msys2.org/docs/environments/>`_.
>> +x86_64 environments are based on GCC, while aarch64 is based on Clang.
>> +
>> +We recommend to use UCRT64 for windows-x86_64 and CLANGARM64 for windows-aarch64
>> +(only available on windows-aarch64 hosts).
> 
> I don't believe we should be recommending ucrt64 for windows. We have two
> testing jobs, one cross compiled from Mingw Fedora, and one native under
> Msys2, both of which are validating mingw64. We have no ucrt64 testing.
> IIUC, Stefan's QEMU installer binaries are also mingw32/mingw64 based,
> not ucrt64.
> 

I have no strong opinion, and was just following default recommendation 
of MSYS2 project to switch to UCRT64.
All msys2 environment use mingw64, but mingw64 can target different 
windows runtime. From what I understand, any difference or bug would be 
a bug for mingw64 project, not for us.

UCRT is now the default target used by mingw64 [1].
More information on this difference here [2].
Migrating to UCRT64 seems to be the best way in the future.

All that said, I'll update this recommendation to MINGW64 for V2 of this 
series,

[1] https://www.mingw-w64.org/changelog/#v1200-2024-05-29
[2] 
https://github.com/mingw-w64/mingw-w64/blob/master/mingw-w64-doc/howto-build/ucrt-vs-msvcrt.txt

> With regards,
> Daniel

Re: [PATCH 12/12] docs: add information on how to setup build environments

[Peter Maydell]

On Thu, 31 Oct 2024 at 04:07, Pierrick Bouvier
<pierrick.bouvier@linaro.org> wrote:
>
> MacOS and Linux are straightforward, but Windows needs a bit more
> details.
>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

We have documentation on the wiki currently about how to
build on various platforms:

https://wiki.qemu.org/Hosts/Linux
https://wiki.qemu.org/Hosts/Mac
https://wiki.qemu.org/Hosts/W32

I agree that we ought to move this into the main documentation.
Some of the information in those wiki pages is probably
out of date, but some of it looks like useful extra detail
that we could incorporate here.

> ---
>  docs/about/build-platforms.rst |   4 +-
>  docs/devel/build-system.rst    | 100 +++++++++++++++++++++++++++++++++

I don't think this is the best place to put this information.
"How do I build from source?" is a question that most beginning
developers and some end users will have. The information in
build-system.rst is mostly about the internals of our
build system and how to modify it, which is a more niche topic.

I would be inclined to put this in a new file, maybe even
in docs/about/ rather than docs/devel.

thanks
-- PMM

Re: [PATCH 12/12] docs: add information on how to setup build environments

[Michael S. Tsirkin]

On Mon, Nov 04, 2024 at 03:58:19PM +0000, Peter Maydell wrote:
> On Thu, 31 Oct 2024 at 04:07, Pierrick Bouvier
> <pierrick.bouvier@linaro.org> wrote:
> >
> > MacOS and Linux are straightforward, but Windows needs a bit more
> > details.
> >
> > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> 
> We have documentation on the wiki currently about how to
> build on various platforms:
> 
> https://wiki.qemu.org/Hosts/Linux
> https://wiki.qemu.org/Hosts/Mac
> https://wiki.qemu.org/Hosts/W32
> 
> I agree that we ought to move this into the main documentation.
> Some of the information in those wiki pages is probably
> out of date, but some of it looks like useful extra detail
> that we could incorporate here.

Also, as that wiki notes:

32 bit Linux target on a 64 bit Linux host would be useful for build bots



> > ---
> >  docs/about/build-platforms.rst |   4 +-
> >  docs/devel/build-system.rst    | 100 +++++++++++++++++++++++++++++++++
> 
> I don't think this is the best place to put this information.
> "How do I build from source?" is a question that most beginning
> developers and some end users will have. The information in
> build-system.rst is mostly about the internals of our
> build system and how to modify it, which is a more niche topic.
> 
> I would be inclined to put this in a new file, maybe even
> in docs/about/ rather than docs/devel.
> 
> thanks
> -- PMM

Re: [PATCH 12/12] docs: add information on how to setup build environments

[Pierrick Bouvier]

On 11/4/24 08:08, Michael S. Tsirkin wrote:
> On Mon, Nov 04, 2024 at 03:58:19PM +0000, Peter Maydell wrote:
>> On Thu, 31 Oct 2024 at 04:07, Pierrick Bouvier
>> <pierrick.bouvier@linaro.org> wrote:
>>>
>>> MacOS and Linux are straightforward, but Windows needs a bit more
>>> details.
>>>
>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>
>> We have documentation on the wiki currently about how to
>> build on various platforms:
>>
>> https://wiki.qemu.org/Hosts/Linux
>> https://wiki.qemu.org/Hosts/Mac
>> https://wiki.qemu.org/Hosts/W32
>>
>> I agree that we ought to move this into the main documentation.
>> Some of the information in those wiki pages is probably
>> out of date, but some of it looks like useful extra detail
>> that we could incorporate here.
> 
> Also, as that wiki notes:
> 
> 32 bit Linux target on a 64 bit Linux host would be useful for build bots
> 

IMHO, and regarding my previous message, I don't think adding details 
about cross compilation would help new comers.
Cross compilation is specific, and most of the people who do it know how 
to do it. Especially when setting up a build bot.
In more, we already have a docker container for this use case 
(debian-i686-cross), without needing to play with host machine.

> 
> 
>>> ---
>>>   docs/about/build-platforms.rst |   4 +-
>>>   docs/devel/build-system.rst    | 100 +++++++++++++++++++++++++++++++++
>>
>> I don't think this is the best place to put this information.
>> "How do I build from source?" is a question that most beginning
>> developers and some end users will have. The information in
>> build-system.rst is mostly about the internals of our
>> build system and how to modify it, which is a more niche topic.
>>
>> I would be inclined to put this in a new file, maybe even
>> in docs/about/ rather than docs/devel.
>>
>> thanks
>> -- PMM
> 

Re: [PATCH 12/12] docs: add information on how to setup build environments

[Pierrick Bouvier]

On 11/4/24 07:58, Peter Maydell wrote:
> On Thu, 31 Oct 2024 at 04:07, Pierrick Bouvier
> <pierrick.bouvier@linaro.org> wrote:
>>
>> MacOS and Linux are straightforward, but Windows needs a bit more
>> details.
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> 
> We have documentation on the wiki currently about how to
> build on various platforms:
> 
> https://wiki.qemu.org/Hosts/Linux
> https://wiki.qemu.org/Hosts/Mac
> https://wiki.qemu.org/Hosts/W32
> 
> I agree that we ought to move this into the main documentation.
> Some of the information in those wiki pages is probably
> out of date, but some of it looks like useful extra detail
> that we could incorporate here.
> 

I noticed that. Is the QEMU wiki considered as "deprecated" (no new 
information there), or more seen as a complement to current manual?

My goal adding this to the documentation was to have a simple (and 
single) "how-to" per platform, instead of trying to cover all the 
possible details and configuration.

Would that be acceptable to add links to the wiki instead of adding all 
information in our documentation?

>> ---
>>   docs/about/build-platforms.rst |   4 +-
>>   docs/devel/build-system.rst    | 100 +++++++++++++++++++++++++++++++++
> 
> I don't think this is the best place to put this information.
> "How do I build from source?" is a question that most beginning
> developers and some end users will have. The information in
> build-system.rst is mostly about the internals of our
> build system and how to modify it, which is a more niche topic.
> 
> I would be inclined to put this in a new file, maybe even
> in docs/about/ rather than docs/devel.
> 

I was not sure, and adding a new file for new developers seems to be the 
best solution. I'll do it.

> thanks
> -- PMM

Thanks

Re: [PATCH 12/12] docs: add information on how to setup build environments

[Peter Maydell]

On Mon, 4 Nov 2024 at 22:05, Pierrick Bouvier
<pierrick.bouvier@linaro.org> wrote:
>
> On 11/4/24 07:58, Peter Maydell wrote:
> > On Thu, 31 Oct 2024 at 04:07, Pierrick Bouvier
> > <pierrick.bouvier@linaro.org> wrote:
> >>
> >> MacOS and Linux are straightforward, but Windows needs a bit more
> >> details.
> >>
> >> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> >
> > We have documentation on the wiki currently about how to
> > build on various platforms:
> >
> > https://wiki.qemu.org/Hosts/Linux
> > https://wiki.qemu.org/Hosts/Mac
> > https://wiki.qemu.org/Hosts/W32
> >
> > I agree that we ought to move this into the main documentation.
> > Some of the information in those wiki pages is probably
> > out of date, but some of it looks like useful extra detail
> > that we could incorporate here.
> >
>
> I noticed that. Is the QEMU wiki considered as "deprecated" (no new
> information there), or more seen as a complement to current manual?

It's in the usual state of all wikis -- it has a lot of information,
some of which is out of date and some of which is still useful,
and none of which is particularly well organized. Some of it
we should definitely move into our main documentation.

> My goal adding this to the documentation was to have a simple (and
> single) "how-to" per platform, instead of trying to cover all the
> possible details and configuration.
>
> Would that be acceptable to add links to the wiki instead of adding all
> information in our documentation?

I think if we want to document the build process in our
"real" documentation (which we should) then we should
remove the wiki pages (or replace their content with
pointers to the docs). Otherwise we just have two
places with the same information. If we decide that
some of the information on the wiki pages is too stale
or too niche to include that's fine.

thanks
-- PMM

Re: [PATCH 00/12] Enable building plugins on Windows with Clang

[Pierrick Bouvier]

On 10/30/24 21:04, Pierrick Bouvier wrote:
> For now, it was only possible to build plugins using GCC on Windows. However,
> windows-aarch64 only supports Clang.
> 
> The first patches (already posted and normally pulled) are fixes and
> prerequisite to build plugins with meson. They are integrated here because it's
> not possible to have two based-on in the cover letter.
> 
> Then, we fix compilation warnings on Windows.
> After that, we can enable linking with Clang on Windows.
> 
> Finally, we enhanced the plugin symbols export list (automatically generated,
> and not a static file), and we report some of the information we discovered on
> the path in the documentation.
> 
> Built and tested on Windows (all msys env)/Linux/MacOS for x86_64 and aarch64
> hosts.
> 
> Pierrick Bouvier (12):
>    scripts: remove erroneous file that breaks git clone on Windows
>    contrib/plugins/cflow: fix warning
>    meson: build contrib/plugins with meson
>    contrib/plugins: remove Makefile for contrib/plugins
>    qga: fix -Wsometimes-uninitialized windows warning
>    qga: fix missing static and prototypes windows warnings
>    win32: use compiler option instead of attribute gcc_struct
>    plugins: enable linking with clang/lld
>    plugins: add missing export for qemu_plugin_num_vcpus
>    plugins: detect qemu plugin API symbols from header
>    plugins: eradicate qemu-plugins.symbols static file
>    docs: add information on how to setup build environments
> 
>   MAINTAINERS                               |   1 +
>   docs/about/build-platforms.rst            |   4 +-
>   docs/devel/build-system.rst               | 100 ++++++++++++++++++++++
>   configure                                 |  18 ----
>   Makefile                                  |  10 ---
>   meson.build                               |  14 ++-
>   include/qemu/compiler.h                   |   7 +-
>   include/qemu/qemu-plugin.h                |   1 +
>   subprojects/libvhost-user/libvhost-user.h |   6 +-
>   contrib/plugins/cflow.c                   |   6 +-
>   qga/commands-windows-ssh.c                |   2 +-
>   contrib/plugins/Makefile                  |  87 -------------------
>   contrib/plugins/meson.build               |  28 ++++++
>   plugins/meson.build                       |  36 ++++++--
>   plugins/qemu-plugins.symbols              |  59 -------------
>   qga/vss-win32/install.cpp                 |   6 +-
>   qga/vss-win32/provider.cpp                |   5 +-
>   qga/vss-win32/requester.cpp               |   8 +-
>   scripts/meson-buildoptions.               |   0
>   scripts/qemu-plugin-symbols.py            |  45 ++++++++++
>   tests/tcg/plugins/meson.build             |   3 +-
>   21 files changed, 238 insertions(+), 208 deletions(-)
>   delete mode 100644 contrib/plugins/Makefile
>   create mode 100644 contrib/plugins/meson.build
>   delete mode 100644 plugins/qemu-plugins.symbols
>   delete mode 100644 scripts/meson-buildoptions.
>   create mode 100755 scripts/qemu-plugin-symbols.py
> 

I won't make a v2 of this series, as the build with clang is currently 
impossible, due to use of gcc_struct attribute. Maintainers have a 
strong preference to wait for official support in clang.

Most of the interesting patches (build system, warning fix) have been 
pulled or will be though.