Repository: rust-lang/rust-bindgen (rust)

Issue:
Mishandle of variadic function with wrapped static functions
<!-- Thanks for filing a bindgen issue! We appreciate it :-) -->

### Input C/C++ Header

```C++
static inline void foo(int flags, ...) {
    // ...
}
```

### Bindgen Invocation

```
$ bindgen --experimental --wrap-static-fns input.h
```

### Actual Results

```rust
extern "C" {
    #[link_name = "foo__extern"]
    pub fn foo(flags: ::std::os::raw::c_int, ...);
}
```

and

```c
#include "input.h"

// Static wrappers

void foo__extern(int flags) { foo(flags); } // At least missing `...`
```

### Expected Results

I expect bindgen to either handle correctly (what ever that would be) or to ignore those bind of functions.

cc @pvdrz 


Pull request:
Do not wrap static variadic functions
Currently there is no way to generate a wrapper for a static variadic function while using the `--wrap-static-fns` feature.

These changes modify `bindgen`'s behavior so such functions are skipped instead of generating invalid code for them. At the same time, a warning is emitted if the `--emit-diagnostics` feature is enabled.

Fixes #2498 

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index bbd4f198..098882a1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -166,9 +166,17 @@
 
 ## Added
 
+* Added the `--generate-cstr` CLI flag to generate string constants as `&CStr`
+  instead of `&[u8]`. (Requires Rust 1.59 or higher.)
+
 ## Changed
+
+* Non-UTF-8 string constants are now generated as references (`&[u8; SIZE]`)
+  instead of arrays (`[u8; SIZE]`) to match UTF-8 strings.
 * Wrappers for static functions that return `void` no longer contain a `return`
   statement and only call the static function instead.
+* The `--wrap-static-fns` option no longer emits wrappers for static variadic
+  functions.
    
 ## Removed
 
@@ -252,7 +260,7 @@
  * The return type is now ommited in signatures of functions returning `void`.
  * Updated the `clap` dependency for `bindgen-cli` to 4.
  * Rewrote the `bindgen-cli` argument parser which could introduce unexpected
-   behavior changes. 
+   behavior changes.
  * The `ParseCallbacks::add_derives` method now receives `DeriveInfo<'_>` as
    argument instead of a `&str`. This type also includes the kind of target type.
 
diff --git a/bindgen-cli/options.rs b/bindgen-cli/options.rs
index 9ab202f1..8e292f95 100644
--- a/bindgen-cli/options.rs
+++ b/bindgen-cli/options.rs
@@ -156,6 +156,9 @@ struct BindgenCommand {
     /// Generate block signatures instead of void pointers.
     #[arg(long)]
     generate_block: bool,
+    /// Generate string constants as `&CStr` instead of `&[u8]`.
+    #[arg(long)]
+    generate_cstr: bool,
     /// Use extern crate instead of use for block.
     #[arg(long)]
     block_extern_crate: bool,
@@ -430,6 +433,7 @@ where
         no_recursive_allowlist,
         objc_extern_crate,
         generate_block,
+        generate_cstr,
         block_extern_crate,
         distrust_clang_mangling,
         builtins,
@@ -755,6 +759,10 @@ where
         builder = builder.generate_block(true);
     }
 
+    if generate_cstr {
+        builder = builder.generate_cstr(true);
+    }
+
     if block_extern_crate {
         builder = builder.block_extern_crate(true);
     }
diff --git a/bindgen/codegen/helpers.rs b/bindgen/codegen/helpers.rs
index 726fe75e..8231e07c 100644
--- a/bindgen/codegen/helpers.rs
+++ b/bindgen/codegen/helpers.rs
@@ -249,12 +249,6 @@ pub(crate) mod ast_ty {
         quote!(#val)
     }
 
-    pub(crate) fn byte_array_expr(bytes: &[u8]) -> TokenStream {
-        let mut bytes: Vec<_> = bytes.to_vec();
-        bytes.push(0);
-        quote! { [ #(#bytes),* ] }
-    }
-
     pub(crate) fn cstr_expr(mut string: String) -> TokenStream {
         string.push('\0');
         let b = proc_macro2::Literal::byte_string(string.as_bytes());
diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs
index 11e49b8d..16757dd8 100644
--- a/bindgen/codegen/mod.rs
+++ b/bindgen/codegen/mod.rs
@@ -57,9 +57,10 @@ use crate::{Entry, HashMap, HashSet};
 use std::borrow::Cow;
 use std::cell::Cell;
 use std::collections::VecDeque;
+use std::ffi::CStr;
 use std::fmt::{self, Write};
 use std::ops;
-use std::str::FromStr;
+use std::str::{self, FromStr};
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub enum CodegenError {
@@ -692,41 +693,48 @@ impl CodeGenerator for Var {
                     });
                 }
                 VarType::String(ref bytes) => {
-                    // Account the trailing zero.
-                    //
+                    let prefix = ctx.trait_prefix();
+
+                    let options = ctx.options();
+                    let rust_features = options.rust_features;
+
+                    let mut cstr_bytes = bytes.clone();
+                    cstr_bytes.push(0);
+                    let len = proc_macro2::Literal::usize_unsuffixed(
+                        cstr_bytes.len(),
+                    );
+                    let cstr = CStr::from_bytes_with_nul(&cstr_bytes).unwrap();
+
                     // TODO: Here we ignore the type we just made up, probably
                     // we should refactor how the variable type and ty ID work.
-                    let len = bytes.len() + 1;
-                    let ty = quote! {
-                        [u8; #len]
-                    };
+                    let array_ty = quote! { [u8; #len] };
+                    let cstr_ty = quote! { ::#prefix::ffi::CStr };
 
-                    match String::from_utf8(bytes.clone()) {
-                        Ok(string) => {
-                            let cstr = helpers::ast_ty::cstr_expr(string);
-                            if ctx
-                                .options()
-                                .rust_features
-                                .static_lifetime_elision
-                            {
-                                result.push(quote! {
-                                    #(#attrs)*
-                                    pub const #canonical_ident : &#ty = #cstr ;
-                                });
-                            } else {
-                                result.push(quote! {
-                                    #(#attrs)*
-                                    pub const #canonical_ident : &'static #ty = #cstr ;
-                                });
-                            }
-                        }
-                        Err(..) => {
-                            let bytes = helpers::ast_ty::byte_array_expr(bytes);
-                            result.push(quote! {
-                                #(#attrs)*
-                                pub const #canonical_ident : #ty = #bytes ;
-                            });
+                    let bytes = proc_macro2::Literal::byte_string(
+                        cstr.to_bytes_with_nul(),
+                    );
+
+                    if rust_features.const_cstr && options.generate_cstr {
+                        result.push(quote! {
+                            #(#attrs)*
+                            #[allow(unsafe_code)]
+                            pub const #canonical_ident: &#cstr_ty = unsafe {
+                                #cstr_ty::from_bytes_with_nul_unchecked(#bytes)
+                            };
+                        });
+                    } else {
+                        let lifetime = if rust_features.static_lifetime_elision
+                        {
+                            None
+                        } else {
+                            Some(quote! { 'static })
                         }
+                        .into_iter();
+
+                        result.push(quote! {
+                            #(#attrs)*
+                            pub const #canonical_ident: &#(#lifetime )*#array_ty = #bytes ;
+                        });
                     }
                 }
                 VarType::Float(f) => {
@@ -4143,10 +4151,26 @@ impl CodeGenerator for Function {
 
         let is_internal = matches!(self.linkage(), Linkage::Internal);
 
-        if is_internal && !ctx.options().wrap_static_fns {
-            // We can't do anything with Internal functions if we are not wrapping them so just
-            // avoid generating anything for them.
-            return None;
+        let signature_item = ctx.resolve_item(self.signature());
+        let signature = signature_item.kind().expect_type().canonical_type(ctx);
+        let signature = match *signature.kind() {
+            TypeKind::Function(ref sig) => sig,
+            _ => panic!("Signature kind is not a Function: {:?}", signature),
+        };
+
+        if is_internal {
+            if !ctx.options().wrap_static_fns {
+                // We cannot do anything with internal functions if we are not wrapping them so
+                // just avoid generating anything for them.
+                return None;
+            }
+
+            if signature.is_variadic() {
+                // We cannot generate wrappers for variadic static functions so we avoid
+                // generating any code for them.
+                variadic_fn_diagnostic(self.name(), item.location(), ctx);
+                return None;
+            }
         }
 
         // Pure virtual methods have no actual symbol, so we can't generate
@@ -4186,13 +4210,6 @@ impl CodeGenerator for Function {
             result.saw_function(seen_symbol_name);
         }
 
-        let signature_item = ctx.resolve_item(self.signature());
-        let signature = signature_item.kind().expect_type().canonical_type(ctx);
-        let signature = match *signature.kind() {
-            TypeKind::Function(ref sig) => sig,
-            _ => panic!("Signature kind is not a Function: {:?}", signature),
-        };
-
         let args = utils::fnsig_arguments(ctx, signature);
         let ret = utils::fnsig_return_ty(ctx, signature);
 
@@ -4400,6 +4417,44 @@ fn unsupported_abi_diagnostic<const VARIADIC: bool>(
     }
 }
 
+fn variadic_fn_diagnostic(
+    fn_name: &str,
+    _location: Option<&crate::clang::SourceLocation>,
+    _ctx: &BindgenContext,
+) {
+    warn!(
+        "Cannot generate wrapper for the static variadic function `{}`.",
+        fn_name,
+    );
+
+    #[cfg(feature = "experimental")]
+    if _ctx.options().emit_diagnostics {
+        use crate::diagnostics::{get_line, Diagnostic, Level, Slice};
+
+        let mut diag = Diagnostic::default();
+
+        diag.with_title(format!("Cannot generate wrapper for the static function `{}`.", fn_name), Level::Warn)
+            .add_annotation("The `--wrap-static-fns` feature does not support variadic functions.", Level::Note)
+            .add_annotation("No code will be generated for this function.", Level::Note);
+
+        if let Some(loc) = _location {
+            let (file, line, col, _) = loc.location();
+
+            if let Some(filename) = file.name() {
+                if let Ok(Some(source)) = get_line(&filename, line) {
+                    let mut slice = Slice::default();
+                    slice
+                        .with_source(source)
+                        .with_location(filename, line, col);
+                    diag.add_slice(slice);
+                }
+            }
+        }
+
+        diag.display()
+    }
+}
+
 fn objc_method_codegen(
     ctx: &BindgenContext,
     method: &ObjCMethod,
diff --git a/bindgen/codegen/serialize.rs b/bindgen/codegen/serialize.rs
index 0baa70a8..1136234e 100644
--- a/bindgen/codegen/serialize.rs
+++ b/bindgen/codegen/serialize.rs
@@ -77,6 +77,8 @@ impl<'a> CSerialize<'a> for Function {
             _ => unreachable!(),
         };
 
+        assert!(!signature.is_variadic());
+
         let name = self.name();
 
         // Function argoments stored as `(name, type_id)` tuples.
diff --git a/bindgen/features.rs b/bindgen/features.rs
index fe6f4158..bee0e859 100644
--- a/bindgen/features.rs
+++ b/bindgen/features.rs
@@ -128,6 +128,9 @@ macro_rules! rust_target_base {
             /// Rust stable 1.47
             /// * `larger_arrays` ([Tracking issue](https://github.com/rust-lang/rust/pull/74060))
             => Stable_1_47 => 1.47;
+            /// Rust stable 1.59
+            /// * `CStr::from_bytes_with_nul_unchecked` in `const` contexts ([PR](https://github.com/rust-lang/rust/pull/54745))
+            => Stable_1_59 => 1.59;
             /// Rust stable 1.64
             ///  * `core_ffi_c` ([Tracking issue](https://github.com/rust-lang/rust/issues/94501))
             => Stable_1_64 => 1.64;
@@ -241,6 +244,9 @@ rust_feature_def!(
     Stable_1_47 {
         => larger_arrays;
     }
+    Stable_1_59 {
+        => const_cstr;
+    }
     Stable_1_64 {
         => core_ffi_c;
     }
diff --git a/bindgen/options/mod.rs b/bindgen/options/mod.rs
index c60da71e..83674e0e 100644
--- a/bindgen/options/mod.rs
+++ b/bindgen/options/mod.rs
@@ -1416,6 +1416,23 @@ options! {
         },
         as_args: "--generate-block",
     },
+    /// Whether to generate strings as `CStr`.
+    generate_cstr: bool {
+        methods: {
+            /// Set whether string constants should be generated as `&CStr` instead of `&[u8]`.
+            ///
+            /// A minimum Rust target of 1.59 is required for this to have any effect as support
+            /// for `CStr::from_bytes_with_nul_unchecked` in `const` contexts is needed.
+            ///
+            /// This option is disabled by default but will become enabled by default in a future
+            /// release, so enabling this is recommended.
+            pub fn generate_cstr(mut self, doit: bool) -> Self {
+                self.options.generate_cstr = doit;
+                self
+            }
+        },
+        as_args: "--generate-cstr",
+    },
     /// Whether to emit `#[macro_use] extern crate block;` instead of `use block;` in the prologue
     /// of the files generated from apple block files.
     block_extern_crate: bool {


Test patch:
diff --git a/bindgen-tests/tests/expectations/tests/libclang-5/constant-evaluate.rs b/bindgen-tests/tests/expectations/tests/libclang-5/constant-evaluate.rs
index 9debe39d..6263faed 100644
--- a/bindgen-tests/tests/expectations/tests/libclang-5/constant-evaluate.rs
+++ b/bindgen-tests/tests/expectations/tests/libclang-5/constant-evaluate.rs
@@ -21,5 +21,5 @@ pub const BAZ: ::std::os::raw::c_longlong = 24;
 pub const fuzz: f64 = 51.0;
 pub const BAZZ: ::std::os::raw::c_char = 53;
 pub const WAT: ::std::os::raw::c_char = 0;
-pub const bytestring: &[u8; 4usize] = b"Foo\0";
-pub const NOT_UTF8: [u8; 5usize] = [240u8, 40u8, 140u8, 40u8, 0u8];
+pub const bytestring: &[u8; 4] = b"Foo\0";
+pub const NOT_UTF8: &[u8; 5] = b"\xF0(\x8C(\0";
diff --git a/bindgen-tests/tests/expectations/tests/libclang-9/constant-evaluate.rs b/bindgen-tests/tests/expectations/tests/libclang-9/constant-evaluate.rs
index 9debe39d..6263faed 100644
--- a/bindgen-tests/tests/expectations/tests/libclang-9/constant-evaluate.rs
+++ b/bindgen-tests/tests/expectations/tests/libclang-9/constant-evaluate.rs
@@ -21,5 +21,5 @@ pub const BAZ: ::std::os::raw::c_longlong = 24;
 pub const fuzz: f64 = 51.0;
 pub const BAZZ: ::std::os::raw::c_char = 53;
 pub const WAT: ::std::os::raw::c_char = 0;
-pub const bytestring: &[u8; 4usize] = b"Foo\0";
-pub const NOT_UTF8: [u8; 5usize] = [240u8, 40u8, 140u8, 40u8, 0u8];
+pub const bytestring: &[u8; 4] = b"Foo\0";
+pub const NOT_UTF8: &[u8; 5] = b"\xF0(\x8C(\0";
diff --git a/bindgen-tests/tests/expectations/tests/macro_const.rs b/bindgen-tests/tests/expectations/tests/macro_const.rs
index de423a2a..eecd8bd7 100644
--- a/bindgen-tests/tests/expectations/tests/macro_const.rs
+++ b/bindgen-tests/tests/expectations/tests/macro_const.rs
@@ -5,10 +5,10 @@
     non_upper_case_globals
 )]
 
-pub const foo: &[u8; 4usize] = b"bar\0";
+pub const foo: &[u8; 4] = b"bar\0";
 pub const CHAR: u8 = 98u8;
 pub const CHARR: u8 = 0u8;
 pub const FLOAT: f64 = 5.09;
 pub const FLOAT_EXPR: f64 = 0.005;
 pub const LONG: u32 = 3;
-pub const INVALID_UTF8: [u8; 5usize] = [240u8, 40u8, 140u8, 40u8, 0u8];
+pub const INVALID_UTF8: &[u8; 5] = b"\xF0(\x8C(\0";
diff --git a/bindgen-tests/tests/expectations/tests/macro_const_1_0.rs b/bindgen-tests/tests/expectations/tests/macro_const_1_0.rs
index e1356611..8523a68a 100644
--- a/bindgen-tests/tests/expectations/tests/macro_const_1_0.rs
+++ b/bindgen-tests/tests/expectations/tests/macro_const_1_0.rs
@@ -5,10 +5,10 @@
     non_upper_case_globals
 )]
 
-pub const foo: &'static [u8; 4usize] = b"bar\0";
+pub const foo: &'static [u8; 4] = b"bar\0";
 pub const CHAR: u8 = 98u8;
 pub const CHARR: u8 = 0u8;
 pub const FLOAT: f64 = 5.09;
 pub const FLOAT_EXPR: f64 = 0.005;
 pub const LONG: u32 = 3;
-pub const INVALID_UTF8: [u8; 5usize] = [240u8, 40u8, 140u8, 40u8, 0u8];
+pub const INVALID_UTF8: &'static [u8; 5] = b"\xF0(\x8C(\0";
diff --git a/bindgen-tests/tests/expectations/tests/strings_array.rs b/bindgen-tests/tests/expectations/tests/strings_array.rs
new file mode 100644
index 00000000..76e8debd
--- /dev/null
+++ b/bindgen-tests/tests/expectations/tests/strings_array.rs
@@ -0,0 +1,10 @@
+#![allow(
+    dead_code,
+    non_snake_case,
+    non_camel_case_types,
+    non_upper_case_globals
+)]
+
+pub const MY_STRING_UTF8: &'static [u8; 14] = b"Hello, world!\0";
+pub const MY_STRING_INTERIOR_NULL: &'static [u8; 7] = b"Hello,\0";
+pub const MY_STRING_NON_UTF8: &'static [u8; 7] = b"ABCDE\xFF\0";
diff --git a/bindgen-tests/tests/expectations/tests/strings_cstr.rs b/bindgen-tests/tests/expectations/tests/strings_cstr.rs
new file mode 100644
index 00000000..e7f275f3
--- /dev/null
+++ b/bindgen-tests/tests/expectations/tests/strings_cstr.rs
@@ -0,0 +1,17 @@
+#![allow(
+    dead_code,
+    non_snake_case,
+    non_camel_case_types,
+    non_upper_case_globals
+)]
+
+#[allow(unsafe_code)]
+pub const MY_STRING_UTF8: &::std::ffi::CStr = unsafe {
+    ::std::ffi::CStr::from_bytes_with_nul_unchecked(b"Hello, world!\0")
+};
+#[allow(unsafe_code)]
+pub const MY_STRING_INTERIOR_NULL: &::std::ffi::CStr =
+    unsafe { ::std::ffi::CStr::from_bytes_with_nul_unchecked(b"Hello,\0") };
+#[allow(unsafe_code)]
+pub const MY_STRING_NON_UTF8: &::std::ffi::CStr =
+    unsafe { ::std::ffi::CStr::from_bytes_with_nul_unchecked(b"ABCDE\xFF\0") };
diff --git a/bindgen-tests/tests/headers/strings_array.h b/bindgen-tests/tests/headers/strings_array.h
new file mode 100644
index 00000000..6a61d710
--- /dev/null
+++ b/bindgen-tests/tests/headers/strings_array.h
@@ -0,0 +1,5 @@
+// bindgen-flags: --rust-target=1.0
+
+const char* MY_STRING_UTF8 = "Hello, world!";
+const char* MY_STRING_INTERIOR_NULL = "Hello,\0World!";
+const char* MY_STRING_NON_UTF8 = "ABCDE\xFF";
diff --git a/bindgen-tests/tests/headers/strings_cstr.h b/bindgen-tests/tests/headers/strings_cstr.h
new file mode 100644
index 00000000..8b1d40e6
--- /dev/null
+++ b/bindgen-tests/tests/headers/strings_cstr.h
@@ -0,0 +1,5 @@
+// bindgen-flags: --rust-target=1.59 --generate-cstr
+
+const char* MY_STRING_UTF8 = "Hello, world!";
+const char* MY_STRING_INTERIOR_NULL = "Hello,\0World!";
+const char* MY_STRING_NON_UTF8 = "ABCDE\xFF";
diff --git a/bindgen-tests/tests/headers/wrap-static-fns.h b/bindgen-tests/tests/headers/wrap-static-fns.h
index 4a1862b0..85dfeac2 100644
--- a/bindgen-tests/tests/headers/wrap-static-fns.h
+++ b/bindgen-tests/tests/headers/wrap-static-fns.h
@@ -44,3 +44,7 @@ static inline enum foo takes_enum(const enum foo f) {
 static inline void nevermore() {
     while (1) { }
 }
+
+static inline int variadic(int x, ...) {
+    return x;
+}