From b9e3d259ef84868c757c809c7d04c6d4bccfa402 Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Sun, 24 Nov 2024 07:40:11 +0100 Subject: [PATCH 1/5] fix signature of Step::steps_between implementations A recent nightly changed the signature of Step::steps_between to match the signature of Iterator::size_hint. This patch changes our implementations to match the new signature. --- src/addr.rs | 39 ++++++++++++++++++----------- src/instructions/tlb.rs | 4 +-- src/structures/paging/page.rs | 10 +++++--- src/structures/paging/page_table.rs | 4 +-- 4 files changed, 35 insertions(+), 22 deletions(-) diff --git a/src/addr.rs b/src/addr.rs index 8afe53f3..fd29a896 100644 --- a/src/addr.rs +++ b/src/addr.rs @@ -241,13 +241,18 @@ impl VirtAddr { // FIXME: Move this into the `Step` impl, once `Step` is stabilized. #[cfg(any(feature = "instructions", feature = "step_trait"))] - pub(crate) fn steps_between_impl(start: &Self, end: &Self) -> Option { - let mut steps = end.0.checked_sub(start.0)?; + pub(crate) fn steps_between_impl(start: &Self, end: &Self) -> (usize, Option) { + let mut steps = if let Some(steps) = end.0.checked_sub(start.0) { + steps + } else { + return (0, None); + }; // Mask away extra bits that appear while jumping the gap. steps &= 0xffff_ffff_ffff; - usize::try_from(steps).ok() + let steps = usize::try_from(steps).ok(); + (steps.unwrap_or(usize::MAX), steps) } // FIXME: Move this into the `Step` impl, once `Step` is stabilized. @@ -360,7 +365,7 @@ impl Sub for VirtAddr { #[cfg(feature = "step_trait")] impl Step for VirtAddr { #[inline] - fn steps_between(start: &Self, end: &Self) -> Option { + fn steps_between(start: &Self, end: &Self) -> (usize, Option) { Self::steps_between_impl(start, end) } @@ -721,43 +726,49 @@ mod tests { #[test] #[cfg(feature = "step_trait")] fn virtaddr_steps_between() { - assert_eq!(Step::steps_between(&VirtAddr(0), &VirtAddr(0)), Some(0)); - assert_eq!(Step::steps_between(&VirtAddr(0), &VirtAddr(1)), Some(1)); - assert_eq!(Step::steps_between(&VirtAddr(1), &VirtAddr(0)), None); + assert_eq!( + Step::steps_between(&VirtAddr(0), &VirtAddr(0)), + (0, Some(0)) + ); + assert_eq!( + Step::steps_between(&VirtAddr(0), &VirtAddr(1)), + (1, Some(1)) + ); + assert_eq!(Step::steps_between(&VirtAddr(1), &VirtAddr(0)), (0, None)); assert_eq!( Step::steps_between( &VirtAddr(0x7fff_ffff_ffff), &VirtAddr(0xffff_8000_0000_0000) ), - Some(1) + (1, Some(1)) ); assert_eq!( Step::steps_between( &VirtAddr(0xffff_8000_0000_0000), &VirtAddr(0x7fff_ffff_ffff) ), - None + (0, None) ); assert_eq!( Step::steps_between( &VirtAddr(0xffff_8000_0000_0000), &VirtAddr(0xffff_8000_0000_0000) ), - Some(0) + (0, Some(0)) ); assert_eq!( Step::steps_between( &VirtAddr(0xffff_8000_0000_0000), &VirtAddr(0xffff_8000_0000_0001) ), - Some(1) + (1, Some(1)) ); assert_eq!( Step::steps_between( &VirtAddr(0xffff_8000_0000_0001), &VirtAddr(0xffff_8000_0000_0000) ), - None + (0, None) ); } @@ -951,7 +962,7 @@ mod proofs { }; // ...then `steps_between` succeeds as well. - assert!(Step::steps_between(&start, &end) == Some(count)); + assert!(Step::steps_between(&start, &end) == (count, Some(count))); } // This harness proves that for all inputs for which `steps_between` @@ -968,7 +979,7 @@ mod proofs { }; // If `steps_between` succeeds... - let Some(count) = Step::steps_between(&start, &end) else { + let Some(count) = Step::steps_between(&start, &end).1 else { return; }; diff --git a/src/instructions/tlb.rs b/src/instructions/tlb.rs index 3f5c4467..4a523483 100644 --- a/src/instructions/tlb.rs +++ b/src/instructions/tlb.rs @@ -315,14 +315,14 @@ where if let Some(mut pages) = self.page_range { while !pages.is_empty() { // Calculate out how many pages we still need to flush. - let count = Page::::steps_between_impl(&pages.start, &pages.end).unwrap(); + let count = Page::::steps_between_impl(&pages.start, &pages.end).0; // Make sure that we never jump the gap in the address space when flushing. let second_half_start = Page::::containing_address(VirtAddr::new(0xffff_8000_0000_0000)); let count = if pages.start < second_half_start { let count_to_second_half = - Page::steps_between_impl(&pages.start, &second_half_start).unwrap(); + Page::steps_between_impl(&pages.start, &second_half_start).0; cmp::min(count, count_to_second_half) } else { count diff --git a/src/structures/paging/page.rs b/src/structures/paging/page.rs index 55720cf4..e71067f5 100644 --- a/src/structures/paging/page.rs +++ b/src/structures/paging/page.rs @@ -160,9 +160,11 @@ impl Page { // FIXME: Move this into the `Step` impl, once `Step` is stabilized. #[cfg(any(feature = "instructions", feature = "step_trait"))] - pub(crate) fn steps_between_impl(start: &Self, end: &Self) -> Option { - VirtAddr::steps_between_impl(&start.start_address, &end.start_address) - .map(|steps| steps / S::SIZE as usize) + pub(crate) fn steps_between_impl(start: &Self, end: &Self) -> (usize, Option) { + let (lower, upper) = VirtAddr::steps_between_impl(&start.start_address, &end.start_address); + let lower = lower / S::SIZE as usize; + let upper = upper.map(|steps| steps / S::SIZE as usize); + (lower, upper) } // FIXME: Move this into the `Step` impl, once `Step` is stabilized. @@ -293,7 +295,7 @@ impl Sub for Page { #[cfg(feature = "step_trait")] impl Step for Page { - fn steps_between(start: &Self, end: &Self) -> Option { + fn steps_between(start: &Self, end: &Self) -> (usize, Option) { Self::steps_between_impl(start, end) } diff --git a/src/structures/paging/page_table.rs b/src/structures/paging/page_table.rs index fe9ebde0..e9069bcc 100644 --- a/src/structures/paging/page_table.rs +++ b/src/structures/paging/page_table.rs @@ -353,8 +353,8 @@ impl From for usize { #[cfg(feature = "step_trait")] impl Step for PageTableIndex { #[inline] - fn steps_between(start: &Self, end: &Self) -> Option { - end.0.checked_sub(start.0).map(usize::from) + fn steps_between(start: &Self, end: &Self) -> (usize, Option) { + Step::steps_between(&start.0, &end.0) } #[inline] From 3d6571458fc48f3b4a81d137536c3b5d6f98a9ad Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Sun, 24 Nov 2024 09:21:58 +0100 Subject: [PATCH 2/5] actually test with slice instead of array Without the as_slice call, the value passed to from_ptr was &[i32; 5] which is *not* a slice. --- src/addr.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/addr.rs b/src/addr.rs index fd29a896..2f894bb5 100644 --- a/src/addr.rs +++ b/src/addr.rs @@ -823,7 +823,10 @@ mod tests { fn test_from_ptr_array() { let slice = &[1, 2, 3, 4, 5]; // Make sure that from_ptr(slice) is the address of the first element - assert_eq!(VirtAddr::from_ptr(slice), VirtAddr::from_ptr(&slice[0])); + assert_eq!( + VirtAddr::from_ptr(slice.as_slice()), + VirtAddr::from_ptr(&slice[0]) + ); } } From 1e5b5a1e45383fd41df90650c9bed2f4c0caa1db Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Sun, 24 Nov 2024 09:23:52 +0100 Subject: [PATCH 3/5] fix tests on 32-bit platforms --- src/addr.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/addr.rs b/src/addr.rs index 2f894bb5..cdc97ffe 100644 --- a/src/addr.rs +++ b/src/addr.rs @@ -669,22 +669,27 @@ mod tests { Step::forward_checked(VirtAddr(0xffff_ffff_ffff_ffff), 1), None ); + #[cfg(target_pointer_width = "64")] assert_eq!( Step::forward(VirtAddr(0x7fff_ffff_ffff), 0x1234_5678_9abd), VirtAddr(0xffff_9234_5678_9abc) ); + #[cfg(target_pointer_width = "64")] assert_eq!( Step::forward(VirtAddr(0x7fff_ffff_ffff), 0x8000_0000_0000), VirtAddr(0xffff_ffff_ffff_ffff) ); + #[cfg(target_pointer_width = "64")] assert_eq!( Step::forward(VirtAddr(0x7fff_ffff_ff00), 0x8000_0000_00ff), VirtAddr(0xffff_ffff_ffff_ffff) ); + #[cfg(target_pointer_width = "64")] assert_eq!( Step::forward_checked(VirtAddr(0x7fff_ffff_ff00), 0x8000_0000_0100), None ); + #[cfg(target_pointer_width = "64")] assert_eq!( Step::forward_checked(VirtAddr(0x7fff_ffff_ffff), 0x8000_0000_0001), None @@ -705,18 +710,22 @@ mod tests { Step::backward(VirtAddr(0xffff_8000_0000_0001), 1), VirtAddr(0xffff_8000_0000_0000) ); + #[cfg(target_pointer_width = "64")] assert_eq!( Step::backward(VirtAddr(0xffff_9234_5678_9abc), 0x1234_5678_9abd), VirtAddr(0x7fff_ffff_ffff) ); + #[cfg(target_pointer_width = "64")] assert_eq!( Step::backward(VirtAddr(0xffff_8000_0000_0000), 0x8000_0000_0000), VirtAddr(0) ); + #[cfg(target_pointer_width = "64")] assert_eq!( Step::backward(VirtAddr(0xffff_8000_0000_0000), 0x7fff_ffff_ff01), VirtAddr(0xff) ); + #[cfg(target_pointer_width = "64")] assert_eq!( Step::backward_checked(VirtAddr(0xffff_8000_0000_0000), 0x8000_0000_0001), None @@ -820,6 +829,7 @@ mod tests { } #[test] + #[cfg(target_pointer_width = "64")] fn test_from_ptr_array() { let slice = &[1, 2, 3, 4, 5]; // Make sure that from_ptr(slice) is the address of the first element From 8fb0668eccf8c917d657f740ac7a416933392b25 Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Sun, 24 Nov 2024 09:24:05 +0100 Subject: [PATCH 4/5] run tests on a 32-bit platform --- .github/workflows/build.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index a406cbba..1216bcab 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -56,7 +56,7 @@ jobs: - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@nightly with: - targets: x86_64-unknown-linux-musl, i686-unknown-linux-gnu, thumbv7em-none-eabihf + targets: x86_64-unknown-linux-musl, i686-unknown-linux-musl, thumbv7em-none-eabihf - run: cargo build @@ -72,9 +72,12 @@ jobs: - name: "Build on non x86_64 platforms" run: | - cargo build --target i686-unknown-linux-gnu --no-default-features --features nightly + cargo build --target i686-unknown-linux-musl --no-default-features --features nightly cargo build --target thumbv7em-none-eabihf --no-default-features --features nightly + - run: cargo test --target i686-unknown-linux-musl --no-default-features --features nightly + if: runner.os == 'Linux' + bootloader-test: name: "Bootloader Integration Test" From b4b66637b7306254447bde805789f1a2cf92b6e5 Mon Sep 17 00:00:00 2001 From: Tom Dohrmann Date: Sun, 24 Nov 2024 09:25:42 +0100 Subject: [PATCH 5/5] fix Page Step impl on 32-bit platforms Previously, we scaled count as a usize, not a u64. This is bad because stepping forward by 0x10_0000 is perfectly fine, yet we were always rejecting this because 0x10_0000 * 0x1000 doesn't fit in usize on 32-bit platforms. Similarly, we would fail to return a step count above or equal to 0x10_0000 because the call to ::steps_between would fail. Never scale usize values, instead, always scale u64 values. To make this easier to work with, this patch adds variants of the Step functions to VirtAddr that take and return u64 instead of usize. This patch also adds some regression tests. --- src/addr.rs | 94 ++++++++++++++++-------- src/structures/paging/page.rs | 133 ++++++++++++++++++++++++++++++++-- 2 files changed, 189 insertions(+), 38 deletions(-) diff --git a/src/addr.rs b/src/addr.rs index cdc97ffe..7ce471ee 100644 --- a/src/addr.rs +++ b/src/addr.rs @@ -240,30 +240,43 @@ impl VirtAddr { } // FIXME: Move this into the `Step` impl, once `Step` is stabilized. - #[cfg(any(feature = "instructions", feature = "step_trait"))] + #[cfg(feature = "step_trait")] pub(crate) fn steps_between_impl(start: &Self, end: &Self) -> (usize, Option) { - let mut steps = if let Some(steps) = end.0.checked_sub(start.0) { - steps + if let Some(steps) = Self::steps_between_u64(start, end) { + let steps = usize::try_from(steps).ok(); + (steps.unwrap_or(usize::MAX), steps) } else { - return (0, None); - }; + (0, None) + } + } + + /// An implementation of steps_between that returns u64. Note that this + /// function always returns the exact bound, so it doesn't need to return a + /// lower and upper bound like steps_between does. + #[cfg(any(feature = "instructions", feature = "step_trait"))] + pub(crate) fn steps_between_u64(start: &Self, end: &Self) -> Option { + let mut steps = end.0.checked_sub(start.0)?; // Mask away extra bits that appear while jumping the gap. steps &= 0xffff_ffff_ffff; - let steps = usize::try_from(steps).ok(); - (steps.unwrap_or(usize::MAX), steps) + Some(steps) } // FIXME: Move this into the `Step` impl, once `Step` is stabilized. #[inline] pub(crate) fn forward_checked_impl(start: Self, count: usize) -> Option { - let offset = u64::try_from(count).ok()?; - if offset > ADDRESS_SPACE_SIZE { + Self::forward_checked_u64(start, u64::try_from(count).ok()?) + } + + /// An implementation of forward_checked that takes u64 instead of usize. + #[inline] + pub(crate) fn forward_checked_u64(start: Self, count: u64) -> Option { + if count > ADDRESS_SPACE_SIZE { return None; } - let mut addr = start.0.checked_add(offset)?; + let mut addr = start.0.checked_add(count)?; match addr.get_bits(47..) { 0x1 => { @@ -279,6 +292,31 @@ impl VirtAddr { Some(unsafe { Self::new_unsafe(addr) }) } + + /// An implementation of backward_checked that takes u64 instead of usize. + #[cfg(feature = "step_trait")] + #[inline] + pub(crate) fn backward_checked_u64(start: Self, count: u64) -> Option { + if count > ADDRESS_SPACE_SIZE { + return None; + } + + let mut addr = start.0.checked_sub(count)?; + + match addr.get_bits(47..) { + 0x1fffe => { + // Jump the gap by sign extending the 47th bit. + addr.set_bits(47.., 0); + } + 0x1fffd => { + // Address underflow + return None; + } + _ => {} + } + + Some(unsafe { Self::new_unsafe(addr) }) + } } impl fmt::Debug for VirtAddr { @@ -376,26 +414,7 @@ impl Step for VirtAddr { #[inline] fn backward_checked(start: Self, count: usize) -> Option { - let offset = u64::try_from(count).ok()?; - if offset > ADDRESS_SPACE_SIZE { - return None; - } - - let mut addr = start.0.checked_sub(offset)?; - - match addr.get_bits(47..) { - 0x1fffe => { - // Jump the gap by sign extending the 47th bit. - addr.set_bits(47.., 0); - } - 0x1fffd => { - // Address underflow - return None; - } - _ => {} - } - - Some(unsafe { Self::new_unsafe(addr) }) + Self::backward_checked_u64(start, u64::try_from(count).ok()?) } } @@ -779,6 +798,21 @@ mod tests { ), (0, None) ); + // Make sure that we handle `steps > u32::MAX` correctly on 32-bit + // targets. On 64-bit targets, `0x1_0000_0000` fits into `usize`, so we + // can return exact lower and upper bounds. On 32-bit targets, + // `0x1_0000_0000` doesn't fit into `usize`, so we only return an lower + // bound of `usize::MAX` and don't return an upper bound. + #[cfg(target_pointer_width = "64")] + assert_eq!( + Step::steps_between(&VirtAddr(0), &VirtAddr(0x1_0000_0000)), + (0x1_0000_0000, Some(0x1_0000_0000)) + ); + #[cfg(not(target_pointer_width = "64"))] + assert_eq!( + Step::steps_between(&VirtAddr(0), &VirtAddr(0x1_0000_0000)), + (usize::MAX, None) + ); } #[test] diff --git a/src/structures/paging/page.rs b/src/structures/paging/page.rs index e71067f5..a51b4df4 100644 --- a/src/structures/paging/page.rs +++ b/src/structures/paging/page.rs @@ -161,17 +161,26 @@ impl Page { // FIXME: Move this into the `Step` impl, once `Step` is stabilized. #[cfg(any(feature = "instructions", feature = "step_trait"))] pub(crate) fn steps_between_impl(start: &Self, end: &Self) -> (usize, Option) { - let (lower, upper) = VirtAddr::steps_between_impl(&start.start_address, &end.start_address); - let lower = lower / S::SIZE as usize; - let upper = upper.map(|steps| steps / S::SIZE as usize); - (lower, upper) + use core::convert::TryFrom; + + if let Some(steps) = + VirtAddr::steps_between_u64(&start.start_address(), &end.start_address()) + { + let steps = steps / S::SIZE; + let steps = usize::try_from(steps).ok(); + (steps.unwrap_or(usize::MAX), steps) + } else { + (0, None) + } } // FIXME: Move this into the `Step` impl, once `Step` is stabilized. #[cfg(any(feature = "instructions", feature = "step_trait"))] pub(crate) fn forward_checked_impl(start: Self, count: usize) -> Option { - let count = count.checked_mul(S::SIZE as usize)?; - let start_address = VirtAddr::forward_checked_impl(start.start_address, count)?; + use core::convert::TryFrom; + + let count = u64::try_from(count).ok()?.checked_mul(S::SIZE)?; + let start_address = VirtAddr::forward_checked_u64(start.start_address, count)?; Some(Self { start_address, size: PhantomData, @@ -304,8 +313,10 @@ impl Step for Page { } fn backward_checked(start: Self, count: usize) -> Option { - let count = count.checked_mul(S::SIZE as usize)?; - let start_address = Step::backward_checked(start.start_address, count)?; + use core::convert::TryFrom; + + let count = u64::try_from(count).ok()?.checked_mul(S::SIZE)?; + let start_address = VirtAddr::backward_checked_u64(start.start_address, count)?; Some(Self { start_address, size: PhantomData, @@ -531,4 +542,110 @@ mod tests { let range_inclusive = PageRangeInclusive { start, end }; assert_eq!(range_inclusive.len(), 51); } + + #[test] + #[cfg(feature = "step_trait")] + fn page_step_forward() { + let test_cases = [ + (0, 0, Some(0)), + (0, 1, Some(0x1000)), + (0x1000, 1, Some(0x2000)), + (0x7fff_ffff_f000, 1, Some(0xffff_8000_0000_0000)), + (0xffff_8000_0000_0000, 1, Some(0xffff_8000_0000_1000)), + (0xffff_ffff_ffff_f000, 1, None), + #[cfg(target_pointer_width = "64")] + (0x7fff_ffff_f000, 0x1_2345_6789, Some(0xffff_9234_5678_8000)), + #[cfg(target_pointer_width = "64")] + (0x7fff_ffff_f000, 0x8_0000_0000, Some(0xffff_ffff_ffff_f000)), + #[cfg(target_pointer_width = "64")] + (0x7fff_fff0_0000, 0x8_0000_00ff, Some(0xffff_ffff_ffff_f000)), + #[cfg(target_pointer_width = "64")] + (0x7fff_fff0_0000, 0x8_0000_0100, None), + #[cfg(target_pointer_width = "64")] + (0x7fff_ffff_f000, 0x8_0000_0001, None), + // Make sure that we handle `steps * PAGE_SIZE > u32::MAX` + // correctly on 32-bit targets. + (0, 0x10_0000, Some(0x1_0000_0000)), + ]; + for (start, count, result) in test_cases { + let start = Page::::from_start_address(VirtAddr::new(start)).unwrap(); + let result = result + .map(|result| Page::::from_start_address(VirtAddr::new(result)).unwrap()); + assert_eq!(Step::forward_checked(start, count), result); + } + } + + #[test] + #[cfg(feature = "step_trait")] + fn page_step_backwards() { + let test_cases = [ + (0, 0, Some(0)), + (0, 1, None), + (0x1000, 1, Some(0)), + (0xffff_8000_0000_0000, 1, Some(0x7fff_ffff_f000)), + (0xffff_8000_0000_1000, 1, Some(0xffff_8000_0000_0000)), + #[cfg(target_pointer_width = "64")] + (0xffff_9234_5678_8000, 0x1_2345_6789, Some(0x7fff_ffff_f000)), + #[cfg(target_pointer_width = "64")] + (0xffff_8000_0000_0000, 0x8_0000_0000, Some(0)), + #[cfg(target_pointer_width = "64")] + (0xffff_8000_0000_0000, 0x7_ffff_ff01, Some(0xff000)), + #[cfg(target_pointer_width = "64")] + (0xffff_8000_0000_0000, 0x8_0000_0001, None), + // Make sure that we handle `steps * PAGE_SIZE > u32::MAX` + // correctly on 32-bit targets. + (0x1_0000_0000, 0x10_0000, Some(0)), + ]; + for (start, count, result) in test_cases { + let start = Page::::from_start_address(VirtAddr::new(start)).unwrap(); + let result = result + .map(|result| Page::::from_start_address(VirtAddr::new(result)).unwrap()); + assert_eq!(Step::backward_checked(start, count), result); + } + } + + #[test] + #[cfg(feature = "step_trait")] + fn page_steps_between() { + let test_cases = [ + (0, 0, 0, Some(0)), + (0, 0x1000, 1, Some(1)), + (0x1000, 0, 0, None), + (0x1000, 0x1000, 0, Some(0)), + (0x7fff_ffff_f000, 0xffff_8000_0000_0000, 1, Some(1)), + (0xffff_8000_0000_0000, 0x7fff_ffff_f000, 0, None), + (0xffff_8000_0000_0000, 0xffff_8000_0000_0000, 0, Some(0)), + (0xffff_8000_0000_0000, 0xffff_8000_0000_1000, 1, Some(1)), + (0xffff_8000_0000_1000, 0xffff_8000_0000_0000, 0, None), + (0xffff_8000_0000_1000, 0xffff_8000_0000_1000, 0, Some(0)), + // Make sure that we handle `steps * PAGE_SIZE > u32::MAX` correctly on 32-bit + // targets. + ( + 0x0000_0000_0000, + 0x0001_0000_0000, + 0x10_0000, + Some(0x10_0000), + ), + // The returned bounds are different when `steps` doesn't fit in + // into `usize`. On 64-bit targets, `0x1_0000_0000` fits into + // `usize`, so we can return exact lower and upper bounds. On + // 32-bit targets, `0x1_0000_0000` doesn't fit into `usize`, so we + // only return an lower bound of `usize::MAX` and don't return an + // upper bound. + #[cfg(target_pointer_width = "64")] + ( + 0x0000_0000_0000, + 0x1000_0000_0000, + 0x1_0000_0000, + Some(0x1_0000_0000), + ), + #[cfg(not(target_pointer_width = "64"))] + (0x0000_0000_0000, 0x1000_0000_0000, usize::MAX, None), + ]; + for (start, end, lower, upper) in test_cases { + let start = Page::::from_start_address(VirtAddr::new(start)).unwrap(); + let end = Page::from_start_address(VirtAddr::new(end)).unwrap(); + assert_eq!(Step::steps_between(&start, &end), (lower, upper)); + } + } }