Skip to content

gh-96143: Add some comments and minor fixes missed in the original PR #96433

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

Merged
merged 2 commits into from
Aug 30, 2022

Conversation

pablogsal
Copy link
Member

@pablogsal pablogsal commented Aug 30, 2022

Automerge-Triggered-By: GH:pablogsal

@pablogsal pablogsal changed the title gh-96132: Add some comments and minor fixes missed in the original PR gh-96143: Add some comments and minor fixes missed in the original PR Aug 30, 2022
Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>

size_t n_copies = mem_size / code_size;
for (size_t i = 0; i < n_copies; i++) {
memcpy(memory + i * code_size, start, code_size * sizeof(char));
}
// Some systems may prevent us from creating executable code on the fly.
// TODO: Call icache invalidation intrinsics if available:
// __builtin___clear_cache/__clear_cache (depending if clang/gcc). This is
Copy link
Contributor

Choose a reason for hiding this comment

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

The kernel would clear the icache for us. Not sure if this would ever be a problem.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's not correct in the general case. You are supposed to call this in aarch64 if you modify the code in some executable segment after it was read at least once. This doesn't apply for us but is not true that in all systems the kernel will clear it for us

Copy link
Member Author

Choose a reason for hiding this comment

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

See the discussion in the previous pr

@pablogsal pablogsal merged commit f49dd54 into python:main Aug 30, 2022
@pablogsal pablogsal deleted the gh-96143 branch August 30, 2022 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants