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

Don't allow construction of non-POD types from Rust #25

Merged
merged 3 commits into from
Oct 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion engine/src/bridge_converter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,13 @@ impl<'a> BridgeConversion<'a> {
let should_be_pod = self.byvalue_checker.is_pod(&tyname);
self.generate_type_alias(tyname, should_be_pod);
if !should_be_pod {
s.fields = syn::Fields::Unit;
// See cxx's opaque::Opaque for rationale for this type... in
// short, it's to avoid being Send/Sync.
s.fields = syn::Fields::Named(parse_quote! {
{
do_not_attempt_to_allocate_nonpod_types: [*const u8; 0],
}
});
}
self.bindgen_items.push(Item::Struct(s));
}
Expand Down
119 changes: 116 additions & 3 deletions engine/src/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,56 @@ fn write_to_file(tdir: &TempDir, filename: &str, content: &str) -> PathBuf {
path
}

/// A positive test, we expect to pass.
fn run_test(
cxx_code: &str,
header_code: &str,
rust_code: TokenStream,
allowed_funcs: &[&str],
allowed_pods: &[&str],
) {
do_run_test(
cxx_code,
header_code,
rust_code,
allowed_funcs,
allowed_pods,
)
.unwrap()
}

fn run_test_expect_fail(
cxx_code: &str,
header_code: &str,
rust_code: TokenStream,
allowed_funcs: &[&str],
allowed_pods: &[&str],
) {
do_run_test(
cxx_code,
header_code,
rust_code,
allowed_funcs,
allowed_pods,
)
.expect_err("Unexpected success");
}

/// In the future maybe the tests will distinguish the exact type of failure expected.
#[derive(Debug)]
enum TestError {
AutoCxx(autocxx_build::Error),
CppBuild(cc::Error),
RsBuild,
}

fn do_run_test(
cxx_code: &str,
header_code: &str,
rust_code: TokenStream,
allowed_funcs: &[&str],
allowed_pods: &[&str],
) -> Result<(), TestError> {
// Step 1: Write the C++ header snippet to a temp file
let tdir = tempdir().unwrap();
write_to_file(&tdir, "input.h", &format!("#pragma once\n{}", header_code));
Expand Down Expand Up @@ -144,7 +187,8 @@ fn run_test(
let target_dir = tdir.path().join("target");
std::fs::create_dir(&target_dir).unwrap();
let target = rust_info::get().target_triple.unwrap();
let mut b = autocxx_build::Builder::new(&rs_path, tdir.path().to_str().unwrap()).unwrap();
let mut b = autocxx_build::Builder::new(&rs_path, tdir.path().to_str().unwrap())
.map_err(TestError::AutoCxx)?;
b.builder()
.file(cxx_path)
.out_dir(&target_dir)
Expand All @@ -154,7 +198,7 @@ fn run_test(
.flag("-std=c++14")
.include(tdir.path())
.try_compile("autocxx-demo")
.unwrap();
.map_err(TestError::CppBuild)?;
// Step 8: use the trybuild crate to build the Rust file.
let r = BUILDER.lock().unwrap().build(
&target_dir,
Expand All @@ -163,10 +207,14 @@ fn run_test(
"input.h",
&rs_path,
);
if r.is_err() {
return Err(TestError::RsBuild); // details of Rust panic are a bit messy to include, and
// not important at the moment.
}
if KEEP_TEMPDIRS {
println!("Tempdir: {:?}", tdir.into_path().to_str());
}
r.unwrap()
Ok(())
}

#[test]
Expand Down Expand Up @@ -990,6 +1038,71 @@ fn test_i32_const() {
run_test(cxx, hdr, rs, &["BOB"], &[]);
}

#[test]
fn test_negative_rs_nonsense() {
// Really just testing the test infrastructure.
let cxx = indoc! {"
"};
let hdr = indoc! {"
#include <cstdint>
const uint32_t BOB = 3;
"};
let rs = quote! {
foo bar
};
run_test_expect_fail(cxx, hdr, rs, &["BOB"], &[]);
}

#[test]
fn test_negative_cpp_nonsense() {
// Really just testing the test infrastructure.
let cxx = indoc! {"
"};
let hdr = indoc! {"
#include <cstdint>
const uint32_t BOB = CAT;
"};
let rs = quote! {
assert_eq!(ffi::BOB, 3);
};
run_test_expect_fail(cxx, hdr, rs, &["BOB"], &[]);
}

#[test]
fn test_negative_make_nonpod() {
let cxx = indoc! {"
uint32_t take_bob(const Bob& a) {
return a.a;
}
std::unique_ptr<Bob> make_bob(uint32_t a) {
auto b = std::make_unique<Bob>();
b->a = a;
return b;
}
"};
let hdr = indoc! {"
#include <cstdint>
#include <memory>
struct Bob {
uint32_t a;
};
std::unique_ptr<Bob> make_bob(uint32_t a);
uint32_t take_bob(const Bob& a);
"};
let rs = quote! {
ffi::cxxbridge::Bob {};
};
let rs2 = quote! {
ffi::cxxbridge::Bob { a: 12 };
};
let rs3 = quote! {
ffi::cxxbridge::Bob { do_not_attempt_to_allocate_nonpod_types: [] };
};
run_test_expect_fail(cxx, hdr, rs, &["take_bob", "Bob", "make_bob"], &[]);
run_test_expect_fail(cxx, hdr, rs2, &["take_bob", "Bob", "make_bob"], &[]);
run_test_expect_fail(cxx, hdr, rs3, &["take_bob", "Bob", "make_bob"], &[]);
}

// Yet to test:
// 1. Make UniquePtr<CxxStrings> in Rust
// 3. Constants
Expand Down