[PATCH] rust/wrapper.h: define memory_order enum

[PATCH] rust/wrapper.h: define memory_order enum

[Manos Pitsidianakis]

Add stub definition of memory_order enum in wrapper.h.

Creating Rust bindings from C code is done by passing the wrapper.h
header to `bindgen`. This fails when library dependencies that use
compiler headers are enabled, and the libclang that bindgen detects does
not match the expected clang version. So far this has only been observed
with the memory_order enum symbols from stdatomic.h. If we add the enum
definition to wrapper.h ourselves, the error does not happen.

Before this commit, if the mismatch happened the following error could
come up:

  /usr/include/liburing/barrier.h:72:10: error: use of undeclared identifier 'memory_order_release'
  /usr/include/liburing/barrier.h:75:9: error: use of undeclared identifier 'memory_order_acquire'
  /usr/include/liburing/barrier.h:75:9: error: use of undeclared identifier 'memory_order_acquire'
  /usr/include/liburing/barrier.h:68:9: error: use of undeclared identifier 'memory_order_relaxed'
  /usr/include/liburing/barrier.h:65:17: error: use of undeclared identifier 'memory_order_relaxed'
  /usr/include/liburing/barrier.h:75:9: error: use of undeclared identifier 'memory_order_acquire'
  /usr/include/liburing/barrier.h:75:9: error: use of undeclared identifier 'memory_order_acquire'
  /usr/include/liburing/barrier.h:72:10: error: use of undeclared identifier 'memory_order_release'
  panicked at [..]/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-cli-0.70.1/main.rs:45:36:
  Unable to generate bindings

To fix this (on my system) I would have to export CLANG_PATH and
LIBCLANG_PATH:

  export CLANG_PATH=/bin/clang-17
  export LIBCLANG_PATH=/usr/lib/llvm-17/lib

With these changes applied, bindgen is successful with both the
environment variables set and unset.

Since we're not using those symbols in the bindings (they are only used
by dependencies) this does not affect the generated bindings in any way.

Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
---
 configure      |  7 -------
 meson.build    |  7 -------
 rust/wrapper.h | 17 +++++++++++++++++
 3 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/configure b/configure
index 72d1a94225051ff6e9da9e9256fc8c30aa728f67..83c7636c9a9711e3a1c860e0aac52249a909eaaf 100755
--- a/configure
+++ b/configure
@@ -2060,10 +2060,3 @@ echo ' "$@"' >>config.status
 chmod +x config.status
 
 rm -r "$TMPDIR1"
-
-if test "$rust" != disabled; then
-  echo '\nINFO: Rust bindings generation with `bindgen` might fail in some cases where'
-  echo 'the detected `libclang` does not match the expected `clang` version/target. In'
-  echo 'this case you must pass the path to `clang` and `libclang` to your build'
-  echo 'command invocation using the environment variables CLANG_PATH and LIBCLANG_PATH'
-fi
diff --git a/meson.build b/meson.build
index d26690ce204016d43848d8a73a2d4351a620c5c2..dd4908501037e23c4c6ab165c5809f623c93360b 100644
--- a/meson.build
+++ b/meson.build
@@ -3956,13 +3956,6 @@ if have_rust and have_system
     bindgen_args += ['--bitfield-enum', enum]
   endforeach
 
-  # TODO: Remove this comment when the clang/libclang mismatch issue is solved.
-  #
-  # Rust bindings generation with `bindgen` might fail in some cases where the
-  # detected `libclang` does not match the expected `clang` version/target. In
-  # this case you must pass the path to `clang` and `libclang` to your build
-  # command invocation using the environment variables CLANG_PATH and
-  # LIBCLANG_PATH
   bindings_rs = import('rust').bindgen(
     input: 'rust/wrapper.h',
     dependencies: common_ss.all_dependencies(),
diff --git a/rust/wrapper.h b/rust/wrapper.h
index 77e40213efb686d23f6b768b78602e4337623280..285d0eb6ad01e227a82f13e17c79390b4c34d37e 100644
--- a/rust/wrapper.h
+++ b/rust/wrapper.h
@@ -30,6 +30,23 @@
  * in order to generate C FFI compatible Rust bindings.
  */
 
+#ifndef __CLANG_STDATOMIC_H
+#define __CLANG_STDATOMIC_H
+/*
+ * Fix potential missing stdatomic.h error in case bindgen does not insert the
+ * correct libclang header paths on its own. We do not use stdatomic.h symbols
+ * in QEMU code, so it's fine to declare dummy types instead.
+ */
+typedef enum memory_order {
+  memory_order_relaxed,
+  memory_order_consume,
+  memory_order_acquire,
+  memory_order_release,
+  memory_order_acq_rel,
+  memory_order_seq_cst,
+} memory_order;
+#endif /* __CLANG_STDATOMIC_H */
+
 #include "qemu/osdep.h"
 #include "qemu/module.h"
 #include "qemu-io.h"

---
base-commit: c155d13167c6ace099e351e28125f9eb3694ae27
change-id: 20241015-rust-wrapper-stdatomic-18d58292c243

--
γαῖα πυρί μιχθήτω

Re: [PATCH] rust/wrapper.h: define memory_order enum

[Paolo Bonzini]

On 10/15/24 14:07, Manos Pitsidianakis wrote:
> Add stub definition of memory_order enum in wrapper.h.
> 
> Creating Rust bindings from C code is done by passing the wrapper.h
> header to `bindgen`. This fails when library dependencies that use
> compiler headers are enabled, and the libclang that bindgen detects does
> not match the expected clang version. So far this has only been observed
> with the memory_order enum symbols from stdatomic.h. If we add the enum
> definition to wrapper.h ourselves, the error does not happen.
> 
> Before this commit, if the mismatch happened the following error could
> come up:
> 
>    /usr/include/liburing/barrier.h:72:10: error: use of undeclared identifier 'memory_order_release'
>    /usr/include/liburing/barrier.h:75:9: error: use of undeclared identifier 'memory_order_acquire'
>    /usr/include/liburing/barrier.h:75:9: error: use of undeclared identifier 'memory_order_acquire'
>    /usr/include/liburing/barrier.h:68:9: error: use of undeclared identifier 'memory_order_relaxed'
>    /usr/include/liburing/barrier.h:65:17: error: use of undeclared identifier 'memory_order_relaxed'
>    /usr/include/liburing/barrier.h:75:9: error: use of undeclared identifier 'memory_order_acquire'
>    /usr/include/liburing/barrier.h:75:9: error: use of undeclared identifier 'memory_order_acquire'
>    /usr/include/liburing/barrier.h:72:10: error: use of undeclared identifier 'memory_order_release'
>    panicked at [..]/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-cli-0.70.1/main.rs:45:36:
>    Unable to generate bindings
> 
> To fix this (on my system) I would have to export CLANG_PATH and
> LIBCLANG_PATH:
> 
>    export CLANG_PATH=/bin/clang-17
>    export LIBCLANG_PATH=/usr/lib/llvm-17/lib
> 
> With these changes applied, bindgen is successful with both the
> environment variables set and unset.
> 
> Since we're not using those symbols in the bindings (they are only used
> by dependencies) this does not affect the generated bindings in any way.

That is still likely to break, depending on what changes in the future 
in the headers that clang provides.

We can do this (or alternatively, limit the number of files included in 
wrapper.h; in this case trying to remove use of io_uring's headers from 
include/block/aio.h) but probably we have to leave the warning in.

Paolo

Re: [PATCH] rust/wrapper.h: define memory_order enum

[Junjie Mao]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 10/15/24 14:07, Manos Pitsidianakis wrote:
>> Add stub definition of memory_order enum in wrapper.h.
>> Creating Rust bindings from C code is done by passing the wrapper.h
>> header to `bindgen`. This fails when library dependencies that use
>> compiler headers are enabled, and the libclang that bindgen detects does
>> not match the expected clang version. So far this has only been observed
>> with the memory_order enum symbols from stdatomic.h. If we add the enum
>> definition to wrapper.h ourselves, the error does not happen.
>> Before this commit, if the mismatch happened the following error could
>> come up:
>>    /usr/include/liburing/barrier.h:72:10: error: use of undeclared identifier
>> 'memory_order_release'
>>    /usr/include/liburing/barrier.h:75:9: error: use of undeclared identifier 'memory_order_acquire'
>>    /usr/include/liburing/barrier.h:75:9: error: use of undeclared identifier 'memory_order_acquire'
>>    /usr/include/liburing/barrier.h:68:9: error: use of undeclared identifier 'memory_order_relaxed'
>>    /usr/include/liburing/barrier.h:65:17: error: use of undeclared identifier 'memory_order_relaxed'
>>    /usr/include/liburing/barrier.h:75:9: error: use of undeclared identifier 'memory_order_acquire'
>>    /usr/include/liburing/barrier.h:75:9: error: use of undeclared identifier 'memory_order_acquire'
>>    /usr/include/liburing/barrier.h:72:10: error: use of undeclared identifier 'memory_order_release'
>>    panicked at [..]/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bindgen-cli-0.70.1/main.rs:45:36:
>>    Unable to generate bindings
>> To fix this (on my system) I would have to export CLANG_PATH and
>> LIBCLANG_PATH:
>>    export CLANG_PATH=/bin/clang-17
>>    export LIBCLANG_PATH=/usr/lib/llvm-17/lib
>> With these changes applied, bindgen is successful with both the
>> environment variables set and unset.
>> Since we're not using those symbols in the bindings (they are only used
>> by dependencies) this does not affect the generated bindings in any way.
>
> That is still likely to break, depending on what changes in the future in the
> headers that clang provides.
>
> We can do this (or alternatively, limit the number of files included in
> wrapper.h; in this case trying to remove use of io_uring's headers from
> include/block/aio.h) but probably we have to leave the warning in.
>
> Paolo

I agree with Paolo's comments. It was reported that using inconsistent
versions of clang and libclang can lead to unsound bindings even they
compile. I would recommend to keep that warning till a proper
resolution, e.g. clang-sys provides consistent paths, is present.

--
Best Regards
Junjie Mao