Browse Source

Add shuttle tests to SVM (#2293)

Lucas Ste 1 year ago
parent
commit
98ded995ce

+ 2 - 0
Cargo.lock

@@ -7584,6 +7584,7 @@ dependencies = [
  "rustc_version 0.4.0",
  "rustc_version 0.4.0",
  "serde",
  "serde",
  "serde_derive",
  "serde_derive",
+ "shuttle",
  "solana-bpf-loader-program",
  "solana-bpf-loader-program",
  "solana-compute-budget",
  "solana-compute-budget",
  "solana-compute-budget-program",
  "solana-compute-budget-program",
@@ -8147,6 +8148,7 @@ dependencies = [
  "rand 0.8.5",
  "rand 0.8.5",
  "rustc-demangle",
  "rustc-demangle",
  "scroll",
  "scroll",
+ "shuttle",
  "thiserror",
  "thiserror",
  "winapi 0.3.9",
  "winapi 0.3.9",
 ]
 ]

+ 13 - 0
ci/buildkite-pipeline.sh

@@ -234,6 +234,19 @@ EOF
       "Stable-SBF skipped as no relevant files were modified"
       "Stable-SBF skipped as no relevant files were modified"
   fi
   fi
 
 
+   # Shuttle tests
+  if affects \
+             .rs$ \
+             Cargo.lock$ \
+             Cargo.toml$ \
+             ^ci/rust-version.sh \
+      ; then
+    command_step shuttle "ci/docker-run-default-image.sh ci/test-shuttle.sh" 10
+  else
+    annotate --style info \
+      "test-shuttle skipped as no relevant files were modified"
+  fi
+
   # Downstream backwards compatibility
   # Downstream backwards compatibility
   if affects \
   if affects \
              .rs$ \
              .rs$ \

+ 7 - 0
ci/test-shuttle.sh

@@ -0,0 +1,7 @@
+#!/usr/bin/env bash
+
+set -eo pipefail
+
+source ci/_
+
+cargo nextest run --profile ci --config-file ./nextest.toml --manifest-path="svm/Cargo.toml" --features="shuttle-test" --test concurrent_tests --jobs 1

+ 1 - 1
program-runtime/Cargo.toml

@@ -57,4 +57,4 @@ frozen-abi = [
     "solana-compute-budget/frozen-abi",
     "solana-compute-budget/frozen-abi",
     "solana-sdk/frozen-abi",
     "solana-sdk/frozen-abi",
 ]
 ]
-shuttle-test = ["solana-type-overrides/shuttle-test"]
+shuttle-test = ["solana-type-overrides/shuttle-test", "solana_rbpf/shuttle-test"]

+ 1 - 0
programs/bpf_loader/Cargo.toml

@@ -48,4 +48,5 @@ targets = ["x86_64-unknown-linux-gnu"]
 shuttle-test = [
 shuttle-test = [
     "solana-type-overrides/shuttle-test",
     "solana-type-overrides/shuttle-test",
     "solana-program-runtime/shuttle-test",
     "solana-program-runtime/shuttle-test",
+    "solana_rbpf/shuttle-test"
 ]
 ]

+ 1 - 1
programs/loader-v4/Cargo.toml

@@ -29,4 +29,4 @@ name = "solana_loader_v4_program"
 targets = ["x86_64-unknown-linux-gnu"]
 targets = ["x86_64-unknown-linux-gnu"]
 
 
 [features]
 [features]
-shuttle-test = ["solana-type-overrides/shuttle-test", "solana-program-runtime/shuttle-test"]
+shuttle-test = ["solana-type-overrides/shuttle-test", "solana-program-runtime/shuttle-test", "solana_rbpf/shuttle-test"]

+ 1 - 0
svm/Cargo.toml

@@ -42,6 +42,7 @@ lazy_static = { workspace = true }
 libsecp256k1 = { workspace = true }
 libsecp256k1 = { workspace = true }
 prost = { workspace = true }
 prost = { workspace = true }
 rand = { workspace = true }
 rand = { workspace = true }
+shuttle = { workspace = true }
 solana-bpf-loader-program = { workspace = true }
 solana-bpf-loader-program = { workspace = true }
 solana-compute-budget-program = { workspace = true }
 solana-compute-budget-program = { workspace = true }
 solana-logger = { workspace = true }
 solana-logger = { workspace = true }

+ 68 - 33
svm/tests/concurrent_tests.rs

@@ -1,21 +1,21 @@
+#![cfg(feature = "shuttle-test")]
+
 use {
 use {
     crate::mock_bank::{deploy_program, MockForkGraph},
     crate::mock_bank::{deploy_program, MockForkGraph},
     mock_bank::MockBankCallback,
     mock_bank::MockBankCallback,
+    shuttle::{
+        sync::{Arc, RwLock},
+        thread, Runner,
+    },
     solana_program_runtime::loaded_programs::ProgramCacheEntryType,
     solana_program_runtime::loaded_programs::ProgramCacheEntryType,
     solana_sdk::pubkey::Pubkey,
     solana_sdk::pubkey::Pubkey,
     solana_svm::transaction_processor::TransactionBatchProcessor,
     solana_svm::transaction_processor::TransactionBatchProcessor,
-    solana_type_overrides::sync::RwLock,
-    std::{
-        collections::{HashMap, HashSet},
-        sync::Arc,
-        thread,
-    },
+    std::collections::{HashMap, HashSet},
 };
 };
 
 
 mod mock_bank;
 mod mock_bank;
 
 
-#[test]
-fn fast_concur_test() {
+fn program_cache_execution(threads: usize) {
     let mut mock_bank = MockBankCallback::default();
     let mut mock_bank = MockBankCallback::default();
     let batch_processor = TransactionBatchProcessor::<MockForkGraph>::new(5, 5, HashSet::new());
     let batch_processor = TransactionBatchProcessor::<MockForkGraph>::new(5, 5, HashSet::new());
     let fork_graph = Arc::new(RwLock::new(MockForkGraph {}));
     let fork_graph = Arc::new(RwLock::new(MockForkGraph {}));
@@ -33,32 +33,67 @@ fn fast_concur_test() {
         .map(|(idx, key)| (*key, idx as u64))
         .map(|(idx, key)| (*key, idx as u64))
         .collect();
         .collect();
 
 
-    for _ in 0..10 {
-        let ths: Vec<_> = (0..4)
-            .map(|_| {
-                let local_bank = mock_bank.clone();
-                let processor = TransactionBatchProcessor::new_from(
-                    &batch_processor,
-                    batch_processor.slot,
-                    batch_processor.epoch,
-                );
-                let maps = account_maps.clone();
-                let programs = programs.clone();
-                thread::spawn(move || {
-                    let result = processor.replenish_program_cache(&local_bank, &maps, false, true);
-                    for key in &programs {
-                        let cache_entry = result.find(key);
-                        assert!(matches!(
-                            cache_entry.unwrap().program,
-                            ProgramCacheEntryType::Loaded(_)
-                        ));
-                    }
-                })
+    let ths: Vec<_> = (0..threads)
+        .map(|_| {
+            let local_bank = mock_bank.clone();
+            let processor = TransactionBatchProcessor::new_from(
+                &batch_processor,
+                batch_processor.slot,
+                batch_processor.epoch,
+            );
+            let maps = account_maps.clone();
+            let programs = programs.clone();
+            thread::spawn(move || {
+                let result = processor.replenish_program_cache(&local_bank, &maps, false, true);
+                for key in &programs {
+                    let cache_entry = result.find(key);
+                    assert!(matches!(
+                        cache_entry.unwrap().program,
+                        ProgramCacheEntryType::Loaded(_)
+                    ));
+                }
             })
             })
-            .collect();
+        })
+        .collect();
 
 
-        for th in ths {
-            th.join().unwrap();
-        }
+    for th in ths {
+        th.join().unwrap();
     }
     }
 }
 }
+
+// Shuttle has its own internal scheduler and the following tests change the way it operates to
+// increase the efficiency in finding problems in the program cache's concurrent code.
+
+// This test leverages the probabilistic concurrency testing algorithm
+// (https://www.microsoft.com/en-us/research/wp-content/uploads/2016/02/asplos277-pct.pdf).
+// It bounds the numbers of preemptions to explore (five in this test) for the four
+// threads we use. We run it for 300 iterations.
+#[test]
+fn test_program_cache_with_probabilistic_scheduler() {
+    shuttle::check_pct(
+        move || {
+            program_cache_execution(4);
+        },
+        300,
+        5,
+    );
+}
+
+// In this case, the scheduler is random and may preempt threads at any point and any time.
+#[test]
+fn test_program_cache_with_random_scheduler() {
+    shuttle::check_random(move || program_cache_execution(4), 300);
+}
+
+// This test explores all the possible thread scheduling patterns that might affect the program
+// cache. There is a limitation to run only 500 iterations to avoid consuming too much CI time.
+#[test]
+fn test_program_cache_with_exhaustive_scheduler() {
+    // The DFS (shuttle::check_dfs) test is only complete when we do not generate random
+    // values in a thread.
+    // Since this is not the case for the execution of jitted program, we can still run the test
+    // but with decreased accuracy.
+    let scheduler = shuttle::scheduler::DfsScheduler::new(Some(500), true);
+    let runner = Runner::new(scheduler, Default::default());
+    runner.run(move || program_cache_execution(4));
+}

+ 1 - 1
svm/tests/mock_bank.rs

@@ -10,13 +10,13 @@ use {
         slot_hashes::Slot,
         slot_hashes::Slot,
     },
     },
     solana_svm::transaction_processing_callback::TransactionProcessingCallback,
     solana_svm::transaction_processing_callback::TransactionProcessingCallback,
+    solana_type_overrides::sync::{Arc, RwLock},
     std::{
     std::{
         cmp::Ordering,
         cmp::Ordering,
         collections::HashMap,
         collections::HashMap,
         env,
         env,
         fs::{self, File},
         fs::{self, File},
         io::Read,
         io::Read,
-        sync::{Arc, RwLock},
     },
     },
 };
 };