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

[release/8.0-staging] Always zero-init if object contains pointers #100426

Merged
merged 1 commit into from
Apr 15, 2024

Conversation

VSadov
Copy link
Member

@VSadov VSadov commented Mar 28, 2024

Backport of #100265 to release/8.0-staging

Customer Impact

  • Customer reported
  • Found internally

This was seen to cause failures due to heap corruption in local run-to-failure runs.

Using GC.AllocateUninitializedArray() API with reference-containing element types on NativeAOT would introduce silent heap corruptions and eventually cause a crash at the next full GC.

Such failures would be extremely difficult to diagnose if happen in a real application.
 

Regression

  • Yes
  • No

Allowing reference types for pinned GC.AllocateUninitializedArray() was a new addition to the public API.
The intended semantics of AllocateUninitializedArray with reference containing element types is to ignore the "Unintialized" part as GC heap does not expect uninitialized object references.

Historically, the check for this combination (reference-containing+uninitialized) was present in CoreCLR, but was not ported to NativeAOT as a part of the above change.

Testing

Regular tests + a test scenario that is sensitive to this scenario was added.

Risk

Low: This is matching the long existing behavior on CoreCLR.

Copy link
Contributor

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

Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

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

approved. we will take for consideration in 8.0.x

@rbhanda rbhanda added this to the 8.0.5 milestone Apr 2, 2024
@rbhanda rbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Apr 2, 2024
@VSadov
Copy link
Member Author

VSadov commented Apr 9, 2024

Waiting for #100622 to merge, so that all tests are green.

@ericstj
Copy link
Member

ericstj commented Apr 15, 2024

@jeffschwMSFT @VSadov - please assess failures and merge today.

@VSadov
Copy link
Member Author

VSadov commented Apr 15, 2024

This is a NativeAOT specific fix for GC.AllocateUninitializedArray()
The failure in System.Net.Security.Tests.CertificateValidationRemoteServer.ConnectWithRevocation_WithCallback on CoreCLR does not look related.

@jkotas
Copy link
Member

jkotas commented Apr 15, 2024

LGTM

@jkotas jkotas merged commit 88be910 into dotnet:release/8.0-staging Apr 15, 2024
147 of 149 checks passed
@VSadov VSadov deleted the port100265 branch April 15, 2024 21:41
@github-actions github-actions bot locked and limited conversation to collaborators May 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-NativeAOT-coreclr Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants