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.

Sign up for GitHub

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

Jump to bottom

Handle call local seperately from regular jump instructions #467

Merged
merged 8 commits into from
May 18, 2024
Merged

Handle call local seperately from regular jump instructions #467

merged 8 commits into from
May 18, 2024

Conversation

Alan-Jowett
Copy link
Collaborator

@Alan-Jowett Alan-Jowett commented May 13, 2024 •
edited

Resolves: #458
Resolves: #450

Modify handling of local calls during x64 JIT.

  1. Change the PC target for jumps to point to instruction after the ```sub rsp,8"
  2. Change the relocation to point to the sub rsp,8 when relocating a local call.

This solves the problem where a jump to the start of a function misaligns the stack.

@Alan-Jowett Alan-Jowett requested a review from hawkinsw May 13, 2024 20:27
@Alan-Jowett Alan-Jowett enabled auto-merge (squash) May 13, 2024 20:27
@coveralls
Copy link

coveralls commented May 14, 2024 •
edited

Coverage Status

coverage: 81.54%. first build
when pulling 711cfc0 on Alan-Jowett:issue458
into ff4b48a on iovisor:main.

@Alan-Jowett Alan-Jowett force-pushed the issue458 branch 3 times, most recently from 783766e to 57ddd60 Compare May 14, 2024 22:17
Handle call local seperately from regular jump instructions
5a34daa 
Signed-off-by: Alan Jowett <alan.jowett@microsoft.com>
@Alan-Jowett
Merge branch 'main' into issue458
4cb9308 
Signed-off-by: Alan Jowett <alanjo@microsoft.com>
hawkinsw
hawkinsw reviewed May 17, 2024
View reviewed changes
Copy link
Collaborator

@hawkinsw hawkinsw left a comment

Choose a reason for hiding this comment

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

Looks good! I just had a few questions just to verify that I am understanding your technique!

vm/ubpf_jit_x86_64.c Show resolved Hide resolved
vm/ubpf_jit_arm64.c Outdated Show resolved Hide resolved
vm/ubpf_jit_x86_64.c Outdated Show resolved Hide resolved
@Alan-Jowett
Fix after local call prolog increased in size
e57df10 
Signed-off-by: Alan Jowett <alanjo@microsoft.com>
@Alan-Jowett Alan-Jowett marked this pull request as draft May 17, 2024 23:05
auto-merge was automatically disabled May 17, 2024 23:05

Pull request was converted to draft

@Alan-Jowett
Copy link
Collaborator Author

Converting to draft until I get this working again.

Alan Jowett and others added 3 commits May 17, 2024 16:16
Zero initialize jit_state
0b58048 
Signed-off-by: Alan Jowett <alan.jowett@microsoft.com>
@Alan-Jowett
Increment num_jumps after inserting into table
da4910b 
Signed-off-by: Alan Jowett <alanjo@microsoft.com>
@Alan-Jowett
Initialize bpf_function_prolog_size correctly
e070f79 
Signed-off-by: Alan Jowett <alanjo@microsoft.com>
@Alan-Jowett Alan-Jowett marked this pull request as ready for review May 18, 2024 00:26
hawkinsw
hawkinsw reviewed May 18, 2024
View reviewed changes
Copy link
Collaborator

@hawkinsw hawkinsw left a comment

Choose a reason for hiding this comment

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

Looks great!!

vm/ubpf_jit_arm64.c Show resolved Hide resolved
vm/ubpf_jit_arm64.c Show resolved Hide resolved
vm/ubpf_jit_arm64.c Show resolved Hide resolved
vm/ubpf_jit_x86_64.c Show resolved Hide resolved
@Alan-Jowett
Update ubpf_jit_arm64.c
b24ea1e 
Assert that prolog size matches.
@Alan-Jowett
Update ubpf_jit_x86_64.c
711cfc0 
Assert that the prolog size matches.
@Alan-Jowett Alan-Jowett merged commit 5a4f6c6 into iovisor:main May 18, 2024
34 checks passed
@Alan-Jowett Alan-Jowett deleted the issue458 branch May 18, 2024 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hawkinsw hawkinsw hawkinsw left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Jump target for PC 0 is wrong x64 JIT emit wrong jump target when target is at start of byte code
3 participants
@Alan-Jowett @coveralls @hawkinsw