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

Issue:
Wrapped statics should generate more portable code
<!-- Thanks for filing a bindgen issue! We appreciate it :-) -->

### Input C/C++ Header

```C
static inline void inline_func(void) {}
void not_inline_func(void);
```

### Bindgen Invocation

```
$ bindgen header.h --experimental --wrap-static-fns --wrap-static-fns-path=wrapper.c
```

### Actual Results

```Rust
/* automatically generated by rust-bindgen 0.64.0 */

extern "C" {
    #[link_name = "\u{1}inline_func__extern"]
    pub fn inline_func();
}
extern "C" {
    pub fn not_inline_func();
}
```

```C
void inline_func__extern(void) asm("inline_func__extern");
void inline_func__extern() { return inline_func(); }
```

### Expected Results

The output uses GCC `asm` labels and is missing an include of `header.h`. This seems to make it needlessly difficult to integrate into a project:

* Lacking the include, projects need to use a compiler flag like `-include` to forcibly include the header. This isn't a standard flag and won't work if, say, building with MSVC.
* Using the GCC-specific `asm` label syntax to change the symbol bypasses the platform's underlying name mangling mechanism (e.g. C symbols are underscore-prefixed on darwin), and also breaks if building with a non-GCC-compatible compiler.

Is there a reason the feature was designed this way?

Adding the include seems a straightforward way to stick to standard C mechanisms.

The non-inline function doesn't require any decoration, so clearly bindgen and rustc are perfectly capable of handling the normal C mangling. If the aim was just so the Rust symbol appears as `inline_func` instead of `inline_func__extern`, https://github.com/rust-lang/rust-bindgen/pull/1063 suggests that, by default, `link_name` still does the default C mangling.

Pull request description:
Let LLVM mangle the link name of static wrappers
Fixes #2445 

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 25e9a169..0a6c86b3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -169,6 +169,9 @@
  * The source file generated when the `--wrap-static-fns` flag is enabled now
    contains `#include` directives with all the input headers and all the source
    code added with the `header_contents` method.
+ * The source file generated when the `--wrap-static-fns` flag no longer uses
+   `asm` labeling and the link name of static wrapper functions is allowed to
+   be mangled.
 ## Removed
  * The following deprecated flags were removed: `--use-msvc-mangling`,
    `--rustfmt-bindings` and `--size_t-is-usize`.
diff --git a/bindgen/codegen/helpers.rs b/bindgen/codegen/helpers.rs
index 9c907ae2..726fe75e 100644
--- a/bindgen/codegen/helpers.rs
+++ b/bindgen/codegen/helpers.rs
@@ -7,7 +7,7 @@ use quote::TokenStreamExt;
 
 pub(crate) mod attributes {
     use proc_macro2::{Ident, Span, TokenStream};
-    use std::str::FromStr;
+    use std::{borrow::Cow, str::FromStr};
 
     pub(crate) fn repr(which: &str) -> TokenStream {
         let which = Ident::new(which, Span::call_site());
@@ -62,10 +62,15 @@ pub(crate) mod attributes {
         }
     }
 
-    pub(crate) fn link_name(name: &str) -> TokenStream {
+    pub(crate) fn link_name<const MANGLE: bool>(name: &str) -> TokenStream {
         // LLVM mangles the name by default but it's already mangled.
         // Prefixing the name with \u{1} should tell LLVM to not mangle it.
-        let name = format!("\u{1}{}", name);
+        let name: Cow<'_, str> = if MANGLE {
+            name.into()
+        } else {
+            format!("\u{1}{}", name).into()
+        };
+
         quote! {
             #[link_name = #name]
         }
diff --git a/bindgen/codegen/mod.rs b/bindgen/codegen/mod.rs
index 352f6db0..f4a64b35 100644
--- a/bindgen/codegen/mod.rs
+++ b/bindgen/codegen/mod.rs
@@ -751,7 +751,7 @@ impl CodeGenerator for Var {
                 link_name,
                 None,
             ) {
-                attrs.push(attributes::link_name(link_name));
+                attrs.push(attributes::link_name::<false>(link_name));
             }
 
             let maybe_mut = if self.is_const() {
@@ -4155,7 +4155,7 @@ impl CodeGenerator for Function {
                 Some(abi),
             )
         {
-            attributes.push(attributes::link_name(link_name));
+            attributes.push(attributes::link_name::<false>(link_name));
             has_link_name_attr = true;
         }
 
@@ -4169,7 +4169,7 @@ impl CodeGenerator for Function {
 
         if is_internal && ctx.options().wrap_static_fns && !has_link_name_attr {
             let name = canonical_name.clone() + ctx.wrap_static_fns_suffix();
-            attributes.push(attributes::link_name(&name));
+            attributes.push(attributes::link_name::<true>(&name));
         }
 
         let ident = ctx.rust_ident(canonical_name);
diff --git a/bindgen/codegen/serialize.rs b/bindgen/codegen/serialize.rs
index dedf2568..ac620235 100644
--- a/bindgen/codegen/serialize.rs
+++ b/bindgen/codegen/serialize.rs
@@ -105,12 +105,6 @@ impl<'a> CSerialize<'a> for Function {
         // The function's return type
         let ret_ty = signature.return_type();
 
-        // Write `ret_ty wrap_name(args) asm("wrap_name");`
-        ret_ty.serialize(ctx, (), stack, writer)?;
-        write!(writer, " {}(", wrap_name)?;
-        serialize_args(&args, ctx, writer)?;
-        writeln!(writer, ") asm(\"{}\");", wrap_name)?;
-
         // Write `ret_ty wrap_name(args) { return name(arg_names)' }`
         ret_ty.serialize(ctx, (), stack, writer)?;
         write!(writer, " {}(", wrap_name)?;


Test patch:
diff --git a/bindgen-tests/tests/expectations/tests/generated/wrap_static_fns.c b/bindgen-tests/tests/expectations/tests/generated/wrap_static_fns.c
index 1057475f..abc52242 100644
--- a/bindgen-tests/tests/expectations/tests/generated/wrap_static_fns.c
+++ b/bindgen-tests/tests/expectations/tests/generated/wrap_static_fns.c
@@ -2,19 +2,11 @@
 
 // Static wrappers
 
-int foo__extern(void) asm("foo__extern");
 int foo__extern(void) { return foo(); }
-int bar__extern(void) asm("bar__extern");
 int bar__extern(void) { return bar(); }
-int takes_ptr__extern(int *arg) asm("takes_ptr__extern");
 int takes_ptr__extern(int *arg) { return takes_ptr(arg); }
-int takes_fn_ptr__extern(int (*f) (int)) asm("takes_fn_ptr__extern");
 int takes_fn_ptr__extern(int (*f) (int)) { return takes_fn_ptr(f); }
-int takes_fn__extern(int (f) (int)) asm("takes_fn__extern");
 int takes_fn__extern(int (f) (int)) { return takes_fn(f); }
-int takes_alias__extern(func f) asm("takes_alias__extern");
 int takes_alias__extern(func f) { return takes_alias(f); }
-int takes_qualified__extern(const int *const *arg) asm("takes_qualified__extern");
 int takes_qualified__extern(const int *const *arg) { return takes_qualified(arg); }
-enum foo takes_enum__extern(const enum foo f) asm("takes_enum__extern");
 enum foo takes_enum__extern(const enum foo f) { return takes_enum(f); }
diff --git a/bindgen-tests/tests/expectations/tests/wrap-static-fns.rs b/bindgen-tests/tests/expectations/tests/wrap-static-fns.rs
index fbfdcb13..0db78054 100644
--- a/bindgen-tests/tests/expectations/tests/wrap-static-fns.rs
+++ b/bindgen-tests/tests/expectations/tests/wrap-static-fns.rs
@@ -6,19 +6,19 @@
 )]
 
 extern "C" {
-    #[link_name = "\u{1}foo__extern"]
+    #[link_name = "foo__extern"]
     pub fn foo() -> ::std::os::raw::c_int;
 }
 extern "C" {
-    #[link_name = "\u{1}bar__extern"]
+    #[link_name = "bar__extern"]
     pub fn bar() -> ::std::os::raw::c_int;
 }
 extern "C" {
-    #[link_name = "\u{1}takes_ptr__extern"]
+    #[link_name = "takes_ptr__extern"]
     pub fn takes_ptr(arg: *mut ::std::os::raw::c_int) -> ::std::os::raw::c_int;
 }
 extern "C" {
-    #[link_name = "\u{1}takes_fn_ptr__extern"]
+    #[link_name = "takes_fn_ptr__extern"]
     pub fn takes_fn_ptr(
         f: ::std::option::Option<
             unsafe extern "C" fn(
@@ -28,7 +28,7 @@ extern "C" {
     ) -> ::std::os::raw::c_int;
 }
 extern "C" {
-    #[link_name = "\u{1}takes_fn__extern"]
+    #[link_name = "takes_fn__extern"]
     pub fn takes_fn(
         f: ::std::option::Option<
             unsafe extern "C" fn(
@@ -41,11 +41,11 @@ pub type func = ::std::option::Option<
     unsafe extern "C" fn(arg1: ::std::os::raw::c_int) -> ::std::os::raw::c_int,
 >;
 extern "C" {
-    #[link_name = "\u{1}takes_alias__extern"]
+    #[link_name = "takes_alias__extern"]
     pub fn takes_alias(f: func) -> ::std::os::raw::c_int;
 }
 extern "C" {
-    #[link_name = "\u{1}takes_qualified__extern"]
+    #[link_name = "takes_qualified__extern"]
     pub fn takes_qualified(
         arg: *const *const ::std::os::raw::c_int,
     ) -> ::std::os::raw::c_int;
@@ -53,6 +53,6 @@ extern "C" {
 pub const foo_BAR: foo = 0;
 pub type foo = ::std::os::raw::c_uint;
 extern "C" {
-    #[link_name = "\u{1}takes_enum__extern"]
+    #[link_name = "takes_enum__extern"]
     pub fn takes_enum(f: foo) -> foo;
 }