[Libraries][Android] Fix alignment for padding System.Decimal #57978
Conversation
|
Looks like x86 Windows and x86 Linux have different alignment for a struct like this https://godbolt.org/z/GvnosE9MK Looks like wasm was special-cased in the test and doesn't work now. |
It seems like then the expected behavior should be an align of 4, so instead, modified the expectations for the tests for Android x86. |
|
@jkotas what's the right interop behavior for System.Decimal on non-Windows platforms? Should it match Windows padding/alignment or the host platform? Specifically, should we make it match Windows x86 when the host is Linux x86? Does CoreCLR make any guarantees here? I saw #38603 so it seems like on coreclr you just follow the normal host ABI, right - and it happens that on Windows it ends up matching the layout of the unmanaged DECIMAL struct? |
Right, Decimal should be treated as ordinary blittable struct. It should not need any special alignment quirks anymore. "Remove Decimal struct special casing in marshal" change looks right to me. |
|
/backport to release/6.0 |
|
Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1182913073 |
ghost
locked as resolved and limited conversation to collaborators
3 participants
Fixes #49872
There is a difference in the current alignment of a decimal type on Android x86 (32bit)
MONO_ABI_ALIGNOF (gpointer)which evaluates to 4 bytes and whats expected, which is 8 bytes. The behavior ofMONO_ABI_ALIGNOF (gpointer)seems correct asgpointerisvoid*and 32-bit aligned pointers on a 32-bit platform makes sense. Instead, it's suspected that usingMONO_ABI_ALIGNOF (gpointer)as the alignment is not correct for Android x86 if we are expecting an align of 8 bytes.Attempting to remove the entire special casing for System Decimal in
marshal.cdidn't seem to work as noted in #49872 (comment)This PR will adhere to the alignment value of 4 for Android x86, remove the special casing for System.Decimal because CoreCLR had removed their special casing, and modifies the expected values in the tests. https://github.com/dotnet/corefx/pull/30690/files?file-filters%5B%5D=.cs#r199212623 makes me suspect these tests have yet to be modified to fully accomodate Linux platforms.