Skip to content

Commit

Permalink
Fix bug in validate_cast_and_convert_metadata (#376)
Browse files Browse the repository at this point in the history
Previously, `validate_cast_and_convert_metadata` incorrectly assumed
that, for slice DSTs, the trailing slice element would always have an
alignment at least as large as the outer type's alignment, and so there
would never be any padding added after the trailing slice. In this
commit, we rewrite `validate_cast_and_convert_metadata` to no longer
assume this behavior.

While we're here, we add the `test_validate_rust_layout` test, which
validates many of our assumptions about Rust's type layout against the
standard library to help catch mismatches like this in the future.
  • Loading branch information
joshlf authored Sep 18, 2023
1 parent e70c25f commit 524b2e2
Show file tree
Hide file tree
Showing 7 changed files with 960 additions and 204 deletions.
23 changes: 19 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,12 @@ jobs:
if: contains(matrix.target, 'i686')

- name: Run tests
run: ./cargo.sh +${{ matrix.toolchain }} test --package ${{ matrix.crate }} --target ${{ matrix.target }} ${{ matrix.features }} --verbose
run: |
./cargo.sh +${{ matrix.toolchain }} test \
--package ${{ matrix.crate }} \
--target ${{ matrix.target }} \
${{ matrix.features }} \
--verbose
# Only run tests when targetting x86 (32- or 64-bit) - we're executing on
# x86_64, so we can't run tests for any non-x86 target.
#
Expand All @@ -163,14 +168,24 @@ jobs:
# Run under both the stacked borrows model (default) and under the tree
# borrows model to ensure we're compliant with both.
for EXTRA_FLAGS in "" "-Zmiri-tree-borrows"; do
# Skip the `ui` test since it invokes the compiler, which we can't do from
# Miri (and wouldn't want to do anyway).
# On `--skip ui`: Skip the `ui` test since it invokes the compiler,
# which we can't do from Miri (and wouldn't want to do anyway).
#
# On `matrix.features == '--all-features': We mark tests which are
# particularly expensive to run under Miri as
# `#[cfg_attr(miri, ignore)]`. We want to run them once per
# toolchain/target combination, but not once per feature set so we
# keep the expensive computation to a minimum.
#
# TODO(#391): Optimize these tests.
MIRIFLAGS="$MIRIFLAGS $EXTRA_FLAGS" ./cargo.sh +${{ matrix.toolchain }} \
miri test \
--package ${{ matrix.crate }} \
--target ${{ matrix.target }} \
${{ matrix.features }} \
-- --skip ui
-- --skip ui \
${{ matrix.features == '--all-features' && '--ignored' || '' }}
done
# Only nightly has a working Miri, so we skip installing on all other
# toolchains.
Expand Down
8 changes: 6 additions & 2 deletions cargo.sh
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ function lookup-version {
esac
}

function get-rustflags {
[ "$1" == nightly ] && echo "--cfg __INTERNAL_USE_ONLY_NIGHLTY_FEATURES_IN_TESTS"
}

case "$1" in
# cargo.sh --version <toolchain-name>
--version)
Expand All @@ -67,14 +71,14 @@ case "$1" in
for toolchain in msrv stable nightly; do
echo "[cargo.sh] running with toolchain: $toolchain" >&2
TOOLCHAIN="$(lookup-version $toolchain)"
cargo "+$TOOLCHAIN" ${@:2}
RUSTFLAGS="$(get-rustflags $toolchain)" cargo "+$TOOLCHAIN" ${@:2}
done
exit 0
;;
# cargo.sh +<toolchain-name> [...]
+*)
TOOLCHAIN="$(lookup-version ${1:1})"
cargo "+$TOOLCHAIN" ${@:2}
RUSTFLAGS="$(get-rustflags ${1:1})" cargo "+$TOOLCHAIN" ${@:2}
;;
*)
print-usage-and-exit
Expand Down
Loading

0 comments on commit 524b2e2

Please sign in to comment.