Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test memory boundaries for opcode execution #38

Closed
vlopes11 opened this issue Oct 25, 2021 · 2 comments · Fixed by #511
Closed

Test memory boundaries for opcode execution #38

vlopes11 opened this issue Oct 25, 2021 · 2 comments · Fixed by #511
Assignees
Labels
fuel-vm Related to the `fuel-vm` crate. question Further information is requested

Comments

@vlopes11
Copy link
Contributor

We are to use words as pairs of encoded instructions.

However, jump instructions are half-word. And we also might interrupt a program (e.g. debug) in a half-word

This means that incrementing by word might lead to invalid memory access in case the code is in the memory boundary.

We need to have a test case covering that - and, if applicable, fix the code so no panic occur.

@Voxelot Voxelot moved this to Todo in Fuel Network Jan 3, 2022
ControlCplusControlV pushed a commit that referenced this issue Nov 24, 2022
ControlCplusControlV pushed a commit that referenced this issue Dec 1, 2022
@mitchmindtree mitchmindtree added the fuel-vm Related to the `fuel-vm` crate. label Dec 9, 2022
xgreenx pushed a commit that referenced this issue Dec 20, 2022
xgreenx pushed a commit that referenced this issue Dec 20, 2022
xgreenx pushed a commit that referenced this issue Dec 20, 2022
xgreenx pushed a commit that referenced this issue Dec 20, 2022
@xgreenx xgreenx added the question Further information is requested label Jun 13, 2023
@xgreenx
Copy link
Collaborator

xgreenx commented Jun 13, 2023

@Dentosal How do you think is this issue actual for us with our current codebase?

@Dentosal
Copy link
Member

How do you think is this issue actual for us with our current codebase?

I'm fairly certain that this isn't an issue anymore. However, it would still be simple to refactor the instruction fetch code so that we could be certain. The current version is somewhat convoluted, as it's supposedly optimized by loading two opcodes at once. I presume that since we don't guarantee instruction alignment anyway, a simpler obviously-correct apporach would have the same performance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fuel-vm Related to the `fuel-vm` crate. question Further information is requested
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants