Repository navigation
Consider that retbuf arg can point to GC heap in fgCreateCallDispatcherAndGetResult() #39815
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
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
81252a1
Add regression test for https://github.com/dotnet/runtime/issues/39581
echesakov 9172d38
Revert "Disable Vector tests failing under GCStress (#39580)"
echesakov bb6998c
Copy to temporary buffer on stack if a struct can be allocated on a G…
echesakov 771d80a
Address feedback in Runtime_39581.il
echesakov 7dcc320
Address feedback in morph.cpp
echesakov 7f8b608
Merge branch 'master' into Runtime_39581
echesakov File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
174 changes: 174 additions & 0 deletions
174
src/tests/JIT/Regression/JitBlue/Runtime_39581/Runtime_39581.il
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,174 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // | ||
| // The test exposes the issue when: | ||
| // | ||
| // 1) methods Callee and Caller have the following structure: | ||
| // | ||
| // S Callee() { produce and return an instance of S } | ||
| // | ||
| // S Caller() { return Callee(); } | ||
| // | ||
| // 2) S is a value type with non-GC fields and has such size that it | ||
| // must be passed via return buffer; | ||
| // | ||
| // 3) call to Callee is transformed to tail call via helper: | ||
| // | ||
| // S result; | ||
| // DispatchTailCalls(&IL_STUB_CallTailCallTarget, (IntPtr)&result, _AddressOfReturnAddress()); | ||
| // return result; | ||
| // | ||
| // 4) Caller is invoked via Reflection. | ||
| // | ||
| // During morph phase the JIT discovers that both Caller and Callee have the return buffer arguments and | ||
| // falsely assumes that the value can be directly passed from Caller to Callee. | ||
| // Here is a problem: DispatchTailCalls helper expects the RetValue argument (in this case, return buffer argument) | ||
| // to always point to the stack. However, during a reflection call the return buffer for S value type can be placed on the GC heap (see condition 2 above). | ||
| // As a consequence, when GC is called at DispatchTailCalls the object corresponding to the return value buffer can be moved, | ||
| // but the pointers passed to DispatchTailCalls will not be updated that leads to Callee using the non-updated location when writing its return value. | ||
| // | ||
| // Note: you will find below that Caller uses localloc - this is done in order to prevent a call to Caller to be transformed into fast tail call. | ||
| // Note: COMPlus_GCStress=3 or COMPlus_GCStress=C are required in order to reliably expose this issue. | ||
|
|
||
| .assembly extern System.Runtime | ||
| { | ||
| } | ||
|
|
||
| .assembly Runtime_39581 | ||
| { | ||
| } | ||
|
|
||
| .class private sequential ansi sealed beforefieldinit int32x8 | ||
| extends [System.Runtime]System.ValueType | ||
| { | ||
| .field public int32 a | ||
| .field public int32 b | ||
| .field public int32 c | ||
| .field public int32 d | ||
| .field public int32 e | ||
| .field public int32 f | ||
| .field public int32 g | ||
| .field public int32 h | ||
| } | ||
|
|
||
| .class private auto ansi beforefieldinit Runtime_39581.Program | ||
| extends [System.Runtime]System.Object | ||
| { | ||
| .method private hidebysig static valuetype int32x8 Callee(uint8* arg0) cil managed noinlining | ||
| { | ||
| .maxstack 2 | ||
| .locals init (valuetype int32x8 V_0) | ||
| IL_0000: ldloca.s V_0 | ||
| IL_0002: initobj int32x8 | ||
| IL_0008: ldloca.s V_0 | ||
| IL_000a: ldc.i4.0 | ||
| IL_000b: stfld int32 int32x8::a | ||
| IL_0010: ldloca.s V_0 | ||
| IL_0012: ldc.i4.1 | ||
| IL_0013: stfld int32 int32x8::b | ||
| IL_0018: ldloca.s V_0 | ||
| IL_001a: ldc.i4.2 | ||
| IL_001b: stfld int32 int32x8::c | ||
| IL_0020: ldloca.s V_0 | ||
| IL_0022: ldc.i4.3 | ||
| IL_0023: stfld int32 int32x8::d | ||
| IL_0028: ldloca.s V_0 | ||
| IL_002a: ldc.i4.4 | ||
| IL_002b: stfld int32 int32x8::e | ||
| IL_0030: ldloca.s V_0 | ||
| IL_0032: ldc.i4.5 | ||
| IL_0033: stfld int32 int32x8::f | ||
| IL_0038: ldloca.s V_0 | ||
| IL_003a: ldc.i4.6 | ||
| IL_003b: stfld int32 int32x8::g | ||
| IL_0040: ldloca.s V_0 | ||
| IL_0042: ldc.i4.7 | ||
| IL_0043: stfld int32 int32x8::h | ||
| IL_0048: ldloc.0 | ||
| IL_0049: ret | ||
| } | ||
|
|
||
| .method public hidebysig static valuetype int32x8 Caller(int32 arg0) cil managed noinlining | ||
| { | ||
| .maxstack 1 | ||
| IL_0000: ldarg.0 | ||
| IL_0001: conv.u | ||
| IL_0002: localloc | ||
| IL_0004: tail. | ||
| IL_0006: call valuetype int32x8 Runtime_39581.Program::Callee(uint8*) | ||
| IL_000b: ret | ||
| } | ||
|
|
||
| .method private hidebysig static int32 Main(string[] args) cil managed | ||
| { | ||
| .entrypoint | ||
| .maxstack 6 | ||
| .locals init (valuetype int32x8 V_0) | ||
| IL_0000: ldtoken Runtime_39581.Program | ||
| IL_0005: call class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle) | ||
| IL_000a: ldstr "Caller" | ||
| IL_000f: ldc.i4.1 | ||
| IL_0010: newarr [System.Runtime]System.Type | ||
| IL_0015: dup | ||
| IL_0016: ldc.i4.0 | ||
| IL_0017: ldtoken [System.Runtime]System.Int32 | ||
| IL_001c: call class [System.Runtime]System.Type [System.Runtime]System.Type::GetTypeFromHandle(valuetype [System.Runtime]System.RuntimeTypeHandle) | ||
| IL_0021: stelem.ref | ||
| IL_0022: call instance class [System.Runtime]System.Reflection.MethodInfo [System.Runtime]System.Type::GetMethod(string, class [System.Runtime]System.Type[]) | ||
| IL_0027: ldnull | ||
| IL_0028: ldc.i4.1 | ||
| IL_0029: newarr [System.Runtime]System.Object | ||
| IL_002e: dup | ||
| IL_002f: ldc.i4.0 | ||
| IL_0030: ldc.i4.1 | ||
| IL_0031: box [System.Runtime]System.Int32 | ||
| IL_0036: stelem.ref | ||
| IL_0037: callvirt instance object [System.Runtime]System.Reflection.MethodBase::Invoke(object, object[]) | ||
| IL_003c: unbox.any int32x8 | ||
| IL_0041: stloc.0 | ||
| IL_0042: ldloc.0 | ||
| IL_0043: ldfld int32 int32x8::a | ||
| IL_0048: brtrue.s IL_008c | ||
|
|
||
| IL_004a: ldloc.0 | ||
| IL_004b: ldfld int32 int32x8::b | ||
| IL_0050: ldc.i4.1 | ||
| IL_0051: bne.un.s IL_008c | ||
|
|
||
| IL_0053: ldloc.0 | ||
| IL_0054: ldfld int32 int32x8::c | ||
| IL_0059: ldc.i4.2 | ||
| IL_005a: bne.un.s IL_008c | ||
|
|
||
| IL_005c: ldloc.0 | ||
| IL_005d: ldfld int32 int32x8::d | ||
| IL_0062: ldc.i4.3 | ||
| IL_0063: bne.un.s IL_008c | ||
|
|
||
| IL_0065: ldloc.0 | ||
| IL_0066: ldfld int32 int32x8::e | ||
| IL_006b: ldc.i4.4 | ||
| IL_006c: bne.un.s IL_008c | ||
|
|
||
| IL_006e: ldloc.0 | ||
| IL_006f: ldfld int32 int32x8::f | ||
| IL_0074: ldc.i4.5 | ||
| IL_0075: bne.un.s IL_008c | ||
|
|
||
| IL_0077: ldloc.0 | ||
| IL_0078: ldfld int32 int32x8::g | ||
| IL_007d: ldc.i4.6 | ||
| IL_007e: bne.un.s IL_008c | ||
|
|
||
| IL_0080: ldloc.0 | ||
| IL_0081: ldfld int32 int32x8::h | ||
| IL_0086: ldc.i4.7 | ||
| IL_0087: bne.un.s IL_008c | ||
|
|
||
| IL_0089: ldc.i4.s 100 | ||
| IL_008b: ret | ||
|
|
||
| IL_008c: ldc.i4.0 | ||
| IL_008d: ret | ||
| } | ||
| } |
13 changes: 13 additions & 0 deletions
13
src/tests/JIT/Regression/JitBlue/Runtime_39581/Runtime_39581.ilproj
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| <Project Sdk="Microsoft.NET.Sdk.IL"> | ||
| <PropertyGroup> | ||
| <OutputType>Exe</OutputType> | ||
| </PropertyGroup> | ||
| <PropertyGroup> | ||
| <DebugType>None</DebugType> | ||
| <Optimize>True</Optimize> | ||
| <AllowUnsafeBlocks>true</AllowUnsafeBlocks> | ||
| </PropertyGroup> | ||
| <ItemGroup> | ||
| <Compile Include="$(MSBuildProjectName).il" /> | ||
| </ItemGroup> | ||
| </Project> |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jakobbotsch Do you remember why this handling was needed here? I understand that on System-V the method that uses return buffer must return an address to the return buffer in rax at the end. But do we use the value in the caller somehow?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if JIT uses the returned pointer, but even if not it is presumably necessary for reverse pinvoke.
This is already what the JIT does for functions with ret buffers so I just did the same.