From 9a0f897d72495855f8e0252f4244c76d7d9157a5 Mon Sep 17 00:00:00 2001 From: Anton Baliasnikov Date: Fri, 31 Jan 2025 12:55:33 +0000 Subject: [PATCH 1/2] feat: add additional valgrind arguments support --- src/lib.rs | 5 +++++ src/platforms/aarch64_linux_gnu.rs | 2 ++ src/platforms/aarch64_linux_musl.rs | 4 ++++ src/platforms/shared.rs | 16 +++++++++++----- src/platforms/x86_64_linux_gnu.rs | 2 ++ src/platforms/x86_64_linux_musl.rs | 4 ++++ src/zksync_llvm/arguments.rs | 4 ++++ src/zksync_llvm/main.rs | 2 ++ 8 files changed, 34 insertions(+), 5 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index f2103eb..74d1476 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -161,6 +161,7 @@ pub fn build( enable_assertions: bool, sanitizer: Option, enable_valgrind: bool, + valgrind_options: Vec, ) -> anyhow::Result<()> { std::fs::create_dir_all(LLVMPath::DIRECTORY_LLVM_TARGET)?; @@ -180,6 +181,7 @@ pub fn build( enable_assertions, sanitizer, enable_valgrind, + valgrind_options, )?; } else if target_env == target_env::TargetEnv::GNU { platforms::x86_64_linux_gnu::build( @@ -195,6 +197,7 @@ pub fn build( enable_assertions, sanitizer, enable_valgrind, + valgrind_options, )?; } else { anyhow::bail!("Unsupported target environment for x86_64 and Linux"); @@ -246,6 +249,7 @@ pub fn build( enable_assertions, sanitizer, enable_valgrind, + valgrind_options, )?; } else if target_env == target_env::TargetEnv::GNU { platforms::aarch64_linux_gnu::build( @@ -261,6 +265,7 @@ pub fn build( enable_assertions, sanitizer, enable_valgrind, + valgrind_options, )?; } else { anyhow::bail!("Unsupported target environment for aarch64 and Linux"); diff --git a/src/platforms/aarch64_linux_gnu.rs b/src/platforms/aarch64_linux_gnu.rs index 3069cf2..268b382 100644 --- a/src/platforms/aarch64_linux_gnu.rs +++ b/src/platforms/aarch64_linux_gnu.rs @@ -30,6 +30,7 @@ pub fn build( enable_assertions: bool, sanitizer: Option, enable_valgrind: bool, + valgrind_options: Vec, ) -> anyhow::Result<()> { crate::utils::check_presence("cmake")?; crate::utils::check_presence("clang")?; @@ -105,6 +106,7 @@ pub fn build( )) .args(crate::platforms::shared::shared_build_opts_valgrind( enable_valgrind, + valgrind_options, )), "LLVM building cmake", )?; diff --git a/src/platforms/aarch64_linux_musl.rs b/src/platforms/aarch64_linux_musl.rs index 95bac6d..9b6b090 100644 --- a/src/platforms/aarch64_linux_musl.rs +++ b/src/platforms/aarch64_linux_musl.rs @@ -30,6 +30,7 @@ pub fn build( enable_assertions: bool, sanitizer: Option, enable_valgrind: bool, + valgrind_options: Vec, ) -> anyhow::Result<()> { crate::utils::check_presence("cmake")?; crate::utils::check_presence("clang")?; @@ -90,6 +91,7 @@ pub fn build( enable_assertions, sanitizer, enable_valgrind, + valgrind_options, )?; Ok(()) @@ -281,6 +283,7 @@ fn build_target( enable_assertions: bool, sanitizer: Option, enable_valgrind: bool, + valgrind_options: Vec, ) -> anyhow::Result<()> { let mut clang_path = host_target_directory.to_path_buf(); clang_path.push("bin/clang"); @@ -360,6 +363,7 @@ fn build_target( )) .args(crate::platforms::shared::shared_build_opts_valgrind( enable_valgrind, + valgrind_options, )), "LLVM target building cmake", )?; diff --git a/src/platforms/shared.rs b/src/platforms/shared.rs index 1d6aaee..b511a14 100644 --- a/src/platforms/shared.rs +++ b/src/platforms/shared.rs @@ -177,12 +177,18 @@ pub fn shared_build_opts_sanitizers(sanitizer: Option) -> Vec /// /// The build options to enable Valgrind for LLVM regression tests. /// -pub fn shared_build_opts_valgrind(enabled: bool) -> Vec { - if enabled { - vec!["-DLLVM_LIT_ARGS='-sv --vg --vg-leak'".to_owned()] - } else { - vec![] +pub fn shared_build_opts_valgrind(enabled: bool, valgrind_options: Vec) -> Vec { + if !enabled { + return vec![]; } + + let vg_args = valgrind_options + .iter() + .map(|opt| format!("--vg-arg='{}'", opt)) + .collect::>() + .join(" "); + + vec![format!("-DLLVM_LIT_ARGS='-sv --vg --vg-leak {}'", vg_args)] } /// diff --git a/src/platforms/x86_64_linux_gnu.rs b/src/platforms/x86_64_linux_gnu.rs index 8ff4499..6db998e 100644 --- a/src/platforms/x86_64_linux_gnu.rs +++ b/src/platforms/x86_64_linux_gnu.rs @@ -30,6 +30,7 @@ pub fn build( enable_assertions: bool, sanitizer: Option, enable_valgrind: bool, + valgrind_options: Vec, ) -> anyhow::Result<()> { crate::utils::check_presence("cmake")?; crate::utils::check_presence("clang")?; @@ -105,6 +106,7 @@ pub fn build( )) .args(crate::platforms::shared::shared_build_opts_valgrind( enable_valgrind, + valgrind_options, )), "LLVM building cmake", )?; diff --git a/src/platforms/x86_64_linux_musl.rs b/src/platforms/x86_64_linux_musl.rs index 1905b79..d7b2186 100644 --- a/src/platforms/x86_64_linux_musl.rs +++ b/src/platforms/x86_64_linux_musl.rs @@ -31,6 +31,7 @@ pub fn build( enable_assertions: bool, sanitizer: Option, enable_valgrind: bool, + valgrind_options: Vec, ) -> anyhow::Result<()> { crate::utils::check_presence("cmake")?; crate::utils::check_presence("clang")?; @@ -91,6 +92,7 @@ pub fn build( enable_assertions, sanitizer, enable_valgrind, + valgrind_options, )?; Ok(()) @@ -281,6 +283,7 @@ fn build_target( enable_assertions: bool, sanitizer: Option, enable_valgrind: bool, + valgrind_options: Vec, ) -> anyhow::Result<()> { let mut clang_path = host_target_directory.to_path_buf(); clang_path.push("bin/clang"); @@ -360,6 +363,7 @@ fn build_target( )) .args(crate::platforms::shared::shared_build_opts_valgrind( enable_valgrind, + valgrind_options, )), "LLVM target building cmake", )?; diff --git a/src/zksync_llvm/arguments.rs b/src/zksync_llvm/arguments.rs index d8ca343..dd3edb0 100644 --- a/src/zksync_llvm/arguments.rs +++ b/src/zksync_llvm/arguments.rs @@ -76,6 +76,10 @@ pub enum Arguments { /// Whether to run LLVM unit tests under valgrind or not. #[arg(long)] enable_valgrind: bool, + + /// Additional valgrind options to pass to the valgrind command. + #[arg(long)] + valgrind_options: Vec, }, /// Checkout the branch specified in `LLVM.lock`. diff --git a/src/zksync_llvm/main.rs b/src/zksync_llvm/main.rs index 79582c8..11f6215 100644 --- a/src/zksync_llvm/main.rs +++ b/src/zksync_llvm/main.rs @@ -55,6 +55,7 @@ fn main_inner() -> anyhow::Result<()> { enable_assertions, sanitizer, enable_valgrind, + valgrind_options, } => { let mut targets = targets .into_iter() @@ -103,6 +104,7 @@ fn main_inner() -> anyhow::Result<()> { enable_assertions, sanitizer, enable_valgrind, + valgrind_options, )?; } Arguments::Checkout { force } => { From 6c6851a02e0c4c4d9c987760f76a46ab3a689e43 Mon Sep 17 00:00:00 2001 From: Anton Baliasnikov Date: Fri, 31 Jan 2025 16:28:26 +0000 Subject: [PATCH 2/2] chore: fix macos tests --- .github/actions/build_and_test/action.yml | 1 + .github/workflows/build-and-test.yaml | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/actions/build_and_test/action.yml b/.github/actions/build_and_test/action.yml index d28c7df..ff6bfcb 100644 --- a/.github/actions/build_and_test/action.yml +++ b/.github/actions/build_and_test/action.yml @@ -11,6 +11,7 @@ runs: set -x cargo install cargo2junit [ "${RUNNER_OS}" = "Windows" ] && ADD_FLAGS="--skip debug_build_with_tests_coverage --skip build_with_sanitizers" + [ "${RUNNER_OS}" = "macOS" ] && export PATH="/opt/homebrew/opt/llvm/bin:${PATH}" cargo test --no-fail-fast -- ${ADD_FLAGS} ${TEST_THREADS} -Z unstable-options --format json \ | cargo2junit | tee "${UNIT_TESTS_RESULTS_XML}" diff --git a/.github/workflows/build-and-test.yaml b/.github/workflows/build-and-test.yaml index 60e2746..786bee9 100644 --- a/.github/workflows/build-and-test.yaml +++ b/.github/workflows/build-and-test.yaml @@ -58,7 +58,7 @@ jobs: - name: Prepare MacOS environment shell: bash if: runner.os == 'macOS' - run: brew install cmake ninja + run: brew install cmake ninja llvm - name: Build and test uses: ./.github/actions/build_and_test