[class] relax assumption about origin of a class (can come from an dynamic image)

[lewurm]

check got introduced with ffc3aaa (in most places).

fixes https://bugzilla.xamarin.com/show_bug.cgi?id=41130

[lewurm]

not sure if it's the right fix, please review and comment 🙂

[lewurm]

this is the actual change

[lambdageek]

So I don't think this approach is correct - SRE MonoImages don't have metadata.

In particular, calls like mono_class_get_first_field_idx must assert for dynamic images:

• For one thing - metadata tokens in SRE are not consecutive for fields of the same class (create two TypeBuilders and interleave their DefineField calls and you'll get metadata tokens that go 1, 3, 5, 7... and 2, 4, 6, 8...).
• For another thing the changes in class.c are benign but misleading - they typically do mono_class_get_first_field_idx and then do calls to mono_metadata_field_info with consecutive indices. None of that code gets called for SRE images (SRE does its own MonoClass setup), but if it did, it would be looking into tables that haven't been initialized. (For SRE Save they'll be filled when you call Save(), for SRE Run they may never be filled in).

I think we probably need to relax some mono_class_has_static_metadata check someplace so that the Ldfld example works, but changing it pervasively doesn't seem right.

[lambdageek]

Quick update. I took the test case bug-41130.cs and changed it to use reflection to create a x = new Foo<int>(); x.value = 42 instance and then to call Bar.Get<int> (x). That works as expected (returns 42).

So the problem is just the MetadataToken property of Foo<A>.value where A is the generic parameter of the generic method static A Bar.Get<A> (Foo<A>).

[lambdageek]

So the problem is in mono_reflection_get_token_checked. It's job is: given a managed reflection object, return the MetadataToken apropriate for that object.

In this case it's called on a System.Reflection.MonoField instance - but it so happens that the SR.MonoField is for a field of a class that was created using reflection. Unfortunately, the only things that a SR.MonoField has access to are the underlying MonoClassField*.

MonoClassField* does not store a metadata token. This makes sense for classes that come from ordinary MonoImages: the fields of a class are assigned consecutive tokens starting from some initial index - that's what mono_class_get_field_token basically computes: get the first metadata token from tables and compute the MonoClassField* offset from &klass.fields[0].

The problem (as I mentioned above) is that for dynamic classes the offset of a MonoClassField* relative to &klass.fields[0] has no relation to its token.

Note that this is not a problem for methods because MonoMethod* has a token field.

I think there are two possible ways to go:

1. Provide a mechanism to get from a SR.MonoField in a dynamic image back to its SRE.FieldBuilder (which stores the token that was assigned at definition time). One way to do this is to have a map in SRE.ModuleBuilder that maps each SRe.FieldBuilder to a baked SR.MonoField. Advantage: no cost for non-SRE cases. Disadvantage: I'm not sure if the map can be done lazily.
2. or just add a token field to MonoClassField. Advantage: no maps; lookup is fast and simple. Disadvantage: all classes get bigger.
3. or maybe add a member to SR.MonoField (either a SRE.FieldBuilder wasFieldBiulder or a int sreToken) if the member is set then use it to return MetadataToken; assume it wasn't an SRE case and dive down to the runtime as we do now.

[lambdageek]

Related: https://bugzilla.xamarin.com/show_bug.cgi?id=33208

Overall the constraints are:

1. from managed: each SR.Module and MetadataToken ought to map uniquely to a SR.MonoField
2. in the runtime: we really want consecutive MonoClassField*s of a MonoClass to have consecutive tokens.

I tried to get something that accomplishes both of those things in #3908 but I never got it reviewed or merged. It might be worth reviving. But I don't think I properly considered an example like the one in 41130.