Skip to content

Enable exceptions on wrong LDELEM arguments #116235

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
Jun 3, 2025

Conversation

janvorli
Copy link
Member

@janvorli janvorli commented Jun 2, 2025

When the array is null, throw NullReferenceException When the index is out of range, throw IndexOutOfRangeException

Until now, it was asserting instead.

When the array is null, throw NullReferenceException
When the index is out of range, throw IndexOutOfRangeException

Until now, it was asserting instead.
@janvorli janvorli added this to the 10.0.0 milestone Jun 2, 2025
@janvorli janvorli self-assigned this Jun 2, 2025
@Copilot Copilot AI review requested due to automatic review settings June 2, 2025 22:58
@janvorli janvorli requested review from BrzVlad and kg as code owners June 2, 2025 22:58
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enables exceptions on wrong LDELEM arguments by replacing assertion failures with explicit exception throwing.

  • Replaces assert(0) with COMPlusThrow(kNullReferenceException) for null array references.
  • Replaces assert(0) with COMPlusThrow(kIndexOutOfRangeException) for out-of-range index values.

@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jun 2, 2025
@filipnavara
Copy link
Member

STELEM below has the same problem. Can you fix it in one go?

@janvorli
Copy link
Member Author

janvorli commented Jun 3, 2025

STELEM below has the same problem. Can you fix it in one go?

Thank you for spotting this! I was just fixing random small issues I've hit in the coreclr codegen bringup tests, so I have missed it. Let me change that too.

@janvorli
Copy link
Member Author

janvorli commented Jun 3, 2025

/ba-g timeout of build of an unrelated NativeAOT leg.

@janvorli janvorli merged commit 8ffd896 into dotnet:main Jun 3, 2025
94 of 96 checks passed
@janvorli janvorli deleted the enable-ldelem-exceptions branch June 3, 2025 13:59
@github-actions github-actions bot locked and limited conversation to collaborators Jul 4, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants