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

Place non-GC statics in the dehydratable section #78795

Merged
merged 1 commit into from
Nov 30, 2022

Conversation

MichalStrehovsky
Copy link
Member

There can be a pointer reloc that points to the class constructor. We need this to stay as a full pointer since non-Gc statics can be allocated at runtime and we don't have guarantees they would be within +-2 GB of the cctor.

Cc @dotnet/ilc-contrib

@ghost
Copy link

ghost commented Nov 24, 2022

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

Issue Details

There can be a pointer reloc that points to the class constructor. We need this to stay as a full pointer since non-Gc statics can be allocated at runtime and we don't have guarantees they would be within +-2 GB of the cctor.

Cc @dotnet/ilc-contrib

Author: MichalStrehovsky
Assignees: -
Labels:

area-NativeAOT-coreclr

Milestone: -

@@ -47,7 +47,7 @@ public bool IsStandardSection
public static readonly ObjectNodeSection FoldableReadOnlyDataSection = new ObjectNodeSection("rdata", SectionType.ReadOnly);
public static readonly ObjectNodeSection TextSection = new ObjectNodeSection("text", SectionType.Executable);
public static readonly ObjectNodeSection TLSSection = new ObjectNodeSection("TLS", SectionType.Writeable);
public static readonly ObjectNodeSection BssSection = new ObjectNodeSection("bss", SectionType.Writeable);
public static readonly ObjectNodeSection BssSection = new ObjectNodeSection("bss", SectionType.Uninitialized);
Copy link
Member Author

Choose a reason for hiding this comment

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

I've confirmed in the CI logs this also fixes the linker warning in multimodule testing: MultiModule.obj : warning LNK4078: multiple '.bss' sections found with different attributes (C0401040). Yay.


// If the data would be placed into an uninitialized section, that's better
// than dehydrating a bunch of zeros.
skipDehydrating |= GetDehydratedSection(factory).Type == SectionType.Uninitialized;
Copy link
Member

Choose a reason for hiding this comment

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

nit: maybe the shared logic could be moved into a helper to ensure it's consistent between GetSection and GetData (I had to read this a couple times to convince myself it was right, but it may just be me).

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried with a bool CanDehydrate(ObjectNodeSection desiredSection) but the shape of the helper (need to pass the section to make a decision) made it a bit weird. I'll keep it like this for now.

@MichalStrehovsky MichalStrehovsky merged commit f1a42cf into dotnet:main Nov 30, 2022
@MichalStrehovsky MichalStrehovsky deleted the nongcstat branch November 30, 2022 05:56
@ghost ghost locked as resolved and limited conversation to collaborators Dec 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants