Fix instruction length calculation. #311
Conversation
crates/polkavm/src/compiler.rs
Outdated
| let offset = self.program_counter_to_machine_code_offset_list.last().unwrap().1 as usize; | ||
| let instruction_length = self.asm.len() - offset; | ||
| let instruction_length = self.asm.len() - self.asm_len_before; |
There was a problem hiding this comment.
Doing this isn't necessarily correct.
In general the point of this check is as follows: we want to estimate how many bytes of native assembly each byte of PVM bytecode can emit at maximum, and then allocate the appropriate amount of address space so that it will all fit even in the worst case. So we need to include the size of the gas metering stub in these calculations somehow. If we apply this change then the size of the gas metering stub will effectively be ignored.
However, I think it should be fine if we move this whole if to the end of this function so that it will include the size of the gas metering stub.
There was a problem hiding this comment.
Thanks. I reverted my changes and instead increased max. instruction length by 10 (the size of the stub). I hope this is correct. The former if already included gas metering stub length.
|
cc @aman4150 The fuzzer should have caught this. |
Previously the length of the gas metering stub was incorrectly added to the length of the instruction that comes after. For long instructions such as 66-byte
divthis resulted in exceeding max. instruction length and assertion failure. This PR fixes the issue.