Merge side table into WASM module #730
Conversation
There was a problem hiding this comment.
Just did a quick review. I think we can also change next_branch_table to return Self::BranchTable<'a, 'm> instead of &'a mut Self::BranchTable<'m>. This will permit removing the branch_table_view field of SideTableView. Also I think we can split this PR:
- One for the change above.
- One for using
[u8]instead of[u16]. - One for serializing/deserializing the side-table.
- One for adding and using merge.
The first 2 can be done in parallel.
| @@ -26,6 +26,23 @@ use crate::toctou::*; | |||
| use crate::util::*; | |||
| use crate::*; | |||
|
|
|||
| pub fn merge(binary: &[u8], side_table: Vec<MetadataEntry>) -> Vec<u8> { | |||
There was a problem hiding this comment.
We can take a slice: &[MetadataEntry].
| let mut wasm = vec![]; | ||
| wasm.extend_from_slice(&binary[0 .. 8]); | ||
| wasm.push(0); | ||
| wasm.extend_from_slice(&side_table.len().to_le_bytes()); |
There was a problem hiding this comment.
This one needs to be leb128 since that's part of wasm spec.
| wasm.push(0); | ||
| wasm.extend_from_slice(&side_table.len().to_le_bytes()); | ||
| for entry in side_table { | ||
| wasm.extend_from_slice(&entry.type_idx.to_le_bytes()); |
There was a problem hiding this comment.
We need to convert the usize to u16 first.
| for entry in side_table { | ||
| wasm.extend_from_slice(&entry.type_idx.to_le_bytes()); | ||
| wasm.extend_from_slice(&entry.parser_range.start.to_le_bytes()); | ||
| wasm.extend_from_slice(&entry.parser_range.end.to_le_bytes()); |
There was a problem hiding this comment.
Same here, usize has not a fixed size, and we expect a u32.
| wasm.extend_from_slice(&binary[0 .. 8]); | ||
| wasm.push(0); | ||
| wasm.extend_from_slice(&side_table.len().to_le_bytes()); | ||
| for entry in side_table { |
There was a problem hiding this comment.
We also need to serialize the indices. Below is just the metadata.
Verifymode #726.I'll debug in the meanwhile.
#46