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

[NativeAOT/ARM] Bail out on IMAGE_REL_BASED_THUMB_BRANCH24 with >24-bit addends #97756

Merged
merged 2 commits into from
Feb 2, 2024

Conversation

filipnavara
Copy link
Member

... and let the linker generate thunks.

Fixes #97750

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Jan 31, 2024
@ghost
Copy link

ghost commented Jan 31, 2024

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

... and let the linker generate thunks.

Fixes #97750

Author: filipnavara
Assignees: -
Labels:

area-NativeAOT-coreclr

Milestone: -

@jkotas
Copy link
Member

jkotas commented Jan 31, 2024

let the linker generate thunks.

Is the linker guaranteed to find space to generate the thunks? How reliable is this fix?

@filipnavara
Copy link
Member Author

filipnavara commented Jan 31, 2024

Is the linker guaranteed to find space to generate the thunks? How reliable is this fix?

It is not guaranteed to find a space but it should be a hard fail if it does not. In practice I expect this should effectively double the size of code before the error is hit again (to 1 << 25 bytes).

We may investigate alternative mitigation strategies but this is the cheap way to increase the size limit.

@jkotas
Copy link
Member

jkotas commented Jan 31, 2024

Are the thunks placed outside the managed code section? If they are part the managed code section, there may be fixes needed in the code manager to ignore them.

@filipnavara
Copy link
Member Author

Are the thunks placed outside the managed code section?

That's a good question. They should not interleave with existing code and I assume they would have to be placed at the end of the same section as the code. That means they will not have reliable unwinding data, and that could be a problem (especially if it picks unwinding code from last method).

I'll put this back to draft for now and verify the assumptions later.

@filipnavara filipnavara marked this pull request as draft January 31, 2024 19:46
@filipnavara
Copy link
Member Author

Are the thunks placed outside the managed code section?

The linker creates a separate .text.thunks section, so it should not interfere. The hijack signal handler can catch the thread inside the thunk but it will bail out since it's not in a managed code at that point.

@filipnavara filipnavara marked this pull request as ready for review February 2, 2024 07:55
Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thank you for checking

@jkotas jkotas merged commit c31fb7f into dotnet:main Feb 2, 2024
106 of 110 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Mar 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-arm32 area-NativeAOT-coreclr community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[NativeAOT/ARM] Assert failure in PutThumb2BlRel24
2 participants