Repository navigation
Conversation
|
Frankly this is a little beyond my understanding and I'm going to try to gather some clearer evidence of how this manifests downstream. cc @pablogsal I assume you are the best reviewer for this though |
pablogsal
left a comment
There was a problem hiding this comment.
I do not think this is the right fix. The byte gives the end marker a separate address from the next function, but it only addresses that specific layout. BOLT can still move the start marker independently or instrument the trampoline itself. In the latter case, it can introduce PC-relative references that become invalid when CPython copies the code to a different address.
CPython uses these bytes as a template for copies in executable memory. The ELF template therefore belongs in .rodata instead of .text. BOLT then treats it as data instead of a native function, which protects both the bytes and the range that CPython copies.
I suggest we change the template section and remove the BOLT-specific test skips as part of the fix. That avoids the extra byte and a trampoline-specific exclusion from BOLT.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
Appreciate the feedback! Thanks! |
CPython copies the trampoline templates into executable memory. Keep the ELF source in .rodata so BOLT preserves its bytes and boundaries.
|
I pushed a fix :) |
|
Tested locally with: |
CPython copies the perf trampoline templates into executable memory at runtime. Store the ELF templates in
.rodataso BOLT treats them as data and preserves their bytes and boundaries.Fixes #128883.