Skip to content

Bring throw helpers to PUSH_COOP_PINVOKE_FRAME plan #123015

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
Merged
janvorli merged 45 commits into from
Jan 16, 2026
Merged

Bring throw helpers to PUSH_COOP_PINVOKE_FRAME plan #123015

janvorli merged 45 commits into from
Jan 16, 2026

Conversation

@am11
Copy link
Member

@am11 am11 commented Jan 8, 2026

Fixes #116375.

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jan 8, 2026
@dotnet-policy-service
Copy link
Contributor

dotnet-policy-service bot commented Jan 8, 2026

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

@am11 am11 force-pushed the feature/il_throw_PUSH_COOP_PINVOKE_FRAME branch from 1a243a6 to 00c4f6a Compare January 8, 2026 22:39
@am11 am11 force-pushed the feature/il_throw_PUSH_COOP_PINVOKE_FRAME branch from b8782d2 to 0d677dd Compare January 8, 2026 22:41
@am11 am11 force-pushed the feature/il_throw_PUSH_COOP_PINVOKE_FRAME branch from 0d677dd to 5359b4c Compare January 8, 2026 22:42
@build-analysis build-analysis bot mentioned this pull request Jan 9, 2026
Open
This was referenced Jan 9, 2026
Open
Open
Open
jkotas
jkotas reviewed Jan 9, 2026
View reviewed changes
jkotas
jkotas reviewed Jan 10, 2026
View reviewed changes
@build-analysis build-analysis bot mentioned this pull request Jan 10, 2026
Open
jkotas
jkotas reviewed Jan 11, 2026
View reviewed changes
jkotas
jkotas reviewed Jan 15, 2026
View reviewed changes
@am11
Copy link
Member Author

am11 commented Jan 15, 2026

@janvorli, FYI, I have brought back ldmxcsr to see if all tests pass with it included. It was failing Exceptions/ForeignThread/ForeignThreadExceptions/ForeignThreadExceptions.cmd tests without it https://helix.dot.net/api/2019-06-17/jobs/91c6751b-23aa-4f7e-b012-635d2ea381fe/workitems/Exceptions/console.

@janvorli
Copy link
Member

janvorli commented Jan 15, 2026

It was failing Exceptions/ForeignThread/ForeignThreadExceptions/ForeignThreadExceptions.cmd tests without it https://helix.dot.net/api/2019-06-17/jobs/91c6751b-23aa-4f7e-b012-635d2ea381fe/workitems/Exceptions/console.

That is due to a different problem - wrong unwind info of the IL_Throw. If you do what @jkotas said, the issue goes away.

@am11
Copy link
Member Author

am11 commented Jan 15, 2026

Thanks.

If you do what @jkotas said, the issue goes away.

Already pushed in 7cb82b1. I will try undoing the other change locally and push once this PR runs complete.

@am11
Copy link
Member Author

am11 commented Jan 15, 2026

@janvorli, win-x64 pri0 tests are now passing. But if I drop ldmxcsr change from 7cb82b1, ForeignThreadExceptions failed. Could you please show me how you addressed @jkotas' feedback; and was it different than how I did?

@am11 am11 marked this pull request as ready for review January 15, 2026 20:08
@am11
Copy link
Member Author

am11 commented Jan 15, 2026

PTAL. One less PAL_VirtualUnwind call (and six more to go before we can let go of libunwind dependency 😁).

@janvorli
Copy link
Member

janvorli commented Jan 15, 2026

Could you please show me how you addressed @jkotas' feedback; and was it different than how I did?

I did it the same way as you, there is no difference in that part. But I've also removed the ldmxcsr from the Context.asm which you still seem to have.

jkotas
jkotas reviewed Jan 15, 2026
View reviewed changes
janvorli
janvorli approved these changes Jan 16, 2026
View reviewed changes
Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

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

LGTM, just few nits. Thank you!

@janvorli
Copy link
Member

janvorli commented Jan 16, 2026

/azp run runtime-coreclr outerloop

@azure-pipelines
Copy link

azure-pipelines bot commented Jan 16, 2026

Azure Pipelines successfully started running 1 pipeline(s).

@am11
Copy link
Member Author

am11 commented Jan 16, 2026

osx-arm64 failure is an existing one; failed in a previous run as well https://dev.azure.com/dnceng-public/public/_build/results?buildId=1255235&view=logs&j=86a95901-e0d9-5eec-3cb6-04a12f80e719&t=27788b50-9372-56cb-e5f4-7bfe19754c51.

#123195

@janvorli
Copy link
Member

janvorli commented Jan 16, 2026

/ba-g the PAL test failure is #123195

@janvorli janvorli merged commit 898e351 into dotnet:main Jan 16, 2026
150 of 152 checks passed
@am11 am11 deleted the feature/il_throw_PUSH_COOP_PINVOKE_FRAME branch January 16, 2026 13:44
@dotnet-maestro dotnet-maestro bot mentioned this pull request Jan 17, 2026
Merged
rosebyte pushed a commit that referenced this pull request Jan 19, 2026
@am11 @janvorli
Bring throw helpers to PUSH_COOP_PINVOKE_FRAME plan (#123015)
244edbe 
Fixes #116375.

---------

Co-authored-by: Jan Vorlicek <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jkotas jkotas jkotas left review comments

@janvorli janvorli janvorli approved these changes

@filipnavara filipnavara Awaiting requested review from filipnavara

@davidwrighton davidwrighton Awaiting requested review from davidwrighton

Assignees

No one assigned

Labels

area-VM-coreclr community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bring IL_{Throw,Rethrow,ThrowExact} to PUSH_COOP_PINVOKE_FRAME plan

3 participants

@am11 @janvorli @jkotas