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

Pass indirect call target in first register #436

Merged
merged 4 commits into from
Nov 12, 2024

Conversation

rinon
Copy link
Collaborator

@rinon rinon commented Oct 8, 2024

Rather than pass the indirect call target in a global static, this change passes the target in the first parameter register to the wrapper. The wrapper then shifts the remaining parameters so that the callee receives the correct parameters.

@rinon rinon requested a review from ayrtonm October 8, 2024 23:57
@ayrtonm
Copy link
Contributor

ayrtonm commented Oct 9, 2024

Rather than pass the indirect call target in a global static, this change passes the target in the first parameter register to the wrapper

I agree this is a better approach than a thread-local in memory.

@rinon rinon linked an issue Oct 9, 2024 that may be closed by this pull request
@rinon rinon force-pushed the am/scrub_registers branch from d9fdc1a to 6372a16 Compare October 11, 2024 21:11
rinon added 4 commits October 11, 2024 14:15
Rather than pass the indirect call target in a global static, this change passes the target in the first parameter register to the wrapper. The wrapper then shifts the remaining parameters so that the callee receives the correct parameters.
@rinon rinon force-pushed the sjc/secure_indirect_calls branch from cbba2f1 to 07e8c3d Compare October 11, 2024 21:15
Copy link
Contributor

@ayrtonm ayrtonm left a comment

Choose a reason for hiding this comment

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

LGTM

@ayrtonm
Copy link
Contributor

ayrtonm commented Oct 28, 2024

I think at some point we'll want to generate scrub registers in GenCallAsm.cpp to avoid the redundant push/pops and avoid spilling the function pointer to the stack in indirect calls, but this is already better than what we currently do since it's only on a protected stack.

@fw-immunant fw-immunant self-requested a review November 12, 2024 19:43
Copy link
Contributor

@fw-immunant fw-immunant 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 wish our handling of stack alignment were less ad-hoc.

@fw-immunant fw-immunant merged commit 79ff3b6 into am/scrub_registers Nov 12, 2024
34 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.

ia2_fn_ptr shouldn't be shared between threads or compartments
3 participants