slice::split_mut does not elide bound checks in release mode

[daniel5151]

As part of an ongoing effort to eliminate all panics from one of my no_std projects, I discovered that slice::split_mut does not elide bounds checks in release mode, whereas slice::split does.

This issue also affects slice::splitn_mut.

The following examples illustrate this issue:

pub fn split(s: &mut [u8]) {
    for s in s.split(|b| *b == b' ') {
        black_box(s);
    }
}

// stable block_box pulled from
// https://docs.rs/bencher/0.1.5/src/bencher/lib.rs.html#590-596
pub fn black_box<T>(dummy: T) -> T {
    unsafe {
        let ret = core::ptr::read_volatile(&dummy);
        core::mem::forget(dummy);
        ret
    }
}

The generated asm for split elides all bounds checks:

playground::split:
	sub	rsp, 16

.LBB0_1:
	test	rsi, rsi
	je	.LBB0_2
	lea	rax, [rdi + 1]
	xor	ecx, ecx

.LBB0_4:
	cmp	byte ptr [rax - 1], 32
	je	.LBB0_7
	add	rcx, 1
	add	rax, 1
	cmp	rsi, rcx
	jne	.LBB0_4
	xor	edx, edx
	mov	rax, rdi
	mov	rcx, rsi
	test	rdi, rdi
	jne	.LBB0_9
	jmp	.LBB0_10

.LBB0_2:
	xor	esi, esi
	mov	rax, rdi
	xor	edx, edx
	xor	ecx, ecx
	test	rdi, rdi
	jne	.LBB0_9
	jmp	.LBB0_10

.LBB0_7:
	mov	rdx, rcx
	not	rdx
	add	rsi, rdx
	mov	dl, 1
	test	rdi, rdi
	je	.LBB0_10

.LBB0_9:
	mov	qword ptr [rsp], rdi
	mov	qword ptr [rsp + 8], rcx
	mov	rcx, qword ptr [rsp + 8]
	mov	rcx, qword ptr [rsp]
	mov	rdi, rax
	test	dl, dl
	jne	.LBB0_1

.LBB0_10:
	add	rsp, 16
	ret

Unfortunately, the bounds check is reintroduced when split_mut is used instead:

pub fn split_mut(s: &mut [u8]) {
    for s in s.split_mut(|b| *b == b' ') {
        black_box(s);
    }
}

playground::split_mut:
	sub	rsp, 24
	lea	r8, [rip + .L__unnamed_1]
	jmp	.LBB0_1

.LBB0_2:
	xor	eax, eax
	mov	rcx, r8
	xor	r9d, r9d
	xor	esi, esi

.LBB0_9:
	mov	qword ptr [rsp + 8], rdi
	mov	qword ptr [rsp + 16], rsi
	mov	rdx, qword ptr [rsp + 16]
	mov	rdx, qword ptr [rsp + 8]
	mov	rsi, rax
	mov	rdi, rcx
	test	r9b, r9b
	je	.LBB0_10

.LBB0_1:
	test	rsi, rsi
	je	.LBB0_2
	lea	rcx, [rdi + 1]
	xor	edx, edx

.LBB0_4:
	cmp	byte ptr [rcx - 1], 32
	je	.LBB0_7
	add	rdx, 1
	add	rcx, 1
	cmp	rsi, rdx
	jne	.LBB0_4
	xor	eax, eax
	mov	rcx, r8
	xor	r9d, r9d
	jmp	.LBB0_9

.LBB0_7:
	cmp	rsi, rdx
	je	.LBB0_11
	mov	rax, rdx
	not	rax
	add	rax, rsi
	mov	r9b, 1
	mov	rsi, rdx
	jmp	.LBB0_9

.LBB0_10:
	add	rsp, 24
	ret

.LBB0_11:
	lea	rdx, [rip + .L__unnamed_2]
	mov	edi, 1
	xor	esi, esi
	call	qword ptr [rip + core::slice::index::slice_start_index_len_fail@GOTPCREL]
	ud2

.L__unnamed_1:

.L__unnamed_3:
	.ascii	"/rustc/9bc8c42bb2f19e745a63f3445f1ac248fb015e53/library/core/src/slice/iter.rs"

.L__unnamed_2:
	.quad	.L__unnamed_3
	.asciz	"N\000\000\000\000\000\000\000z\002\000\000\037\000\000"

It seems that the following code in the SplitMut iterator is responsible for introducing the bounds check (specifically the array index on line 640):

rust/library/core/src/slice/iter.rs

Lines 630 to 643 in 673d0db

	let idx_opt = {
	// work around borrowck limitations
	let pred = &mut self.pred;
	self.v.iter().position(|x| (*pred)(x))
	};
	match idx_opt {
	None => self.finish(),
	Some(idx) => {
	let tmp = mem::replace(&mut self.v, &mut []);
	let (head, tail) = tmp.split_at_mut(idx);
	self.v = &mut tail[1..];
	Some(head)
	}
	}

Meta

rustc --version --verbose:

rustc 1.52.1 (9bc8c42bb 2021-05-09)
binary: rustc
commit-hash: 9bc8c42bb2f19e745a63f3445f1ac248fb015e53
commit-date: 2021-05-09
host: x86_64-unknown-linux-gnu
release: 1.52.1
LLVM version: 12.0.0

Also tested on 1.55.0-nightly (2021-06-13 f586d79d183d144e0cbf) from the Rust playground.

[leonardo-m]

Nice catch.

As part of an ongoing effort to eliminate all panics from one of my no_std projects

Oh, I'm doing the same (but in regular std code).
stdsort is full of bound tests (issue #84173).
I'm dreaming of having red squiggles under divisions/mods/rems and array/slice accesses where a bound test hasn't being removed. And built-in annotations like https://crates.io/crates/no-panic . And more :)

[inquisitivecrystal]

@rustbot label +A-codegen +A-slice +T-libs +T-compiler

[saethlin]

@rustbot claim

[daniel5151]

Awesome, thanks for looking into this @saethlin!