| Message ID | 20241015-rust-wrapper-stdatomic-v1-1-f22c0bd31140@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | rust/wrapper.h: define memory_order enum | expand |
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
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
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"
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(-) --- base-commit: c155d13167c6ace099e351e28125f9eb3694ae27 change-id: 20241015-rust-wrapper-stdatomic-18d58292c243 -- γαῖα πυρί μιχθήτω