[Tracking] Mono support for InlineArrayAttribute

[lambdageek]

This issue tracks the work for implementing Mono support for InlineArrayAttribute #61135 and the related C# features such as params ReadOnlySpan, safe fixed size buffers, stackalloc of reference types, etc.

Required work: Completed in .NET8

• Implement struct layout support for InlineArrayAttribute
  This should work for the JIT/interp and also for the AOT compiler
• Implement GC descriptor for boxed structs with InlineArrayAttribute
• [mono][debugger] Implement Mono debugger support for structs with InlineArrayAttribute #84071
• JS Interop generator: Disallow marshaling structs with InlineArrayAttribute in JSImport/JSExport

Nice to have: Future Release.

• Implement support for fast well-known custom attribute lookup in Mono.
  (This isn't strictly necessary, but we already have a number of custom attributes that we check for on every class, and it would be nice to have a single fast lookup function (CoreCLR uses perfect hashing for this))

[dotnet-issue-labeler]

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.

[lambdageek]

/cc @SamMonoRT @kotlarmilos @thaystg

[am11]

Does this supersede #48113?

[lambdageek]

Does this supersede #48113?

Good catch - I forgot about that one. 🥲
But also I think the details of the API proposal changed enough that it's worth tracking in a new issue.

[lambdageek]

One interesting technical challenge is how to extract the Length property from InlineArrayAttribute without triggering class loading and managed object construction for possibly unrelated custom attributes on every class.

We have already a custom attribute iterator mono_class_metadata_foreach_custom_attr that avoids triggering assembly loads and so on. It takes a callback that gets the method token of the constructor and the namespace and name of the custom attribute. I think what we need to do is to also pass to it the custom attribute row index so that it can find the blob data and length.

Roughly like this:

1. Extend mono_class_metadata_foreach_custom_attr to pass the custom attribute row index to the MonoAssemblyMetadatacustomAttrIterFunc callback.
2. Extract the loop body in mono_custom_attrs_from_index_checked into a helper function:
   void mono_metadata_custom_attrs_get_blob_info (MonoImage *image, uint32_t custom_attr_idx, MonoMethod **out_ctor, uint32_t *out_blob_size, const char** out_blob) that gets passed a custom attribute row index and returns the MonoMethod a blob size and the start of the blob that encodes the custom attribute data in that one row in the custom attribute table.
3. Then call mono_reflection_create_custom_attr_data_args_noalloc passing it the ctor, blob size and blob data pointer - this will return a data structure containing the property values.

It might also be possible to just pass the blob pointer directly to the MonoAsemblyMetadataCustomAttrIterFunc callback directly from mono_class_metadata_foreach_custom_attr without bothering with the custom attribute row index - all the relevant info (method token for the ctor, blob data size, blob data ptr) should be retrievable by mono_class_metadata_foreach_custom_attr. Then if we know we're looking at InlineArrayAttribute, we can call mono_method_get_method_checked on the method token to get a MonoMethod* for the ctor and then just pass all the right args to mono_reflection_create_custom_attr_data_args_noalloc directly in the iterator callback.

[kotlarmilos]

metadata_foreach_custom_attr_from_index has inlined loop of mono_custom_attrs_from_index_checked which makes it easy to create MonoCustomAttrEntry struct and pass it to the callback function, where if the attribute exists mono_reflection_create_custom_attr_data_args_noalloc is invoked. Does it make sense to extract the loop body in mono_custom_attrs_from_index_checked into mono_metadata_custom_attrs_get_blob_info which returns MonoCustomAttrEntry * instead of out arguments?

The current PoC retrieves the Length property from InlineArrayAttribute in metadata_foreach_custom_attr_from_index. As the function iterates through all attributes, it might be inefficient. Also, for further extensibility I suggest adding a helper function with an array of attributes that has a value, like gboolean attr_has_value (char *attr_name)

[lambdageek]

metadata_foreach_custom_attr_from_index has inlined loop of mono_custom_attrs_from_index_checked which makes it easy to create MonoCustomAttrEntry struct and pass it to the callback function, where if the attribute exists mono_reflection_create_custom_attr_data_args_noalloc is invoked. Does it make sense to extract the loop body in mono_custom_attrs_from_index_checked into mono_metadata_custom_attrs_get_blob_info which returns MonoCustomAttrEntry * instead of out arguments?

The current PoC retrieves the Length property from InlineArrayAttribute in metadata_foreach_custom_attr_from_index. As the function iterates through all attributes, it might be inefficient. Also, for further extensibility I suggest adding a helper function with an array of attributes that has a value, like gboolean attr_has_value (char *attr_name)

I think this is generally the right direction to head in, but there are a couple of issues:

1. I don't think it's a good idea for metadata_foreach_custom_attr_from_index to know anything about InlineArrayAttribute. Ideally it's just a generic traversal mechanism. The logic to deal with InlineArrayAttribute should be part of the callback that is passed to foreach by the class-init.c code.
2. MonoCustomAttrEntry is not a great struct to use for this particular case. The issue is that to create a MonoMethod from a method token, we have to create and initialize the class in which the method is defined; and that may require us to load the assembly in which the class is defined. In the code, as you have it, we only try to load InlineArrayAttribute's ctor, but if we make foreach less specialized, it would not be right to try to create a MonoMethod for every attribute that we see.

So I really think more should be delegated to the callback

But overall the rest of the structure looks reasonable