[PATCH v4 0/4] Fix sanitizer errors with clang 18.1.1

[PATCH v4 0/4] Fix sanitizer errors with clang 18.1.1

[Akihiko Odaki]

I upgraded my Fedora Asahi Remix from 39 to 40 and found new sanitizer
errors with clang it ships so here are fixes.

The patch "meson: Drop the .fa library prefix" may have a broad impact
to the build system so please tell me if you have a concern with it.

To: Michael Tokarev <mjt@tls.msk.ru>
To: Laurent Vivier <laurent@vivier.eu>
To: Paolo Bonzini <pbonzini@redhat.com>
To: Marc-André Lureau <marcandre.lureau@redhat.com>
To: Daniel P. Berrangé <berrange@redhat.com>
To: Thomas Huth <thuth@redhat.com>
To: Philippe Mathieu-Daudé <philmd@linaro.org>
To: Alex Bennée <alex.bennee@linaro.org>
To: Wainer dos Santos Moschetta <wainersm@redhat.com>
To: Beraldo Leal <bleal@redhat.com>
To: Richard Henderson <richard.henderson@linaro.org>
To: Laurent Vivier <lvivier@redhat.com>
Cc: qemu-devel@nongnu.org
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Changes in v4:
- Fixed function pointer problems instead of ignoring them.
- Made references to allocations static instead of incompletely freeing
  them for qemu-keymap.
- s/prefix/suffix/ for patch "meson: Drop the .fa library suffix".
- Link to v3: https://lore.kernel.org/r/20240522-xkb-v3-0-c429de860fa1@daynix.com

Changes in v3:
- Moved changes that should belong to patch "meson: Drop the .fa library
  prefix" from patch "meson: Add -fno-sanitize=function".
- Link to v2: https://lore.kernel.org/r/20240522-xkb-v2-0-67b54fa7c98f@daynix.com

Changes in v2:
- Added more patches and converted them to a series.
- Link to v1: https://lore.kernel.org/r/20240501-xkb-v1-1-f046d8e11a2b@daynix.com

---
Akihiko Odaki (4):
      qemu-keymap: Make references to allocations static
      lockable: Do not cast function pointers
      qapi: Do not cast function pointers
      meson: Drop the .fa library suffix

 docs/devel/build-system.rst         |  5 -----
 meson.build                         | 17 ++---------------
 include/qapi/clone-visitor.h        | 37 ++++++++++++++++++++++++-------------
 include/qemu/lockable.h             | 23 +++++++++++++++++++----
 qapi/qapi-clone-visitor.c           | 30 ++++--------------------------
 qemu-keymap.c                       |  8 +++-----
 stubs/blk-exp-close-all.c           |  2 +-
 .gitlab-ci.d/buildtest-template.yml |  2 --
 .gitlab-ci.d/buildtest.yml          |  2 --
 gdbstub/meson.build                 |  2 --
 tcg/meson.build                     |  2 --
 tests/Makefile.include              |  2 +-
 tests/qtest/libqos/meson.build      |  1 -
 13 files changed, 54 insertions(+), 79 deletions(-)
---
base-commit: c25df57ae8f9fe1c72eee2dab37d76d904ac382e
change-id: 20240501-xkb-258483ccc5d8

Best regards,
-- 
Akihiko Odaki <akihiko.odaki@daynix.com>

[PATCH v4 1/4] qemu-keymap: Make references to allocations static

[Akihiko Odaki]

LeakSanitizer complains about allocations whose references are held
only by automatic variables. It is possible to free them to suppress
the complaints, but it is a chore to make sure they are freed in all
exit paths so make them static instead.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 qemu-keymap.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/qemu-keymap.c b/qemu-keymap.c
index 8c80f7a4ed65..701e4332af87 100644
--- a/qemu-keymap.c
+++ b/qemu-keymap.c
@@ -154,9 +154,9 @@ static xkb_mod_mask_t get_mod(struct xkb_keymap *map, const char *name)
 
 int main(int argc, char *argv[])
 {
-    struct xkb_context *ctx;
-    struct xkb_keymap *map;
-    struct xkb_state *state;
+    static struct xkb_context *ctx;
+    static struct xkb_keymap *map;
+    static struct xkb_state *state;
     xkb_mod_index_t mod, mods;
     int rc;
 
@@ -234,8 +234,6 @@ int main(int argc, char *argv[])
 
     state = xkb_state_new(map);
     xkb_keymap_key_for_each(map, walk_map, state);
-    xkb_state_unref(state);
-    state = NULL;
 
     /* add quirks */
     fprintf(outfile,

-- 
2.45.1

[PATCH v4 2/4] lockable: Do not cast function pointers

[Akihiko Odaki]

-fsanitize=undefined complains if function pointers are casted. It
also prevents enabling teh strict mode of CFI which is currently
disabled with -fsanitize-cfi-icall-generalize-pointers.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2345
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 include/qemu/lockable.h | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/include/qemu/lockable.h b/include/qemu/lockable.h
index 9823220446d9..c1b097c44879 100644
--- a/include/qemu/lockable.h
+++ b/include/qemu/lockable.h
@@ -43,15 +43,30 @@ qemu_null_lockable(void *x)
     return NULL;
 }
 
+#define QML_FUNC_(name)                                           \
+    static inline void qemu_lockable_ ## name ## _lock(void *x)   \
+    {                                                             \
+        qemu_ ## name ## _lock(x);                                \
+    }                                                             \
+    static inline void qemu_lockable_ ## name ## _unlock(void *x) \
+    {                                                             \
+        qemu_ ## name ## _unlock(x);                              \
+    }
+
+QML_FUNC_(mutex)
+QML_FUNC_(rec_mutex)
+QML_FUNC_(co_mutex)
+QML_FUNC_(spin)
+
 /*
  * In C, compound literals have the lifetime of an automatic variable.
  * In C++ it would be different, but then C++ wouldn't need QemuLockable
  * either...
  */
-#define QML_OBJ_(x, name) (&(QemuLockable) {                            \
-        .object = (x),                                                  \
-        .lock = (QemuLockUnlockFunc *) qemu_ ## name ## _lock,          \
-        .unlock = (QemuLockUnlockFunc *) qemu_ ## name ## _unlock       \
+#define QML_OBJ_(x, name) (&(QemuLockable) {        \
+        .object = (x),                              \
+        .lock = qemu_lockable_ ## name ## _lock,    \
+        .unlock = qemu_lockable_ ## name ## _unlock \
     })
 
 /**

-- 
2.45.1

[PATCH v4 3/4] qapi: Do not cast function pointers

[Akihiko Odaki]

-fsanitize=undefined complains if function pointers are casted. It
also prevents enabling teh strict mode of CFI which is currently
disabled with -fsanitize-cfi-icall-generalize-pointers.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2346
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 include/qapi/clone-visitor.h | 37 ++++++++++++++++++++++++-------------
 qapi/qapi-clone-visitor.c    | 30 ++++--------------------------
 2 files changed, 28 insertions(+), 39 deletions(-)

diff --git a/include/qapi/clone-visitor.h b/include/qapi/clone-visitor.h
index adf9a788e232..ebc182b034d7 100644
--- a/include/qapi/clone-visitor.h
+++ b/include/qapi/clone-visitor.h
@@ -11,6 +11,7 @@
 #ifndef QAPI_CLONE_VISITOR_H
 #define QAPI_CLONE_VISITOR_H
 
+#include "qapi/error.h"
 #include "qapi/visitor.h"
 
 /*
@@ -20,11 +21,8 @@
  */
 typedef struct QapiCloneVisitor QapiCloneVisitor;
 
-void *qapi_clone(const void *src, bool (*visit_type)(Visitor *, const char *,
-                                                     void **, Error **));
-void qapi_clone_members(void *dst, const void *src, size_t sz,
-                        bool (*visit_type_members)(Visitor *, void *,
-                                                   Error **));
+Visitor *qapi_clone_visitor_new(void);
+Visitor *qapi_clone_members_visitor_new(void);
 
 /*
  * Deep-clone QAPI object @src of the given @type, and return the result.
@@ -32,10 +30,18 @@ void qapi_clone_members(void *dst, const void *src, size_t sz,
  * Not usable on QAPI scalars (integers, strings, enums), nor on a
  * QAPI object that references the 'any' type.  Safe when @src is NULL.
  */
-#define QAPI_CLONE(type, src)                                           \
-    ((type *)qapi_clone(src,                                            \
-                        (bool (*)(Visitor *, const char *, void **,     \
-                                  Error **))visit_type_ ## type))
+#define QAPI_CLONE(type, src)                                   \
+    ({                                                          \
+        Visitor *v_;                                            \
+        type *dst_ = (type *) (src); /* Cast away const */      \
+                                                                \
+        if (dst_) {                                             \
+            v_ = qapi_clone_visitor_new();                      \
+            visit_type_ ## type(v_, NULL, &dst_, &error_abort); \
+            visit_free(v_);                                     \
+        }                                                       \
+        dst_;                                                   \
+    })
 
 /*
  * Copy deep clones of @type members from @src to @dst.
@@ -43,9 +49,14 @@ void qapi_clone_members(void *dst, const void *src, size_t sz,
  * Not usable on QAPI scalars (integers, strings, enums), nor on a
  * QAPI object that references the 'any' type.
  */
-#define QAPI_CLONE_MEMBERS(type, dst, src)                              \
-    qapi_clone_members(dst, src, sizeof(type),                          \
-                       (bool (*)(Visitor *, void *,                     \
-                                 Error **))visit_type_ ## type ## _members)
+#define QAPI_CLONE_MEMBERS(type, dst, src)                                \
+    ({                                                                    \
+        Visitor *v_;                                                      \
+                                                                          \
+        v_ = qapi_clone_members_visitor_new();                            \
+        *(type *)(dst) = *(src);                                          \
+        visit_type_ ## type ## _members(v_, (type *)(dst), &error_abort); \
+        visit_free(v_);                                                   \
+    })
 
 #endif
diff --git a/qapi/qapi-clone-visitor.c b/qapi/qapi-clone-visitor.c
index c45c5caa3b89..bbf953698f38 100644
--- a/qapi/qapi-clone-visitor.c
+++ b/qapi/qapi-clone-visitor.c
@@ -149,7 +149,7 @@ static void qapi_clone_free(Visitor *v)
     g_free(v);
 }
 
-static Visitor *qapi_clone_visitor_new(void)
+Visitor *qapi_clone_visitor_new(void)
 {
     QapiCloneVisitor *v;
 
@@ -174,31 +174,9 @@ static Visitor *qapi_clone_visitor_new(void)
     return &v->visitor;
 }
 
-void *qapi_clone(const void *src, bool (*visit_type)(Visitor *, const char *,
-                                                     void **, Error **))
+Visitor *qapi_clone_members_visitor_new(void)
 {
-    Visitor *v;
-    void *dst = (void *) src; /* Cast away const */
-
-    if (!src) {
-        return NULL;
-    }
-
-    v = qapi_clone_visitor_new();
-    visit_type(v, NULL, &dst, &error_abort);
-    visit_free(v);
-    return dst;
-}
-
-void qapi_clone_members(void *dst, const void *src, size_t sz,
-                        bool (*visit_type_members)(Visitor *, void *,
-                                                   Error **))
-{
-    Visitor *v;
-
-    v = qapi_clone_visitor_new();
-    memcpy(dst, src, sz);
+    Visitor *v = qapi_clone_visitor_new();
     to_qcv(v)->depth++;
-    visit_type_members(v, dst, &error_abort);
-    visit_free(v);
+    return v;
 }

-- 
2.45.1

[PATCH v4 4/4] meson: Drop the .fa library suffix

[Akihiko Odaki]

The non-standard .fa library suffix breaks the link source
de-duplication done by Meson so drop it.

The lack of link source de-duplication causes AddressSanitizer to
complain ODR violations, and makes GNU ld abort when combined with
clang's LTO.

Previously, the non-standard suffix was necessary for fork-fuzzing.
Meson wraps all standard-suffixed libraries with --start-group and
--end-group. This made a fork-fuzz.ld linker script wrapped as well and
broke builds. Commit d2e6f9272d33 ("fuzz: remove fork-fuzzing
scaffolding") dropped fork-fuzzing so we can now restore the standard
suffix.

The occurences of the suffix were detected and removed by performing
a tree-wide search with 'fa' and .fa (note the quotes and dot).

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 docs/devel/build-system.rst         |  5 -----
 meson.build                         | 17 ++---------------
 stubs/blk-exp-close-all.c           |  2 +-
 .gitlab-ci.d/buildtest-template.yml |  2 --
 .gitlab-ci.d/buildtest.yml          |  2 --
 gdbstub/meson.build                 |  2 --
 tcg/meson.build                     |  2 --
 tests/Makefile.include              |  2 +-
 tests/qtest/libqos/meson.build      |  1 -
 9 files changed, 4 insertions(+), 31 deletions(-)

diff --git a/docs/devel/build-system.rst b/docs/devel/build-system.rst
index 09caf2f8e199..5baf027b7614 100644
--- a/docs/devel/build-system.rst
+++ b/docs/devel/build-system.rst
@@ -236,15 +236,10 @@ Subsystem sourcesets:
   are then turned into static libraries as follows::
 
     libchardev = static_library('chardev', chardev_ss.sources(),
-                                name_suffix: 'fa',
                                 build_by_default: false)
 
     chardev = declare_dependency(link_whole: libchardev)
 
-  As of Meson 0.55.1, the special ``.fa`` suffix should be used for everything
-  that is used with ``link_whole``, to ensure that the link flags are placed
-  correctly in the command line.
-
 Target-independent emulator sourcesets:
   Various general purpose helper code is compiled only once and
   the .o files are linked into all output binaries that need it.
diff --git a/meson.build b/meson.build
index 91a0aa64c640..d6549722b50d 100644
--- a/meson.build
+++ b/meson.build
@@ -3462,14 +3462,12 @@ endif
 qom_ss = qom_ss.apply({})
 libqom = static_library('qom', qom_ss.sources() + genh,
                         dependencies: [qom_ss.dependencies()],
-                        name_suffix: 'fa',
                         build_by_default: false)
 qom = declare_dependency(link_whole: libqom)
 
 event_loop_base = files('event-loop-base.c')
 event_loop_base = static_library('event-loop-base',
                                  sources: event_loop_base + genh,
-                                 name_suffix: 'fa',
                                  build_by_default: false)
 event_loop_base = declare_dependency(link_whole: event_loop_base,
                                      dependencies: [qom])
@@ -3703,7 +3701,6 @@ qemu_syms = custom_target('qemu.syms', output: 'qemu.syms',
 authz_ss = authz_ss.apply({})
 libauthz = static_library('authz', authz_ss.sources() + genh,
                           dependencies: [authz_ss.dependencies()],
-                          name_suffix: 'fa',
                           build_by_default: false)
 
 authz = declare_dependency(link_whole: libauthz,
@@ -3712,7 +3709,6 @@ authz = declare_dependency(link_whole: libauthz,
 crypto_ss = crypto_ss.apply({})
 libcrypto = static_library('crypto', crypto_ss.sources() + genh,
                            dependencies: [crypto_ss.dependencies()],
-                           name_suffix: 'fa',
                            build_by_default: false)
 
 crypto = declare_dependency(link_whole: libcrypto,
@@ -3722,13 +3718,11 @@ io_ss = io_ss.apply({})
 libio = static_library('io', io_ss.sources() + genh,
                        dependencies: [io_ss.dependencies()],
                        link_with: libqemuutil,
-                       name_suffix: 'fa',
                        build_by_default: false)
 
 io = declare_dependency(link_whole: libio, dependencies: [crypto, qom])
 
 libmigration = static_library('migration', sources: migration_files + genh,
-                              name_suffix: 'fa',
                               build_by_default: false)
 migration = declare_dependency(link_with: libmigration,
                                dependencies: [zlib, qom, io])
@@ -3738,7 +3732,6 @@ block_ss = block_ss.apply({})
 libblock = static_library('block', block_ss.sources() + genh,
                           dependencies: block_ss.dependencies(),
                           link_depends: block_syms,
-                          name_suffix: 'fa',
                           build_by_default: false)
 
 block = declare_dependency(link_whole: [libblock],
@@ -3748,7 +3741,6 @@ block = declare_dependency(link_whole: [libblock],
 blockdev_ss = blockdev_ss.apply({})
 libblockdev = static_library('blockdev', blockdev_ss.sources() + genh,
                              dependencies: blockdev_ss.dependencies(),
-                             name_suffix: 'fa',
                              build_by_default: false)
 
 blockdev = declare_dependency(link_whole: [libblockdev],
@@ -3757,13 +3749,11 @@ blockdev = declare_dependency(link_whole: [libblockdev],
 qmp_ss = qmp_ss.apply({})
 libqmp = static_library('qmp', qmp_ss.sources() + genh,
                         dependencies: qmp_ss.dependencies(),
-                        name_suffix: 'fa',
                         build_by_default: false)
 
 qmp = declare_dependency(link_whole: [libqmp])
 
 libchardev = static_library('chardev', chardev_ss.sources() + genh,
-                            name_suffix: 'fa',
                             dependencies: chardev_ss.dependencies(),
                             build_by_default: false)
 
@@ -3771,7 +3761,6 @@ chardev = declare_dependency(link_whole: libchardev)
 
 hwcore_ss = hwcore_ss.apply({})
 libhwcore = static_library('hwcore', sources: hwcore_ss.sources() + genh,
-                           name_suffix: 'fa',
                            build_by_default: false)
 hwcore = declare_dependency(link_whole: libhwcore)
 common_ss.add(hwcore)
@@ -3807,8 +3796,7 @@ common_all = static_library('common',
                             sources: common_ss.all_sources() + genh,
                             include_directories: common_user_inc,
                             implicit_include_directories: false,
-                            dependencies: common_ss.all_dependencies(),
-                            name_suffix: 'fa')
+                            dependencies: common_ss.all_dependencies())
 
 feature_to_c = find_program('scripts/feature_to_c.py')
 
@@ -3909,8 +3897,7 @@ foreach target : target_dirs
                  objects: objects,
                  include_directories: target_inc,
                  c_args: c_args,
-                 build_by_default: false,
-                 name_suffix: 'fa')
+                 build_by_default: false)
 
   if target.endswith('-softmmu')
     execs = [{
diff --git a/stubs/blk-exp-close-all.c b/stubs/blk-exp-close-all.c
index 1c7131676392..2f68e06d7d05 100644
--- a/stubs/blk-exp-close-all.c
+++ b/stubs/blk-exp-close-all.c
@@ -1,7 +1,7 @@
 #include "qemu/osdep.h"
 #include "block/export.h"
 
-/* Only used in programs that support block exports (libblockdev.fa) */
+/* Only used in programs that support block exports (libblockdev.a) */
 void blk_exp_close_all(void)
 {
 }
diff --git a/.gitlab-ci.d/buildtest-template.yml b/.gitlab-ci.d/buildtest-template.yml
index 22045add8064..69e468a576ba 100644
--- a/.gitlab-ci.d/buildtest-template.yml
+++ b/.gitlab-ci.d/buildtest-template.yml
@@ -45,10 +45,8 @@
     exclude:
       - build/**/*.p
       - build/**/*.a.p
-      - build/**/*.fa.p
       - build/**/*.c.o
       - build/**/*.c.o.d
-      - build/**/*.fa
 
 .common_test_job_template:
   extends: .base_job_template
diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index cfdff175c389..c156e6f1d90e 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -178,10 +178,8 @@ build-previous-qemu:
     exclude:
       - build-previous/**/*.p
       - build-previous/**/*.a.p
-      - build-previous/**/*.fa.p
       - build-previous/**/*.c.o
       - build-previous/**/*.c.o.d
-      - build-previous/**/*.fa
   needs:
     job: amd64-opensuse-leap-container
   variables:
diff --git a/gdbstub/meson.build b/gdbstub/meson.build
index da5721d8452b..c91e398ae726 100644
--- a/gdbstub/meson.build
+++ b/gdbstub/meson.build
@@ -19,13 +19,11 @@ gdb_system_ss = gdb_system_ss.apply({})
 
 libgdb_user = static_library('gdb_user',
                              gdb_user_ss.sources() + genh,
-                             name_suffix: 'fa',
                              c_args: '-DCONFIG_USER_ONLY',
                              build_by_default: false)
 
 libgdb_system = static_library('gdb_system',
                                 gdb_system_ss.sources() + genh,
-                                name_suffix: 'fa',
                                 build_by_default: false)
 
 gdb_user = declare_dependency(link_whole: libgdb_user)
diff --git a/tcg/meson.build b/tcg/meson.build
index 8251589fd4e9..f941413d5801 100644
--- a/tcg/meson.build
+++ b/tcg/meson.build
@@ -31,7 +31,6 @@ tcg_ss = tcg_ss.apply({})
 
 libtcg_user = static_library('tcg_user',
                              tcg_ss.sources() + genh,
-                             name_suffix: 'fa',
                              c_args: '-DCONFIG_USER_ONLY',
                              build_by_default: false)
 
@@ -41,7 +40,6 @@ user_ss.add(tcg_user)
 
 libtcg_system = static_library('tcg_system',
                                 tcg_ss.sources() + genh,
-                                name_suffix: 'fa',
                                 c_args: '-DCONFIG_SOFTMMU',
                                 build_by_default: false)
 
diff --git a/tests/Makefile.include b/tests/Makefile.include
index c9d1674bd070..d39d5dd6a43e 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -87,7 +87,7 @@ distclean-tcg: $(DISTCLEAN_TCG_TARGET_RULES)
 .PHONY: check-venv check-avocado check-acceptance check-acceptance-deprecated-warning
 
 # Build up our target list from the filtered list of ninja targets
-TARGETS=$(patsubst libqemu-%.fa, %, $(filter libqemu-%.fa, $(ninja-targets)))
+TARGETS=$(patsubst libqemu-%.a, %, $(filter libqemu-%.a, $(ninja-targets)))
 
 TESTS_VENV_TOKEN=$(BUILD_DIR)/pyvenv/tests.group
 TESTS_RESULTS_DIR=$(BUILD_DIR)/tests/results
diff --git a/tests/qtest/libqos/meson.build b/tests/qtest/libqos/meson.build
index 3aed6efcb8d1..45b81c83ade3 100644
--- a/tests/qtest/libqos/meson.build
+++ b/tests/qtest/libqos/meson.build
@@ -68,7 +68,6 @@ if have_virtfs
 endif
 
 libqos = static_library('qos', libqos_srcs + genh,
-                        name_suffix: 'fa',
                         build_by_default: false)
 
 qos = declare_dependency(link_whole: libqos)

-- 
2.45.1

Re: [PATCH v4 1/4] qemu-keymap: Make references to allocations static

[Philippe Mathieu-Daudé]

On 24/5/24 07:35, Akihiko Odaki wrote:
> LeakSanitizer complains about allocations whose references are held
> only by automatic variables. It is possible to free them to suppress
> the complaints, but it is a chore to make sure they are freed in all
> exit paths so make them static instead.
> 
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>   qemu-keymap.c | 8 +++-----
>   1 file changed, 3 insertions(+), 5 deletions(-)

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

Re: [PATCH v4 2/4] lockable: Do not cast function pointers

[Philippe Mathieu-Daudé]

On 24/5/24 07:35, Akihiko Odaki wrote:
> -fsanitize=undefined complains if function pointers are casted. It
> also prevents enabling teh strict mode of CFI which is currently
> disabled with -fsanitize-cfi-icall-generalize-pointers.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2345
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>   include/qemu/lockable.h | 23 +++++++++++++++++++----
>   1 file changed, 19 insertions(+), 4 deletions(-)

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

Re: [PATCH v4 2/4] lockable: Do not cast function pointers

[Philippe Mathieu-Daudé]

On 27/5/24 11:57, Philippe Mathieu-Daudé wrote:
> On 24/5/24 07:35, Akihiko Odaki wrote:
>> -fsanitize=undefined complains if function pointers are casted. It
>> also prevents enabling teh strict mode of CFI which is currently

s/teh/the/ (also next patch)

>> disabled with -fsanitize-cfi-icall-generalize-pointers.
>>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2345
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>> ---
>>   include/qemu/lockable.h | 23 +++++++++++++++++++----
>>   1 file changed, 19 insertions(+), 4 deletions(-)
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 

Re: [PATCH v4 3/4] qapi: Do not cast function pointers

[Markus Armbruster]

Akihiko Odaki <akihiko.odaki@daynix.com> writes:

> -fsanitize=undefined complains if function pointers are casted. It
> also prevents enabling teh strict mode of CFI which is currently

Typo: the

> disabled with -fsanitize-cfi-icall-generalize-pointers.

The above describes the problem the patch solves.  Good!  Two
suggestions:

1. Quote the error message.

2. Briefly describe the solution as well.  Perhaps:

  The problematic casts are necessary to pass visit_type_T() and
  visit_type_T_members() as callbacks to qapi_clone() and
  qapi_clone_members(), respectively.  Open-code these two functions to
  avoid the callbacks, and thus the type casts.

> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2346
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Always kind of sad to move implementation code to headers, but getting
rid of the function pointer casts makes sense, and I don't have better
ideas for doing that.

With an improved commit message
Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [PATCH v4 0/4] Fix sanitizer errors with clang 18.1.1

[Thomas Huth]

On 24/05/2024 07.35, Akihiko Odaki wrote:
> I upgraded my Fedora Asahi Remix from 39 to 40 and found new sanitizer
> errors with clang it ships so here are fixes.
> 
> The patch "meson: Drop the .fa library prefix" may have a broad impact
> to the build system so please tell me if you have a concern with it.
> 
> To: Michael Tokarev <mjt@tls.msk.ru>
> To: Laurent Vivier <laurent@vivier.eu>
> To: Paolo Bonzini <pbonzini@redhat.com>
> To: Marc-André Lureau <marcandre.lureau@redhat.com>
> To: Daniel P. Berrangé <berrange@redhat.com>
> To: Thomas Huth <thuth@redhat.com>
> To: Philippe Mathieu-Daudé <philmd@linaro.org>
> To: Alex Bennée <alex.bennee@linaro.org>
> To: Wainer dos Santos Moschetta <wainersm@redhat.com>
> To: Beraldo Leal <bleal@redhat.com>
> To: Richard Henderson <richard.henderson@linaro.org>
> To: Laurent Vivier <lvivier@redhat.com>
> Cc: qemu-devel@nongnu.org
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> 
> Changes in v4:
> - Fixed function pointer problems instead of ignoring them.
> - Made references to allocations static instead of incompletely freeing
>    them for qemu-keymap.
> - s/prefix/suffix/ for patch "meson: Drop the .fa library suffix".
> - Link to v3: https://lore.kernel.org/r/20240522-xkb-v3-0-c429de860fa1@daynix.com
> 
> Changes in v3:
> - Moved changes that should belong to patch "meson: Drop the .fa library
>    prefix" from patch "meson: Add -fno-sanitize=function".
> - Link to v2: https://lore.kernel.org/r/20240522-xkb-v2-0-67b54fa7c98f@daynix.com
> 
> Changes in v2:
> - Added more patches and converted them to a series.
> - Link to v1: https://lore.kernel.org/r/20240501-xkb-v1-1-f046d8e11a2b@daynix.com
> 
> ---
> Akihiko Odaki (4):
>        qemu-keymap: Make references to allocations static
>        lockable: Do not cast function pointers
>        qapi: Do not cast function pointers
>        meson: Drop the .fa library suffix

FYI, I'll try to pick up patches 1 - 3 for my next pull request (updating 
the commit description of patch 3 according to Markus' suggestions).

Patch 4 does not apply cleanly anymore, so it needs a respin, but I'd also 
like to see this going through Paolo's meson tree if possible.

  Thomas