Store a full WasmFeatures inside of BinaryReader (#1548)

* Move `WasmFeatures` to a standalone module in `wasmparser`

This makes the type available even without the `validate` feature since
it doesn't have any intrinsic dependencies on validation.

* Extend `WasmFeatures` with methods for testing features

A bit nicer than `.contains(WasmFeatures::FEATURE)` and abstracts the
implementation for possible future iterations such as forcing features
to always be on.

* Store a full `WasmFeatures` inside of `BinaryReader`

Historically wasmparser has had a pretty clean split between validation
and parsing with the idea that the parser produces a bunch of stuff and
validation weeds it all out depending on activated features. Parsing has
historically not had access to the set of active features to have it
work the same no matter what. Over time, though, this has become quite
the burden.

The integration with the spec test repo in this repository tries to
make sure that any invalid wasm binary matches the same error message,
with a degree of flexibility. Not being able to take wasm features into
account when parsing makes this quite difficult. For example WebAssembly
binaries invalid before a feature is implemented might be valid after a
feature is implemented. They might also have an entirely different class
of error after a feature is implemented. Effectively WebAssembly does
not guarantee stability of the error message for invalid wasms as it
evolves, only that valid wasms all remain interpreted the same way.

Wasmparser has historically had a number of hacks around this. The
`roundtrip.rs` test suite has a very large function trying to match
error messages between wasmparser and the spec interpreter. Lots of
special cases happen here for wasmparser's feature-agnostic parser when
run against the spec test suite where each proposal has its own copy of
the spec interpreter, possibly with subtly different binary decoders.
Wasmparser additionally has hacks present in the public API such as
`BinaryReader::allow_memarg64` and the `table_byte` field of the
`CallIndirect` instruction. These have historically exclusively been
around to get the spec tests passing but otherwise serve no purpose.

Overall, there's been a lot of pain historically from not being able to
understand active wasm features when parsing wasm. It's quite easy to
write a code path along the lines of "if this feature is active parse
this way otherwise parse this way", but the features have never been
available to test this. Additionally `BinaryReader` instances were
created in quite a few locations throughout `wasmparser` which would
mean threading these around would be quite difficult. This commit
changes all of this.

This commit replaces the `allow_memarg64: bool` field of `BinaryReader`
with `features: WasmFeatures`. This means that all instances of
`BinaryReader` have access to all activated features and will be able to
parse differently when a feature is disabled or enabled. This should
make it much easier to pass the various stages of spec tests depending
on what happens to be merged into the spec interpreter.

To assist with threading this value around and to ensure it's accurate
the `new_with_offset` constructor of `BinaryReader` was removed and the
only remaining `new` constructor now requires that features must be
specified. Much of `wasmparser` has now additionally been refactored to
take a `BinaryReader` as a constructor rather than a slice-and-offset.
This centralizes the construction of `BinaryReader` and cuts down on
constant conversions in-and-out of a `BinaryReader`. For example much of
wasmparser now forwards along `BinaryReader` instances as-is. This is a
large-ish API change, too, for anyone constructing these manually
(shouldn't affect those exclusively using the validator).

The intention is that it's generally ok to pass `WasmFeatures::all()` to
the construction of a `BinaryReader`. For example the
`wasmparser::Parser` type defaults to having all features active. This
means that errors will be different (or not present) for newer wasm
features but they'll all still get filtered out by the validator. The
features in the `BinaryReader` are not intended to be a strict gating
procedure such that it's an exact wasm MVP parser, for example, instead
only being available as necessary for spec tests and other relevant edge
cases.

* Fix a rebase conflict

* Add missing calls to `shrink` for linking/reloc parsers

* Fix fuzzer compilation

Author: alexcrichton

crates/wasm-encoder/src/core/code.rs

@@ -83,11 +83,13 @@ impl CodeSection {
     /// into a new code section encoder:
     ///
     /// ```
+    /// # use wasmparser::{BinaryReader, WasmFeatures, CodeSectionReader};
     /// //                  id, size, # entries, entry
     /// let code_section = [10, 6,    1,         4, 0, 65, 0, 11];
     ///
     /// // Parse the code section.
-    /// let reader = wasmparser::CodeSectionReader::new(&code_section, 0).unwrap();
+    /// let reader = BinaryReader::new(&code_section, 0, WasmFeatures::all());
+    /// let reader = CodeSectionReader::new(reader).unwrap();
     /// let body = reader.into_iter().next().unwrap().unwrap();
     /// let body_range = body.range();
     ///

crates/wasm-metadata/src/lib.rs

@@ -9,8 +9,8 @@ use std::mem;
 use std::ops::Range;
 use wasm_encoder::{ComponentSection as _, ComponentSectionId, Encode, Section};
 use wasmparser::{
-    ComponentNameSectionReader, KnownCustom, NameSectionReader, Parser, Payload::*,
-    ProducersSectionReader,
+    BinaryReader, ComponentNameSectionReader, KnownCustom, NameSectionReader, Parser, Payload::*,
+    ProducersSectionReader, WasmFeatures,
 };
 
 /// A representation of a WebAssembly producers section.
@@ -64,7 +64,8 @@ impl Producers {
     }
     /// Read the producers section from a Wasm binary.
     pub fn from_bytes(bytes: &[u8], offset: usize) -> Result<Self> {
-        let section = ProducersSectionReader::new(bytes, offset)?;
+        let reader = BinaryReader::new(bytes, offset, WasmFeatures::all());
+        let section = ProducersSectionReader::new(reader)?;
         let mut fields = IndexMap::new();
         for field in section.into_iter() {
             let field = field?;
@@ -603,7 +604,8 @@ impl<'a> ModuleNames<'a> {
     /// Read a name section from a WebAssembly binary. Records the module name, and all other
     /// contents of name section, for later serialization.
     pub fn from_bytes(bytes: &'a [u8], offset: usize) -> Result<ModuleNames<'a>> {
-        let section = NameSectionReader::new(bytes, offset);
+        let reader = BinaryReader::new(bytes, offset, WasmFeatures::all());
+        let section = NameSectionReader::new(reader);
         let mut s = Self::empty();
         for name in section.into_iter() {
             let name = name?;
@@ -688,7 +690,8 @@ impl<'a> ComponentNames<'a> {
     /// Read a component-name section from a WebAssembly binary. Records the component name, as
     /// well as all other component name fields for later serialization.
     pub fn from_bytes(bytes: &'a [u8], offset: usize) -> Result<ComponentNames<'a>> {
-        let section = ComponentNameSectionReader::new(bytes, offset);
+        let reader = BinaryReader::new(bytes, offset, WasmFeatures::all());
+        let section = ComponentNameSectionReader::new(reader);
         let mut s = Self::empty();
         for name in section.into_iter() {
             let name = name?;

crates/wasm-mutate/src/info.rs

@@ -5,7 +5,7 @@ use crate::{
 use std::collections::HashSet;
 use std::ops::Range;
 use wasm_encoder::{RawSection, SectionId};
-use wasmparser::{Chunk, Parser, Payload};
+use wasmparser::{BinaryReader, Chunk, Parser, Payload, WasmFeatures};
 
 /// Provides module information for future usage during mutation
 /// an instance of ModuleInfo could be user to determine which mutation could be applied
@@ -220,7 +220,8 @@ impl<'a> ModuleInfo<'a> {
     pub fn has_nonempty_code(&self) -> bool {
         if let Some(section) = self.code {
             let section_data = self.raw_sections[section].data;
-            wasmparser::CodeSectionReader::new(section_data, 0)
+            let reader = BinaryReader::new(section_data, 0, WasmFeatures::all());
+            wasmparser::CodeSectionReader::new(reader)
                 .map(|r| r.count() != 0)
                 .unwrap_or(false)
         } else {
@@ -247,21 +248,12 @@ impl<'a> ModuleInfo<'a> {
         });
     }
 
-    pub fn get_type_section(&self) -> Option<RawSection<'a>> {
-        let idx = self.types?;
-        Some(self.raw_sections[idx])
-    }
-
     pub fn get_code_section(&self) -> RawSection<'a> {
         self.raw_sections[self.code.unwrap()]
     }
 
-    pub fn get_exports_section(&self) -> RawSection<'a> {
-        self.raw_sections[self.exports.unwrap()]
-    }
-
-    pub fn get_data_section(&self) -> RawSection<'a> {
-        self.raw_sections[self.data.unwrap()]
+    pub fn get_binary_reader(&self, i: usize) -> wasmparser::BinaryReader<'a> {
+        BinaryReader::new(self.raw_sections[i].data, 0, WasmFeatures::all())
     }
 
     pub fn has_exports(&self) -> bool {
@@ -281,11 +273,6 @@ impl<'a> ModuleInfo<'a> {
         self.global_types.len()
     }
 
-    /// Returns the global section bytes as a `RawSection` instance
-    pub fn get_global_section(&self) -> RawSection {
-        self.raw_sections[self.globals.unwrap()]
-    }
-
     /// Insert a new section as the `i`th section in the Wasm module.
     pub fn insert_section(
         &self,

crates/wasm-mutate/src/mutators/add_function.rs

@@ -21,8 +21,8 @@ impl Mutator for AddFunctionMutator {
         // (Re)encode the function section and add this new entry.
         let mut func_sec_enc = wasm_encoder::FunctionSection::new();
         if let Some(func_sec_idx) = config.info().functions {
-            let raw_func_sec = config.info().raw_sections[func_sec_idx];
-            let reader = wasmparser::FunctionSectionReader::new(raw_func_sec.data, 0)?;
+            let reader = config.info().get_binary_reader(func_sec_idx);
+            let reader = wasmparser::FunctionSectionReader::new(reader)?;
             for x in reader {
                 func_sec_enc.function(x?);
             }
@@ -33,12 +33,11 @@ impl Mutator for AddFunctionMutator {
         // this function.
         let mut code_sec_enc = wasm_encoder::CodeSection::new();
         if let Some(code_sec_idx) = config.info().code {
-            let raw_code_sec = config.info().raw_sections[code_sec_idx];
-            let reader = wasmparser::CodeSectionReader::new(raw_code_sec.data, 0)?;
+            let reader = config.info().get_binary_reader(code_sec_idx);
+            let reader = wasmparser::CodeSectionReader::new(reader)?;
             for body in reader {
                 let body = body?;
-                let range = body.range();
-                code_sec_enc.raw(&raw_code_sec.data[range.start..range.end]);
+                code_sec_enc.raw(body.as_bytes());
             }
         }
         let func_ty = match &config.info().types_map[usize::try_from(ty_idx).unwrap()] {

crates/wasm-mutate/src/mutators/add_type.rs

@@ -52,9 +52,10 @@ impl Mutator for AddTypeMutator {
         }
 
         let mut types = wasm_encoder::TypeSection::new();
-        if let Some(old_types) = config.info().get_type_section() {
+        if let Some(old_types) = config.info().types {
             // Copy the existing types section over into the encoder.
-            let reader = wasmparser::TypeSectionReader::new(old_types.data, 0)?;
+            let reader = config.info().get_binary_reader(old_types);
+            let reader = wasmparser::TypeSectionReader::new(reader)?;
             for ty in reader.into_iter_err_on_gc_types() {
                 let ty = ty?;
                 let params = ty

crates/wasm-mutate/src/mutators/codemotion.rs

@@ -62,9 +62,9 @@ impl CodemotionMutator {
         config: &mut WasmMutate,
         mutators: &[Box<dyn AstMutator>],
     ) -> crate::Result<(Function, u32)> {
-        let original_code_section = config.info().get_code_section();
-
-        let sectionreader = CodeSectionReader::new(original_code_section.data, 0)?;
+        let original_code_section = config.info().code.unwrap();
+        let reader = config.info().get_binary_reader(original_code_section);
+        let sectionreader = CodeSectionReader::new(reader)?;
         let function_count = sectionreader.count();
         let function_to_mutate = config.rng().gen_range(0..function_count);
 
@@ -75,8 +75,7 @@ impl CodemotionMutator {
         for fidx in (function_to_mutate..function_count).chain(0..function_to_mutate) {
             config.consume_fuel(1)?;
             let reader = all_readers[fidx as usize].clone();
-            let mut operatorreader = reader.get_operators_reader()?;
-            operatorreader.allow_memarg64(true);
+            let operatorreader = reader.get_operators_reader()?;
 
             let operators = operatorreader
                 .into_iter_with_offsets()
@@ -101,7 +100,7 @@ impl CodemotionMutator {
                         &ast,
                         &self.copy_locals(reader)?,
                         &operators,
-                        original_code_section.data,
+                        config.info().raw_sections[original_code_section].data,
                     )?;
                     return Ok((newfunc, fidx));
                 }
@@ -143,16 +142,16 @@ impl Mutator for CodemotionMutator {
         let (newfunc, function_to_mutate) = self.random_mutate(config, &mutators)?;
 
         let mut codes = CodeSection::new();
-        let code_section = config.info().get_code_section();
-        let sectionreader = CodeSectionReader::new(code_section.data, 0)?;
+        let code_section = config.info().get_binary_reader(config.info().code.unwrap());
+        let sectionreader = CodeSectionReader::new(code_section)?;
 
         for (fidx, reader) in sectionreader.into_iter().enumerate() {
             let reader = reader?;
             if fidx as u32 == function_to_mutate {
                 log::trace!("Mutating function {}", fidx);
                 codes.function(&newfunc);
             } else {
-                codes.raw(&code_section.data[reader.range().start..reader.range().end]);
+                codes.raw(reader.as_bytes());
             }
         }
         let module = config

crates/wasm-mutate/src/mutators/custom.rs

@@ -28,9 +28,8 @@ impl Mutator for CustomSectionMutator {
         assert!(!custom_section_indices.is_empty());
 
         let custom_section_index = *custom_section_indices.choose(config.rng()).unwrap();
-        let old_custom_section = &config.info().raw_sections[custom_section_index];
-        let old_custom_section =
-            wasmparser::CustomSectionReader::new(old_custom_section.data, 0).unwrap();
+        let reader = config.info().get_binary_reader(custom_section_index);
+        let old_custom_section = wasmparser::CustomSectionReader::new(reader).unwrap();
 
         let name_string;
         let data_vec;

crates/wasm-mutate/src/mutators/function_body_unreachable.rs

@@ -19,8 +19,9 @@ impl Mutator for FunctionBodyUnreachable {
     ) -> Result<Box<dyn Iterator<Item = Result<Module>> + 'a>> {
         let mut codes = CodeSection::new();
 
-        let code_section = config.info().get_code_section();
-        let reader = CodeSectionReader::new(code_section.data, 0)?;
+        let code_section = config.info().code.unwrap();
+        let reader = config.info().get_binary_reader(code_section);
+        let reader = CodeSectionReader::new(reader)?;
 
         let count = reader.count();
         let function_to_mutate = config.rng().gen_range(0..count);
@@ -38,7 +39,7 @@ impl Mutator for FunctionBodyUnreachable {
 
                 codes.function(&f);
             } else {
-                codes.raw(&code_section.data[f.range().start..f.range().end]);
+                codes.raw(f.as_bytes());
             }
         }
 

crates/wasm-mutate/src/mutators/modify_const_exprs.rs

@@ -155,7 +155,8 @@ impl Mutator for ConstExpressionMutator {
                 let mutate_idx = config.rng().gen_range(0..num_total);
                 let section = config.info().globals.ok_or(skip_err)?;
                 let mut new_section = GlobalSection::new();
-                let reader = GlobalSectionReader::new(config.info().raw_sections[section].data, 0)?;
+                let reader = config.info().get_binary_reader(section);
+                let reader = GlobalSectionReader::new(reader)?;
                 let mut translator = InitTranslator {
                     config,
                     skip_inits: 0,
@@ -179,8 +180,8 @@ impl Mutator for ConstExpressionMutator {
                 let mutate_idx = config.rng().gen_range(0..num_total);
                 let section = config.info().elements.ok_or(skip_err)?;
                 let mut new_section = ElementSection::new();
-                let reader =
-                    ElementSectionReader::new(config.info().raw_sections[section].data, 0)?;
+                let reader = config.info().get_binary_reader(section);
+                let reader = ElementSectionReader::new(reader)?;
                 let mut translator = InitTranslator {
                     config,
                     skip_inits: 0,

crates/wasm-mutate/src/mutators/modify_data.rs

@@ -18,7 +18,9 @@ impl Mutator for ModifyDataMutator {
         config: &'a mut WasmMutate,
     ) -> Result<Box<dyn Iterator<Item = Result<Module>> + 'a>> {
         let mut new_section = DataSection::new();
-        let reader = DataSectionReader::new(config.info().get_data_section().data, 0)?;
+        let section_idx = config.info().data.unwrap();
+        let reader = config.info().get_binary_reader(section_idx);
+        let reader = DataSectionReader::new(reader)?;
 
         // Select an arbitrary data segment to modify.
         let data_to_modify = config.rng().gen_range(0..reader.count());