From 8dac328ea724175d74aaf71d21e8d53cadef00be Mon Sep 17 00:00:00 2001 From: adam-rhebo Date: Wed, 12 Jun 2019 11:30:10 +0200 Subject: [PATCH] Remove hashbrown and use BTree{Map,Set} from the alloc crate (#187) * Remove hashbrown and use BTree{Map,Set} from the alloc crate wasmi-validation must handle untrusted input and hence we switch from Hash{Set,Map} (whether std's or hashbrown's) to BTree{Set,Map} to avoid algorithmic complexity attacks while retaining no_std support. Closes #183 * Improve memory locality of checking for duplicate exports Using a sorted slice gives us the same O(N log N) worst case execution time as using a BTreeMap, but using a single allocation as with HashMap, so that we should see better memory locality and hence better constant factors when checking for duplicate exports. --- Cargo.toml | 3 --- src/imports.rs | 9 +++------ src/lib.rs | 2 -- src/module.rs | 9 +++------ validation/Cargo.toml | 5 +---- validation/src/lib.rs | 29 ++++++++++++++++------------- 6 files changed, 23 insertions(+), 34 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 6c1d87c..e9e01e5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,7 +13,6 @@ exclude = [ "/res/*", "/tests/*", "/fuzz/*", "/benches/*" ] [dependencies] wasmi-validation = { version = "0.1", path = "validation", default-features = false } parity-wasm = { version = "0.31", default-features = false } -hashbrown = { version = "0.1.8", optional = true } memory_units = "0.3.0" libm = { version = "0.1.2", optional = true } @@ -30,10 +29,8 @@ std = [ "wasmi-validation/std", ] # Enable for no_std support -# hashbrown only works on no_std core = [ "wasmi-validation/core", - "hashbrown/nightly", "libm" ] diff --git a/src/imports.rs b/src/imports.rs index 5cd72eb..125b84e 100644 --- a/src/imports.rs +++ b/src/imports.rs @@ -1,10 +1,7 @@ #[allow(unused_imports)] use alloc::prelude::v1::*; -#[cfg(not(feature = "std"))] -use hashbrown::HashMap; -#[cfg(feature = "std")] -use std::collections::HashMap; +use alloc::collections::BTreeMap; use func::FuncRef; use global::GlobalRef; @@ -106,7 +103,7 @@ pub trait ImportResolver { /// [`ImportResolver`]: trait.ImportResolver.html /// [`ModuleImportResolver`]: trait.ModuleImportResolver.html pub struct ImportsBuilder<'a> { - modules: HashMap, + modules: BTreeMap, } impl<'a> Default for ImportsBuilder<'a> { @@ -119,7 +116,7 @@ impl<'a> ImportsBuilder<'a> { /// Create an empty `ImportsBuilder`. pub fn new() -> ImportsBuilder<'a> { ImportsBuilder { - modules: HashMap::new(), + modules: BTreeMap::new(), } } diff --git a/src/lib.rs b/src/lib.rs index c4bdc4a..ccbc90a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -114,8 +114,6 @@ extern crate assert_matches; #[cfg(test)] extern crate wabt; -#[cfg(not(feature = "std"))] -extern crate hashbrown; extern crate memory_units as memory_units_crate; extern crate parity_wasm; diff --git a/src/module.rs b/src/module.rs index 5c5d9a9..dbcbc99 100644 --- a/src/module.rs +++ b/src/module.rs @@ -5,10 +5,7 @@ use core::cell::RefCell; use core::fmt; use Trap; -#[cfg(not(feature = "std"))] -use hashbrown::HashMap; -#[cfg(feature = "std")] -use std::collections::HashMap; +use alloc::collections::BTreeMap; use core::cell::Ref; use func::{FuncBody, FuncInstance, FuncRef}; @@ -162,7 +159,7 @@ pub struct ModuleInstance { funcs: RefCell>, memories: RefCell>, globals: RefCell>, - exports: RefCell>, + exports: RefCell>, } impl ModuleInstance { @@ -173,7 +170,7 @@ impl ModuleInstance { tables: RefCell::new(Vec::new()), memories: RefCell::new(Vec::new()), globals: RefCell::new(Vec::new()), - exports: RefCell::new(HashMap::new()), + exports: RefCell::new(BTreeMap::new()), } } diff --git a/validation/Cargo.toml b/validation/Cargo.toml index e4595c2..55f0367 100644 --- a/validation/Cargo.toml +++ b/validation/Cargo.toml @@ -9,7 +9,6 @@ description = "Wasm code validator" [dependencies] parity-wasm = { version = "0.31", default-features = false } -hashbrown = { version = "0.1.8", optional = true } [dev-dependencies] assert_matches = "1.1" @@ -17,6 +16,4 @@ assert_matches = "1.1" [features] default = ["std"] std = ["parity-wasm/std"] -core = [ - "hashbrown/nightly" -] +core = [] diff --git a/validation/src/lib.rs b/validation/src/lib.rs index 8a562a6..4815b0a 100644 --- a/validation/src/lib.rs +++ b/validation/src/lib.rs @@ -27,15 +27,10 @@ use core::fmt; #[cfg(feature = "std")] use std::error; -#[cfg(not(feature = "std"))] -use hashbrown::HashSet; -#[cfg(feature = "std")] -use std::collections::HashSet; - use self::context::ModuleContextBuilder; use parity_wasm::elements::{ - BlockType, External, FuncBody, GlobalEntry, GlobalType, InitExpr, Instruction, Internal, - MemoryType, Module, ResizableLimits, TableType, Type, ValueType, + BlockType, ExportEntry, External, FuncBody, GlobalEntry, GlobalType, InitExpr, Instruction, + Internal, MemoryType, Module, ResizableLimits, TableType, Type, ValueType, }; pub mod context; @@ -250,13 +245,21 @@ pub fn validate_module(module: &Module) -> Result>(); + + export_names.sort_unstable(); + + for (fst, snd) in export_names.iter().zip(export_names.iter().skip(1)) { + if fst == snd { + return Err(Error(format!("duplicate export {}", fst))); } + } + + for export in export_section.entries() { match *export.internal() { Internal::Function(function_index) => { context.require_function(function_index)?;