Use [u8] instead of [u16] to not rely on alignment
#736
Conversation
crates/interpreter/src/side_table.rs
Outdated
| branch_table_view: Default::default(), | ||
| }) | ||
| } | ||
|
|
||
| pub fn metadata(&self, func_idx: usize) -> Metadata<'m> { | ||
| Metadata( | ||
| &self.metadata[self.indices[func_idx] as usize .. self.indices[func_idx + 1] as usize], | ||
| &self.metadata | ||
| [self.indices[func_idx * 2] as usize .. self.indices[(func_idx + 1) * 2] as usize], |
There was a problem hiding this comment.
This is wrong, we need to read 2 bytes. We could use a parse_u16(offset: usize) -> u16 helper which simply does u16::from_le_bytes(self.binary[offset..][..2].try_into().unwrap()) or something like that.
Also, we probably shouldn't panic if the side table is corrupted during verification. If some functions are used both in Check and Use mode, then we need to make them generic.
There was a problem hiding this comment.
It seems this function is only used in next_branch_table() for the Verify mode. Do you mean we should return Result<Metadata<'m>, Error> instead?
There was a problem hiding this comment.
It's because Module::side_table is currently wrong. It will be the SideTableView eventually, and then it will use this function to access the parser range and branch table.
Yes, I mean we should return MResult<Metadata<'m>, M>. And Module::new_unchecked() should do the initial parsing. This could be done in another PR though.
There was a problem hiding this comment.
Do you basically mean prepare() should return SideTableView?
There was a problem hiding this comment.
No, prepare() is not concerned. It's only Module::new_unchecked(). More generally, it's only relevant for parsing (views) and not formatting (prepare()).
zhouwfang
force-pushed
the
use-u8-array
branch
from
ab0ac3a to
aadb20f
Compare
| Ok(SideTableView { | ||
| func_idx: 0, | ||
| indices: parse_side_table_field(parser)?, | ||
| metadata: parse_side_table_field(parser)?, | ||
| indices: &binary[2 .. indices_end], |
There was a problem hiding this comment.
The TODO below also applies to this function. In particular, we should use .get(2 .. indices_end) to return an error instead of panicking.
| @@ -65,15 +59,23 @@ impl<'m> Metadata<'m> { | |||
| } | |||
|
|
|||
| pub fn branch_table(&self) -> &[BranchTableEntry] { | |||
| bytemuck::cast_slice(&self.0[5 ..]) | |||
| let entry_size = size_of::<BranchTableEntry>(); | |||
| assert_eq!( | |||
There was a problem hiding this comment.
Same here, we probably want to use MResult.
#46