Skip to content

[NativeAOT] Ensure that frozen objects respect the minimum object size - #76031

Merged
jkotas merged 3 commits into
dotnet:mainfrom
filipnavara:frozen-heap-objsize
Sep 23, 2022
Merged

[NativeAOT] Ensure that frozen objects respect the minimum object size#76031
jkotas merged 3 commits into
dotnet:mainfrom
filipnavara:frozen-heap-objsize

Conversation

@filipnavara

Copy link
Copy Markdown
Member

Ref: #75421 (comment)

When System.Memory.Tests was compiled with NativeAOT it produced incorrect data in the frozen heap section for some empty objects:

First object:

(lldb) image lookup -va 0x10b1c5058
Address: System.Memory.Tests[0x0000000100d7c058] (System.Memory.Tests.__DATA.__data + 268888)
Summary: __FrozenObj_xunit_assert_Xunit_Sdk_AssertEqualityComparer_1___c<S_P_CoreLib_System_ReadOnlyMemory_1<Int32>>1
Module: file = "/Users/teamcity/runtime/runtime/artifacts/bin/System.Memory.Tests/Release/net7.0/native/System.Memory.Tests", arch = "x86_64"
Symbol: id = {0x00061771}, range = [0x000000010b1c5058-0x000000010b1c5068), mangled="__FrozenObj_xunit_assert_Xunit_Sdk_AssertEqualityComparer_1___c<S_P_CoreLib_System_ReadOnlyMemory_1<Int32>>1"

Next object:

 Address: System.Memory.Tests[0x0000000100d7c068] (System.Memory.Tests.__DATA.__data + 268904)
Summary: __FrozenObj_S_P_CoreLib_System_Array_1_ArrayEnumerator<S_P_CoreLib_System_Nullable_1<Int32>>3
Module: file = "/Users/teamcity/runtime/runtime/artifacts/bin/System.Memory.Tests/Release/net7.0/native/System.Memory.Tests", arch = "x86_64"
Symbol: id = {0x00061772}, range = [0x000000010b1c5068-0x000000010b1c5088), mangled="__FrozenObj_S_P_CoreLib_System_Array_1_ArrayEnumerator<S_P_CoreLib_System_Nullable_1<Int32>>3"

The dumped vtable of the first object:

(lldb) x/8 0x10b3f3798
0x10b3f3798: 0x52000000 0x00000018 0x0b2b94f8 0x00000001
0x10b3f37a8: 0x00000003 0x63cb6bdc 0x0a6746e0 0x00000001

Second word is the size (0x00000018) but the difference between the address of the two objects in the data is only 0x10. This fixes the IL Compiler to properly align empty objects and prevents a GC crash as runtime.

@ghostghost added area-NativeAOT-coreclr community-contribution Indicates that the PR has been added by a community member labels Sep 22, 2022
@filipnavara

Copy link
Copy Markdown
MemberAuthor

It solves the problem but I am not sure about the preferred code organisation. I'd have preferred to use EETypeNode.BaseSize directly in ObjectInstance.WriteContent to emit the padding but there is an indirection through IEETypeNode so I couldn't just access the member. This patch feels a bit finicky.

Also, once the details are ironed out, is this candidate for release/7.0 fix?

Comment threadsrc/coreclr/tools/Common/TypeSystem/Common/TargetDetails.cs Outdated
Comment threadsrc/coreclr/tools/aot/ILCompiler.Compiler/Compiler/TypePreinit.cs Outdated

@jkotasjkotas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Thank you!

@jkotas

Copy link
Copy Markdown
Member

/azp run runtime-extra-platforms

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@jkotas

Copy link
Copy Markdown
Member

The failures are #75699

@jkotas
jkotas merged commit a8e4a4c into dotnet:mainSep 23, 2022
@jkotas

Copy link
Copy Markdown
Member

/backport to release/7.0

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/3109813547

@jkotas

Copy link
Copy Markdown
Member

@filipnavara Thank you!

@filipnavara

Copy link
Copy Markdown
MemberAuthor

Thanks for help with it!

@ghostghost locked as resolved and limited conversation to collaborators Oct 23, 2022
@filipnavara
filipnavara deleted the frozen-heap-objsize branch June 5, 2025 07:50
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-NativeAOT-coreclrcommunity-contributionIndicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@filipnavara@jkotas