Leverage new MemorySizeAttribute in metadata

[AArnott]

I'm not sure what it does yet, but we should see what it can offer the C# projection.

[KevinCathcart]

I'm not sure what it does yet

It appears you used to know what it did. perhaps reviewing #225 could help you remember.

The attribute points to the parameter that specifies the memory size in byte that the called function may access starting from pointer parameter it is attached to. It seems to have at least three use cases.

The simplest use case is pointers representing arrays. This is basically the same as [NativeArrayInfo]'s CountParamIndex, except it is always measured in units of bytes. For byte* arrays both cases are synonyms. It also gets used with void* for buffers. If it were to be used with other array element sizes, it would be count*sizeof(ElementType), but I don't think that actually happens anywhere.

The second use case is for a version of the extensible struct pattern (like where the struct has a field that indicates its own size), without having the size field in the struct itself but instead as a separate parameter.

The third use case use case is rather esoteric. It applies when an output parameter of a function is a struct containing a flexible array member (or the pre-standard equivalent, known as the "struct hack", where the last field of the struct is an array of unspecified size, determined by how much you malloc/alloca). In that case, you need to specify the total size of the buffer available to write the struct to (since the struct size can vary), and it needs to fail if the buffer you provided is too small. For an example, this scenario happens with the AccessCheck family of APIs in conjunction with the struct Windows.Win32.Security.PRIVILEGE_SET.

Flexible array member structs are obviously a bit obnoxious to project into other languages since they violate the concept of structs being fixed sized. CsWin32 does not appear to handle these well right now. It sees the flexible array member as a fixed sized inline array of size 1, because that is how they are being represented in the metadata. That would probably need to be improved before this third use case could be handled.

Hope this helps.

[jnm2]

You used to know what it did: see #225

Could come across as a bit rough.

[KevinCathcart]

Could come across as a bit rough.

Fair point. It was intended as a pointer to help Andrew remember what must have slipped his mind in the subsequent time, given everything else done since. Adjusted the wording to be harder to misinterpret the intention.

The first two cases I outlined should hopefully be at least a little familiar, since both got mentioned in the issue that originally led to the new attribute. The third case however might be new. I couldn't find much in the code or issues about the flexible array members or the struct hack, and there definitely seems to be nothing in any of the repositories about how this attribute can relate to those.

[jnm2]

We discussed the third case in #117 and microsoft/win32metadata#266 with the BITMAPINFO struct as an example.

[KevinCathcart]

Yes and no. When i was saying it was new, I meant very specifically with respect to this attribute.

Those issues you referenced discussed the struct hack, but did not really seem to discuss the case where they are being used as an output parameter of an api. They most certainly did not point out that those apis will almost always have a buffer size argument often with this newish attribute on it.

What was being discussed there was mostly some basic details of the shape for the representation (with plenty of emphasis on the fixed sized array cases too), and any Metadata improvements for flagging these structs. Hence why I said "not much".

Despite its size Issue 117 was still a relatively cursory look at variable size structs, mostly because a key concern there was getting something workable for downlevel frameworks that don't have span, or only have slow span, which is less safe in some scenarios.

Another consideration not brought up there is if there was anything that could or should be done to help the user allocate memory for those variable sized structs. The answer of course is that there is plenty that could be done to help there, if deemed desireable.

Example of what could be done

For example, the code could provide a factory method that takes total byte size (or even element count, and calculate the size), allocates a byte array of that size, gets a ref to element 0, uses Unsafe.As to turn it into a ref of the struct type, and returns it as a ref return. Unlike the span stuff, nothing in such an allocation helper should cause GC problems, even on old platforms. Interior pointers located as local variables have been supported by the framework basically forever, due to refs to array elements. The GC won't care about changing a managed pointer of one value type into another, especially if both are blitable, so contain no object references.

Similarly, relatively little was said about if any changes to the friendly overloads would help improve things for the (not much that is both feasible and sensible springs to mind for the output case).

[AArnott]

Thank you @KevinCathcart for enumerating the 3 scenarios we have. That was enlightening. The first case (representing arrays) in irritating because it overlaps with NativeArrayInfoAttribute yet does it ambiguously so I don't know whether the pointer is to an array or just one struct. I filed microsoft/win32metadata#1555 to track that. I think all such uses should switch to NativeArrayInfoAttribute to resolve the ambiguity.

Cases 2 and 3 seem to be the in and out variants of variable size structs. In that case, it's not clear to me that CsWin32 should do anything to assist in this case, since we can't emit sizeof code or similar to find the size of a struct whose size can actually vary. Even in the versioned structs, if the user passes in a pointer to a FOO2 as an argument to a FOO* parameter, CsWin32 had better not inject sizeof(FOO) as the automated value for the size parameter.

In summary, I think I should close this issue as fixed with #923, which seems like the only safe use of this attribute (and hopefully going away soon anyway).