Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bindgen: use bindgen to provide Rust bindings to C - v9 #12578

Closed
wants to merge 7 commits into from

Conversation

jasonish
Copy link
Member

Previous PR: #12466

  • Check for minimum version of bindgen.

Require a minimum version of 0.66.0.
Follow Rust convention of using a "sys" crate for bindings to C
functions. The bindings don't exist yet, but will be generated by
bindgen and put into this crate.
Bindgen works by processing a header file which includes all other
header files it should generate bindings for. For this I've created
bindgen.h which just includes app-layer-protos.h for now as an
example.

These bindings are then generated and saved in the "suricata-sys"
crate and become availale as "suricata_sys::sys".
Have bindgen generate bindings for app-layer-protos.h, then use the
generated definitions of AppProto/AppProtoEnum instead if defining
them ourselves.

This header was chosen as its used by Rust, and its a simple header
with no circular dependencies.
Regenerates the `sys.rs` and looks for any difference. Check will fail
if there is a difference.
We don't keep bindgen's autogenerated do not edit line as it contains
the bindgen version which could break the CI check for out of date
bindings. So add our own do not edit line.
Copy link

codecov bot commented Feb 13, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.71%. Comparing base (3831843) to head (dd07183).
Report is 14 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #12578      +/-   ##
==========================================
- Coverage   80.72%   80.71%   -0.01%     
==========================================
  Files         929      929              
  Lines      259062   259062              
==========================================
- Hits       209119   209100      -19     
- Misses      49943    49962      +19     
Flag Coverage Δ
fuzzcorpus 56.96% <100.00%> (+0.01%) ⬆️
livemode 19.39% <0.00%> (+<0.01%) ⬆️
pcap 44.16% <100.00%> (-0.03%) ⬇️
suricata-verify 63.39% <100.00%> (+0.01%) ⬆️
unittests 58.36% <33.33%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 24720

@victorjulien
Copy link
Member

See this fail on one of my arm 32bit runners, but not on all of them

test sys::bindgen_test_layout_SCAppLayerPlugin_ ... FAILED
test sys::bindgen_test_layout_SCCapturePlugin_ ... FAILED
test sys::bindgen_test_layout_SCCapturePlugin___bindgen_ty_1 ... FAILED
test sys::bindgen_test_layout_SCPlugin_ ... FAILED
failures:
---- sys::bindgen_test_layout_SCAppLayerPlugin_ stdout ----
thread 'sys::bindgen_test_layout_SCAppLayerPlugin_' panicked at sys/src/sys.rs:294:5:
assertion `left == right` failed: Size of: SCAppLayerPlugin_
  left: 32
 right: 56
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
---- sys::bindgen_test_layout_SCCapturePlugin_ stdout ----
thread 'sys::bindgen_test_layout_SCCapturePlugin_' panicked at sys/src/sys.rs:199:5:
assertion `left == right` failed: Size of: SCCapturePlugin_
  left: 28
 right: 56
---- sys::bindgen_test_layout_SCCapturePlugin___bindgen_ty_1 stdout ----
thread 'sys::bindgen_test_layout_SCCapturePlugin___bindgen_ty_1' panicked at sys/src/sys.rs:164:5:
assertion `left == right` failed: Size of: SCCapturePlugin___bindgen_ty_1
  left: 8
 right: 16
---- sys::bindgen_test_layout_SCPlugin_ stdout ----
thread 'sys::bindgen_test_layout_SCPlugin_' panicked at sys/src/sys.rs:70:5:
assertion `left == right` failed: Size of: SCPlugin_
  left: 16
 right: 32
failures:
    sys::bindgen_test_layout_SCAppLayerPlugin_
    sys::bindgen_test_layout_SCCapturePlugin_
    sys::bindgen_test_layout_SCCapturePlugin___bindgen_ty_1
    sys::bindgen_test_layout_SCPlugin_
test result: FAILED. 0 passed; 4 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

@victorjulien
Copy link
Member

The job is using alpine:latest, which uses musl. Perhaps that is related.

@victorjulien
Copy link
Member

Also fails my freebsd builder

cd /var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/../../rust && \
	CARGO_HOME="/var/tmp/gitlab_runner/.cargo" CARGO_TARGET_DIR="/var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/rust/target" SURICATA_LUA_SYS_HEADER_DST="/var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/rust/gen" \
	/usr/local/bin/cargo build --release  \
		--features "ja3 ja4  " 
error: failed to write /var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/rust/Cargo.lock
Caused by:
  failed to open: /var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/rust/Cargo.lock
Caused by:
  Permission denied (os error 13)
gmake[3]: Leaving directory '/var/tmp/gitlab_runner/builds/Tys3Svbf/0/inliniac/suricata-ci/suricata/suricata-8.0.0-dev/_build/sub/rust'
gmake[3]: *** [Makefile:736: all-local] Error 101

@jasonish
Copy link
Member Author

See this fail on one of my arm 32bit runners, but not on all of them

test sys::bindgen_test_layout_SCAppLayerPlugin_ ... FAILED
test sys::bindgen_test_layout_SCCapturePlugin_ ... FAILED
test sys::bindgen_test_layout_SCCapturePlugin___bindgen_ty_1 ... FAILED
test sys::bindgen_test_layout_SCPlugin_ ... FAILED
failures:
---- sys::bindgen_test_layout_SCAppLayerPlugin_ stdout ----
thread 'sys::bindgen_test_layout_SCAppLayerPlugin_' panicked at sys/src/sys.rs:294:5:
assertion `left == right` failed: Size of: SCAppLayerPlugin_
  left: 32
 right: 56
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
---- sys::bindgen_test_layout_SCCapturePlugin_ stdout ----
thread 'sys::bindgen_test_layout_SCCapturePlugin_' panicked at sys/src/sys.rs:199:5:
assertion `left == right` failed: Size of: SCCapturePlugin_
  left: 28
 right: 56
---- sys::bindgen_test_layout_SCCapturePlugin___bindgen_ty_1 stdout ----
thread 'sys::bindgen_test_layout_SCCapturePlugin___bindgen_ty_1' panicked at sys/src/sys.rs:164:5:
assertion `left == right` failed: Size of: SCCapturePlugin___bindgen_ty_1
  left: 8
 right: 16
---- sys::bindgen_test_layout_SCPlugin_ stdout ----
thread 'sys::bindgen_test_layout_SCPlugin_' panicked at sys/src/sys.rs:70:5:
assertion `left == right` failed: Size of: SCPlugin_
  left: 16
 right: 32
failures:
    sys::bindgen_test_layout_SCAppLayerPlugin_
    sys::bindgen_test_layout_SCCapturePlugin_
    sys::bindgen_test_layout_SCCapturePlugin___bindgen_ty_1
    sys::bindgen_test_layout_SCPlugin_
test result: FAILED. 0 passed; 4 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

Interesting. I wonder if the size of a Rust c_int and a C int differ between Rust and C. This is a sanity check unit test generated by bindgen. The bindings look just like as if we were to do them by hand. So either they're wrong on this specific build, or the sanity check is wrong, so will require further investigation.

Appears there are some known cases where the generated tests don't work on a different size arch, but the bindings themselves are fine.

@jasonish
Copy link
Member Author

See this fail on one of my arm 32bit runners, but not on all of them

test sys::bindgen_test_layout_SCAppLayerPlugin_ ... FAILED
test sys::bindgen_test_layout_SCCapturePlugin_ ... FAILED
test sys::bindgen_test_layout_SCCapturePlugin___bindgen_ty_1 ... FAILED
test sys::bindgen_test_layout_SCPlugin_ ... FAILED
failures:
---- sys::bindgen_test_layout_SCAppLayerPlugin_ stdout ----
thread 'sys::bindgen_test_layout_SCAppLayerPlugin_' panicked at sys/src/sys.rs:294:5:
assertion `left == right` failed: Size of: SCAppLayerPlugin_
  left: 32
 right: 56
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
---- sys::bindgen_test_layout_SCCapturePlugin_ stdout ----
thread 'sys::bindgen_test_layout_SCCapturePlugin_' panicked at sys/src/sys.rs:199:5:
assertion `left == right` failed: Size of: SCCapturePlugin_
  left: 28
 right: 56
---- sys::bindgen_test_layout_SCCapturePlugin___bindgen_ty_1 stdout ----
thread 'sys::bindgen_test_layout_SCCapturePlugin___bindgen_ty_1' panicked at sys/src/sys.rs:164:5:
assertion `left == right` failed: Size of: SCCapturePlugin___bindgen_ty_1
  left: 8
 right: 16
---- sys::bindgen_test_layout_SCPlugin_ stdout ----
thread 'sys::bindgen_test_layout_SCPlugin_' panicked at sys/src/sys.rs:70:5:
assertion `left == right` failed: Size of: SCPlugin_
  left: 16
 right: 32
failures:
    sys::bindgen_test_layout_SCAppLayerPlugin_
    sys::bindgen_test_layout_SCCapturePlugin_
    sys::bindgen_test_layout_SCCapturePlugin___bindgen_ty_1
    sys::bindgen_test_layout_SCPlugin_
test result: FAILED. 0 passed; 4 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

Interesting. I wonder if the size of a Rust c_int and a C int differ between Rust and C. This is a sanity check unit test generated by bindgen. The bindings look just like as if we were to do them by hand. So either they're wrong on this specific build, or the sanity check is wrong, so will require further investigation.

Appears there are some known cases where the generated tests don't work on a different size arch, but the bindings themselves are fine.

Appears the generated tests check for the pointer width on the machine type generating the bindings. So its a good sanity check, but only on the same architecture. Worst case we disable automake test generation (cbindgen doesn't have them either), or see if we can easily gate them to the arch generating the bindings.

@jasonish
Copy link
Member Author

Appears to only be the tests that differ when the bindings are generated on arm32.

diff --git a/rust/sys/src/sys.rs b/rust/sys/src/sys.rs
index 91dae8dc0..7717f89e1 100644
--- a/rust/sys/src/sys.rs
+++ b/rust/sys/src/sys.rs
@@ -69,53 +69,33 @@ fn bindgen_test_layout_SCPlugin_() {
     let ptr = UNINIT.as_ptr();
     assert_eq!(
         ::std::mem::size_of::<SCPlugin_>(),
-        32usize,
-        concat!("Size of: ", stringify!(SCPlugin_))
+        16usize,
+        "Size of SCPlugin_"
     );
     assert_eq!(
         ::std::mem::align_of::<SCPlugin_>(),
-        8usize,
-        concat!("Alignment of ", stringify!(SCPlugin_))
+        4usize,
+        "Alignment of SCPlugin_"
     );
     assert_eq!(
         unsafe { ::std::ptr::addr_of!((*ptr).name) as usize - ptr as usize },
         0usize,
-        concat!(
-            "Offset of field: ",
-            stringify!(SCPlugin_),
-            "::",
-            stringify!(name)
-        )
+        "Offset of field: SCPlugin_::name"
     );
     assert_eq!(
         unsafe { ::std::ptr::addr_of!((*ptr).license) as usize - ptr as usize },
-        8usize,
-        concat!(
-            "Offset of field: ",
-            stringify!(SCPlugin_),
-            "::",
-            stringify!(license)
-        )
+        4usize,
+        "Offset of field: SCPlugin_::license"
     );
     assert_eq!(
         unsafe { ::std::ptr::addr_of!((*ptr).author) as usize - ptr as usize },
-        16usize,
-        concat!(
-            "Offset of field: ",
-            stringify!(SCPlugin_),
-            "::",
-            stringify!(author)
-        )
+        8usize,
+        "Offset of field: SCPlugin_::author"
     );
     assert_eq!(
         unsafe { ::std::ptr::addr_of!((*ptr).Init) as usize - ptr as usize },
-        24usize,
-        concat!(
-            "Offset of field: ",
-            stringify!(SCPlugin_),
-            "::",
-            stringify!(Init)
-        )
+        12usize,
+        "Offset of field: SCPlugin_::Init"
     );
 }
 #[doc = " Structure to define a Suricata plugin."]
@@ -163,33 +143,23 @@ fn bindgen_test_layout_SCCapturePlugin___bindgen_ty_1() {
     let ptr = UNINIT.as_ptr();
     assert_eq!(
         ::std::mem::size_of::<SCCapturePlugin___bindgen_ty_1>(),
-        16usize,
-        concat!("Size of: ", stringify!(SCCapturePlugin___bindgen_ty_1))
+        8usize,
+        "Size of SCCapturePlugin___bindgen_ty_1"
     );
     assert_eq!(
         ::std::mem::align_of::<SCCapturePlugin___bindgen_ty_1>(),
-        8usize,
-        concat!("Alignment of ", stringify!(SCCapturePlugin___bindgen_ty_1))
+        4usize,
+        "Alignment of SCCapturePlugin___bindgen_ty_1"
     );
     assert_eq!(
         unsafe { ::std::ptr::addr_of!((*ptr).tqe_next) as usize - ptr as usize },
         0usize,
-        concat!(
-            "Offset of field: ",
-            stringify!(SCCapturePlugin___bindgen_ty_1),
-            "::",
-            stringify!(tqe_next)
-        )
+        "Offset of field: SCCapturePlugin___bindgen_ty_1::tqe_next"
     );
     assert_eq!(
         unsafe { ::std::ptr::addr_of!((*ptr).tqe_prev) as usize - ptr as usize },
-        8usize,
-        concat!(
-            "Offset of field: ",
-            stringify!(SCCapturePlugin___bindgen_ty_1),
-            "::",
-            stringify!(tqe_prev)
-        )
+        4usize,
+        "Offset of field: SCCapturePlugin___bindgen_ty_1::tqe_prev"
     );
 }
 #[test]
@@ -198,73 +168,43 @@ fn bindgen_test_layout_SCCapturePlugin_() {
     let ptr = UNINIT.as_ptr();
     assert_eq!(
         ::std::mem::size_of::<SCCapturePlugin_>(),
-        56usize,
-        concat!("Size of: ", stringify!(SCCapturePlugin_))
+        28usize,
+        "Size of SCCapturePlugin_"
     );
     assert_eq!(
         ::std::mem::align_of::<SCCapturePlugin_>(),
-        8usize,
-        concat!("Alignment of ", stringify!(SCCapturePlugin_))
+        4usize,
+        "Alignment of SCCapturePlugin_"
     );
     assert_eq!(
         unsafe { ::std::ptr::addr_of!((*ptr).name) as usize - ptr as usize },
         0usize,
-        concat!(
-            "Offset of field: ",
-            stringify!(SCCapturePlugin_),
-            "::",
-            stringify!(name)
-        )
+        "Offset of field: SCCapturePlugin_::name"
     );
     assert_eq!(
         unsafe { ::std::ptr::addr_of!((*ptr).Init) as usize - ptr as usize },
-        8usize,
-        concat!(
-            "Offset of field: ",
-            stringify!(SCCapturePlugin_),
-            "::",
-            stringify!(Init)
-        )
+        4usize,
+        "Offset of field: SCCapturePlugin_::Init"
     );
     assert_eq!(
         unsafe { ::std::ptr::addr_of!((*ptr).ThreadInit) as usize - ptr as usize },
-        16usize,
-        concat!(
-            "Offset of field: ",
-            stringify!(SCCapturePlugin_),
-            "::",
-            stringify!(ThreadInit)
-        )
+        8usize,
+        "Offset of field: SCCapturePlugin_::ThreadInit"
     );
     assert_eq!(
         unsafe { ::std::ptr::addr_of!((*ptr).ThreadDeinit) as usize - ptr as usize },
-        24usize,
-        concat!(
-            "Offset of field: ",
-            stringify!(SCCapturePlugin_),
-            "::",
-            stringify!(ThreadDeinit)
-        )
+        12usize,
+        "Offset of field: SCCapturePlugin_::ThreadDeinit"
     );
     assert_eq!(
         unsafe { ::std::ptr::addr_of!((*ptr).GetDefaultMode) as usize - ptr as usize },
-        32usize,
-        concat!(
-            "Offset of field: ",
-            stringify!(SCCapturePlugin_),
-            "::",
-            stringify!(GetDefaultMode)
-        )
+        16usize,
+        "Offset of field: SCCapturePlugin_::GetDefaultMode"
     );
     assert_eq!(
         unsafe { ::std::ptr::addr_of!((*ptr).entries) as usize - ptr as usize },
-        40usize,
-        concat!(
-            "Offset of field: ",
-            stringify!(SCCapturePlugin_),
-            "::",
-            stringify!(entries)
-        )
+        20usize,
+        "Offset of field: SCCapturePlugin_::entries"
     );
 }
 pub type SCCapturePlugin = SCCapturePlugin_;
@@ -293,83 +233,48 @@ fn bindgen_test_layout_SCAppLayerPlugin_() {
     let ptr = UNINIT.as_ptr();
     assert_eq!(
         ::std::mem::size_of::<SCAppLayerPlugin_>(),
-        56usize,
-        concat!("Size of: ", stringify!(SCAppLayerPlugin_))
+        32usize,
+        "Size of SCAppLayerPlugin_"
     );
     assert_eq!(
         ::std::mem::align_of::<SCAppLayerPlugin_>(),
         8usize,
-        concat!("Alignment of ", stringify!(SCAppLayerPlugin_))
+        "Alignment of SCAppLayerPlugin_"
     );
     assert_eq!(
         unsafe { ::std::ptr::addr_of!((*ptr).version) as usize - ptr as usize },
         0usize,
-        concat!(
-            "Offset of field: ",
-            stringify!(SCAppLayerPlugin_),
-            "::",
-            stringify!(version)
-        )
+        "Offset of field: SCAppLayerPlugin_::version"
     );
     assert_eq!(
         unsafe { ::std::ptr::addr_of!((*ptr).name) as usize - ptr as usize },
         8usize,
-        concat!(
-            "Offset of field: ",
-            stringify!(SCAppLayerPlugin_),
-            "::",
-            stringify!(name)
-        )
+        "Offset of field: SCAppLayerPlugin_::name"
     );
     assert_eq!(
         unsafe { ::std::ptr::addr_of!((*ptr).Register) as usize - ptr as usize },
-        16usize,
-        concat!(
-            "Offset of field: ",
-            stringify!(SCAppLayerPlugin_),
-            "::",
-            stringify!(Register)
-        )
+        12usize,
+        "Offset of field: SCAppLayerPlugin_::Register"
     );
     assert_eq!(
         unsafe { ::std::ptr::addr_of!((*ptr).KeywordsRegister) as usize - ptr as usize },
-        24usize,
-        concat!(
-            "Offset of field: ",
-            stringify!(SCAppLayerPlugin_),
-            "::",
-            stringify!(KeywordsRegister)
-        )
+        16usize,
+        "Offset of field: SCAppLayerPlugin_::KeywordsRegister"
     );
     assert_eq!(
         unsafe { ::std::ptr::addr_of!((*ptr).logname) as usize - ptr as usize },
-        32usize,
-        concat!(
-            "Offset of field: ",
-            stringify!(SCAppLayerPlugin_),
-            "::",
-            stringify!(logname)
-        )
+        20usize,
+        "Offset of field: SCAppLayerPlugin_::logname"
     );
     assert_eq!(
         unsafe { ::std::ptr::addr_of!((*ptr).confname) as usize - ptr as usize },
-        40usize,
-        concat!(
-            "Offset of field: ",
-            stringify!(SCAppLayerPlugin_),
-            "::",
-            stringify!(confname)
-        )
+        24usize,
+        "Offset of field: SCAppLayerPlugin_::confname"
     );
     assert_eq!(
         unsafe { ::std::ptr::addr_of!((*ptr).Logger) as usize - ptr as usize },
-        48usize,
-        concat!(
-            "Offset of field: ",
-            stringify!(SCAppLayerPlugin_),
-            "::",
-            stringify!(Logger)
-        )
+        28usize,
+        "Offset of field: SCAppLayerPlugin_::Logger"
     );
 }
 pub type SCAppLayerPlugin = SCAppLayerPlugin_;

@jasonish
Copy link
Member Author

Replaced by #12589

@jasonish jasonish closed this Feb 16, 2025
@jasonish jasonish deleted the bindgen-cli/v9 branch February 17, 2025 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants