mockall icon indicating copy to clipboard operation
mockall copied to clipboard

External functions seem to be "overwritten" by mocks when exported

Open marcusklaas opened this issue 1 year ago • 11 comments

I am working on a project with multiple packages, where one package exposes a module containing an extern "C" block with a number of foreign functions. And it seems that once I put an #[mockall::automock] attribute on this module, I can no longer access the original function.

To illustrate the problem, imagine a project with 2 crates: test and test_sys. The latter exposes a single foreign function, and test imports that (mocked) module and tries to call the foreign function.

  • test_sys/src/lib.rs:

    pub mod outer {
        #[mockall::automock]
        pub mod inner {
            extern "C" {
                pub fn foo(val: i32) -> i32;
            }
        }
    }
    
  • test/src/lib.rs

    #[test]
    fn test() {
        unsafe { 
            test_sys::outer::inner::foo(5i32);
        }
    }
    

Now when I run that test, I get the following error message:

thread 'test' panicked at packages\test_sys\src\lib.rs:2:5:
mock_inner::foo(5): No matching expectation found

Suggesting that we called the mocked function rather than the original, which is not what we intended to do!

Looking at the macro expanded source of test_sys, it does seem like automock created a new mock_inner module and left the original inner module in place. So I'm not sure why we end up calling the mocked variant when we're not using #[double] or anything. The only thing I can think of is that maybe the use of #[no_mangle] on the definition of the mock function is throwing the linker for a loop or something?

Or maybe it's something else entirely, of course. In any case, I'd be very grateful for your guidance!

Edit: it indeed appears to be a linker issue. On our CI, which uses a different linker, this is now throwing multiple symbol definition errors for these external functions.

marcusklaas avatar Aug 20 '24 13:08 marcusklaas

Can you reproduce the problem with a single crate, or are 2 crates always required? A linker issue would be very "interesting"...

asomers avatar Aug 20 '24 14:08 asomers

Woops, good catch. It turns out that multiple crates are not required. This snippet exhibits the same behavior:

pub mod outer {
    #[mockall::automock]
    pub mod inner {
        extern "C" {
            pub fn foo(val: i32) -> i32;
        }
    }
}

#[test]
fn test() {
    unsafe { 
        outer::inner::foo(5i32);
    }
}

marcusklaas avatar Aug 20 '24 15:08 marcusklaas

I've improved the regression test for this feature, and it fails in the way that you describe. Could you please show me the error that your CI linker shows you? And what linker is it?

asomers avatar Aug 20 '24 17:08 asomers

The linker in our CI is ld. And the error message looks like this:

/_work/path/c_source_file.c:152: multiple definition of `function_being_mocked'; /_work/rust_path/target/debug/deps/libtest_sys-32ed19e140a7bf2e.libtest_sys.73e7146e6d253a1-cgu.08.rcgu.o:/_work/rust_path/target/debug/deps/libtest-sys-a43af6f4aa6f3d3a/out/bindings.rs:39768: first defined here
          collect2: error: ld returned 1 exit status

So it appears it's the symbol of the original C function that's clashing with the symbol of the mocked function (presumably because its symbol isn't being mangled?)

Edit: ~~so maybe we are calling the right Rust function, and it's the Rust function that is actually calling the mock instead of the foreign C func? That'd be kinda cool.~~ Edit 2: not sure if that makes sense.

marcusklaas avatar Aug 20 '24 17:08 marcusklaas

The bug was introduced by #504 . Fixing it without breaking the intention of that PR will be tricky. There are 3 distinct use cases that we need to satisfy:

  • Most users don't care about the ABI or name mangling of their mock functions. Their unit tests will only call the mock function, not the C one. And for their regular builds, the mock function won't be defined.
  • You are different because you want to export public mocks from your crate. So the crate needs to both include the mock function and link to the real one. Removing the #[no_mangle] is sufficient to fix your case.
  • But @Enes1313 wants to define mock functions in Rust and call them from C. So he requires the #[no_mangle]. He also requires that his unit test builds not bind to the real C function.

One solution would be to remove the #[no_mangle], and require users like @Enes1313 to write their own #[no_mangle] wrapper functions. But I'm not sure that's the best solution. I'll have to give it some more thought.

asomers avatar Aug 20 '24 17:08 asomers

I'm still new to Rust. That's why I can't add comments. I will follow changes.

Enes1313 avatar Aug 30 '24 14:08 Enes1313

@Enes1313 could you please provide some more information about how your project is structured? Do you build a cdylib crate containing the mock function, and call that from a C program? If so, then how do you set the expectations? Or do you build a Rust program that links to a C library, and you pass the mock function to the C library as a function pointer?

asomers avatar Aug 31 '24 14:08 asomers

My main goal is a system to prepare unit and integration tests in Rust language for C projects.

For example:

There are a.h, a.c ("a" module), b.h, b.c ("b" module) files. a.c . The a.c file uses b.h.. Using bindgen, I generated signatures from a.h and b.h. (a.rs, b.rs) Using mockall, I want to generate b_mock.rs. Then, I want to test for "a" module in Rust. (unit_test_a.rs) unit_test_a.rs uses a.rs and b_mock.rs. It call a function in "a" module. The function from "a" module call a function in "b_mock.rs".

I haven't looked at what I'm talking about for a long time. I may be remembering incorrectly, but I think it was like I said. I was planning to develop a tool that would make this work easier. Maybe I don't have to write a tool. It could be a document.

Enes1313 avatar Aug 31 '24 23:08 Enes1313

@Enes1313 could you please share an example of the command that you use to link the C objects and Rust objects together?

asomers avatar Sep 02 '24 14:09 asomers

It will take some time because my files are elsewhere. I will share a repo with you soon. @asomers

Enes1313 avatar Sep 03 '24 08:09 Enes1313

@asomers

Hi again, I created two repos about my project.

The project is at the very beginning.

test-with-rust: https://github.com/Enes1313/test-with-rust Test C Project : https://github.com/Enes1313/test-c-project-for-rust

1- Place the two projects under one directory. (If you don't want this, change "project_path" in Cargo.toml) 2- install clang for bindgen 3- My rust toolchain : nightly-x86_64-unknown-linux-gnu (default) 4- Most things, including compile_commands, are not currently used and are not automated. (So when you see some codes, don't focus on the reason.) 5- Two test files are not running at the same time. I haven't set it up yet. That's why there are comment lines in test_util_example.rs. If you want to test "test_util_example.rs", Go to previous commit.

Enes1313 avatar Sep 03 '24 19:09 Enes1313