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

NativeAOT RC2 crash with GVMs on linux-x64 #77070

Closed
agocke opened this issue Oct 15, 2022 · 4 comments · Fixed by #80601
Closed

NativeAOT RC2 crash with GVMs on linux-x64 #77070

agocke opened this issue Oct 15, 2022 · 4 comments · Fixed by #80601

Comments

@agocke
Copy link
Member

agocke commented Oct 15, 2022

Output:

Process terminated. Generic virtual method pointer lookup failure.

Declaring type handle: MethodTable:0x00005608B8D60CF8
Target type handle: MethodTable:0x00005608B8DCE250
Method name: VisitDictionary
Instantiation:
Argument 00000000: MethodTable:0x00005608B8D79B30

backtrace

* thread #1, name = 'dnvm', stop reason = signal SIGABRT
  * frame #0: 0x00007fde39ce1a7c libc.so.6`pthread_kill + 300
    frame #1: 0x00007fde39c8d476 libc.so.6`raise + 22
    frame #2: 0x00007fde39c737f3 libc.so.6`abort + 211
    frame #3: 0x00005608b845b609 dnvm`SystemNative_Abort at pal_threading.c:286:5
    frame #4: 0x00005608b85f8044 dnvm`S_P_CoreLib_System_RuntimeExceptionHelpers__FailFast_1 at RuntimeExceptionHelpers.cs:281
    frame #5: 0x00005608b878fc2e dnvm`S_P_TypeLoader_Internal_Runtime_TypeLoader_TypeLoaderEnvironment__ResolveGenericVirtualMethodTarget_Static at TypeLoaderEnvironment.GVMResolution.cs:628
    frame #6: 0x00005608b878e82f dnvm`S_P_TypeLoader_Internal_Runtime_TypeLoader_TypeLoaderEnvironment__TryGetGenericVirtualTargetForTypeAndSlot at TypeLoaderEnvironment.GVMResolution.cs:100
    frame #7: 0x00005608b86c4aa5 dnvm`S_P_CoreLib_Internal_Runtime_CompilerServices_GenericVirtualMethodSupport__GVMLookupForSlotWorker at GenericVirtualMethodSupport.cs:31
    frame #8: 0x00005608b86c4b5e dnvm`S_P_CoreLib_Internal_Runtime_CompilerServices_GenericVirtualMethodSupport__GVMLookupForSlotWorker at GenericVirtualMethodSupport.cs:47
    frame #9: 0x00005608b86c4d52 dnvm`S_P_CoreLib_Internal_Runtime_CompilerServices_GenericVirtualMethodSupport__GVMLookupForSlot at GenericVirtualMethodSupport.cs:89
    frame #10: 0x00005608b86e36a0 dnvm`S_P_CoreLib_System_Runtime_TypeLoaderExports___c___GVMLookupForSlot_b__14_0 at TypeLoaderExports.cs:144
    frame #11: 0x00005608b8699e65 dnvm`S_P_CoreLib_System_Runtime_TypeLoaderExports__CacheMiss_0 at TypeLoaderExports.cs:218
    frame #12: 0x00005608b8699d85 dnvm`S_P_CoreLib_System_Runtime_TypeLoaderExports__GVMLookupForSlot at TypeLoaderExports.cs:142
    frame #13: 0x00005608b88ac7c5 dnvm`Serde_Serde_Json_JsonDeserializer__DeserializeDictionary<System___Canon__System___Canon> at JsonDeserializer.cs:94
    frame #14: 0x00005608b887a06f dnvm`dnvm_Dnvm_Update_Release_SerdeVisitor__Serde_IDeserializeVisitor_Dnvm_Update_Release__VisitDictionary<Serde_Serde_Json_JsonDeserializer_DeDictionary> at Dnvm.Update.Release.IDeserialize.cs:33

Figuring out how to upload the exe and core dump

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Oct 15, 2022
@agocke agocke added area-NativeAOT-coreclr and removed untriaged New issue has not been triaged by the area owner labels Oct 15, 2022
@agocke agocke added this to the 7.0.x milestone Oct 15, 2022
@agocke
Copy link
Member Author

agocke commented Oct 15, 2022

core.zip
dnvm.zip

@agocke
Copy link
Member Author

agocke commented Oct 20, 2022

I've got a minimal repro and some more info about this:

using System.Collections.Generic;

// Regression test for https://github.com/dotnet/runtime/issues/77070
public static class Gvm77070
{
    public static int Run()
    {
        var deserializer = new JsonDeserializer();
        var visitor = new Release.SerdeVisitor();
        deserializer.DeserializeDictionary<Release, Release.SerdeVisitor>(visitor);
        return 100;
    }

    public partial record struct Release
    {
        public sealed class SerdeVisitor : IDeserializeVisitor<Release>
        {
            void IDeserializeVisitor<Release>.VisitDictionary<D>(ref D d)
            {
                d.GetNextValue<DictWrap<string, StringWrap, string>>();
            }
        }
    }

    public record StringWrap(string Value)
        : IDeserialize
    {
        public static void Deserialize<D>(ref D deserializer)
            where D : IDeserializer
        {
        }
    }

    public readonly struct DictWrap<TKey, TKeyWrap, TValue> : IDeserialize
        where TKey : notnull
        where TKeyWrap : IDeserialize
    {
        static void IDeserialize.Deserialize<D>(ref D deserializer)
        {
            deserializer.DeserializeDictionary<Dictionary<TKey, TValue>, Visitor>(new Visitor());
        }
        private sealed class Visitor : IDeserializeVisitor<Dictionary<TKey, TValue>>
        {
            void IDeserializeVisitor<Dictionary<TKey, TValue>>.VisitDictionary<D>(ref D d)
            {
                d.GetNextValue<TKeyWrap>();
            }
        }
    }

    public interface IDeserializeVisitor<T>
    {
        void VisitDictionary<D>(ref D d) where D : IDeserializeDictionary;
    }

    public interface IDeserialize
    {
        static abstract void Deserialize<D>(ref D deserializer) where D : IDeserializer;
    }

    public interface IDeserializeDictionary
    {
        void GetNextValue<D>() where D : IDeserialize;
    }

    public interface IDeserializer
    {
        void DeserializeDictionary<T, V>(V v) where V : class, IDeserializeVisitor<T>;
    }
    public sealed partial class JsonDeserializer : IDeserializer
    {
        public void DeserializeDictionary<T, V>(V v) where V : class, IDeserializeVisitor<T>
        {
            var map = new DeDictionary(this);
            v.VisitDictionary(ref map);
        }

        private struct DeDictionary : IDeserializeDictionary
        {
            private JsonDeserializer _deserializer;
            public DeDictionary(JsonDeserializer de)
            {
                _deserializer = de;
            }

            public void GetNextValue<D>() where D : IDeserialize
            {
                D.Deserialize(ref _deserializer);
            }
        }
    }
}

The problem appears to be something about static interface methods with generics. It looks like this is blowing up in the type loader, which doesn't know how to work with this method specification. I'm not sure what the fix is, but I'm still looking into it.

@MichalStrehovsky
Copy link
Member

Workaround is the following RD.XML (I constructed thing by looking up the symbols associated with the addresses in the FailFast) - it pregenerates the method dictionary we're failing to build at runtime:

<Directives>
  <Application>
    <Assembly Name="repro">
      <Type Name="Gvm77070+DictWrap`3+Visitor[[System.String, mscorlib],[Gvm77070+StringWrap],[System.String,mscorlib]]">
        <Method Name="Gvm77070.IDeserializeVisitor&lt;System.Collections.Generic.Dictionary&lt;TKey,TValue>>.VisitDictionary" Dynamic="Required All">
          <GenericArgument Name="Gvm77070+JsonDeserializer+DeDictionary" />
        </Method>
      </Type>
    </Assembly>
  </Application>
</Directives>

I looked at this in a debugger myself and yep, this needs to be fixed in the type loader (we need to implement the missing dictionary kind that this is throwing about) and it is a hard fix:

 	reproNative.exe!S_P_CoreLib_System_Runtime_EH__RhThrowEx() Line 556	Unknown
 	[External Code]	
>	reproNative.exe!S_P_TypeLoader_Internal_Runtime_TypeLoader_GenericDictionaryCell__ParseAndCreateCell() Line 1879	Unknown
 	reproNative.exe!S_P_TypeLoader_Internal_Runtime_TypeLoader_GenericDictionaryCell__BuildDictionary() Line 1071	Unknown
 	reproNative.exe!S_P_TypeLoader_Internal_Runtime_TypeLoader_TypeBuilder__ParseNativeLayoutInfo() Line 486	Unknown
 	reproNative.exe!S_P_TypeLoader_Internal_Runtime_TypeLoader_TypeBuilder__PrepareMethod() Line 223	Unknown
 	reproNative.exe!S_P_TypeLoader_Internal_Runtime_TypeLoader_GenericDictionaryCell_MethodCell__Prepare() Line 659	Unknown
 	reproNative.exe!S_P_TypeLoader_Internal_Runtime_TypeLoader_GenericDictionaryCell__BuildDictionary() Line 1075	Unknown
 	reproNative.exe!S_P_TypeLoader_Internal_Runtime_TypeLoader_TypeBuilder__ParseNativeLayoutInfo() Line 486	Unknown
 	reproNative.exe!S_P_TypeLoader_Internal_Runtime_TypeLoader_TypeBuilder__PrepareMethod() Line 223	Unknown
 	reproNative.exe!S_P_TypeLoader_Internal_Runtime_TypeLoader_TypeBuilder__BuildMethod() Line 1654	Unknown
 	reproNative.exe!S_P_TypeLoader_Internal_Runtime_TypeLoader_TypeBuilder__TryBuildGenericMethod_0() Line 2071	Unknown
 	reproNative.exe!S_P_TypeLoader_Internal_Runtime_TypeLoader_TypeLoaderEnvironment__TryGetGenericVirtualMethodPointer() Line 382	Unknown

We need to call into GVM resolution logic as part of type loading and build whatever GVM resolution logic came up with. Either that or generate something that will allow us to lazily resolve this once it's needed.

Touching anything in the type loader is pretty high risk. To set expectations, the fix might be too risky for servicing.

@agocke agocke modified the milestones: 7.0.x, 8.0.0 Nov 18, 2022
agocke added a commit to serdedotnet/serde that referenced this issue Nov 19, 2022
Struct visitors are useful (no allocations), but they're tricky, as
side-effects may not be preserved when they are copied by the underlying
deserializer.

Most importantly, a Native AOT bug
(dotnet/runtime#77070) currently causes some
static abstract interface calls to fail when they have to be loaded at
runtime. Generic specialization can avoid this. By using a struct
visitor for some compound calls (dictionaries, enumerables, nullables)
the likelihood of hitting this is reduced.
MichalStrehovsky added a commit to MichalStrehovsky/runtime that referenced this issue Nov 28, 2022
If the program hits the conditions in dotnet#77070, generate a failfast message that makes it possible to create a workaround.

Instead of:

```
Process terminated. Generic virtual method pointer lookup failure.

Declaring type handle: MethodTable:0x00007FF66E8587B8
Target type handle: MethodTable:0x00007FF66E858810
Method name: Serialize
Instantiation:
  Argument 00000000: MethodTable:0x00007FF66E85DA08
```

Generate:

```
Process terminated. Generic virtual method pointer lookup failure.

Declaring type handle: EETypeRva:0x005438B8(MemoryPackFormatter2`1[MemPackObject])
Target type handle: EETypeRva:0x00543910(MemoryPackableFormatter2`1[MemPackObject])
Method name: Serialize
Instantiation:
  Argument 00000000: EETypeRva:0x00529B38(System.Buffers.ArrayBufferWriter`1[System.Byte])
```

The workaround is then:

```xml
<Directives>
  <Application>
    <Assembly Name="repro">
      <Type Name="MemoryPackableFormatter2`1[[MemPackObject]]">
        <Method Name="Serialize" Dynamic="Required All">
          <GenericArgument Name="System.Buffers.ArrayBufferWriter`1[[System.Byte, mscorlib]],System.Memory" />
        </Method>
      </Type>
    </Assembly>
  </Application>
</Directives>
```
MichalStrehovsky added a commit that referenced this issue Nov 28, 2022
If the program hits the conditions in #77070, generate a failfast message that makes it possible to create a workaround.

Instead of:

```
Process terminated. Generic virtual method pointer lookup failure.

Declaring type handle: MethodTable:0x00007FF66E8587B8
Target type handle: MethodTable:0x00007FF66E858810
Method name: Serialize
Instantiation:
  Argument 00000000: MethodTable:0x00007FF66E85DA08
```

Generate:

```
Process terminated. Generic virtual method pointer lookup failure.

Declaring type handle: EETypeRva:0x005438B8(MemoryPackFormatter2`1[MemPackObject])
Target type handle: EETypeRva:0x00543910(MemoryPackableFormatter2`1[MemPackObject])
Method name: Serialize
Instantiation:
  Argument 00000000: EETypeRva:0x00529B38(System.Buffers.ArrayBufferWriter`1[System.Byte])
```

The workaround is then:

```xml
<Directives>
  <Application>
    <Assembly Name="repro">
      <Type Name="MemoryPackableFormatter2`1[[MemPackObject]]">
        <Method Name="Serialize" Dynamic="Required All">
          <GenericArgument Name="System.Buffers.ArrayBufferWriter`1[[System.Byte, mscorlib]],System.Memory" />
        </Method>
      </Type>
    </Assembly>
  </Application>
</Directives>
```
github-actions bot pushed a commit that referenced this issue Nov 28, 2022
If the program hits the conditions in #77070, generate a failfast message that makes it possible to create a workaround.

Instead of:

```
Process terminated. Generic virtual method pointer lookup failure.

Declaring type handle: MethodTable:0x00007FF66E8587B8
Target type handle: MethodTable:0x00007FF66E858810
Method name: Serialize
Instantiation:
  Argument 00000000: MethodTable:0x00007FF66E85DA08
```

Generate:

```
Process terminated. Generic virtual method pointer lookup failure.

Declaring type handle: EETypeRva:0x005438B8(MemoryPackFormatter2`1[MemPackObject])
Target type handle: EETypeRva:0x00543910(MemoryPackableFormatter2`1[MemPackObject])
Method name: Serialize
Instantiation:
  Argument 00000000: EETypeRva:0x00529B38(System.Buffers.ArrayBufferWriter`1[System.Byte])
```

The workaround is then:

```xml
<Directives>
  <Application>
    <Assembly Name="repro">
      <Type Name="MemoryPackableFormatter2`1[[MemPackObject]]">
        <Method Name="Serialize" Dynamic="Required All">
          <GenericArgument Name="System.Buffers.ArrayBufferWriter`1[[System.Byte, mscorlib]],System.Memory" />
        </Method>
      </Type>
    </Assembly>
  </Application>
</Directives>
```
MichalStrehovsky added a commit to MichalStrehovsky/runtime that referenced this issue Dec 23, 2022
For whatever reason, the logic that walks the inheritance hierarchy and deals with the differences between interface GVM and class GVM calls was in CoreLib.

Move it to the type loader because we'll need it for dotnet#77070.
jkotas pushed a commit that referenced this issue Dec 23, 2022
)

* Move all of generic virtual method resolution to the type loader

For whatever reason, the logic that walks the inheritance hierarchy and deals with the differences between interface GVM and class GVM calls was in CoreLib.

Move it to the type loader because we'll need it for #77070.

* Oh the conversion to EEType was important
MichalStrehovsky added a commit that referenced this issue Jan 2, 2023
Generic virtual method resolution currently happens on top of `RuntimeTypeHandle`s (i.e. the runtime type system). This works as long as we only need to do generic virtual method resolution on top of types that exist in the runtime.

In order to lay foundation for fixing #77070, we need to be able to resolve not just with types that have a type handle, but also for types that we're in the process of building (e.g. due to MakeGeneric, or due to another generic virtual method dispatch).

This pull request rewrites generic virtual method dispatch to happen on top of the managed type system, instead of the runtime type system. It's a bit more than just a mechanical replacement because the existing algorithm was set up in a confusing way, with `ref` parameters that were changing what we were resolving and an odd `slotChanged` logic that I don't fully understand. I replaced the `slotChanged` with the straightforward thing (if we resolve interface method to a virtual method on a type, find the implementation of the virtual method on the current type). Tests seem to be passing. Also instead of a bunch of `ref` parameters we now just pass a `MethodDesc`.

This also includes a compiler change, because in order for the managed type system's casting logic to work, we need to be able to find out the variance of parameters on generic definitions. The compiler change is about generating this info.
carlossanlop pushed a commit that referenced this issue Jan 4, 2023
If the program hits the conditions in #77070, generate a failfast message that makes it possible to create a workaround.

Instead of:

```
Process terminated. Generic virtual method pointer lookup failure.

Declaring type handle: MethodTable:0x00007FF66E8587B8
Target type handle: MethodTable:0x00007FF66E858810
Method name: Serialize
Instantiation:
  Argument 00000000: MethodTable:0x00007FF66E85DA08
```

Generate:

```
Process terminated. Generic virtual method pointer lookup failure.

Declaring type handle: EETypeRva:0x005438B8(MemoryPackFormatter2`1[MemPackObject])
Target type handle: EETypeRva:0x00543910(MemoryPackableFormatter2`1[MemPackObject])
Method name: Serialize
Instantiation:
  Argument 00000000: EETypeRva:0x00529B38(System.Buffers.ArrayBufferWriter`1[System.Byte])
```

The workaround is then:

```xml
<Directives>
  <Application>
    <Assembly Name="repro">
      <Type Name="MemoryPackableFormatter2`1[[MemPackObject]]">
        <Method Name="Serialize" Dynamic="Required All">
          <GenericArgument Name="System.Buffers.ArrayBufferWriter`1[[System.Byte, mscorlib]],System.Memory" />
        </Method>
      </Type>
    </Assembly>
  </Application>
</Directives>
```

Co-authored-by: Michal Strehovský <MichalStrehovsky@users.noreply.github.com>
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jan 13, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jan 16, 2023
@dotnet dotnet locked as resolved and limited conversation to collaborators Feb 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants