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

System.TypeLoadException thrown by the Runtime when using structs and generics #11179

Closed
improbablejan opened this issue Oct 2, 2018 · 16 comments

Comments

@improbablejan
Copy link

The following snippet of code, when compiled and run as a standalone program, will compile just fine but throw a runtime exception:

struct A
{
    B<A> b;

    static void Main(string[] args)
    {
        A a = new A();
    }
}

struct B<T>
{
    C<T> c;
}

class C<T>
{
    T t;
}

Exception thrown:
Unhandled Exception: System.TypeLoadException: Could not load type 'A' from assembly 'dotnet, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null'.

I have tested this with the newest public release of .NET Core I could find - 2.2.100-preview2-009404.

The same issue also occurs when building a standard .NET Framework project; I have tested targeting versions 4.5.2 and 4.7.2. Running the executable produced for .NET with mono (Mono JIT compiler version 5.14.0 (Visual Studio built mono)) works just fine - the program prints nothing and exits with code 0.

The same problem occurs if similar code is compiled into a DLL that's later loaded as a library. However, this is the minimum repro case I could come up with. Should I separately file a bug for .NET Framework? This page suggests I should do so, but points to a "retired" web page.

@poizan42
Copy link
Contributor

poizan42 commented Oct 2, 2018

Your declaration is infinitely recursive. If every instance of struct A should contain B<A> which contains C<A> which contains A, then the struct would have to be infinitely big, that is obviously not possible (in this case it is otherwise empty, but empty structs in .net takes 1 byte anyways).

You could question whether Roslyn should detect this and fail with CS0523, but note that it can only do so if all the types are declared in the same assembly.

Sorry, I didn't notice C being a class.

@improbablejan
Copy link
Author

improbablejan commented Oct 2, 2018

C is a class, which breaks the infinite recursion. A more minimal example (apologies) which highlights the problem better and makes it more obvious that there is no infinite recursion going on is this:

struct A
{
    B<A> b;

    static void Main(string[] args)
    {
        A a = new A();
    }
}

struct B<T>
{
}

@MichalStrehovsky
Copy link
Member

The second example is #6924, but it's possible the first one is related too.

@improbablejan
Copy link
Author

Thanks @MichalStrehovsky

That does look like the same issue. I feel like the title in https://github.com/dotnet/coreclr/issues/7957 is a bit too narrow. https://github.com/dotnet/coreclr/issues/7957#issuecomment-411838562 describes a similar example to my original. The issue seems to be a cyclic recursion in type parameters of structs causing issues even if there is no cyclic reference in the struct data layout itself.

@improbablejan
Copy link
Author

I poked around the codebase, and believe the issue is the following:
When A gets type loaded, we need to calculate the size and alignment requirements of the struct, which involves calculating the alignment requirements and size of non-static fields. Because B<A> is a value type, this forces type loading B<A>, and is in general unavoidable. In order to type load a generic type, the classloader first type loads all the type arguments, and then uses them for type substitution to instantiate the generic. This forces A to be type loaded again while it's still being loaded, detects the cycle, and throws.

My understanding of the codebase is very pedestrian, but I thought of two possible fixes:

  • Change the resolution of generic type parameters to be calculated lazily as needed. This sounds like a big change with knock-on effects, and so probably isn't feasible.
  • Separate size and alignment calculations, use lazy type parameter evaluation there.

Would any of these options be feasible/acceptable? Do you think there are other, better options?

@Korporal
Copy link

Korporal commented May 2, 2019

@gafter

Why does nobody seem particularly interested in addressing this very fundamental bug - the current inability of the C# compiler to correctly process legal source code? I would have thought that these kinds of bugs would be given a high priority since this is quite legal source code.

@svick
Copy link
Contributor

svick commented May 2, 2019

@Korporal It seems to me that this is a runtime issue, which means that the C# compiler is behaving correctly and that there is no point in appealing to members of the C# compiler team.

@gafter
Copy link
Member

gafter commented May 2, 2019

@Korporal I agree with you totally... this needs to get the appropriate priority. I believe it will. See also

@Korporal
Copy link

Korporal commented May 3, 2019

@Korporal It seems to me that this is a runtime issue, which means that the C# compiler is behaving correctly and that there is no point in appealing to members of the C# compiler team.

@svick - Thank you for that explanation, but please understand I have no idea who is on which team, nor do I know if some prominent individual has or does not have influence over certain decisions. As you'll see Neil has participated in discussions in this forum; confining remarks to "team members" only is also not a rule that I've been asked to follow.

Finally I see no reason for you to presumptuously speak on behalf of others whom I've politely addressed in a post, it is for them to respond or not respond as they see fit, I'm sure Neil is quite capable of this.

Finally I'm sure it likely is a runtime issue as you point out but again I do not know the underlying cause nor do I have the deeper insights that others like your good self may have and so for all I know this could be due to the runtime falling victim to incorrect compiler output - I simply do not know at the time I posted this.

Thank you.

@svick
Copy link
Contributor

svick commented May 3, 2019

@Korporal I apologize. I did not mean to tell you how you have to behave, I was just trying to explain what I thought was an effective way to behave. Turns out I was wrong.

@Korporal
Copy link

Korporal commented May 3, 2019

@svick - That's OK, it easy to misconstrue someone's intentions here sometimes! NP.

@karelz
Copy link
Member

karelz commented Jun 5, 2019

@davidwrighton @RussKeldorph there seems to be bunch of duplicates - see https://github.com/dotnet/coreclr/issues/20220#issuecomment-488859255
It seems to hit quite a few customers now. Is it correctly triaged to Future instead of 3.0?

cc @jkotas

@jkotas
Copy link
Member

jkotas commented Jun 5, 2019

Is it correctly triaged to Future instead of 3.0?

Yes. This implementation limitation of CoreCLR Typeloader goes pretty deep. It is not easy to fix.

@karelz
Copy link
Member

karelz commented Jun 5, 2019

Thanks @jkotas! Would it make sense to clean up the duplicates and make one central issue with clarification of the limitations as you posted above?

@jkotas
Copy link
Member

jkotas commented Jun 5, 2019

This is duplicate of #6924

@jkotas jkotas closed this as completed Jun 5, 2019
@jkotas
Copy link
Member

jkotas commented Jun 5, 2019

For the record, the repro from the top can be simplified to:

struct A
{
    B<A> b;

    static void Main(string[] args)
    {
        A a = new A();
    }
}

struct B<T>
{
    object c;
}

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the Future milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants