Generate extern wrappers for inlined functions #2335

pvdrz · 2022-11-04T18:02:43Z

If bindgen finds a static or static inline function and the new --wrap-static-fns option is enabled, then:

It will generate a new c source files with functions that wrap the non-external functions.
Rename the link_name of the bindings generated for non-external functions so they link against the wrappers instead.

The following options were also added:

--wrap-static-fns-suffix=<suffix>: Adds <suffix> to the name of each wrapper function (__extern is used by default).
--wrap-static-fns-path=<path>: writes the generated c code at <path>.c or <path>.cpp accordingly (/tmp/bindgen/extern is used by default).

Given that the C code serialization is experimental and only supports a very limited set of C functions, we also added a new --experimental flag and "experimental" feature and all the three new options were put behind this feature.

Fixes #1090.

If you're interested in how to use this feature see: #2405

bors-servo · 2022-11-09T15:07:05Z

☔ The latest upstream changes (presumably ed3aa90) made this pull request unmergeable. Please resolve the merge conflicts.

bors-servo · 2022-11-22T16:44:23Z

☔ The latest upstream changes (presumably 046d6f9) made this pull request unmergeable. Please resolve the merge conflicts.

bors-servo · 2022-11-24T16:19:57Z

☔ The latest upstream changes (presumably 95fd17b) made this pull request unmergeable. Please resolve the merge conflicts.

bors-servo · 2022-12-09T21:18:50Z

☔ The latest upstream changes (presumably c17c292) made this pull request unmergeable. Please resolve the merge conflicts.

bors-servo · 2023-01-12T20:40:41Z

☔ The latest upstream changes (presumably ed2d06e) made this pull request unmergeable. Please resolve the merge conflicts.

alex · 2023-01-22T19:10:18Z

Question: Is it possible this same strategy can be extended to support (some) macros?

pvdrz · 2023-01-23T14:33:15Z

Question: Is it possible this same strategy can be extended to support (some) macros?

what do you mean by this? Like to process function-like macros in a certain way?

alex · 2023-01-23T14:40:12Z

Yeah, for a macro that's function-like (e.g., https://github.com/openssl/openssl/blob/163b2bcd8b2e5cd149dfc8dce1ca096805559379/include/openssl/err.h#L198) automatically generate a C function that calls it and can be linked. I guess the user of bindgen would need to provide a signature.

Or maybe we should just send PRs uptream to convert things to use static inline and then use this :-)

pvdrz · 2023-01-23T14:45:50Z

we might be able to reuse some parts of this for that (or at least some things in a conceptual sense). We can continue this discussion over here: #753

If bindgen finds an inlined function and the `--generate-extern-functions` options is enabled, then: - It will generate two new source and header files with external functions that wrap the inlined functions. - Rerun `Bindings::generate` using the new header file to include these wrappers in the generated bindings. The following additional options were added: - `--extern-function-suffix=<suffix>`: Adds <suffix> to the name of each external wrapper function (`__extern` is used by default). - `--extern-functions-file-name=<name>`: Uses <name> as the file name for the header and source files (`extern` is used by default). - `--extern-function-directory=<dir>`: Creates the source and header files inside <dir> (`/tmp/bindgen` is used by default). The C code serialization is experimental and only supports a very limited set of C functions. Fixes rust-lang#1090.

test(static inlined): pretty print the diff Issue: rust-langGH-1090 test(static inlined): refactor paths once again - Because directory structure is confusing to the author of this commit rust-langGH-1090 test(static inlined): refactor test files - Expected files should be under tests/expectations/generated - Remove extern_stub.h because we can reuse the header from generate-extern-functions.h test(static inlined): diff test; generated files refactor(static inlined): integration test Issue: rust-langGH-1090 rust-langGH-1090: Integration tests integration test: document the GH issue integration test (macro): same order in impl as the trait for consistency integration test: move each setup into its own function, just to save face inline static func integration test resolve accidental conflict

emilio

So one thing that came up while looking at this... Is the second run even necessary? Can't we just have the suffix added needed on the first run, and rely on someone providing that symbol?

Might be overlooking something tho.

bindgen-cli/options.rs

bindgen/codegen/mod.rs

bindgen/ir/serialize.rs

bindgen/ir/function.rs

pvdrz · 2023-02-02T22:40:56Z

After playing with c constness I was able to achieve the following. If you pass this to bindgen:

typedef int (*OPENSSL_sk_cmp_func)(const void **a, const void **b);
typedef int (*sk_void_cmp_func)(const void **, const void **);

static inline int sk_void_call_cmp_func(OPENSSL_sk_cmp_func cmp_func, const void *const *a,
                      const void *const *b) {
  const void *a_ptr = (const void *)*a;
  const void *b_ptr = (const void *)*b;
  return ((sk_void_cmp_func)cmp_func)(&a_ptr, &b_ptr);
}

it will emit this wrapper:

int sk_void_call_cmp_func__extern(OPENSSL_sk_cmp_func cmp_func, const void *const *a, const void *const *b) asm("sk_void_call_cmp_func__extern");
int sk_void_call_cmp_func__extern(OPENSSL_sk_cmp_func cmp_func, const void *const *a, const void *const *b) { return sk_void_call_cmp_func(cmp_func, a, b); }

alex · 2023-02-02T22:42:13Z

Thanks, looks like that does resolve the const errors. I don't want to get too optimistic, but I think we're close:

/Users/alex_gaynor/projects/boringssl/build/rust/wrapper.c:17:48: error: this function declaration is not a prototype [-Werror,-Wstrict-prototypes]
struct stack_st_void * sk_void_new_null__extern() asm("sk_void_new_null__extern");
                                               ^
                                                void
/Users/alex_gaynor/projects/boringssl/build/rust/wrapper.c:18:24: error: no previous prototype for function 'sk_void_new_null__extern' [-Werror,-Wmissing-prototypes]
struct stack_st_void * sk_void_new_null__extern() { return sk_void_new_null(); }
                       ^
/Users/alex_gaynor/projects/boringssl/build/rust/wrapper.c:17:24: note: this declaration is not a prototype; add 'void' to make it a prototype for a zero-parameter function
struct stack_st_void * sk_void_new_null__extern() asm("sk_void_new_null__extern");
                       ^
                                                void
/Users/alex_gaynor/projects/boringssl/build/rust/wrapper.c:18:48: error: this old-style function definition is not preceded by a prototype [-Werror,-Wstrict-prototypes]
struct stack_st_void * sk_void_new_null__extern() { return sk_void_new_null(); }
                                               ^

I think the fix is that for 0-argument functions, the asm() line needs to make the signature be (void) instead of ().

alex · 2023-02-02T22:49:09Z

It builds!!!!

I have to wire everything up and make sure it works, but thank you so much for your efforts here. I'm super excited.

pvdrz · 2023-02-02T22:53:25Z

I think the internet is mad at us and all the server request from github workflows are timing out 😅

I'm going to leave it like this for today and re-run CI tomorrow.

…e naturally This PR uses the in-development bindgen support for static inline functions (rust-lang/rust-bindgen#2335) + an in-development boringssl patch (https://boringssl-review.googlesource.com/c/boringssl/+/56505) to allow using boringssl with rust-openssl without needing a .cargo/config override

alex · 2023-02-02T23:49:56Z

Just wanted to confirm that everything is now working end-to-end.

…e naturally This PR uses the in-development bindgen support for static inline functions (rust-lang/rust-bindgen#2335) + an in-development boringssl patch (https://boringssl-review.googlesource.com/c/boringssl/+/56505) to allow using boringssl with rust-openssl without needing a .cargo/config override

emilio

From a skim it looks good, let's try this. r=me with the nits below, please squash the commits before landing as needed. Thanks for working on this, it took a while but it's great!

bindgen/codegen/mod.rs

bindgen/codegen/serialize.rs

…e naturally This PR uses the in-development bindgen support for static inline functions (rust-lang/rust-bindgen#2335) + an in-development boringssl patch (https://boringssl-review.googlesource.com/c/boringssl/+/56505) to allow using boringssl with rust-openssl without needing a .cargo/config override

SeleDreams · 2023-02-12T03:51:36Z

one thing I notice is that the headers don't get included in the generated C file. Is it intended ?
the source should probably include the h file used during the bindgen command

pvdrz · 2023-02-12T04:11:05Z

one thing I notice is that the headers don't get included in the generated C file. Is it intended ? the source should probably include the h file used during the bindgen command

see #2405

SeleDreams · 2023-02-12T04:17:07Z

one thing I notice is that the headers don't get included in the generated C file. Is it intended ? the source should probably include the h file used during the bindgen command

see #2405

the issue is that as i'm building for the DS which has a pretty specific toolchain, i have to use the cc crate to build the C file
atm i have this command, which succeeds if i modify the C file to include the header
cc::Build::new().file("src/arm7_bindings.c").compiler(format!("{}/devkitARM/bin/arm-none-eabi-gcc",dkp_path)).include(format!("{}/libnds/include",dkp_path)).include(format!("{}/devkitARM/lib/gcc/arm-none-eabi/12.2.0/include",dkp_path)).compile("bindings");
but i dunno if it's even possible to force the cc command to include a header for the c file

pvdrz · 2023-02-12T16:48:00Z

I'm guessing that .include("the_input_headers.h") should work

SeleDreams · 2023-02-12T16:49:46Z

The include command of CC is to include a directory weirdly, but i found out i could pass raw arguments to CC so i was able to force the include this way Le dim. 12 févr. 2023 à 17:48, Christian Poveda Ruiz < ***@***.***> a écrit :

…

I'm guessing that .include("the_input_headers.h") should work — Reply to this email directly, view it on GitHub <#2335 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AD4UF4OXVSTFOZEW4NVIXTTWXEH4XANCNFSM6AAAAAARXNQWCE> . You are receiving this because you commented.Message ID: ***@***.***>

pvdrz · 2023-02-12T17:59:14Z

I'm not that familiar with cc but from the docs it seems something like .file("input.h") should work

pvdrz requested a review from emilio November 4, 2022 18:02

pvdrz force-pushed the generate-inline-functions branch from c4fb085 to 039d7d0 Compare November 4, 2022 20:28

pvdrz marked this pull request as draft November 4, 2022 20:55

pvdrz force-pushed the generate-inline-functions branch from 039d7d0 to aab6160 Compare November 8, 2022 20:08

pvdrz marked this pull request as ready for review November 8, 2022 20:08

pvdrz force-pushed the generate-inline-functions branch from aab6160 to 65ff041 Compare November 9, 2022 15:21

pvdrz force-pushed the generate-inline-functions branch from 65ff041 to 9b7ccf5 Compare November 22, 2022 21:27

pvdrz force-pushed the generate-inline-functions branch from 9b7ccf5 to 0736ef4 Compare November 24, 2022 16:30

pvdrz force-pushed the generate-inline-functions branch from 0736ef4 to e9abb71 Compare December 9, 2022 21:40

pvdrz force-pushed the generate-inline-functions branch from e9abb71 to 4b5fec4 Compare January 16, 2023 17:23

amanjeev assigned amanjeev and unassigned amanjeev Jan 16, 2023

pvdrz force-pushed the generate-inline-functions branch from 4b5fec4 to d5d96e2 Compare January 16, 2023 19:49

pvdrz and others added 3 commits January 24, 2023 14:37

add the experimental feature

101cde0

pvdrz force-pushed the generate-inline-functions branch from 4efcc6f to 101cde0 Compare January 24, 2023 19:53

emilio requested changes Jan 25, 2023

View reviewed changes

bindgen-cli/options.rs Outdated Show resolved Hide resolved

bindgen/codegen/mod.rs Outdated Show resolved Hide resolved

bindgen/ir/serialize.rs Outdated Show resolved Hide resolved

bindgen/ir/function.rs Outdated Show resolved Hide resolved

pvdrz added 3 commits January 26, 2023 12:14

Rename options

61b2b44

Fixup: Rename options

65a80e0

merge filename and directory arguments

eb823b8

use void for empty args

9326863

alex mentioned this pull request Feb 2, 2023

fixes #1768 -- integrate boringssl into the build process more naturally sfackler/rust-openssl#1806

Closed

amanjeev approved these changes Feb 6, 2023

View reviewed changes

pass amanfmt

5a9e5ca

emilio approved these changes Feb 7, 2023

View reviewed changes

bindgen/codegen/mod.rs Outdated Show resolved Hide resolved

bindgen/codegen/mod.rs Outdated Show resolved Hide resolved

bindgen/codegen/serialize.rs Outdated Show resolved Hide resolved

bindgen/codegen/serialize.rs Outdated Show resolved Hide resolved

some nits

572d1a4

pvdrz merged commit 2be14a3 into rust-lang:main Feb 7, 2023

pvdrz deleted the generate-inline-functions branch February 7, 2023 15:13

jrmuizel mentioned this pull request Feb 7, 2023

Handle inline functions better valarauca/xed-sys#1

Open

ojeda mentioned this pull request Feb 7, 2023

bindgen wanted features & bugfixes Rust-for-Linux/linux#353

Open

25 tasks

Meziu mentioned this pull request May 15, 2023

Create a safe SVC module rust3ds/ctru-rs#99

Closed

DeltaF1 mentioned this pull request May 15, 2023

Expose static inline functions to bindings rust3ds/ctru-rs#123

Closed

ammarfaizi2 mentioned this pull request Aug 12, 2023

liburing-ffi headers axboe/liburing#921

Closed

jktjkt mentioned this pull request Sep 15, 2023

Make tree_data static functions available in api CESNET/libyang#2101

Merged

midnightveil mentioned this pull request Oct 21, 2024

A minimal Rust based sdmmc driver for odroidc4 au-ts/sddf#273

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generate extern wrappers for inlined functions #2335

Generate extern wrappers for inlined functions #2335

pvdrz commented Nov 4, 2022 •

edited

Loading

bors-servo commented Nov 9, 2022

bors-servo commented Nov 22, 2022

bors-servo commented Nov 24, 2022

bors-servo commented Dec 9, 2022

bors-servo commented Jan 12, 2023

alex commented Jan 22, 2023

pvdrz commented Jan 23, 2023

alex commented Jan 23, 2023

pvdrz commented Jan 23, 2023

emilio left a comment

pvdrz commented Feb 2, 2023

alex commented Feb 2, 2023

alex commented Feb 2, 2023

pvdrz commented Feb 2, 2023

alex commented Feb 2, 2023

emilio left a comment

SeleDreams commented Feb 12, 2023 •

edited

Loading

pvdrz commented Feb 12, 2023

SeleDreams commented Feb 12, 2023

pvdrz commented Feb 12, 2023

SeleDreams commented Feb 12, 2023 via email

pvdrz commented Feb 12, 2023

Generate extern wrappers for inlined functions #2335

Generate extern wrappers for inlined functions #2335

Conversation

pvdrz commented Nov 4, 2022 • edited Loading

bors-servo commented Nov 9, 2022

bors-servo commented Nov 22, 2022

bors-servo commented Nov 24, 2022

bors-servo commented Dec 9, 2022

bors-servo commented Jan 12, 2023

alex commented Jan 22, 2023

pvdrz commented Jan 23, 2023

alex commented Jan 23, 2023

pvdrz commented Jan 23, 2023

emilio left a comment

Choose a reason for hiding this comment

pvdrz commented Feb 2, 2023

alex commented Feb 2, 2023

alex commented Feb 2, 2023

pvdrz commented Feb 2, 2023

alex commented Feb 2, 2023

emilio left a comment

Choose a reason for hiding this comment

SeleDreams commented Feb 12, 2023 • edited Loading

pvdrz commented Feb 12, 2023

SeleDreams commented Feb 12, 2023

pvdrz commented Feb 12, 2023

SeleDreams commented Feb 12, 2023 via email

pvdrz commented Feb 12, 2023

pvdrz commented Nov 4, 2022 •

edited

Loading

SeleDreams commented Feb 12, 2023 •

edited

Loading