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

src/interpreter.rs: Fix arithmetic left/right shift implementation (mask offset) #102

Merged
merged 1 commit into from
Mar 15, 2024

Conversation

pcy190
Copy link
Contributor

@pcy190 pcy190 commented Mar 13, 2024

Fix #101

We mask the mask offset for arithmetic shifts operaitons. The current version of the BPF Instruction Set Specification specifies that "Shift operations use a mask of 0x3F (63) for 64-bit operations and 0x1F (31) for 32-bit operations" 0.

@pcy190
Copy link
Contributor Author

pcy190 commented Mar 14, 2024

Seems that the casting should be rewritten.

Copy link
Owner

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need some tests coming with this patch, I wrote some here (feel free to add on top of your PR). Based on this tests, I've got one comment inline below.

I bumped the version in the Appveyor workflow, so it should pass if you rebase your PR.

src/interpreter.rs Outdated Show resolved Hide resolved
src/interpreter.rs Show resolved Hide resolved
@qmonnet
Copy link
Owner

qmonnet commented Mar 14, 2024

And thanks a lot!!

…ask offset)

We mask the mask offset for arithmetic shifts operaitons. The current
version of the BPF Instruction Set Specification specifies that
"Shift operations use a mask of 0x3F (63) for 64-bit operations and
0x1F (31) for 32-bit operations" [0].

[0]: https://www.ietf.org/archive/id/draft-thaler-bpf-isa-02.html#section-3.1-2.2.16.1.1

Signed-off-by: ret2happy <[email protected]>
@qmonnet qmonnet merged commit adea893 into qmonnet:main Mar 15, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistences in arithmetic shift implementation (mask offset)
2 participants