From 63e10b3e5fc47b041464e6fedb29d22da7d85f14 Mon Sep 17 00:00:00 2001 From: newpavlov Date: Wed, 13 Jun 2018 15:49:19 +0300 Subject: [PATCH 1/8] inline optimization, limit relaxation --- flif/src/lib.rs | 4 ++-- flif/src/numbers/near_zero.rs | 12 +++++++----- flif/src/numbers/rac.rs | 1 + 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/flif/src/lib.rs b/flif/src/lib.rs index 20219b5c..26c1b618 100644 --- a/flif/src/lib.rs +++ b/flif/src/lib.rs @@ -85,7 +85,7 @@ pub struct Limits { pub metadata_count: usize, /// max number of pixels: `width * height * frames` (default: 226) pub pixels: usize, - /// max number of MANIAC nodes (default: 4096) + /// max number of MANIAC nodes (default: 214) pub maniac_nodes: usize, } @@ -95,7 +95,7 @@ impl Default for Limits { metadata_chunk: 1 << 20, metadata_count: 8, pixels: 1 << 26, - maniac_nodes: 4096, + maniac_nodes: 1 << 14, } } } diff --git a/flif/src/numbers/near_zero.rs b/flif/src/numbers/near_zero.rs index 7f2c6928..9aeff702 100644 --- a/flif/src/numbers/near_zero.rs +++ b/flif/src/numbers/near_zero.rs @@ -19,16 +19,18 @@ impl NearZeroCoder for R { max: I, context: &mut ChanceTable, ) -> Result { - if min > I::zero() { - Ok(read_near_zero_inner(self, I::zero(), max - min, context)? + min) + let (min, max, delta) = if min > I::zero() { + (I::zero(), max - min, min) } else if max < I::zero() { - Ok(read_near_zero_inner(self, min - max, I::zero(), context)? + max) + (min - max, I::zero(), max) } else { - read_near_zero_inner(self, min, max, context) - } + (min, max, I::zero()) + }; + Ok(read_near_zero_inner(self, min, max, context)? + delta) } } +#[inline(always)] fn read_near_zero_inner( read: &mut R, min: I, diff --git a/flif/src/numbers/rac.rs b/flif/src/numbers/rac.rs index b306e441..15a1896b 100644 --- a/flif/src/numbers/rac.rs +++ b/flif/src/numbers/rac.rs @@ -57,6 +57,7 @@ impl RacRead for Rac { self.get(chance) } + #[inline(always)] fn read(&mut self, context: &mut ChanceTable, entry: ChanceTableEntry) -> Result { let chance = context.get_chance(entry); let transformed_chance = Self::apply_chance(u32::from(chance), self.range); From f9917bf487572dac35b873ef8d9fa32819fa1c0e Mon Sep 17 00:00:00 2001 From: newpavlov Date: Wed, 13 Jun 2018 16:23:55 +0300 Subject: [PATCH 2/8] branchless read_near_zero --- flif/src/numbers/near_zero.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/flif/src/numbers/near_zero.rs b/flif/src/numbers/near_zero.rs index 9aeff702..80f53cce 100644 --- a/flif/src/numbers/near_zero.rs +++ b/flif/src/numbers/near_zero.rs @@ -3,6 +3,8 @@ use num_traits::PrimInt; use numbers::chances::{ChanceTable, ChanceTableEntry}; use numbers::rac::RacRead; +use std::cmp; + pub trait NearZeroCoder { fn read_near_zero( &mut self, @@ -19,13 +21,9 @@ impl NearZeroCoder for R { max: I, context: &mut ChanceTable, ) -> Result { - let (min, max, delta) = if min > I::zero() { - (I::zero(), max - min, min) - } else if max < I::zero() { - (min - max, I::zero(), max) - } else { - (min, max, I::zero()) - }; + let delta = cmp::min(max, cmp::max(I::zero(), min)); + let min = min - delta; + let max = max - delta; Ok(read_near_zero_inner(self, min, max, context)? + delta) } } From 73e79dc7de939f3afb068986324b61682ddc31cd Mon Sep 17 00:00:00 2001 From: newpavlov Date: Wed, 13 Jun 2018 17:20:03 +0300 Subject: [PATCH 3/8] fixed potential overflow --- flif/src/decoder.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/flif/src/decoder.rs b/flif/src/decoder.rs index 213185aa..47773600 100644 --- a/flif/src/decoder.rs +++ b/flif/src/decoder.rs @@ -91,13 +91,12 @@ fn identify_internal(mut reader: R, limits: Limits) -> Result<(FlifInfo // read the first header let main_header = Header::from_reader(&mut reader)?; let frames = main_header.num_frames as usize; - let pixels = main_header.width * main_header.height * frames; - if pixels > limits.pixels { - Err(Error::LimitViolation(format!( - "number of pixels exceeds limit: {} vs {}", - pixels, limits.pixels, - )))? - } + frames.checked_mul(main_header.width) + .and_then(|val| val.checked_mul(main_header.height)) + .and_then(|pixels| if pixels > limits.pixels { None } else { Some(()) }) + .ok_or(Error::LimitViolation(format!( + "number of pixels exceeds limit: {}", limits.pixels, + )))?; // read the metadata chunks let (metadata, non_optional_byte) = Metadata::all_from_reader(&mut reader, &limits)?; From 1f943a26d9a30073f9cbe90889f6890d78371abb Mon Sep 17 00:00:00 2001 From: newpavlov Date: Wed, 13 Jun 2018 17:31:20 +0300 Subject: [PATCH 4/8] fmt --- flif/src/decoder.rs | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/flif/src/decoder.rs b/flif/src/decoder.rs index 47773600..da42edbb 100644 --- a/flif/src/decoder.rs +++ b/flif/src/decoder.rs @@ -91,12 +91,25 @@ fn identify_internal(mut reader: R, limits: Limits) -> Result<(FlifInfo // read the first header let main_header = Header::from_reader(&mut reader)?; let frames = main_header.num_frames as usize; - frames.checked_mul(main_header.width) - .and_then(|val| val.checked_mul(main_header.height)) - .and_then(|pixels| if pixels > limits.pixels { None } else { Some(()) }) - .ok_or(Error::LimitViolation(format!( - "number of pixels exceeds limit: {}", limits.pixels, - )))?; + let pixels = frames + .checked_mul(main_header.width) + .and_then(|val| val.checked_mul(main_header.height)); + match pixels { + Some(pix) if pix > limits.pixels => { + Err(Error::LimitViolation(format!( + "number of pixels exceeds limit: {}/{}", + pix, + limits.pixels, + )))?; + } + None => { + Err(Error::LimitViolation(format!( + "number of pixels exceeds limit: overflow/{}", + limits.pixels, + )))?; + } + Some(_) => (), + } // read the metadata chunks let (metadata, non_optional_byte) = Metadata::all_from_reader(&mut reader, &limits)?; From 8b0ddbb765685815b967c1e29231d12d1025080f Mon Sep 17 00:00:00 2001 From: newpavlov Date: Wed, 13 Jun 2018 17:48:57 +0300 Subject: [PATCH 5/8] fmt2 --- flif/src/decoder.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/flif/src/decoder.rs b/flif/src/decoder.rs index da42edbb..3a6eaa31 100644 --- a/flif/src/decoder.rs +++ b/flif/src/decoder.rs @@ -98,8 +98,7 @@ fn identify_internal(mut reader: R, limits: Limits) -> Result<(FlifInfo Some(pix) if pix > limits.pixels => { Err(Error::LimitViolation(format!( "number of pixels exceeds limit: {}/{}", - pix, - limits.pixels, + pix, limits.pixels, )))?; } None => { From 407948663a9b7ef21ab7700ed1a1cf554b285cad Mon Sep 17 00:00:00 2001 From: newpavlov Date: Thu, 14 Jun 2018 21:44:52 +0300 Subject: [PATCH 6/8] fix overflow bug --- flif/src/components/header.rs | 47 ++++++++++++++++++++++++++++++++--- flif/src/decoder.rs | 21 +--------------- 2 files changed, 44 insertions(+), 24 deletions(-) diff --git a/flif/src/components/header.rs b/flif/src/components/header.rs index 43b1a4fb..03568b54 100644 --- a/flif/src/components/header.rs +++ b/flif/src/components/header.rs @@ -8,6 +8,7 @@ use numbers::chances::UpdateTable; use numbers::rac::RacRead; use numbers::symbol::UniformSymbolCoder; use numbers::FlifReadExt; +use Limits; #[derive(Eq, PartialEq, Debug, Clone, Copy)] pub enum BytesPerChannel { @@ -26,8 +27,44 @@ pub struct Header { pub num_frames: u32, } +/// Helper function for reading width, height and num_frames +fn read_varint(reader: &mut R, delta: u32) -> Result { + reader + .read_varint::() + .and_then(|v| { + v.checked_add(delta).ok_or_else(|| { + Error::LimitViolation( + "number of pixels exceeds limit: overflow".to_string() + ) + }) + }) +} + +/// Check if number of pixels uphelds provided limit +fn check_limit(width: usize, height: usize, frames: u32, limit: usize) -> Result<()> { + let frames = frames as usize; + let pixels = frames + .checked_mul(width) + .and_then(|val| val.checked_mul(height)); + match pixels { + Some(pix) if pix > limit => { + Err(Error::LimitViolation(format!( + "number of pixels exceeds limit: {}/{}", + pix, limit, + ))) + } + None => { + Err(Error::LimitViolation( + "number of pixels exceeds limit: overflow".to_string() + )) + } + Some(_) => Ok(()), + } +} + impl Header { - pub(crate) fn from_reader(mut reader: R) -> Result { + + pub(crate) fn from_reader(mut reader: R, limits: &Limits) -> Result { // first read in some magic let mut magic_buf = [0; 4]; reader.read_exact(&mut magic_buf)?; @@ -67,15 +104,17 @@ impl Header { desc: "bytes per channel was not a valid value", })?, }; - let width = 1 + reader.read_varint::()?; - let height = 1 + reader.read_varint::()?; + let width = read_varint(&mut reader, 1)? as usize; + let height = read_varint(&mut reader, 1)? as usize; let num_frames = if animated { - 2 + reader.read_varint::()? + read_varint(&mut reader, 2)? } else { 1 }; + check_limit(width, height, num_frames, limits.pixels)?; + Ok(Header { interlaced, channels, diff --git a/flif/src/decoder.rs b/flif/src/decoder.rs index 3a6eaa31..9f2c2ee3 100644 --- a/flif/src/decoder.rs +++ b/flif/src/decoder.rs @@ -89,26 +89,7 @@ impl Decoder { fn identify_internal(mut reader: R, limits: Limits) -> Result<(FlifInfo, Rac)> { // read the first header - let main_header = Header::from_reader(&mut reader)?; - let frames = main_header.num_frames as usize; - let pixels = frames - .checked_mul(main_header.width) - .and_then(|val| val.checked_mul(main_header.height)); - match pixels { - Some(pix) if pix > limits.pixels => { - Err(Error::LimitViolation(format!( - "number of pixels exceeds limit: {}/{}", - pix, limits.pixels, - )))?; - } - None => { - Err(Error::LimitViolation(format!( - "number of pixels exceeds limit: overflow/{}", - limits.pixels, - )))?; - } - Some(_) => (), - } + let main_header = Header::from_reader(&mut reader, &limits)?; // read the metadata chunks let (metadata, non_optional_byte) = Metadata::all_from_reader(&mut reader, &limits)?; From 6ba1718c3f75b3017737c52c9933d2b9ed331c18 Mon Sep 17 00:00:00 2001 From: newpavlov Date: Thu, 14 Jun 2018 21:55:06 +0300 Subject: [PATCH 7/8] usize -> u32 for limits and image sizes --- flif/src/components/header.rs | 11 ++++---- flif/src/components/metadata.rs | 6 ++--- flif/src/decoding_image.rs | 45 ++++++++++++++++----------------- flif/src/lib.rs | 14 +++++----- flif/src/maniac/mod.rs | 2 +- 5 files changed, 38 insertions(+), 40 deletions(-) diff --git a/flif/src/components/header.rs b/flif/src/components/header.rs index 03568b54..6b74af38 100644 --- a/flif/src/components/header.rs +++ b/flif/src/components/header.rs @@ -22,8 +22,8 @@ pub struct Header { pub interlaced: bool, pub channels: ColorSpace, pub bytes_per_channel: BytesPerChannel, - pub width: usize, - pub height: usize, + pub width: u32, + pub height: u32, pub num_frames: u32, } @@ -41,8 +41,7 @@ fn read_varint(reader: &mut R, delta: u32) -> Result { } /// Check if number of pixels uphelds provided limit -fn check_limit(width: usize, height: usize, frames: u32, limit: usize) -> Result<()> { - let frames = frames as usize; +fn check_limit(width: u32, height: u32, frames: u32, limit: u32) -> Result<()> { let pixels = frames .checked_mul(width) .and_then(|val| val.checked_mul(height)); @@ -104,8 +103,8 @@ impl Header { desc: "bytes per channel was not a valid value", })?, }; - let width = read_varint(&mut reader, 1)? as usize; - let height = read_varint(&mut reader, 1)? as usize; + let width = read_varint(&mut reader, 1)?; + let height = read_varint(&mut reader, 1)?; let num_frames = if animated { read_varint(&mut reader, 2)? diff --git a/flif/src/components/metadata.rs b/flif/src/components/metadata.rs index c0fd19d8..97ace1f7 100644 --- a/flif/src/components/metadata.rs +++ b/flif/src/components/metadata.rs @@ -29,13 +29,13 @@ impl Metadata { mut reader: R, limits: &Limits, ) -> Result<(Vec, u8)> { - let mut ret = Vec::with_capacity(limits.metadata_count); + let mut ret = Vec::with_capacity(limits.metadata_count as usize); let required_type = loop { match Self::from_reader(&mut reader, limits)? { MetadataType::Optional(metadata) => ret.push(metadata), MetadataType::Required(byte) => break byte, } - if ret.len() > limits.metadata_count { + if ret.len() > limits.metadata_count as usize { Err(Error::LimitViolation(format!( "number of metadata entries exceeds limit: {}", limits.metadata_count, @@ -70,7 +70,7 @@ impl Metadata { }; let chunk_size = reader.read_varint()?; - if chunk_size > limits.metadata_chunk { + if chunk_size > limits.metadata_chunk as usize { Err(Error::LimitViolation(format!( "requested metadata chunk size exceeds limit: {} vs {}", chunk_size, limits.metadata_chunk, diff --git a/flif/src/decoding_image.rs b/flif/src/decoding_image.rs index 72faac83..b9665208 100644 --- a/flif/src/decoding_image.rs +++ b/flif/src/decoding_image.rs @@ -10,8 +10,8 @@ use FlifInfo; pub use decoder::Decoder; pub(crate) struct DecodingImage { - height: usize, - width: usize, + height: u32, + width: u32, channels: ColorSpace, data: Vec, } @@ -49,35 +49,36 @@ type Maniac<'a> = ChannelSet>>; // safety criterias defined by `debug_assert`s impl DecodingImage { pub fn new(info: &FlifInfo) -> DecodingImage { + let pixels = (info.header.height * info.header.width) as usize; DecodingImage { height: info.header.height, width: info.header.width, channels: info.header.channels, - data: vec![Pixel::default(); info.header.height * info.header.width], + data: vec![Pixel::default(); pixels], } } - fn get_idx(&self, x: usize, y: usize) -> usize { - (self.width * y) + x + fn check_data(&self) -> bool { + self.data.len() == (self.width * self.height) as usize + } + + fn get_idx(&self, x: u32, y: u32) -> usize { + ((self.width * y) + x) as usize } pub fn get_data(&self) -> &[Pixel] { &self.data } - unsafe fn get_val(&self, x: usize, y: usize, chan: Channel) -> ColorValue { - debug_assert!( - x < self.width && y < self.height && self.data.len() == self.width * self.height - ); + unsafe fn get_val(&self, x: u32, y: u32, chan: Channel) -> ColorValue { + debug_assert!(x < self.width && y < self.height && self.check_data()); self.data.get_unchecked(self.get_idx(x, y))[chan] } - unsafe fn get_edge_vicinity(&self, x: usize, y: usize, chan: Channel) -> EdgePixelVicinity { - debug_assert!( - x < self.width && y < self.height && self.data.len() == self.width * self.height - ); + unsafe fn get_edge_vicinity(&self, x: u32, y: u32, chan: Channel) -> EdgePixelVicinity { + debug_assert!(x < self.width && y < self.height && self.check_data()); EdgePixelVicinity { - pixel: *self.data.get_unchecked((self.width * y) + x), + pixel: *self.data.get_unchecked(self.get_idx(x, y)), is_rgba: self.channels == ColorSpace::RGBA, chan, top: if y != 0 { @@ -113,16 +114,16 @@ impl DecodingImage { } } - unsafe fn get_core_vicinity(&self, x: usize, y: usize, chan: Channel) -> CorePixelVicinity { + unsafe fn get_core_vicinity(&self, x: u32, y: u32, chan: Channel) -> CorePixelVicinity { debug_assert!( x < self.width - 1 && y < self.height && x > 1 && y > 1 - && self.data.len() == self.width * self.height + && self.check_data() ); CorePixelVicinity { - pixel: *self.data.get_unchecked((self.width * y) + x), + pixel: *self.data.get_unchecked(self.get_idx(x, y)), chan, is_rgba: self.channels == ColorSpace::RGBA, top: self.get_val(x, y - 1, chan), @@ -136,8 +137,8 @@ impl DecodingImage { unsafe fn process_edge_pixel( &mut self, - x: usize, - y: usize, + x: u32, + y: u32, chan: Channel, maniac: &mut Maniac, rac: &mut Rac, @@ -146,9 +147,7 @@ impl DecodingImage { where E: FnMut(EdgePixelVicinity, &mut Maniac, &mut Rac) -> Result, { - debug_assert!( - x < self.width && y < self.height && self.data.len() == self.width * self.height - ); + debug_assert!(x < self.width && y < self.height && self.check_data()); let pix_vic = self.get_edge_vicinity(x, y, chan); let val = edge_f(pix_vic, maniac, rac)?; let idx = self.get_idx(x, y); @@ -172,7 +171,7 @@ impl DecodingImage { { let width = self.width; let height = self.height; - debug_assert!(self.data.len() == width * height); + debug_assert!(self.check_data()); // special case for small images if width <= 3 || height <= 2 { for y in 0..height { diff --git a/flif/src/lib.rs b/flif/src/lib.rs index 26c1b618..a6cee4de 100644 --- a/flif/src/lib.rs +++ b/flif/src/lib.rs @@ -64,7 +64,7 @@ impl Flif { }; let width = self.info.header.width; let height = self.info.header.height; - let mut data = Vec::with_capacity(n * width * height); + let mut data = Vec::with_capacity((n * width * height) as usize); for vals in self.image_data.get_data().iter() { for channel in self.info.header.channels { @@ -80,13 +80,13 @@ impl Flif { #[derive(Copy, Clone, Debug)] pub struct Limits { /// max size of the compressed metadata in bytes (default: 1 MB) - pub metadata_chunk: usize, + pub metadata_chunk: u32, /// max number of metadata entries (default: 8) - pub metadata_count: usize, - /// max number of pixels: `width * height * frames` (default: 226) - pub pixels: usize, - /// max number of MANIAC nodes (default: 214) - pub maniac_nodes: usize, + pub metadata_count: u32, + /// max number of pixels: `width * height * frames` (default: 67M = 226) + pub pixels: u32, + /// max number of MANIAC nodes (default: 16384 = 214) + pub maniac_nodes: u32, } impl Default for Limits { diff --git a/flif/src/maniac/mod.rs b/flif/src/maniac/mod.rs index 7325bc51..e89f1475 100644 --- a/flif/src/maniac/mod.rs +++ b/flif/src/maniac/mod.rs @@ -109,7 +109,7 @@ impl<'a> ManiacTree<'a> { _ => break, }; - if result_vec.len() > limits.maniac_nodes { + if result_vec.len() > limits.maniac_nodes as usize { Err(Error::LimitViolation(format!( "number of maniac nodes exceeds limit" )))?; From 91f3d4910e3ca626440b42dd884997603329ec7a Mon Sep 17 00:00:00 2001 From: newpavlov Date: Thu, 14 Jun 2018 22:02:21 +0300 Subject: [PATCH 8/8] fmt --- flif/src/components/header.rs | 31 +++++++++++-------------------- flif/src/decoding_image.rs | 8 +------- 2 files changed, 12 insertions(+), 27 deletions(-) diff --git a/flif/src/components/header.rs b/flif/src/components/header.rs index 6b74af38..4202990b 100644 --- a/flif/src/components/header.rs +++ b/flif/src/components/header.rs @@ -29,15 +29,11 @@ pub struct Header { /// Helper function for reading width, height and num_frames fn read_varint(reader: &mut R, delta: u32) -> Result { - reader - .read_varint::() - .and_then(|v| { - v.checked_add(delta).ok_or_else(|| { - Error::LimitViolation( - "number of pixels exceeds limit: overflow".to_string() - ) - }) + reader.read_varint::().and_then(|v| { + v.checked_add(delta).ok_or_else(|| { + Error::LimitViolation("number of pixels exceeds limit: overflow".to_string()) }) + }) } /// Check if number of pixels uphelds provided limit @@ -46,23 +42,18 @@ fn check_limit(width: u32, height: u32, frames: u32, limit: u32) -> Result<()> { .checked_mul(width) .and_then(|val| val.checked_mul(height)); match pixels { - Some(pix) if pix > limit => { - Err(Error::LimitViolation(format!( - "number of pixels exceeds limit: {}/{}", - pix, limit, - ))) - } - None => { - Err(Error::LimitViolation( - "number of pixels exceeds limit: overflow".to_string() - )) - } + Some(pix) if pix > limit => Err(Error::LimitViolation(format!( + "number of pixels exceeds limit: {}/{}", + pix, limit, + ))), + None => Err(Error::LimitViolation( + "number of pixels exceeds limit: overflow".to_string(), + )), Some(_) => Ok(()), } } impl Header { - pub(crate) fn from_reader(mut reader: R, limits: &Limits) -> Result { // first read in some magic let mut magic_buf = [0; 4]; diff --git a/flif/src/decoding_image.rs b/flif/src/decoding_image.rs index b9665208..69f8ef9e 100644 --- a/flif/src/decoding_image.rs +++ b/flif/src/decoding_image.rs @@ -115,13 +115,7 @@ impl DecodingImage { } unsafe fn get_core_vicinity(&self, x: u32, y: u32, chan: Channel) -> CorePixelVicinity { - debug_assert!( - x < self.width - 1 - && y < self.height - && x > 1 - && y > 1 - && self.check_data() - ); + debug_assert!(x < self.width - 1 && y < self.height && x > 1 && y > 1 && self.check_data()); CorePixelVicinity { pixel: *self.data.get_unchecked(self.get_idx(x, y)), chan,