feat: add additional valgrind arguments support (#48)

matter-labs · Feb 3, 2025 · 090f5d5 · 090f5d5
1 parent 2c3e923
commit 090f5d5
Show file tree

Hide file tree

Showing 10 changed files with 36 additions and 6 deletions.
diff --git 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
@@ -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

diff --git a/src/lib.rs b/src/lib.rs
@@ -161,6 +161,7 @@ pub fn build(
     enable_assertions: bool,
     sanitizer: Option<sanitizer::Sanitizer>,
     enable_valgrind: bool,
+    valgrind_options: Vec<String>,
 ) -> 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
@@ -30,6 +30,7 @@ pub fn build(
     enable_assertions: bool,
     sanitizer: Option<Sanitizer>,
     enable_valgrind: bool,
+    valgrind_options: Vec<String>,
 ) -> 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
@@ -30,6 +30,7 @@ pub fn build(
     enable_assertions: bool,
     sanitizer: Option<Sanitizer>,
     enable_valgrind: bool,
+    valgrind_options: Vec<String>,
 ) -> 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<Sanitizer>,
     enable_valgrind: bool,
+    valgrind_options: Vec<String>,
 ) -> 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
@@ -177,12 +177,18 @@ pub fn shared_build_opts_sanitizers(sanitizer: Option<Sanitizer>) -> Vec<String>
 ///
 /// The build options to enable Valgrind for LLVM regression tests.
 ///
-pub fn shared_build_opts_valgrind(enabled: bool) -> Vec<String> {
-    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<String>) -> Vec<String> {
+    if !enabled {
+        return vec![];
     }
+
+    let vg_args = valgrind_options
+        .iter()
+        .map(|opt| format!("--vg-arg='{}'", opt))
+        .collect::<Vec<_>>()
+        .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
@@ -30,6 +30,7 @@ pub fn build(
     enable_assertions: bool,
     sanitizer: Option<Sanitizer>,
     enable_valgrind: bool,
+    valgrind_options: Vec<String>,
 ) -> 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
@@ -31,6 +31,7 @@ pub fn build(
     enable_assertions: bool,
     sanitizer: Option<Sanitizer>,
     enable_valgrind: bool,
+    valgrind_options: Vec<String>,
 ) -> 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<Sanitizer>,
     enable_valgrind: bool,
+    valgrind_options: Vec<String>,
 ) -> 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
@@ -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<String>,
     },
 
     /// Checkout the branch specified in `LLVM.lock`.

diff --git 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 } => {