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

ClangSharpPInvokeGenerator ignores __align #409

Closed
qmfrederik opened this issue Dec 3, 2022 · 3 comments
Closed

ClangSharpPInvokeGenerator ignores __align #409

qmfrederik opened this issue Dec 3, 2022 · 3 comments

Comments

@qmfrederik
Copy link
Contributor

I'm using ClangSharpPInvokeGenerator to generate the P/Invoke declarations for FreeRDP. Some of the structs make heavy use of __declspec(align(8)) / __attribute__((aligned(8))) to align all fields to an 8-byte boundary. An example can be seen here: https://github.com/FreeRDP/FreeRDP/blob/master/include/freerdp/settings.h#L962

I would have expected ClangSharpPInvokeGenerator to use StructLayout.Explicit on the struct and add a FieldOffset attribute to each field; but this is currently not the case.

I'm working around this by post-processing of the generate source files. Is there any way to have ClangSharpPInvokeGenerator generate the correct layout for these structs?

@qmfrederik
Copy link
Contributor Author

I assume that getting ClangSharpPInvokeGenerator to automatically calculate the offset of all fields of a struct is not an easy task. An alternative may be to have ClangSharpPInvokeGenerator provide annotations (AlignAttribute or similar) on the various fields, so that the metadata is preserved, and downstream tools could do post-processing?

@tannergooding
Copy link
Member

tannergooding commented Dec 6, 2022

ClangSharp should probably do something here to better support such "custom" layouts.

That being said, it's worth noting .NET doesn't provide a way to control alignment. So even if the layout is correct, that won't guarantee the struct is allocated on an 8-byte memory boundary (.NET only guarantees alignment that is no greater than 8; it will be dependent on the packing of the type, which is can be no greater than the natural packing).

It's also worth noting that there will be cases where such "custom" layouts result in definitions that are impossible to represent using a single .NET struct. Consider for example a struct S1 { void* _field1; size_t _field2; } and then a struct S2 { ALIGN64 S1 _field1; ALIGN64 BOOL _field2; }. In this scenario, S2._field1 is offset 0. But S2._field2 is either offset 8 or offset 16, depending on if you're targeting a 32-bit or 64-bit platform.
-- It's really easy to get these kinds of things wrong, and its really "better" to not define such structs in the first place. Expose APIs to get the relevant data or define a well-spec'd layout that works with memcpy and directly reading offsets from a void* instead.

It's also worth noting that such custom layouts is going to pessimize codegen by the JIT, by a lot, as explicit layouts presume aliasing. The size of this struct in particular, coupled with the number of fields and sheer amount of wasted space is going to hurt things as well.

@tannergooding
Copy link
Member

Since you can't really change what the C side is doing, this is the kind of scenario where doing some kind of "custom" translation for this type is probably better.

In particular, this is probably a case where you'd want to do some fixed ulong _data[....] and then expose a bunch of properties matching the field names that directly access the relevant offset.

It might be possible/better for ClangSharp to have some functionality built in to assist with that.

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

No branches or pull requests

2 participants