Skip to content

Commit 8928de7

Browse files
committed
Auto merge of #52972 - RalfJung:from_raw_parts_align, r=alexcrichton
debug_assert to ensure that from_raw_parts is only used properly aligned This does not help nearly as much as I would hope because everybody uses the distributed libstd which is compiled without debug assertions. For this reason, I am not sure if this is even worth it. OTOH, this would have caught the misalignment fixed by #42789 *if* there had been any tests actually using ZSTs with alignment >1 (we have a CI runner which has debug assertions in libstd enabled), and it seems to currently [fail in the rg testsuite](https://ci.appveyor.com/project/rust-lang/rust/build/1.0.8403/job/v7dfdcgn8ay5j6sb). So maybe it is worth it, after all. I have seen the attribute `#[rustc_inherit_overflow_checks]` in some places, does that make it so that the *caller's* debug status is relevant? Is there a similar attribute for `debug_assert!`? That could even subsume `rustc_inherit_overflow_checks`: Something like `rustc_inherit_debug_flag` could affect *all* places that change the generated code depending on whether we are in debug or release mode. In fact, given that we have to keep around the MIR for generic functions anyway, is there ever a reason *not* to handle the debug flag that way? I guess currently we apply debug flags like `cfg` so this is dropped early during the MIR pipeline? EDIT: I learned from @eddyb that because of how `debug_assert!` works, this is not realistic. Well, we could still have it for the rustc CI runs and then maybe, eventually, when libstd gets compiled client-side and there is both a debug and a release build... then this will also benefit users.^^
2 parents a9fe312 + 1001b2b commit 8928de7

File tree

8 files changed

+28
-3
lines changed

8 files changed

+28
-3
lines changed

.travis.yml

+2
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ matrix:
4646
# slow to run.
4747

4848
# OSX builders running tests, these run the full test suite.
49+
# NO_DEBUG_ASSERTIONS=1 to make them go faster, but also do have some
50+
# runners that run `//ignore-debug` tests.
4951
#
5052
# Note that the compiler is compiled to target 10.8 here because the Xcode
5153
# version that we're using, 8.2, cannot compile LLVM for OSX 10.7.

src/libcore/slice/mod.rs

+4
Original file line numberDiff line numberDiff line change
@@ -1785,6 +1785,7 @@ impl<T> [T] {
17851785
(self, &[], &[])
17861786
} else {
17871787
let (left, rest) = self.split_at(offset);
1788+
// now `rest` is definitely aligned, so `from_raw_parts_mut` below is okay
17881789
let (us_len, ts_len) = rest.align_to_offsets::<U>();
17891790
(left,
17901791
from_raw_parts(rest.as_ptr() as *const U, us_len),
@@ -1837,6 +1838,7 @@ impl<T> [T] {
18371838
(self, &mut [], &mut [])
18381839
} else {
18391840
let (left, rest) = self.split_at_mut(offset);
1841+
// now `rest` is definitely aligned, so `from_raw_parts_mut` below is okay
18401842
let (us_len, ts_len) = rest.align_to_offsets::<U>();
18411843
let mut_ptr = rest.as_mut_ptr();
18421844
(left,
@@ -3878,6 +3880,7 @@ unsafe impl<'a, T> TrustedRandomAccess for ExactChunksMut<'a, T> {
38783880
#[inline]
38793881
#[stable(feature = "rust1", since = "1.0.0")]
38803882
pub unsafe fn from_raw_parts<'a, T>(data: *const T, len: usize) -> &'a [T] {
3883+
debug_assert!(data as usize % mem::align_of::<T>() == 0, "attempt to create unaligned slice");
38813884
Repr { raw: FatPtr { data, len } }.rust
38823885
}
38833886

@@ -3891,6 +3894,7 @@ pub unsafe fn from_raw_parts<'a, T>(data: *const T, len: usize) -> &'a [T] {
38913894
#[inline]
38923895
#[stable(feature = "rust1", since = "1.0.0")]
38933896
pub unsafe fn from_raw_parts_mut<'a, T>(data: *mut T, len: usize) -> &'a mut [T] {
3897+
debug_assert!(data as usize % mem::align_of::<T>() == 0, "attempt to create unaligned slice");
38943898
Repr { raw: FatPtr { data, len} }.rust_mut
38953899
}
38963900

src/libcore/tests/slice.rs

+14
Original file line numberDiff line numberDiff line change
@@ -986,3 +986,17 @@ fn test_align_to_non_trivial() {
986986
assert_eq!(aligned.len(), 4);
987987
assert_eq!(prefix.len() + suffix.len(), 2);
988988
}
989+
990+
#[test]
991+
fn test_align_to_empty_mid() {
992+
use core::mem;
993+
994+
// Make sure that we do not create empty unaligned slices for the mid part, even when the
995+
// overall slice is too short to contain an aligned address.
996+
let bytes = [1, 2, 3, 4, 5, 6, 7];
997+
type Chunk = u32;
998+
for offset in 0..4 {
999+
let (_, mid, _) = unsafe { bytes[offset..offset+1].align_to::<Chunk>() };
1000+
assert_eq!(mid.as_ptr() as usize % mem::align_of::<Chunk>(), 0);
1001+
}
1002+
}

src/test/codegen/vec-clear.rs

+1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11+
// ignore-debug: the debug assertions get in the way
1112
// compile-flags: -O
1213

1314
#![crate_type = "lib"]

src/test/codegen/vec-iter-collect-len.rs

+1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11+
// ignore-debug: the debug assertions get in the way
1112
// no-system-llvm
1213
// compile-flags: -O
1314
#![crate_type="lib"]

src/test/codegen/vec-optimizes-away.rs

+1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010
//
11+
// ignore-debug: the debug assertions get in the way
1112
// no-system-llvm
1213
// compile-flags: -O
1314
#![crate_type="lib"]

src/tools/cargotest/main.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ const TEST_REPOS: &'static [Test] = &[
3333
Test {
3434
name: "ripgrep",
3535
repo: "https://github.com/BurntSushi/ripgrep",
36-
sha: "b65bb37b14655e1a89c7cd19c8b011ef3e312791",
36+
sha: "ad9befbc1d3b5c695e7f6b6734ee1b8e683edd41",
3737
lock: None,
3838
packages: &[],
3939
},

src/tools/compiletest/src/header.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -615,12 +615,14 @@ impl Config {
615615
common::DebugInfoLldb => name == "lldb",
616616
common::Pretty => name == "pretty",
617617
_ => false,
618-
} || (self.target != self.host && name == "cross-compile") ||
618+
} ||
619+
(self.target != self.host && name == "cross-compile") ||
619620
match self.compare_mode {
620621
Some(CompareMode::Nll) => name == "compare-mode-nll",
621622
Some(CompareMode::Polonius) => name == "compare-mode-polonius",
622623
None => false,
623-
}
624+
} ||
625+
(cfg!(debug_assertions) && name == "debug")
624626
} else {
625627
false
626628
}

0 commit comments

Comments
 (0)