Uh oh!
There was an error while loading. Please reload this page.
[Mono] Fix offset calculation for nested struct, when pinvoke is enabled - #91292
Conversation
fanyang-mono
commented
Aug 29, 2023
This should be backported to .NET8/7/6. |
lambdageek
left a comment
There was a problem hiding this comment.
The test can be simplified - you don't need any methods in the struct definitions - just fields.
The test can also go into src/tests/Interop/StructMarshalling/PInvoke/ and can be updated like the other .csproj files in that directory to use the .cpp file to implement a C++ function that takes the right kind of argument struct.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
fanyang-mono
commented
Aug 30, 2023
Newly added test failed on wasm with interpreter. Investigating it now. |
fanyang-mono
commented
Aug 30, 2023
The wasm apps created for runtime tests weren't including c++ file/library needed by the test. See details in #64127. Disabled newly added test on wasm. |
fanyang-mono
commented
Aug 30, 2023
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
fanyang-mono
commented
Aug 31, 2023
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
fanyang-mono
commented
Aug 31, 2023
Library test failures were related to #91410. |
fanyang-mono
commented
Aug 31, 2023
/backport to release/8.0 |
Started backporting to release/8.0: https://github.com/dotnet/runtime/actions/runs/6041888778 |
fanyang-mono
commented
Aug 31, 2023
/backport to release/7.0-staging |
fanyang-mono
commented
Aug 31, 2023
/backport to release/6.0-staging |
Started backporting to release/7.0-staging: https://github.com/dotnet/runtime/actions/runs/6041900557 |
Started backporting to release/6.0-staging: https://github.com/dotnet/runtime/actions/runs/6041901510 |
@fanyang-mono backporting to release/7.0-staging failed, the patch most likely resulted in conflicts: $ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Fix offset calculation for nested struct
Applying: Add a testfor nested struct with pinvoke
Applying: Move and update test with real c++ implementation
Applying: Exclude newly added test from wasm
Using index info to reconstruct a base tree...
M src/tests/issues.targets
Falling back to patching base and 3-way merge...
Auto-merging src/tests/issues.targets
CONFLICT (content): Merge conflict in src/tests/issues.targets
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0004 Exclude newly added test from wasm
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128Please backport manually! |
@fanyang-mono an error occurred while backporting to release/7.0-staging, please check the run log for details! Error: git am failed, most likely due to a merge conflict. |
@fanyang-mono backporting to release/6.0-staging failed, the patch most likely resulted in conflicts: $ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Fix offset calculation for nested struct
Using index info to reconstruct a base tree...
M src/mono/mono/mini/mini-amd64.c
Falling back to patching base and 3-way merge...
Auto-merging src/mono/mono/mini/mini-amd64.c
Applying: Add a testfor nested struct with pinvoke
Applying: Move and update test with real c++ implementation
Applying: Exclude newly added test from wasm
Using index info to reconstruct a base tree...
M src/tests/issues.targets
Falling back to patching base and 3-way merge...
Auto-merging src/tests/issues.targets
CONFLICT (content): Merge conflict in src/tests/issues.targets
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0004 Exclude newly added test from wasm
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128Please backport manually! |
@fanyang-mono an error occurred while backporting to release/6.0-staging, please check the run log for details! Error: git am failed, most likely due to a merge conflict. |
Fixes: #90955
This is an existing bug for both JIT and AOT. Basically, the issue was that the offset from one layer above wasn't carried on when calculating offset for nested struct fields. I've added a test to lock down this behavior and verified that the test failed without this change.