Pin the inkwell version to latest stable on master

[tewaro]

The current master branch of inkwell seems to change a lot and for compatibility issues it might help to pin the commit.

I was having trouble building this as well because the macro for llvm_versions now uses different syntax so I fixed that as well.

Please let me know what steps I can take to help get this merged, I really like the idea of this tool and this allows me to use it for my research.

[jamesmth]

You are right, we should pin to a specific commit, I overlooked that!

[jamesmth]

Most MacOS tests failed and some Linux tests failed as well.

For MacOS, it seems that TheDan64/inkwell@5c9f7fc fixes the issue. For Linux, however, we may have to pin to an older commit (one that would not break MacOS builds either).

It may help you to know that I was using TheDan64/inkwell@5d5a531 the last time the CI was run and succeeded.

[tewaro]

Cool I'll start the search from there thanks!

[tewaro]

is there some sort of docker style tests for this? the different linux instances/would you be opposed to introducing that so I can build the linux builds locally?

[jamesmth]

There are no docker images for testing the Linux builds, since I wanted to keep the CI workflow similar between MacOS, Windows and Linux.

If you look at the .github/workflows/linux.yml file, you can reproduce the Linux test locally with a few commands. Running the test 4 (which fails) with llvm-12 for instance:

$ wget -O llvm.tar.gz https://github.com/jamesmth/llvm-project/releases/download/v12.0.1-rust-1.55/llvm-lld-12.0.1-rust-1.55-linux-x86_64.tar.gz
$ tar xf llvm.tar.gz
$ LLVM_SYS_120_PREFIX=$PWD/llvm cargo b --manifest-path tests/plugins/Cargo.toml -p plugin* --no-default-features --features target-x86,llvm12-0
$ llvm/bin/opt --load-pass-plugin="tests/plugins/target/debug/libplugin4.so" --passes="mpass,function(fpass)" tests/test.ll -disable-output

[jamesmth]

Anyway, I fixed the issue with Linux tests with the following modifications:

Replacing these lines:

llvm-plugin-rs/llvm-plugin/src/analysis.rs

Lines 74 to 81 in 31cc6f2

	unsafe {
	let res = crate::get_function_analysis_cached_result(
	self.inner,
	id,
	function.as_value_ref().cast(),
	);
	(!res.is_null()).then_some(Box::leak(Box::from_raw(res.cast())))
	}

With:

let res = crate::get_function_analysis_cached_result(
     self.inner,
     id,
     function.as_value_ref().cast(),
);

if !res.is_null() {
    let res = unsafe { Box::leak(Box::from_raw(res.cast())) };
    Some(res)
} else {
    None
}}

And replacing this line:

llvm-plugin-rs/llvm-plugin/src/analysis.rs

Line 217 in 31cc6f2

unsafe { (!res.is_null()).then_some(Box::leak(Box::from_raw(res.cast()))) }

With:

if !res.is_null() {
    let res = unsafe { Box::leak(Box::from_raw(res.cast())) };
    Some(res)
} else {
    None
}

If you apply these changes it should pass the CI tests (hopefully).

[jamesmth]

Just apply rustfmt and we are good to merge!