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

Plugged System.Activator.CreateInstance methods for types with parameterless constructors. #2997

Merged
merged 3 commits into from
Apr 12, 2024

Conversation

Guillermo-Santos
Copy link
Contributor

This PR add the plugs for System.Activator.CreateInstance<T>() and System.Activator.CreateInstance(Type T, bool, bool) two methods that are used to create generic types.

With System.Activator.CreateInstance<T>() the following start to work on cosmos:

// A generic method to get an instance of type T
public static T Get<T>() where T : class, new()
{
    return new T();
}

Limitations

  1. Constructors may not be on the compilation result:At the moment Constructors, as any other method, are not included on the compilation result if never called, this could cause stack corruption or null reference exceptions when calling one of these methods or when using its result.

To avoid this, you would need to add a dummy variable somewhere (BeforeRun recommended) to make IL2CPU include the ctor on the compilation.

var dummy = new MyClass();
var myVar = Activator.CreateInstance<MyClass>();//This will work as ctor is included on the compilation.
  1. ref struct should not be supported: If the type you are trying to instantiate is a ref struct these methods should throw a NotSupportedException (as far as I know, is because ref struct should never be boxed), but since, at the moment, we cannot check if a type is a ref struct (System.Type.IsByRefLike field not plugged) we just process the ref struct and return it as a normal struct or class.

@zarlo
Copy link
Member

zarlo commented Apr 10, 2024

ref struct should not be supported: If the type you are trying to instantiate is a ref struct these methods should throw a NotSupportedException (as far as I know, is because ref struct should never be boxed), but since, at the moment, we cannot check if a type is a ref struct (System.Type.IsByRefLike field not plugged) we just process the ref struct and return it as a normal struct or class.

@Guillermo-Santos if it has not already been done could you make a issue for this so we can track it

}

// nonPublic is ignored since, at the moment, we cannot know if a ctor is public or not
// and we would get a Stack Corruption or Null reference exception if the ctor is not present either way.
Copy link
Member

Choose a reason for hiding this comment

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

is there a way to prevent the Stack Corruption and just throw an exception (we should try and match what .net does whti what exception is thrown)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem is, we do not have method info, we do not know what method is a ctor and what method is not (and their parameters) at runtime. To make it all work as intended the Vtable should be reworked.

Copy link
Contributor Author

@Guillermo-Santos Guillermo-Santos Apr 10, 2024

Choose a reason for hiding this comment

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

What i mean for stack corruption or null reference, is that a method other than the ctor was called or for some reason the null check of the ctor throws (happened while testing but should not happend now). This could happend either because there is no parameterless constructor or that ctor is not the address at index 0 (till now, it always have been at index 0, but the possibility of it changing is still there).

@Guillermo-Santos
Copy link
Contributor Author

ref struct should not be supported: If the type you are trying to instantiate is a ref struct these methods should throw a NotSupportedException (as far as I know, is because ref struct should never be boxed), but since, at the moment, we cannot check if a type is a ref struct (System.Type.IsByRefLike field not plugged) we just process the ref struct and return it as a normal struct or class.

@Guillermo-Santos if it has not already been done could you make a issue for this so we can track it

Will do

Copy link
Member

@quajak quajak left a comment

Choose a reason for hiding this comment

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

Great work!

@quajak quajak merged commit f929ce2 into CosmosOS:master Apr 12, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants