Skip to content

Conversation

jkotas
Copy link
Member

@jkotas jkotas commented Sep 14, 2025

No description provided.

@Copilot Copilot AI review requested due to automatic review settings September 14, 2025 03:19
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 changes the type of the numElements parameter in RhpGcAlloc from uintptr_t to intptr_t for consistency across the codebase. This aligns the parameter type with how the value is actually used, particularly in array allocation scenarios where negative values are checked for overflow conditions.

Key changes:

  • Update function signature in implementation files
  • Update corresponding comments in assembly files

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.

File Description
src/coreclr/vm/gchelpers.cpp Updates RhpGcAllocMaybeFrozen function signature parameter type
src/coreclr/nativeaot/Runtime/portable.cpp Updates RhpGcAlloc declaration and AllocateObject function signature
src/coreclr/nativeaot/Runtime/GCHelpers.cpp Updates RhpGcAlloc function signature in implementation
Multiple assembly files Updates function signature comments in various architecture-specific allocation helpers

@jkotas
Copy link
Member Author

jkotas commented Sep 14, 2025

Implementations check for less than zero in number of places: https://github.com/dotnet/runtime/pull/119691/files#diff-547cf137c0fe434ed40dcf035ff12ee3fe81180847cb5851ec67667c0d0e8701R143-R144 . These places checked are unreachable with uintptr_t argument type. I do not think that this is fixing any real bug, but it is changing the exact exception message in some cases.

@jkotas
Copy link
Member Author

jkotas commented Sep 14, 2025

cc @filipnavara

@jkotas jkotas requested a review from radekdoulik September 14, 2025 14:42
@jkotas
Copy link
Member Author

jkotas commented Sep 14, 2025

@radekdoulik I have noticed this while reviewing your PR

@radekdoulik radekdoulik merged commit ec4a8b0 into dotnet:main Sep 15, 2025
99 checks passed
@jkotas jkotas deleted the gcalloc-intptr branch September 15, 2025 06:25
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