Switch to bindgen's static inline support
This removes the need to hand-write rust_wrapper.c, because bindgen can
generate it for us. bindgen 0.65 or later is needed. Earlier versions of
this were buggy.
I've also removed the claim that bssl-sys is somehow a solution for
version skew. That was the original thinking from Android, but it hasn't
worked out. The version skew solution is simply "use bindgen, don't
handwrite bindings".
Android are quite behind their originaly July timeline for adding the
build half of this mechanism, but as this is now in the way of other
work, we're going to proceed with using this now. There is now a
unsupported_inline_wrappers cfg that Android can set to use the old
mechanism.
Update-Note: Rust support now requires your build correctly handle
--wrap-static-fns. On Android, you will need to enable the
unsupported_inline_wrappers cfg option until b/290347127 is fixed.
Chromium doesn't actually use any of the inline functions yet, so we can
handle --wrap-static-fns asynchronously, but I have a CL ready to enable
that.
Fixed: 596
Change-Id: I51fd1108a8c17a06f1bdd9171ebf352cea871723
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/58985
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
diff --git a/rust/bssl-sys/CMakeLists.txt b/rust/bssl-sys/CMakeLists.txt
index d17a8f1..0ed1c95 100644
--- a/rust/bssl-sys/CMakeLists.txt
+++ b/rust/bssl-sys/CMakeLists.txt
@@ -1,19 +1,15 @@
-# Additional interop for things like macros and inlined functions.
-add_library(rust_wrapper STATIC rust_wrapper.c)
-target_link_libraries(rust_wrapper crypto)
-
# Generate architecture-specific wrappers. bindgen must be called from
# ${CMAKE_BINARY_DIR}, with the output path as a relative path. bindgen writes
# the depfile using the same syntax as the command-line argument, and ninja
# requires a path relative to the top-level build directory.
-set(wrapper wrapper_${RUST_BINDINGS}.rs)
-binary_dir_relative_path(${wrapper} wrapper_relative)
-binary_dir_relative_path(${wrapper}.d depfile_relative)
+set(wrapper_rs wrapper_${RUST_BINDINGS}.rs)
+binary_dir_relative_path(${wrapper_rs} wrapper_rs_relative)
+binary_dir_relative_path(${wrapper_rs}.d depfile_relative)
add_custom_command(
- OUTPUT ${wrapper}
+ OUTPUT ${wrapper_rs} wrapper.c
COMMAND ${BINDGEN_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/wrapper.h
- -o ${wrapper_relative}
+ -o ${wrapper_rs_relative}
--depfile=${depfile_relative}
--no-derive-default
--enable-function-attribute-detection
@@ -37,13 +33,23 @@
# https://github.com/rust-lang/rust-bindgen/issues/2508), we can
# switch to that.
--allowlist-file=".*[[:punct:]]include[[:punct:]]openssl[[:punct:]].*\\.h"
- --allowlist-file=".*[[:punct:]]rust_wrapper\\.h"
+ --experimental
+ --wrap-static-fns
+ --wrap-static-fns-path="${CMAKE_CURRENT_BINARY_DIR}/wrapper.c"
-- # these are LLVM arg passthroughs
-I${PROJECT_SOURCE_DIR}/include
# https://doc.rust-lang.org/nightly/rustc/platform-support.html
--target=${RUST_BINDINGS}
DEPENDS wrapper.h
- DEPFILE ${CMAKE_CURRENT_BINARY_DIR}/${wrapper}.d
+ DEPFILE ${CMAKE_CURRENT_BINARY_DIR}/${wrapper_rs}.d
WORKING_DIRECTORY ${CMAKE_BINARY_DIR}
)
-add_custom_target(bssl_sys ALL DEPENDS ${wrapper})
+
+add_library(rust_wrapper STATIC wrapper.c)
+if(CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR CMAKE_COMPILER_IS_GNUCXX)
+ target_compile_options(rust_wrapper PRIVATE "-Wno-missing-prototypes")
+endif()
+target_include_directories(rust_wrapper PRIVATE ${CMAKE_CURRENT_SOURCE_DIR})
+target_link_libraries(rust_wrapper crypto ssl)
+
+add_custom_target(bssl_sys ALL DEPENDS ${wrapper_rs} rust_wrapper)
diff --git a/rust/bssl-sys/README.md b/rust/bssl-sys/README.md
index c988813..13b7290 100644
--- a/rust/bssl-sys/README.md
+++ b/rust/bssl-sys/README.md
@@ -1,7 +1,7 @@
bssl-sys
============
-A low-level binding crate for Rust that moves in lockstop with BoringSSL. BoringSSL explicitly does not have a stable ABI, `bssl-sys` is the solution for preventing subtle-memory corruption bugs due to version skew.
+A low-level binding crate for Rust that moves in lockstop with BoringSSL.
### How it works
`bssl-sys` uses `bindgen` as part of the cmake build process to generate Rust compatibility shims for the targeted platform. It is important to generate it for the correct platform because `bindgen` uses LLVM information for alignment which varies depending on architecture.
diff --git a/rust/bssl-sys/rust_wrapper.h b/rust/bssl-sys/rust_wrapper.h
index 632622a..55d5a6f 100644
--- a/rust/bssl-sys/rust_wrapper.h
+++ b/rust/bssl-sys/rust_wrapper.h
@@ -23,8 +23,10 @@
// The following functions are wrappers over inline functions and macros in
-// BoringSSL, which bindgen cannot currently correctly bind. These wrappers
-// ensure changes to the functions remain in lockstep with the Rust versions.
+// BoringSSL. These are not necessary, as bindgen has long supported
+// --wrap-static-fns, however Android is still missing support for this. (See
+// b/290347127.) These manual wrappers are, temporarily, retained for Android,
+// but this codepath is no longer tested or supported by BoringSSL.
int ERR_GET_LIB_RUST(uint32_t packed_error);
int ERR_GET_REASON_RUST(uint32_t packed_error);
int ERR_GET_FUNC_RUST(uint32_t packed_error);
diff --git a/rust/bssl-sys/src/lib.rs b/rust/bssl-sys/src/lib.rs
index 06b907c..718509e 100644
--- a/rust/bssl-sys/src/lib.rs
+++ b/rust/bssl-sys/src/lib.rs
@@ -6,14 +6,17 @@
include!(env!("BINDGEN_RS_FILE"));
// TODO(crbug.com/boringssl/596): Remove these wrappers.
+#[cfg(unsupported_inline_wrappers)]
pub fn ERR_GET_LIB(packed_error: u32) -> i32 {
unsafe { ERR_GET_LIB_RUST(packed_error) }
}
+#[cfg(unsupported_inline_wrappers)]
pub fn ERR_GET_REASON(packed_error: u32) -> i32 {
unsafe { ERR_GET_REASON_RUST(packed_error) }
}
+#[cfg(unsupported_inline_wrappers)]
pub fn ERR_GET_FUNC(packed_error: u32) -> i32 {
unsafe { ERR_GET_FUNC_RUST(packed_error) }
}
