Possibly unsupported static constructor (C++)

[Belval]

Possibly problematic C++ code

static auto Create(const EncryptionParameters &parms,  bool expand_mod_chain = true)
{
   return std::shared_ptr<SEALContext>(
       new SEALContext(parms, expand_mod_chain, 
       MemoryManager::GetPool()));
}

Bindgen Invocation

let bindings = bindgen::Builder::default()
        .generate_inline_functions(true)
        .derive_default(true)
        .header("./seal/src/seal/seal.h")
        .clang_arg("-I./seal/src/")
        .clang_arg("-std=c++17")
        .clang_arg("-x")
        .clang_arg("c++")
        .opaque_type("std::.*")
        .whitelist_type("seal::.*")
        .whitelist_function("seal::.*")
        .generate()
        .expect("Unable to generate bindings");

then

// Building the EncryptionParameters object
let mut ep: seal_EncryptionParameters = Default::default();
seal_EncryptionParameters_EncryptionParameters(&mut ep, seal_scheme_type_BFV);
...
// Construct the context
let mut ctx = seal_SEALContext::Create(&ep, true);
//OR
let mut ctx = seal_SEALContext_Create(&ep, true);

I tried all combinations that i could find in the bindings, there doesn't seem to be a right way to do it.

Actual Results

error: process didn't exit successfully: ... (signal: 11, SIGSEGV: invalid memory reference)

I used rust-gdb to see what was causing it, and it seems to be during the assignation? of the MemoryPool from the EncryptionParameters.

#0  0x00005555555b2e36 in std::__shared_ptr<seal::util::MemoryPool, (__gnu_cxx::_Lock_policy)2>::operator= (this=0x7ffff7a77540) at /usr/include/c++/8.2.1/bits/shared_ptr_base.h:1058
#1  0x00005555555b2e85 in std::shared_ptr<seal::util::MemoryPool>::operator= (this=0x7ffff7a77540) at /usr/include/c++/8.2.1/bits/shared_ptr.h:103
#2  0x00005555555b2eaf in seal::MemoryPoolHandle::operator= (this=0x7ffff7a77540, assign=...) at ./seal/src/seal/memorymanager.h:121
#3  0x00005555555b2dce in seal::MemoryPoolHandle::MemoryPoolHandle (this=0x7ffff7a77540, copy=...) at ./seal/src/seal/memorymanager.h:96
#4  0x00005555555b5e02 in seal::EncryptionParameters::EncryptionParameters (this=0x7ffff7a77540, copy=...) at ./seal/src/seal/encryptionparams.h:103
#5  0x00005555555b622b in seal::SEALContext::Create (parms=..., expand_mod_chain=true) at ./seal/src/seal/context.h:329
#6  0x000055555556f697 in seal_rs::seal_SEALContext::Create (parms=0x7ffff7a77668, expand_mod_chain=true) at src/./bindings.rs:7098
#7  0x00005555555ae446 in seal_rs::example1 () at src/lib.rs:22
#8  0x000055555556f4da in seal_rs::example1::{{closure}} () at src/lib.rs:9
#9  0x00005555555af28e in core::ops::function::FnOnce::call_once () at /build/rust/src/rustc-1.32.0-src/src/libcore/ops/function.rs:238

Expected Results

I expect it to run without segmentation fault. If static constructor are supported.

[emilio]

Hmm, this is probably an ABI mismatch somewhere. If you could create a reduced test-case I'd be happy to take a look.

[emilio]

Oh, actually, that's a bit more surprising, it's crashing on the C++ side... Do layout tests pass for EncryptionParameters?

[Belval]

All layout tests are passing.

What would a satisfactory reduced test case look like? I just want to make sure that whatever I provide will actually help you (or anyone else) approach the issue.

[emilio]

A standalone repo / gist that reproduces the issue with as little C++ / Rust code as possible would be great.

[emilio]

Or maybe even a patch to this repo's bindgen-integration tests, which already have all the build setup.

[Belval]

It's not minimal yet (I think) but I trimmed a lot of fat.

See: https://github.com/Belval/seal-bindgen-test-case

You can get the error simply by running cargo test

[emilio]

Ah, I can repro that, thanks! So the issue is an ABI issue, and it's a bit self-inflicted unfortunately :(.

So the function prototype bindgen generates returns [u64; 2], but the actual type returned is std::shared_ptr, which is of course a non-trivial type and all that, so the ABI is different. Let me try to confirm that / fix it.

[emilio]

Ok, so I took a look at it, I got the STL to not complain (#1513), and then used the replaces= feature so we have a decent std::shared_ptr:

diff --git a/Cargo.toml b/Cargo.toml
index b766ef1..bd60449 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -6,7 +6,7 @@ build = "build.rs"
 
 [build-dependencies]
 git2 = "0.8"
-bindgen = "0.42.2"
+bindgen = { path = "/home/emilio/src/moz/rust-bindgen" }
 cc = "1.0"
 
 [dependencies]
diff --git a/build.rs b/build.rs
index 7606151..8f23d2b 100644
--- a/build.rs
+++ b/build.rs
@@ -32,12 +32,11 @@ fn main() {
     let bindings = bindgen::Builder::default()
         .generate_inline_functions(true)
         .derive_default(true)
-        .header("./seal/src/seal/seal.h")
+        .header("src/bindings.h")
         .clang_arg("-I./seal/src/")
         .clang_arg("-std=c++17")
         .clang_arg("-x")
         .clang_arg("c++")
-        .opaque_type("std::.*")
         .whitelist_type("seal::.*")
         .whitelist_function("seal::.*")
         .generate()
diff --git a/src/bindings.h b/src/bindings.h
new file mode 100644
index 0000000..1ce8790
--- /dev/null
+++ b/src/bindings.h
@@ -0,0 +1,12 @@
+#include <seal/seal.h>
+
+/// <div rustbindgen replaces="std::shared_ptr"></div>
+template<typename T>
+class simple_shared_ptr {
+ public:
+  T* ptr;
+  void* count;
+};
+
+static_assert(sizeof(simple_shared_ptr<int>) == sizeof(std::shared_ptr<int>), "");
+static_assert(alignof(simple_shared_ptr<int>) == alignof(std::shared_ptr<int>), "");

(You could use the same for the util_Ptr stuff btw, I'd think).

Anyhow, with that the issue persists. I could confirm there's an ABI issue. From Rust:

(rr) p &ep
$3 = (*mut seal_rs::seal_EncryptionParameters) 0x7ffc6413c580
(rr) down
#5  0x00005560b40fdc42 in seal::SEALContext::Create (parms=..., expand_mod_chain=128) at ./seal/src/seal/context.h:24
(rr) p &parms
$4 = (const seal::EncryptionParameters *) 0x0

So I think this is basically a dupe of #778 / rust-lang/rust#38258, and the workaround is creating an extern "C" wrapper until there's a way to tell rustc to use the right calling convention :(

[Belval]

Apologies if I misunderstand, but there is already an extern "C" to create the SEALContext object:

extern "C" {
    #[link_name = "\u{1}_ZN4seal11SEALContext6CreateERKNS_20EncryptionParametersEb"]
    pub fn seal_SEALContext_Create(
        parms: *const seal_EncryptionParameters,
        expand_mod_chain: bool,
    ) -> [u64; 2usize];
}

but it doesn't work either because of the [u64; 2usize] return type. You say that we cannot return std::shared_ptr because of it's too complex. Which is why to patched the type with simple_share_pointer. Why wouldn't it work with that class?

I guess my question is, what do you mean by creating an extern "C" wrapper? In what ways would it be different from the already existing one?

Thank you for taking the time to investigate the issue.

[emilio]

The issue is that, when returning the shared ptr, the itanium C++ ABI says that it is non-trivial for the purpose of calls (since it has a destructor). Then, returning a shared_ptr is supposed to go via:

https://itanium-cxx-abi.github.io/cxx-abi/abi.html#value-parameter

Point 1, that is:

If the parameter type is non-trivial for the purposes of calls, the caller must allocate space for a temporary and pass that temporary by reference.

Regardless of whether it's a struct or a class, rust won't let us do that, and will always use registers for small structs.

What I mean is that you can write a C function that doesn't return the complex type, for example:

extern "C" SealContext* sealrs_Context_Create(const EncryptionParameters* parms, bool expand_mod_chain) {
    return new SealContext(...);
}

And you could call that function, and it'll work.

Another alternative would be something like:

extern "C" void sealrs_Context_Create(const EncryptionParameters* parms, bool expand_mod_chain, shared_ptr<SealContext>* out) {
    *out = SealContext::Create(..);
}

[Belval]

Oh ok it is clearer now. Thank you!

[Belval]

I did what I thought was you described here but it does not seem to work.

I create a bindings.h file in which I put my extern "C" function:

#include <seal/seal.h>

using namespace seal;

namespace bindings 
{
  extern "C" SEALContext* SEALContext_Create(const EncryptionParameters* parms, bool expand_mod_chain) {
    return SEALContext::Create(*parms, expand_mod_chain).get();
  }
}

Then I edited my bindgen config like this:

    // Generate the bindings
    let bindings = bindgen::Builder::default()
        .generate_inline_functions(true)
        .derive_default(true)
        .header("src/bindings.h")
        .clang_arg("-I./seal/src/")
        .clang_arg("-std=c++17")
        .clang_arg("-x")
        .clang_arg("c++")
        .opaque_type("std::.*")
        .whitelist_type("seal::.*")
        .whitelist_function("seal::.*")
        .whitelist_type("bindings::.*")
        .whitelist_function("bindings::.*")
        .generate()
        .expect("Unable to generate bindings");

Setting my header file as "bindings.h" with it having an include for "seal.h". The generated bindings seems to be right and include fn bindings_SEALContext_Create() and everything it had previously.

Yet when I run it I get this error message:

error: linking with `cc` failed: exit code: 1
...
/home/edouard/Git/seal-rs/target/debug/build/seal-rs-bb56035b3c8ff5e1/out/libseal.a(bindings.o) in archive is not an object

Which is unclear to me because it seems to imply that cc didn't include it in the final library.

[emilio]

I'm unsure you can define an inline extern "C" function, you probably need a new cpp file or something. Also, you're returning a freed pointer, that's no good.