-
Notifications
You must be signed in to change notification settings - Fork 785
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
Extra tail instruction corrupts stack? #13447
Comments
Sounds like JIT is kicking in and something breaks, need to look at codegen. I presume it works with --optimize- and/or --tailcalls-? |
Also, does it reproduce on net5? |
Yup, works with optimizations, but tailcalls disabled. Don't know about 5, don't have it installed, sorry. |
Yeah, gonna test it. It is a bit weird bug tbh. |
Hm, it seems I cannot reproduce it on
(didn't include logs for other TFMs, since they're the same) Same goes with
@kerams what SDK versions do you see this bug on? |
I see the problem in your console dump :)
The last byte should always be |
Hold on, I misunderstood the issue, sorry. |
Smells like a codegen bug, I can't repro it when I remove that ; Assembly listing for method Write:writeString(String,Stream)
G_M26705_IG01: ;; offset=0000H
55 push rbp
4157 push r15
4156 push r14
4154 push r12
57 push rdi
56 push rsi
53 push rbx
4883EC60 sub rsp, 96
488D6C2420 lea rbp, [rsp+20H]
C5D857E4 vxorps xmm4, xmm4
C5F97F6510 vmovdqa xmmword ptr [rbp+10H], xmm4
C5F97F6520 vmovdqa xmmword ptr [rbp+20H], xmm4
C5F97F6530 vmovdqa xmmword ptr [rbp+30H], xmm4
48B80AD0F514A8090000 mov rax, 0x9A814F5D00A
48894508 mov qword ptr [rbp+08H], rax
488BF1 mov rsi, rcx
488BFA mov rdi, rdx
;; size=58 bbWeight=1 PerfScore 15.83
G_M26705_IG02: ;; offset=003AH
48B9C80B4088C1010000 mov rcx, 0x1C188400BC8 ; const ptr
488B19 mov rbx, gword ptr [rcx]
488BCB mov rcx, rbx
8B5608 mov edx, dword ptr [rsi+8]
FF15C5DB1100 call [UTF8EncodingSealed:GetMaxByteCount(int):int:this]
448BF0 mov r14d, eax
4585F6 test r14d, r14d
741E je SHORT G_M26705_IG04
4183C60F add r14d, 15
49C1EE04 shr r14, 4
4883C420 add rsp, 32
;; size=45 bbWeight=1 PerfScore 10.00
G_M26705_IG03: ;; offset=0067H
6A00 push 0
6A00 push 0
49FFCE dec r14
75F7 jne SHORT G_M26705_IG03
4883EC20 sub rsp, 32
4C8D742420 lea r14, [rsp+20H]
;; size=18 bbWeight=1 PerfScore 4.00
G_M26705_IG04: ;; offset=0079H
85C0 test eax, eax
0F8C11010000 jl G_M26705_IG09
;; size=8 bbWeight=1 PerfScore 1.25
G_M26705_IG05: ;; offset=0081H
448BF8 mov r15d, eax
488BCB mov rcx, rbx
488D560C lea rdx, bword ptr [rsi+12]
448B4608 mov r8d, dword ptr [rsi+8]
48895530 mov bword ptr [rbp+30H], rdx
44894538 mov dword ptr [rbp+38H], r8d
4C897520 mov bword ptr [rbp+20H], r14
44897D28 mov dword ptr [rbp+28H], r15d
488D5530 lea rdx, [rbp+30H]
4C8D4520 lea r8, [rbp+20H]
FF15D3D71100 call [UTF8Encoding:GetBytes(ReadOnlySpan`1,Span`1):int:this]
8BF0 mov esi, eax
48B96001F908FB7F0000 mov rcx, 0x7FFB08F90160 ; PrintfFormat`5
E8C264CF5F call CORINFO_HELP_NEWSFAST
488BD8 mov rbx, rax
48BA28924088C1010000 mov rdx, 0x1C188409228 ; "Byte - %d, in total %d"
488B12 mov rdx, gword ptr [rdx]
488D4B08 lea rcx, bword ptr [rbx+8]
E83972E2FF call CORINFO_HELP_ASSIGN_REF
33C0 xor rax, rax
48894310 mov gword ptr [rbx+16], rax
48894318 mov gword ptr [rbx+24], rax
FF15E1461D00 call [Console:get_Out():TextWriter]
488BD0 mov rdx, rax
4C8BC3 mov r8, rbx
48B9A8FFF808FB7F0000 mov rcx, 0x7FFB08F8FFA8 ; PrintfModule:PrintFormatLineToTextWriter(TextWriter,PrintfFormat`4):FSharpFunc`2
FF1523661D00 call [PrintfModule:PrintFormatLineToTextWriter(TextWriter,PrintfFormat`4):__Canon]
488BD8 mov rbx, rax
48B96802F908FB7F0000 mov rcx, 0x7FFB08F90268 ; writeString@63
E87164CF5F call CORINFO_HELP_NEWSFAST
4C8BE0 mov r12, rax
498D4C2408 lea rcx, bword ptr [r12+8]
488BD3 mov rdx, rbx
E8F171E2FF call CORINFO_HELP_ASSIGN_REF
4585FF test r15d, r15d
7475 je SHORT G_M26705_IG10
450FB606 movzx r8, byte ptr [r14]
448BCE mov r9d, esi
498BD4 mov rdx, r12
48B9E005F908FB7F0000 mov rcx, 0x7FFB08F905E0 ; FSharpFunc`2:InvokeFast(FSharpFunc`2,ubyte,int):Unit
FF15DA644000 call [FSharpFunc`2:InvokeFast(FSharpFunc`2,ubyte,int):__Canon]
413BF7 cmp esi, r15d
774F ja SHORT G_M26705_IG09
;; size=194 bbWeight=1.00 PerfScore 37.25
G_M26705_IG06: ;; offset=0143H
488BCF mov rcx, rdi
4C897510 mov bword ptr [rbp+10H], r14
897518 mov dword ptr [rbp+18H], esi
488D5510 lea rdx, [rbp+10H]
E852354400 call ILStubClass:IL_STUB_StoreTailCallArgs(Object,ReadOnlySpan`1)
488D4D78 lea rcx, bword ptr [rbp+78H]
48BAC0C3FA08FB7F0000 mov rdx, 0x7FFB08FAC3C0 ; code for ILStubClass:IL_STUB_CallTailCallTarget
4533C0 xor r8d, r8d
FF1543ACFCFF call [RuntimeHelpers:DispatchTailCalls(long,long,long)]
48B90AD0F514A8090000 mov rcx, 0x9A814F5D00A
48394D08 cmp qword ptr [rbp+08H], rcx
7405 je SHORT G_M26705_IG07
E89E6BA95F call CORINFO_HELP_FAIL_FAST
;; size=63 bbWeight=1 PerfScore 12.00
G_M26705_IG07: ;; offset=0182H
90 nop
;; size=1 bbWeight=1 PerfScore 0.25
G_M26705_IG08: ;; offset=0183H
488D6540 lea rsp, [rbp+40H]
5B pop rbx
5E pop rsi
5F pop rdi
415C pop r12
415E pop r14
415F pop r15
5D pop rbp
C3 ret
;; size=15 bbWeight=1 PerfScore 5.00
G_M26705_IG09: ;; offset=0192H
FF15B8453E00 call [ThrowHelper:ThrowArgumentOutOfRangeException()]
CC int3
;; size=7 bbWeight=0 PerfScore 0.00
G_M26705_IG10: ;; offset=0199H
E84260A95F call CORINFO_HELP_RNGCHKFAIL
CC int3
;; size=6 bbWeight=0 PerfScore 0.00
; Total bytes of code 415, prolog size 52, PerfScore 127.48, instruction count 106, allocated bytes for code 419 (MethodHash=f10797ae) for method Write:writeString(String,Stream)
; ============================================================ @jakobbotsch does it ring a bell to you? |
The call is passing a |
@EgorBo @jakobbotsch thanks for looking into it! Fix should be straightforward, we can start from here: fsharp/src/Compiler/CodeGen/IlxGen.fs Lines 4319 to 4352 in 2c8ebd6
|
Looking at https://github.com/kerams/repro/blob/master/src/FableBinarySerializationBug.Server/Program.fs#L60 this is not really a bug, though we still might decide to fix it. stackalloc is just "dragons beware" in F#. It gives a warning saying that. We could perhaps turn off tailcalls from any method using stackalloc (or give a late codgen warning or error when that occurs). But really, if you use stackalloc you have to heed the warning. We can extend the warning to talk about tailcalls |
To be safe, we can skip emitting
Additionally, we can also put Update: it already has the attribute, nvm wdyt @dsyme? |
Right, but as far as I am concerned, the warning implies that the compiler won't prevent me from shooting myself in the foor here, which is fair enough. The warning in no way makes it clear that all bets are off and the F# compiler might shoot me in the foot. The equivalent C# code is perfectly safe (the function does not limit the amount of bytes that are going to be allocated to make the repro simpler). |
Yeah, C# is not emitting I personally think that we need to do both - a warning, implying that Update: it already has the attribute, nvm |
Interestingly, I am not able to reproduce the behavior on Mac. For both
FYI @EgorBo |
The error will only surface if |
Afaik, tests were done on Intel Mac, not arm one. |
The same argument applies, SysV ABI (used on Intel Macs) probably does not need the portable tailcall helper for the call to |
Repro steps
Clone https://github.com/kerams/repro/, open Program.fs, target either
net6.0
ornet7.0
, run in Release mode.Expected behavior
Printed 10 times.
Actual behavior
This is printed the first 2 times:
Known workarounds
Remove
[<Struct>]
fromMyResult
.OR
Put
()
at the end of thewriteString
function. The before and after diff is here. That extra tail instruction is a bit suspicious.Related information
dotnet --info
.NET SDK: Version: 7.0.100-preview.5.22307.18 Commit: bd8b088037Runtime Environment:
OS Name: Windows
OS Version: 10.0.19044
OS Platform: Windows
RID: win10-x64
Base Path: C:\Program Files\dotnet\sdk\7.0.100-preview.5.22307.18\
global.json file:
Not found
Host:
Version: 7.0.0-preview.5.22301.12
Architecture: x64
Commit: 425fedc0fb
.NET SDKs installed:
6.0.400-preview.22301.10 [C:\Program Files\dotnet\sdk]
7.0.100-preview.5.22307.18 [C:\Program Files\dotnet\sdk]
.NET runtimes installed:
Microsoft.AspNetCore.App 6.0.5 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 7.0.0-preview.5.22303.8 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.NETCore.App 6.0.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 7.0.0-preview.5.22301.12 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.WindowsDesktop.App 6.0.5 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 7.0.0-preview.5.22302.5 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
The text was updated successfully, but these errors were encountered: