From d1ecee96bfb7fb52b67358302c524a8c0193dc23 Mon Sep 17 00:00:00 2001 From: Ulrik Sverdrup Date: Wed, 24 Aug 2016 18:57:45 +0200 Subject: [PATCH 1/2] memrchr: Correct aligned offset computation The memrchr fallback did not compute the offset correctly. It was intentioned to land on usize-aligned addresses but did not. This was suspected to resulted in a crash on ARMv7 platform! This bug affected non-linux platforms. I think like this, if we have a slice with pointer `ptr` and length `len`, we want to find the last usize-aligned offset in the slice. The correct computation should be: For example if ptr = 1 and len = 6, and size_of::() is 4: [ x x x x x x ] 1 2 3 4 5 6 ^-- last aligned address at offset 3 from the start. The last aligned address is ptr + len - (ptr + len) % usize_size. Compute offset from the start as: offset = len - (ptr + len) % usize_size = 6 - (1 + 6) % 4 = 6 - 3 = 3. I believe the function's return value was always correct previously, if the platform supported unaligned addresses. --- src/libstd/memchr.rs | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/libstd/memchr.rs b/src/libstd/memchr.rs index a408b4378e19e..89b77a7d6614e 100644 --- a/src/libstd/memchr.rs +++ b/src/libstd/memchr.rs @@ -209,7 +209,7 @@ mod fallback { let end_align = (ptr as usize + len) & (usize_bytes - 1); let mut offset; if end_align > 0 { - offset = len - cmp::min(usize_bytes - end_align, len); + offset = len - cmp::min(end_align, len); if let Some(index) = text[offset..].iter().rposition(|elt| *elt == x) { return Some(offset + index); } @@ -309,6 +309,17 @@ mod fallback { fn no_match_reversed() { assert_eq!(None, memrchr(b'a', b"xyz")); } + + #[test] + fn each_alignment_reversed() { + let mut data = [1u8; 64]; + let needle = 2; + let pos = 40; + data[pos] = needle; + for start in 0..16 { + assert_eq!(Some(pos - start), memrchr(needle, &data[start..])); + } + } } #[cfg(test)] @@ -385,4 +396,15 @@ mod tests { fn no_match_reversed() { assert_eq!(None, memrchr(b'a', b"xyz")); } + + #[test] + fn each_alignment() { + let mut data = [1u8; 64]; + let needle = 2; + let pos = 40; + data[pos] = needle; + for start in 0..16 { + assert_eq!(Some(pos - start), memchr(needle, &data[start..])); + } + } } From 8295c5056da0be355c86b29d1d4eed469920e73c Mon Sep 17 00:00:00 2001 From: Ulrik Sverdrup Date: Wed, 24 Aug 2016 21:41:23 +0200 Subject: [PATCH 2/2] memrchr: Use a conditional instead of subtracting a complicated min This makes the critical calculation easier to understand. --- src/libstd/memchr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstd/memchr.rs b/src/libstd/memchr.rs index 89b77a7d6614e..03f55f7ad6186 100644 --- a/src/libstd/memchr.rs +++ b/src/libstd/memchr.rs @@ -209,7 +209,7 @@ mod fallback { let end_align = (ptr as usize + len) & (usize_bytes - 1); let mut offset; if end_align > 0 { - offset = len - cmp::min(end_align, len); + offset = if end_align >= len { 0 } else { len - end_align }; if let Some(index) = text[offset..].iter().rposition(|elt| *elt == x) { return Some(offset + index); }