Skip to content
Merged
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
1a810db
Drop unsafe loops. On Rust 1.27 performance degradation is within mea…
Shnatsel Jun 22, 2018
488db57
Exit earlier on invalid run length, without leaving behind a buffer w…
Shnatsel Jun 24, 2018
a5e29c8
Fix formatting
Shnatsel Jun 24, 2018
bc3916d
Fix formatting in one more place
Shnatsel Jun 24, 2018
3715c4c
Add semicolon
Shnatsel Jun 24, 2018
11c73f2
run_len_dist: validate that dist is not 0. Fixes information disclosu…
Shnatsel Jun 25, 2018
8f22601
::std::intrinsics now requires 'core_intrinsics' feature, not 'core'
Shnatsel Jun 25, 2018
4e5cc94
Remove use of 'abort' intrinsic now that panic!() is optimized by the…
Shnatsel Jun 25, 2018
dd20d3e
Remove 'unstable' feature from Cargo.toml
Shnatsel Jun 25, 2018
d56abee
Replace panic!() with unreachable!(). Thank you @hauleth for the advice
Shnatsel Jun 25, 2018
61c2c1c
Revert "Remove 'unstable' feature from Cargo.toml"
Shnatsel Jun 26, 2018
d476136
Add an assert() before the last remaining unsafe {}; no performance p…
Shnatsel Jun 26, 2018
fdee4de
Add another assert against potentially reading from uninitialized memory
Shnatsel Jun 26, 2018
9c8f582
Add a comment explaining why `if dist < 1` check is there
Shnatsel Jun 26, 2018
a2955c8
Add an assert for dist correctness right before it's used. Surprising…
Shnatsel Jun 26, 2018
f4418ea
More comments warning the unwary about the one and only unsafe block
Shnatsel Jun 26, 2018
baf9c7e
Drop a wall of text in a comment, it is not helpful. There is an asse…
Shnatsel Jun 26, 2018
a36a517
Explain what each assert actually does
Shnatsel Jun 26, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 22 additions & 60 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,6 @@
//! }
//! ```

#![cfg_attr(feature = "unstable", feature(core))]

extern crate adler32;

use std::cmp;
Expand Down Expand Up @@ -182,16 +180,6 @@ struct BitStream<'a> {
state: BitState,
}

// Use this instead of triggering a panic (that will unwind).
#[cfg(feature = "unstable")]
fn abort() -> ! {
unsafe { ::std::intrinsics::abort() }
}
#[cfg(not(feature = "unstable"))]
fn abort() -> ! {
panic!()
}

#[cfg(debug)]
macro_rules! debug { ($($x:tt)*) => (println!($($x)*)) }
#[cfg(not(debug))]
Expand Down Expand Up @@ -224,10 +212,7 @@ impl<'a> BitStream<'a> {
return false;
}
if n > 8 && self.state.n < n {
if n > 16 {
// HACK(eddyb) in place of a static assert.
abort();
}
assert!(n <= 16);
if !self.use_byte() {
return false;
}
Expand All @@ -248,10 +233,7 @@ impl<'a> BitStream<'a> {
}

fn take(&mut self, n: u8) -> Option<u8> {
if n > 8 {
// HACK(eddyb) in place of a static assert.
abort();
}
assert!(n <= 8);
self.take16(n).map(|v: u16| v as u8)
}

Expand Down Expand Up @@ -404,7 +386,7 @@ impl CodeLengthReader {
self.result.push(0);
}
}
_ => abort(),
_ => unreachable!(),
}
}
Ok(true)
Expand Down Expand Up @@ -625,6 +607,10 @@ impl InflateStream {
fn run_len_dist(&mut self, len: u16, dist: u16) -> Result<Option<u16>, String> {
debug!("RLE -{}; {} (cap={} len={})", dist, len,
self.buffer.capacity(), self.buffer.len());
if dist < 1 {
return Err("invalid run length in stream".to_owned());
}
// `buffer_size` is used for validating `unsafe` below, handle with care
let buffer_size = self.buffer.capacity() as u16;
let len = if self.pos < dist {
// Handle copying from ahead, until we hit the end reading.
Expand All @@ -638,26 +624,13 @@ impl InflateStream {
return Err("run length distance is bigger than the window size".to_owned());
}
let forward = buffer_size - dist;
// assert for unsafe code:
if pos_end + forward > self.buffer.len() as u16 {
return Err("invalid run length in stream".to_owned());
}
unsafe {
// HACK(eddyb) avoid bound checks, LLVM can't optimize these.
let buffer = self.buffer.as_mut_ptr();
let dst_end = buffer.offset(pos_end as isize);
let mut dst = buffer.offset(self.pos as isize);
let mut src = dst.offset(forward as isize);
while dst < dst_end {
*dst = *src;
dst = dst.offset(1);
src = src.offset(1);
}

for i in self.pos as usize..pos_end as usize {
self.buffer[i] = self.buffer[i + forward as usize];
}
// for i in self.pos as usize..pos_end as usize {
// self.buffer[i] = self.buffer[i + forward as usize]
// }
//
self.pos = pos_end;
left
} else {
Expand All @@ -671,32 +644,23 @@ impl InflateStream {
(buffer_size, Some(pos_end - buffer_size))
};

if self.pos < dist && pos_end > self.pos {
return Err("invalid run length in stream".to_owned());
}

if self.buffer.len() < pos_end as usize {
// ensure the buffer length will not exceed the amount of allocated memory
assert!(pos_end <= buffer_size);
// ensure that the uninitialized chunk of memory will be fully overwritten
assert!(self.pos as usize <= self.buffer.len());
unsafe {
self.buffer.set_len(pos_end as usize);
}
}

// assert for unsafe code:
if self.pos < dist && pos_end > self.pos {
return Err("invalid run length in stream".to_owned());
}
unsafe {
// HACK(eddyb) avoid bound checks, LLVM can't optimize these.
let buffer = self.buffer.as_mut_ptr();
let dst_end = buffer.offset(pos_end as isize);
let mut dst = buffer.offset(self.pos as isize);
let mut src = dst.offset(-(dist as isize));
while dst < dst_end {
*dst = *src;
dst = dst.offset(1);
src = src.offset(1);
}
assert!(dist > 0); // validation against reading uninitialized memory
for i in self.pos as usize..pos_end as usize {
self.buffer[i] = self.buffer[i - dist as usize];
}
// for i in self.pos as usize..pos_end as usize {
// self.buffer[i] = self.buffer[i - dist as usize]
// }
//
self.pos = pos_end;
Ok(left)
}
Expand All @@ -713,9 +677,7 @@ impl InflateStream {
if (self.pos as usize) < self.buffer.len() {
self.buffer[self.pos as usize] = b;
} else {
if (self.pos as usize) != self.buffer.len() {
abort();
}
assert_eq!(self.pos as usize, self.buffer.len());
self.buffer.push(b);
}
self.pos += 1;
Expand Down