dotnet / runtimelab

This repo is for experimentation and exploring new ideas that may or may not make it into the main dotnet/runtime repo.
MIT License
1.36k stars 188 forks source link

[NativeAOT-LLVM] [Question]: Should GCPointerMap have a provision for adding optional fields to GCStaticEETypeNode #2606

Open yowl opened 3 weeks ago

yowl commented 3 weeks ago

WIth this code

    public static class InteropReturnArea
    {
        [InlineArray(3)]
        internal struct ReturnArea
        {
            private ulong buffer;

            internal unsafe nint AddressOfReturnArea()
            {
                return (nint)Unsafe.AsPointer(ref buffer);
            }
        }

        [ThreadStatic]
        [FixedAddressValueType]
        internal static ReturnArea returnArea = default;

ReturnArea should be align 8, so GCStaticEETypeNode should be written with an optional field OptionalFieldsFlag I assume. The mechanism seems to be to do with GCPointerMap but there is nothing present to do this. Am I off track here or is there something missing?

Thanks

jkotas commented 3 weeks ago

Should GCPointerMap have a provision for adding optional fields to GCStaticEETypeNode

I do not think we want to have a generic provision for arbitrary optional fields in GCPointerMap.

We may want to track the RequiresAlign8 in GCPointerMap (more convenient); or RequiresAlign8 can be tracked independently from GCPointerMap.

GCStaticEETypeNode for regular statics has the same problem, but it is masked by the thread statics being allocated on PINNED_OBJECT_HEAP that happens to guarantee 8-byte alignment even on 32-bit platforms. A quick work around for thread statics would be to allocate all thread statics conservatively with 8-byte alignment.

Linux arm32 is probably affected by this bug as well. We may want to fix it in upstream. cc @filipnavara

jkotas commented 3 weeks ago

I have opened https://github.com/dotnet/runtime/issues/103234 for the upstream bug on Linux arm32

filipnavara commented 3 weeks ago

Linux arm32 is probably affected by this bug as well. We may want to fix it in upstream. cc @filipnavara

Sounds like it. I am not sure I fully understand the scope of the issue yet. Generally, Align8 was one of the last items on the https://github.com/dotnet/runtime/issues/97729 list.