Minimal supported rust version (#567)

Add MSRV to the manifest file and test the MSRV in CI workflows using a matrix

Author: qinsoon

.github/scripts/ci-build.sh

@@ -11,10 +11,7 @@ for_all_features "cargo build"
 # Build release
 for_all_features "cargo build --release"
 
-# For x86_64-linux, also see if we can build for i686
+# target-specific features
 if [[ $arch == "x86_64" && $os == "linux" ]]; then
-    cargo build --target i686-unknown-linux-gnu
-    for_all_features "cargo build --target i686-unknown-linux-gnu"
-    for_all_features "cargo build --release --target i686-unknown-linux-gnu"
     cargo build --features perf_counter
-fi
+fi

.github/scripts/ci-style.sh

@@ -13,14 +13,12 @@ for_all_features "cargo clippy --tests"
 # check for dummyvm
 cargo clippy --manifest-path=vmbindings/dummyvm/Cargo.toml
 
-# For x86_64-linux, also check for i686
+# target-specific features
 if [[ $arch == "x86_64" && $os == "linux" ]]; then
-    for_all_features "cargo clippy --target i686-unknown-linux-gnu"
-    for_all_features "cargo clippy --release --target i686-unknown-linux-gnu"
     cargo clippy --features perf_counter
     cargo clippy --release --features perf_counter
     cargo clippy --tests --features perf_counter
-fi 
+fi
 
 # check format
 cargo fmt -- --check

.github/scripts/ci-test.sh

@@ -4,9 +4,8 @@ export RUST_BACKTRACE=1
 
 for_all_features "cargo test"
 
-# For x86_64-linux, also check for i686
+# target-specific features
 if [[ $arch == "x86_64" && $os == "linux" ]]; then
-    for_all_features "cargo test --target i686-unknown-linux-gnu"
     cargo test --features perf_counter
 fi
 

.github/workflows/cargo-msrv.yml

@@ -0,0 +1,26 @@
+name: Minimal Supported Rust Version
+
+on:
+  pull_request:
+    branches:
+      - master
+
+jobs:
+  msrv:
+    runs-on: ubuntu-18.04
+    steps:
+      - uses: actions/checkout@v2
+      - name: Install Rust toolchain
+        uses: actions-rs/toolchain@v1
+        with:
+          toolchain: stable
+          override: true
+      - name: Install cargo-msrv
+        run: cargo install cargo-msrv
+      # Verify the MSRV defined in Cargo.toml
+      - name: Verify MSRV
+        run: cargo msrv verify
+      # If the previous step fails, find MSRV
+      - name: Find MSRV
+        if: failure()
+        run: cargo msrv

.github/workflows/pre-review-ci.yml

@@ -6,16 +6,40 @@ on:
       - master
 
 jobs:
+  # Setup dynamic test matrix
+  setup-test-matrix:
+    runs-on: ubuntu-latest
+    outputs:
+      rust: ${{ steps.rust.outputs.array }}
+    steps:
+      - uses: actions/checkout@v2
+      # Get rust version
+      - id: rust
+        run: |
+          export MSRV=`cargo read-manifest | python -c 'import json,sys; print(json.load(sys.stdin)["rust_version"])'`
+          export TEST=`cat rust-toolchain`
+          echo "::set-output name=array::[\"$MSRV\", \"$TEST\"]"
+
   pre-code-review-checks:
-    runs-on: ubuntu-18.04
-    timeout-minutes: 30
+    needs: setup-test-matrix
+    strategy:
+      fail-fast: false
+      matrix:
+        target:
+          - { os: ubuntu-18.04, triple: x86_64-unknown-linux-gnu }
+          - { os: ubuntu-18.04, triple: i686-unknown-linux-gnu }
+        rust: ${{ fromJson(needs.setup-test-matrix.outputs.rust )}}
+
+    name: ${{ matrix.target.triple }} / ${{ matrix.rust }}
+    runs-on: ${{ matrix.target.os }}
+
     steps:
       - uses: actions/checkout@v2
-      - name: Install latest nightly
+      - name: Install Rust
         uses: actions-rs/toolchain@v1
         with:
+          toolchain: ${{ matrix.rust }}-${{ matrix.target.triple }}
           components: rustfmt, clippy
-          target: i686-unknown-linux-gnu
           # This overwrites the default toolchain with the toolchain specified above.
           override: true
 

Cargo.toml

@@ -10,6 +10,7 @@ repository = "https://github.com/mmtk/mmtk-core"
 readme = "README.md"
 categories = ["memory-management"]
 keywords = ["gc", "garbage", "collection", "garbage-collection", "allocation"]
+rust-version = "1.57.0"
 
 [lib]
 name = "mmtk"

README.md

@@ -20,12 +20,9 @@ We maintain an up to date list of the prerequisite for building MMTk and its bin
 
 ## Build
 
-MMTk can build with a stable Rust toolchain. However,
-as the Rust language and its libraries (crates) are frequently evolving, we recommend using the toolchain specified in the [`rust-toolchain`](rust-toolchain) file.
+MMTk can build with a stable Rust toolchain. The minimal supported Rust version is 1.57.0, and MMTk is tested with 1.59.0.
 
 ```console
-$ export RUSTUP_TOOLCHAIN=`cat rust-toolchain`
-
 $ cargo build
 ```
 

examples/build.py

@@ -39,6 +39,16 @@ def exec_and_redirect(args, env=None):
     if p.returncode != 0:
         exit(p.returncode)
 
+# Get the active toolchain, something like this: stable-x86_64-unknown-linux-gnu
+active_toolchain = str(subprocess.check_output(["rustup", "show", "active-toolchain"]).decode('utf-8')).split(' ')[0]
+print("Active rust toolchain: " + active_toolchain)
+if "x86_64" in active_toolchain:
+    m32 = False
+elif "i686" in active_toolchain:
+    m32 = True
+else:
+    print("Unknown toolchain: " + active_toolchain)
+    sys.exit(1)
 
 system = platform.system()
 assert system == "Darwin" or system == "Linux"
@@ -71,40 +81,21 @@ def exec_and_redirect(args, env=None):
 shutil.copyfile("{}/target/release/libmmtk_dummyvm{}".format(vmbinding, SUFFIX),
                 "./libmmtk{}".format(SUFFIX))
 
-if system == "Linux":
-    exec_and_redirect(cmd + ["--target=i686-unknown-linux-gnu"])
-    exec_and_redirect(
-        cmd + ["--release", "--target=i686-unknown-linux-gnu"])
-    shutil.copyfile(
-        "{}/target/i686-unknown-linux-gnu/release/libmmtk_dummyvm{}".format(vmbinding, SUFFIX),
-        "./libmmtk_32{}".format(SUFFIX))
-
-exec_and_redirect([
+cmd = [
     "clang",
     "-lmmtk",
     "-L.",
     "-I{}/api".format(vmbinding),
     "-O3",
     "-o",
     "test_mmtk",
-    "./examples/main.c"])
-
-if system == "Linux":
-    exec_and_redirect([
-        "clang",
-        "-lmmtk_32",
-        "-L.",
-        "-I{}/api".format(vmbinding),
-        "-O3", "-m32",
-        "-o",
-        "test_mmtk_32",
-        "./examples/main.c"])
+]
+if m32:
+    cmd.append("-m32")
+cmd.append("./examples/main.c")
+exec_and_redirect(cmd)
 
 for plan in PLANS:
     exec_and_redirect(["./test_mmtk"], env={LIBRARY_PATH: ".", "MMTK_PLAN": plan})
-    if system == "Linux":
-        exec_and_redirect(["./test_mmtk_32"], env={LIBRARY_PATH: ".", "MMTK_PLAN": plan})
 
 os.remove("./test_mmtk")
-if system == "Linux":
-    os.remove("./test_mmtk_32")

rust-toolchain

@@ -1 +1 @@
-stable
+1.59.0

src/util/metadata/side_metadata/helpers.rs

@@ -47,10 +47,11 @@ pub(crate) fn ensure_munmap_contiguos_metadata_space(
     spec: &SideMetadataSpec,
 ) -> usize {
     // nearest page-aligned starting address
-    let mmap_start = address_to_meta_address(spec, start).align_down(BYTES_IN_PAGE);
+    let metadata_start = address_to_meta_address(spec, start);
+    let mmap_start = metadata_start.align_down(BYTES_IN_PAGE);
     // nearest page-aligned ending address
-    let mmap_size =
-        address_to_meta_address(spec, start + size).align_up(BYTES_IN_PAGE) - mmap_start;
+    let metadata_size = (size + ((1 << addr_rshift(spec)) - 1)) >> addr_rshift(spec);
+    let mmap_size = (metadata_start + metadata_size).align_up(BYTES_IN_PAGE) - mmap_start;
     if mmap_size > 0 {
         ensure_munmap_metadata(mmap_start, mmap_size);
     }
@@ -70,10 +71,11 @@ pub(crate) fn try_mmap_contiguous_metadata_space(
     debug_assert!(size % BYTES_IN_PAGE == 0);
 
     // nearest page-aligned starting address
-    let mmap_start = address_to_meta_address(spec, start).align_down(BYTES_IN_PAGE);
+    let metadata_start = address_to_meta_address(spec, start);
+    let mmap_start = metadata_start.align_down(BYTES_IN_PAGE);
     // nearest page-aligned ending address
-    let mmap_size =
-        address_to_meta_address(spec, start + size).align_up(BYTES_IN_PAGE) - mmap_start;
+    let metadata_size = (size + ((1 << addr_rshift(spec)) - 1)) >> addr_rshift(spec);
+    let mmap_size = (metadata_start + metadata_size).align_up(BYTES_IN_PAGE) - mmap_start;
     if mmap_size > 0 {
         if !no_reserve {
             MMAPPER.ensure_mapped(mmap_start, mmap_size >> LOG_BYTES_IN_PAGE)

vmbindings/dummyvm/src/object_model.rs

@@ -9,7 +9,10 @@ pub struct VMObjectModel {}
 
 // This is intentionally set to a non-zero value to see if it breaks.
 // Change this if you want to test other values.
+#[cfg(target_pointer_width = "64")]
 pub const OBJECT_REF_OFFSET: usize = 6;
+#[cfg(target_pointer_width = "32")]
+pub const OBJECT_REF_OFFSET: usize = 2;
 
 impl ObjectModel<DummyVM> for VMObjectModel {
     const GLOBAL_LOG_BIT_SPEC: VMGlobalLogBitSpec = VMGlobalLogBitSpec::in_header(0);

vmbindings/dummyvm/src/tests/handle_mmap_oom.rs

@@ -3,14 +3,18 @@ use mmtk::util::opaque_pointer::*;
 use mmtk::util::memory;
 use crate::DummyVM;
 
+#[cfg(target_pointer_width = "32")]
+const LARGE_SIZE: usize = 4_294_967_295;
+#[cfg(target_pointer_width = "64")]
+const LARGE_SIZE: usize = 1_000_000_000_000;
+
 #[test]
 pub fn test_handle_mmap_oom() {
     let panic_res = std::panic::catch_unwind(move || {
         let start = unsafe { Address::from_usize(0x100_0000 )};
-        let one_terabyte = 1000000000000;
         // mmap 1 terabyte memory - we expect this will fail due to out of memory.
         // If that's not the case, increase the size we mmap.
-        let mmap_res = memory::dzmmap_noreplace(start, one_terabyte);
+        let mmap_res = memory::dzmmap_noreplace(start, LARGE_SIZE);
 
         memory::handle_mmap_error::<DummyVM>(mmap_res.err().unwrap(), VMThread::UNINITIALIZED);
     });