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

Assertion failed 'lastBlockILEndOffset < beginOffs' #99543

Closed
filipnavara opened this issue Mar 11, 2024 · 19 comments · Fixed by #99662
Closed

Assertion failed 'lastBlockILEndOffset < beginOffs' #99543

filipnavara opened this issue Mar 11, 2024 · 19 comments · Fixed by #99662
Assignees
Labels
arch-x64 arch-x86 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone

Comments

@filipnavara
Copy link
Member

Building Loader\classloader\StaticVirtualMethods\GenericContext\GenericContextTest in NativeAOT win-x86 Debug mode yields:

ILC: D:\runtime\src\coreclr\jit\scopeinfo.cpp:1566
ILC: D:\runtime\src\coreclr\jit\scopeinfo.cpp:1566
ILC: Assertion failed 'lastBlockILEndOffset < beginOffs' in 'TestEntrypoint:Test_Ldftn_NonGenericNonGenericValuetype_GenericOverString_GenericOverString_GenericMethodOverString[System.__Canon]()' during 'Generate code' (IL size 47; hash 0xab6c92eb; MinOpts)

ILC: Assertion failed 'lastBlockILEndOffset < beginOffs' in 'TestEntrypoint:Test_Ldftn_GenericOverReferenceType_ClassAGenericValuetype_GenericOverInt32_GenericOverObject_NormalMethod[System.__Canon]()' during 'Generate code' (IL size 47; hash 0x89d04558; MinOpts)

ILC: D:\runtime\src\coreclr\jit\scopeinfo.cpp:1566
ILC: Assertion failed 'lastBlockILEndOffset < beginOffs' in 'TestEntrypoint:Test_Ldftn_GenericOverReferenceType_ClassAGenericClass_GenericOverString_GenericOverString_NormalMethod[System.__Canon]()' during 'Generate code' (IL size 47; hash 0xa314cc02; MinOpts)

Native stack trace:

[0x0]   KERNELBASE!wil::details::DebugBreak+0x2   0x3ba503b388   0x7ffd9f8eb9e9   
[0x1]   clrjit_win_aot_x86_x64!assertAbort+0x249   0x3ba503b390   0x7ffd9fb6c5c4   
[0x2]   clrjit_win_aot_x86_x64!CodeGen::siOpenScopesForNonTrackedVars+0xa4   0x3ba503d4c0   0x7ffd9fb6be2b   
[0x3]   clrjit_win_aot_x86_x64!CodeGen::siBeginBlock+0x15b   0x3ba503d510   0x7ffd9f8a2358   
[0x4]   clrjit_win_aot_x86_x64!CodeGen::genCodeForBBlist+0xaf8   0x3ba503d550   0x7ffd9f8969c3   
[0x5]   clrjit_win_aot_x86_x64!CodeGen::genGenerateMachineCode+0x683   0x3ba503da00   0x7ffd9f88a6cc   
[0x6]   clrjit_win_aot_x86_x64!CodeGenPhase::DoPhase+0x2c   0x3ba503da60   0x7ffd9fb3b157   
[0x7]   clrjit_win_aot_x86_x64!Phase::Run+0x87   0x3ba503daa0   0x7ffd9f88a688   
[0x8]   clrjit_win_aot_x86_x64!DoPhase+0x58   0x3ba503db20   0x7ffd9f89627b   

JIT dump for TestEntrypoint:Test_Ldftn_GenericOverTypeParameterGenericValuetype_GenericOverString_CuriouslyRecurringGeneric_GenericMethodOverTypeParameter:
GenericContextTest.txt

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 11, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Mar 11, 2024
@JulieLeeMSFT JulieLeeMSFT added this to the 9.0.0 milestone Mar 12, 2024
@JulieLeeMSFT
Copy link
Member

@TIHan PTAL.

@TIHan
Copy link
Contributor

TIHan commented Mar 12, 2024

Will do

@JulieLeeMSFT JulieLeeMSFT removed the untriaged New issue has not been triaged by the area owner label Mar 12, 2024
@TIHan
Copy link
Contributor

TIHan commented Mar 12, 2024

For Loader\classloader\StaticVirtualMethods\GenericContext\GenericContextTest, assertion also occurs in x64 debug:

 Number of test projects in group 1: 0
  Number of test projects in group 2: 0
  Building managed test group 2: "c:\work\runtime\\dotnet.cmd" msbuild c:\work\runtime\src\tests\build.proj /t:Build "/p:TargetArchitecture=x64" "/p:Configuration=Debug" "/p:LibrariesConfiguration=Release" "/p:TasksConfiguration=Debug" "/p:TargetOS=windows" "/p:ToolsOS
  =" "/p:PackageOS=" "/p:RuntimeFlavor=coreclr" "/p:RuntimeVariant=" "/p:CLRTestBuildAllTargets=" "/p:UseCodeFlowEnforcement=" "/p:__TestGroupToBuild=2" "/p:__SkipRestorePackages=1" /nodeReuse:false /maxcpucount /bl:c:\work\runtime\artifacts\/log/Debug/InnerManagedTest
  Build.2.binlog "/p:DefaultBuildAllTarget=BuildNativeAot"
  C:\work\runtime\.dotnet
  MSBuild version 17.10.0-preview-24101-01+07fd5d51f for .NET

    Number of test projects in group 2: 0
  Number of test projects in group 3: 2
  Building managed test group 3: "c:\work\runtime\\dotnet.cmd" msbuild c:\work\runtime\src\tests\build.proj /t:Build "/p:TargetArchitecture=x64" "/p:Configuration=Debug" "/p:LibrariesConfiguration=Release" "/p:TasksConfiguration=Debug" "/p:TargetOS=windows" "/p:ToolsOS
  =" "/p:PackageOS=" "/p:RuntimeFlavor=coreclr" "/p:RuntimeVariant=" "/p:CLRTestBuildAllTargets=" "/p:UseCodeFlowEnforcement=" "/p:__TestGroupToBuild=3" "/p:__SkipRestorePackages=1" /nodeReuse:false /maxcpucount /bl:c:\work\runtime\artifacts\/log/Debug/InnerManagedTest
  Build.3.binlog "/p:DefaultBuildAllTarget=BuildNativeAot"
  C:\work\runtime\.dotnet
  MSBuild version 17.10.0-preview-24101-01+07fd5d51f for .NET

    Number of test projects in group 3: 2
    ILLink.RoslynAnalyzer -> c:\work\runtime\artifacts\bin\ILLink.RoslynAnalyzer\Debug\netstandard2.0\ILLink.RoslynAnalyzer.dll
    ILLink.CodeFixProvider -> c:\work\runtime\artifacts\bin\ILLink.CodeFixProvider\Debug\netstandard2.0\ILLink.CodeFixProvider.dll
    Mono.Linker -> c:\work\runtime\artifacts\bin\Mono.Linker\ref\Debug\net9.0\illink.dll
    Mono.Linker -> c:\work\runtime\artifacts\bin\Mono.Linker\Debug\net9.0\illink.dll
    ILLink.Tasks -> c:\work\runtime\artifacts\bin\ILLink.Tasks\Debug\net9.0\ILLink.Tasks.dll
    GenericContextCommonAndImplementation -> c:\work\runtime\artifacts\tests\coreclr\windows.x64.Debug\Loader\classloader\StaticVirtualMethods\GenericContext\GenericContextTest\GenericContextCommonAndImplementation\GenericContextCommonAndImplementation.dll
    GenericContextCommonCs -> c:\work\runtime\artifacts\tests\coreclr\windows.x64.Debug\Loader\classloader\StaticVirtualMethods\GenericContext\GenericContextCommonCs\GenericContextCommonCs.dll
    GenericContextTest -> c:\work\runtime\artifacts\tests\coreclr\windows.x64.Debug\Loader\classloader\StaticVirtualMethods\GenericContext\GenericContextTest\GenericContextTest\GenericContextTest.dll
    Generating native code
    ILC: C:\work\runtime\src\coreclr\jit\scopeinfo.cpp:1566
    ILC: C:\work\runtime\src\coreclr\jit\scopeinfo.cpp:1566
    ILC: Assertion failed 'lastBlockILEndOffset < beginOffs' in 'TestEntrypoint:Test_Ldftn_GenericOverStructGenericValuetype_GenericOverString_GenericOverString_GenericMethodOverString[System.__Canon]()' during 'Generate code' (IL size 47; hash 0x23ad0f52; MinOpts)

    ILC: Assertion failed 'lastBlockILEndOffset < beginOffs' in 'TestEntrypoint:Test_Ldftn_NonGenericNonGenericValuetype_GenericOverInt32_CuriouslyRecurringGeneric_GenericMethodOverString[System.__Canon]()' during 'Generate code' (IL size 47; hash 0x49a2e267; MinOpts)

    ILC: C:\work\runtime\src\coreclr\jit\scopeinfo.cpp:1566
    ILC: Assertion failed 'lastBlockILEndOffset < beginOffs' in 'TestEntrypoint:Test_Ldftn_GenericOverReferenceType_ClassAGenericValuetype_GenericOverConstrainedType_NonGeneric_NormalMethod[System.__Canon,System.__Canon]()' during 'Generate code' (IL size 47; hash 0x71
  0bd4fc; MinOpts)

    ILC: C:\work\runtime\src\coreclr\jit\scopeinfo.cpp:1566
    ILC: Assertion failed 'lastBlockILEndOffset < beginOffs' in 'TestEntrypoint:Test_Ldftn_GenericOverGenericStructOverTypeParameterGenericValuetype_GenericOverInt32_GenericOverObject_NormalMethod[System.__Canon]()' during 'Generate code' (IL size 47; hash 0x00be97a2;
  MinOpts)

    ILC: C:\work\runtime\src\coreclr\jit\scopeinfo.cpp:1566
    ILC: Assertion failed 'lastBlockILEndOffset < beginOffs' in 'TestEntrypoint:Test_Ldftn_GenericOverStructGenericClass_GenericOverInt32_GenericOverObject_GenericMethodOverTypeParameter[System.__Canon]()' during 'Generate code' (IL size 47; hash 0x9e4e8b0c; MinOpts)

    ILC: C:\work\runtime\src\coreclr\jit\scopeinfo.cpp:1566
    ILC: Assertion failed 'lastBlockILEndOffset < beginOffs' in 'TestEntrypoint:Test_Ldftn_GenericOverStructGenericClass_GenericOverConstrainedType_CuriouslyRecurringGeneric_GenericMethodOverInt[System.__Canon,System.__Canon]()' during 'Generate code' (IL size 47; hash
   0x9a477855; MinOpts)

    ILC: C:\work\runtime\src\coreclr\jit\scopeinfo.cpp:1566
    ILC: Assertion failed 'lastBlockILEndOffset < beginOffs' in 'TestEntrypoint:Test_Ldftn_NonGenericNonGenericClass_GenericOverInt32_NonGeneric_GenericMethodOverTypeParameter[System.__Canon]()' during 'Generate code' (IL size 47; hash 0xd3a24e21; MinOpts)

    ILC: C:\work\runtime\src\coreclr\jit\scopeinfo.cpp:1566
    ILC: Assertion failed 'lastBlockILEndOffset < beginOffs' in 'TestEntrypoint:Test_Ldftn_GenericOverReferenceType_ClassAGenericClass_GenericOverString_GenericOverString_GenericMethodOverInt[System.__Canon]()' during 'Generate code' (IL size 47; hash 0xf4ad2bdd; MinOp
  ts)

    ILC: C:\work\runtime\src\coreclr\jit\scopeinfo.cpp:1566
    ILC: Assertion failed 'lastBlockILEndOffset < beginOffs' in 'TestEntrypoint:Test_Ldftn_GenericOverTypeParameterGenericClass_GenericOverInt32_GenericOverString_GenericMethodOverTypeParameter[System.__Canon]()' during 'Generate code' (IL size 47; hash 0x5d1c83aa; Min
  Opts)

    ILC: C:\work\runtime\src\coreclr\jit\scopeinfo.cpp:1566
    ILC: Assertion failed 'lastBlockILEndOffset < beginOffs' in 'TestEntrypoint:Test_Ldftn_GenericOverStructGenericValuetype_GenericOverConstrainedType_NonGeneric_GenericMethodOverInt[System.__Canon,System.__Canon]()' during 'Generate code' (IL size 47; hash 0xff512ad7
  ; MinOpts)

    ILC: C:\work\runtime\src\coreclr\jit\scopeinfo.cpp:1566
    ILC: Assertion failed 'lastBlockILEndOffset < beginOffs' in 'TestEntrypoint:Test_Ldftn_NonGenericNonGenericClass_GenericOverConstrainedType_GenericOverString_GenericMethodOverInt[System.__Canon,System.__Canon]()' during 'Generate code' (IL size 62; hash 0xa570c667;
   MinOpts)

    ILC: C:\work\runtime\src\coreclr\jit\scopeinfo.cpp:1566
    ILC: Assertion failed 'lastBlockILEndOffset < beginOffs' in 'TestEntrypoint:Test_Ldftn_GenericOverGenericStructOverTypeParameterGenericClass_GenericOverString_CuriouslyRecurringGeneric_GenericMethodOverTypeParameter[System.__Canon]()' during 'Generate code' (IL siz
  e 47; hash 0x2002870e; MinOpts)

    ILC: C:\work\runtime\src\coreclr\jit\scopeinfo.cpp:1566
    ILC: Assertion failed 'lastBlockILEndOffset < beginOffs' in 'TestEntrypoint:Test_Ldftn_GenericOverReferenceType_ClassAGenericValuetype_GenericOverConstrainedType_GenericOverObject_GenericMethodOverTypeParameter[System.__Canon,System.__Canon]()' during 'Generate cod
  e' (IL size 62; hash 0xd6c990b5; MinOpts)

    ILC: C:\work\runtime\src\coreclr\jit\scopeinfo.cpp:1566
    ILC: Assertion failed 'lastBlockILEndOffset < beginOffs' in 'TestEntrypoint:Test_Ldftn_GenericOverTypeParameterGenericValuetype_GenericOverString_NonGeneric_GenericMethodOverString[System.__Canon]()' during 'Generate code' (IL size 47; hash 0xd4f2af40; MinOpts)

    ILC: C:\work\runtime\src\coreclr\jit\scopeinfo.cpp:1566
    ILC: Assertion failed 'lastBlockILEndOffset < beginOffs' in 'TestEntrypoint:Test_Ldftn_NonGenericNonGenericValuetype_GenericOverString_GenericOverObject_GenericMethodOverInt[System.__Canon]()' during 'Generate code' (IL size 47; hash 0x4893fb2d; MinOpts)

    ILC: C:\work\runtime\src\coreclr\jit\scopeinfo.cpp:1566
    ILC: Assertion failed 'lastBlockILEndOffset < beginOffs' in 'TestEntrypoint:Test_Ldftn_GenericOverTypeParameterGenericValuetype_GenericOverString_CuriouslyRecurringGeneric_GenericMethodOverInt[System.__Canon]()' during 'Generate code' (IL size 47; hash 0x51a8bed9;
  MinOpts)

    ILC: C:\work\runtime\src\coreclr\jit\scopeinfo.cpp:1566
    ILC: Assertion failed 'lastBlockILEndOffset < beginOffs' in 'TestEntrypoint:Test_Ldftn_GenericOverReferenceType_ClassAGenericValuetype_GenericOverInt32_GenericOverObject_GenericMethodOverInt[System.__Canon]()' during 'Generate code' (IL size 47; hash 0x9547e707; Mi
  nOpts)

    ILC: C:\work\runtime\src\coreclr\jit\scopeinfo.cpp:1566
    ILC: Assertion failed 'lastBlockILEndOffset < beginOffs' in 'TestEntrypoint:Test_Ldftn_GenericOverGenericStructOverTypeParameterGenericValuetype_GenericOverString_NonGeneric_GenericMethodOverTypeParameter[System.__Canon]()' during 'Generate code' (IL size 47; hash
  0xd82f01cc; MinOpts)

    ILC: C:\work\runtime\src\coreclr\jit\scopeinfo.cpp:1566
    ILC: Assertion failed 'lastBlockILEndOffset < beginOffs' in 'TestEntrypoint:Test_Ldftn_NonGenericNonGenericValuetype_GenericOverConstrainedType_CuriouslyRecurringGeneric_GenericMethodOverTypeParameter[System.__Canon,System.__Canon]()' during 'Generate code' (IL siz
  e 47; hash 0x927f6c73; MinOpts)

    ILC: C:\work\runtime\src\coreclr\jit\scopeinfo.cpp:1566
    ILC: Assertion failed 'lastBlockILEndOffset < beginOffs' in 'TestEntrypoint:Test_Ldftn_NonGenericNonGenericClass_GenericOverConstrainedType_CuriouslyRecurringGeneric_GenericMethodOverTypeParameter[System.__Canon,System.__Canon]()' during 'Generate code' (IL size 47
  ; hash 0xd30ca96e; MinOpts)

    ILC: C:\work\runtime\src\coreclr\jit\scopeinfo.cpp:1566
    ILC: Assertion failed 'lastBlockILEndOffset < beginOffs' in 'TestEntrypoint:Test_Ldftn_GenericOverGenericStructOverTypeParameterGenericValuetype_GenericOverString_CuriouslyRecurringGeneric_GenericMethodOverTypeParameter[System.__Canon]()' during 'Generate code' (IL
   size 47; hash 0xd2d11753; MinOpts)

    ILC: C:\work\runtime\src\coreclr\jit\scopeinfo.cpp:1566
    ILC: Assertion failed 'lastBlockILEndOffset < beginOffs' in 'TestEntrypoint:Test_Ldftn_GenericOverTypeParameterGenericValuetype_GenericOverString_CuriouslyRecurringGeneric_GenericMethodOverString[System.__Canon]()' during 'Generate code' (IL size 47; hash 0x03b569b
  f; MinOpts)

    ILC: C:\work\runtime\src\coreclr\jit\scopeinfo.cpp:1566
    ILC: Assertion failed 'lastBlockILEndOffset < beginOffs' in 'TestEntrypoint:Test_Ldftn_GenericOverTypeParameterGenericValuetype_GenericOverString_CuriouslyRecurringGeneric_GenericMethodOverTypeParameter[System.__Canon]()' during 'Generate code' (IL size 47; hash 0x
  1f1113fb; MinOpts)

@TIHan
Copy link
Contributor

TIHan commented Mar 12, 2024

@filipnavara , your JIT dump link is giving a 404 - I guess GitHub got messed up?

@TIHan TIHan added the arch-x64 label Mar 12, 2024
@TIHan
Copy link
Contributor

TIHan commented Mar 13, 2024

This assertion occurs even in a December 2023 commit - so it's likely been this way for a while.

@TIHan
Copy link
Contributor

TIHan commented Mar 13, 2024

Dump from one of the x64 assertions:
dump.txt

Edit: Ok, so GitHub is showing 404 for 'dump.txt' even when uploading new files.

@filipnavara
Copy link
Member Author

I suspected it's not x86 specific. Thanks for looking into it.

@AndyAyersMS
Copy link
Member

@TIHan can you give a short description of what the problem is and where in the JIT it happens?

@TIHan
Copy link
Contributor

TIHan commented Mar 13, 2024

For JIT debug compilations, when code-gen'ing a basic block, if there was a previous block, we expect its ending IL offset to be less than the current block's beginning IL offset. These IL offsets and blocks themselves correspond to user code.

However, the JIT can create new basic blocks on its own that don't correspond to user code, in which case the new block needs to be marked with the BBF_INTERNAL flag. Because BBF_INTERNAL blocks do not correspond to a user code block, their beginning and ending IL offsets must remain set at BAD_IL_OFFSET, which I believe is the max value of int32.

The issue here is that some blocks that are marked with BBF_INTERNAL have their ending IL offset not set to BAD_IL_OFFSET. The code below shows how this can occur:

https://github.com/dotnet/runtime/blob/main/src/coreclr/jit/fgbasic.cpp#L4810

        IL_OFFSET splitPointILOffset = fgFindBlockILOffset(newBlock);

        curr->bbCodeOffsEnd  = max(curr->bbCodeOffs, splitPointILOffset);
        newBlock->bbCodeOffs = min(splitPointILOffset, newBlock->bbCodeOffsEnd);

splitPointILOffset is not guaranteed to be BAD_IL_OFFSET if newBlock is BBF_INTERNAL. So the line:

newBlock->bbCodeOffs = min(splitPointILOffset, newBlock->bbCodeOffsEnd);

causes bbCodeOffs to not be BAD_IL_OFFSET for a BBF_INTERNAL since splitPointILOffset would be less than BAD_IL_OFFSET.

@AndyAyersMS
Copy link
Member

The issue here is that some blocks that are marked with BBF_INTERNAL have their ending IL offset not set to BAD_IL_OFFSET. The code below shows how this can occur:

Can you share the dump?

For debug code if we have statements with IL offsets, it seems like the blocks they are in should also have IL offsets.

Seems like that means that curr is also BBF_INTERNAL and has an IL offset, or at least has statements with IL offsets.

@TIHan
Copy link
Contributor

TIHan commented Mar 15, 2024

You should be able to get the dump, GitHub is working again it looks like.
https://github.com/dotnet/runtime/files/14580404/dump.txt

curr is a BBF_INTERNAL and it has statements that have IL offsets which is why fgFindBlockILOffset returns a valid offset.

@AndyAyersMS
Copy link
Member

I am trying to determine what the right fix looks like.

It seems like the root cause is that we are putting IL-offset carrying statements into blocks that are marked as BBF_INTERNAL and not giving those blocks proper IL offsets. I suspect the IL offsets were set "correctly" this may cause us to lose debug information that we need to preserve.

So one possible fix is to stop putting IL offset carrying statements into internal blocks, by making sure the blocks are not marked as internal, and getting the offsets right.

Another possible fix is to erase the debug info (at least from the block), which more or less what you're proposing. If we go this route, I think we should examine debug emission and make sure nothing is getting lost.

Whatever direction we go in we should update the post-phase diagnostic checker so it spots these problems right away.

Can you look at the first option? Things seem ok here:

Trees after Set block order

----------------------------------------------------------------------------------------------------------------------------------------------
BBnum BBid ref try hnd preds           weight   [IL range]   [jump]              [EH region]        [flags]
----------------------------------------------------------------------------------------------------------------------------------------------
BB01 [0000]  1                             1    [???..???)-> BB02      (always)                     i keep internal q
BB02 [0001]  1       BB01                  1    [000..01F)-> BB04      (always)                     i hascall q
BB04 [0003]  1       BB02                  1    [???..???)-> BB06,BB05 ( cond )                     i internal hascall q
BB05 [0004]  1       BB04                  0.80 [???..???)-> BB03      (always)                     i internal hascall gcsafe
BB06 [0005]  1       BB04                  0.20 [???..???)-> BB03      (always)                     i internal hascall gcsafe q
BB03 [0002]  2       BB05,BB06             1    [01F..02F)             (return)                     i internal hascall gcsafe
----------------------------------------------------------------------------------------------------------------------------------------------

but not just after (seemingly BB19 is the troublemaker, so something in this expansion is not treating debug offsets the way it should).

*************** Finishing PHASE Expand static init
Trees after Expand static init
----------------------------------------------------------------------------------------------------------------------------------------------
BBnum BBid ref try hnd preds           weight   [IL range]   [jump]              [EH region]        [flags]
----------------------------------------------------------------------------------------------------------------------------------------------
BB01 [0000]  1                             1    [???..???)-> BB02      (always)                     i keep internal q
BB02 [0001]  1       BB01                  1    [???..???)-> BB07,BB09 ( cond )                     i hascall
BB09 [0008]  1       BB02                  0    [???..???)-> BB07      (always)                     rare internal q
BB07 [0006]  2       BB02,BB09             1    [000..01F)-> BB04      (always)                     i hascall q
BB04 [0003]  1       BB07                  1    [???..???)-> BB10,BB12 ( cond )                     i internal hascall
BB12 [0011]  1       BB04                  0    [???..???)-> BB10      (always)                     rare internal q
BB10 [0009]  2       BB04,BB12             1    [???..???)-> BB06,BB05 ( cond )                     i internal hascall q
BB05 [0004]  1       BB10                  0.80 [???..???)-> BB13,BB15 ( cond )                     i internal hascall gcsafe
BB15 [0014]  1       BB05                  0    [???..???)-> BB13      (always)                     rare internal q
BB13 [0012]  2       BB05,BB15             0.80 [???..???)-> BB03      (always)                     i internal hascall gcsafe
BB06 [0005]  1       BB10                  0.20 [???..???)-> BB16,BB18 ( cond )                     i internal hascall gcsafe
BB18 [0017]  1       BB06                  0    [???..???)-> BB16      (always)                     rare internal q
BB16 [0015]  2       BB06,BB18             0.20 [???..???)-> BB19,BB21 ( cond )                     i internal hascall gcsafe
BB21 [0020]  1       BB16                  0    [???..???)-> BB19      (always)                     rare internal q
BB19 [0018]  2       BB16,BB21             0.20 [013..???)-> BB03      (always)                     i internal hascall gcsafe q
BB03 [0002]  2       BB13,BB19             1    [01F..02F)             (return)                     i internal hascall gcsafe
----------------------------------------------------------------------------------------------------------------------------------------------

@TIHan
Copy link
Contributor

TIHan commented Mar 15, 2024

Can you look at the first option?

Sure, I can look.

@TIHan
Copy link
Contributor

TIHan commented Mar 19, 2024

The BFF_INTERNAL blocks are initially created in the callindirecttransformer, specifically the fat-pointer transformer. It creates new basic blocks and marks them as BFF_INTERNAL - however, they contain statements that do contain valid IL offsets. These statements are copied from another statement that have valid IL offsets. Examples: https://github.com/dotnet/runtime/blob/main/src/coreclr/jit/indirectcalltransformer.cpp#L399
https://github.com/dotnet/runtime/blob/main/src/coreclr/jit/indirectcalltransformer.cpp#L414

@AndyAyersMS Should we unmark BFF_INTERNAL from these new blocks since they will contain statements with valid IL offsets? It won't fix the issue if we do. Still looking into what is going on with the offsets.

@AndyAyersMS
Copy link
Member

If we are required to duplicate user statements (looks like we are) then some options are:

  1. if the debug format allows us to map multiple binary ranges to IL offsets, then we arguably should make the blocks be non-internal, and figure out how to fix the assert to tolerate these cases. This would give the best debugger experience. However we might find that the debug format for locals does not tolerate having multiple ranges for a local. I suspect the alternative blocks will always be nearby in debug code so we ought to be able to report local scope changes as one range, but this may be a bit tricky to pull off.
  2. if the debug format does not allow us to map multiple binary ranges to IL offsets or has simplistic assumptions for local scopes, then we can pick one copy as the "winner" and report it, and in the other copies suppress the IL offsets. This might be done by suppressing IL offsets for internal blocks, like in your initial proposed change.
  3. we might be able to hoist most of the user statements up above the point where we have to branch but the call itself would be problematic. This is where stubs come in handy, but sounds like in NAOT that's not an option.

Whichever way we go we should clarify the behavior of the jit with respect to internal blocks. For instance, we might consider to enforcing the invariant that internal blocks and their statements do not have IL offsets, or (perhaps simpler) that internal blocks do not have IL offsets and we ignore any IL offsets for statements inside (for purposes of debug emission).

@TIHan
Copy link
Contributor

TIHan commented Mar 20, 2024

AFAIK, the debug format should allow to map multiple ranges, so I think it's fine that we copy user statements around.

I did a lot of investigation into this issue. I believe the problem really is how we are updating the IL offsets in fgSplitBlockAfterStatement.

I tweaked the original solution in #99662 where instead of checking for BBF_INTERNAL, it will check if the original block's ending offset was BAD_IL_OFFSET. If it was, then we do not update the IL offsets.

@AndyAyersMS
Copy link
Member

Does this mean we may lose a bit of debugging fidelity? Since this is for scopes then impact might be that at certain breakpoints or stepping, locals that should be visible suddenly are not?

Should we add a diagnostic check that no BBF_INTERNAL block has valid IL offsets?

@TIHan
Copy link
Contributor

TIHan commented Mar 20, 2024

If it is common to have blocks that have its ending offset set to BAD_IL_OFFSET and contain statements that have valid IL offsets, then maybe.

Another solution is to set the block's ending offset to BAD_IL_OFFSET only in expanding static init call. This would limit any risk to losing fidelity.

@TIHan
Copy link
Contributor

TIHan commented Mar 20, 2024

I made a compromise. I am now updating the IL offsets in fgExpandStaticInitForCall which also solves the issue.

Before:

BB01 [0000]  1                             1    [???..???)-> BB02      (always)                     i keep internal q
BB02 [0001]  1       BB01                  1    [???..???)-> BB07,BB09 ( cond )                     i hascall
BB09 [0008]  1       BB02                  0    [???..???)-> BB07      (always)                     rare internal q
BB07 [0006]  2       BB02,BB09             1    [000..01F)-> BB04      (always)                     i hascall q
BB04 [0003]  1       BB07                  1    [???..???)-> BB10,BB12 ( cond )                     i internal hascall
BB12 [0011]  1       BB04                  0    [???..???)-> BB10      (always)                     rare internal q
BB10 [0009]  2       BB04,BB12             1    [???..???)-> BB06,BB05 ( cond )                     i internal hascall q
BB05 [0004]  1       BB10                  0.80 [???..???)-> BB13,BB15 ( cond )                     i internal hascall gcsafe
BB15 [0014]  1       BB05                  0    [???..???)-> BB13      (always)                     rare internal q
BB13 [0012]  2       BB05,BB15             0.80 [???..???)-> BB03      (always)                     i internal hascall gcsafe
BB06 [0005]  1       BB10                  0.20 [???..???)-> BB16,BB18 ( cond )                     i internal hascall gcsafe
BB18 [0017]  1       BB06                  0    [???..???)-> BB16      (always)                     rare internal q
BB16 [0015]  2       BB06,BB18             0.20 [???..???)-> BB19,BB21 ( cond )                     i internal hascall gcsafe
BB21 [0020]  1       BB16                  0    [???..???)-> BB19      (always)                     rare internal q
BB19 [0018]  2       BB16,BB21             0.20 [013..???)-> BB03      (always)                     i internal hascall gcsafe q
BB03 [0002]  2       BB13,BB19             1    [01F..02F)             (return)                     i internal hascall gcsafe

After:

BB01 [0000]  1                             1    [???..???)-> BB02      (always)                     i keep internal q
BB02 [0001]  1       BB01                  1    [???..???)-> BB07,BB09 ( cond )                     i hascall
BB09 [0008]  1       BB02                  0    [???..???)-> BB07      (always)                     rare internal q
BB07 [0006]  2       BB02,BB09             1    [000..???)-> BB04      (always)                     i hascall q
BB04 [0003]  1       BB07                  1    [???..???)-> BB10,BB12 ( cond )                     i internal hascall
BB12 [0011]  1       BB04                  0    [???..???)-> BB10      (always)                     rare internal q
BB10 [0009]  2       BB04,BB12             1    [013..???)-> BB06,BB05 ( cond )                     i internal hascall q
BB05 [0004]  1       BB10                  0.80 [???..???)-> BB13,BB15 ( cond )                     i internal hascall gcsafe
BB15 [0014]  1       BB05                  0    [???..???)-> BB13      (always)                     rare internal q
BB13 [0012]  2       BB05,BB15             0.80 [013..???)-> BB03      (always)                     i internal hascall gcsafe
BB06 [0005]  1       BB10                  0.20 [???..???)-> BB16,BB18 ( cond )                     i internal hascall gcsafe
BB18 [0017]  1       BB06                  0    [???..???)-> BB16      (always)                     rare internal q
BB16 [0015]  2       BB06,BB18             0.20 [013..013)-> BB19,BB21 ( cond )                     i internal hascall gcsafe
BB21 [0020]  1       BB16                  0    [013..013)-> BB19      (always)                     rare internal q
BB19 [0018]  2       BB16,BB21             0.20 [013..013)-> BB03      (always)                     i internal hascall gcsafe q
BB03 [0002]  2       BB13,BB19             1    [01F..02F)             (return)                     i internal hascall gcsafe

We actually gain some IL offsets here for few a few blocks. The only block that appears to lose fidelity is BB07 - which its internal statement gets modified in fgExpandStaticInitForCall anyway - so I don't know what the implications are of that for debug fidelity.

Before fgExpandStaticInitForCall:

BB02:
N005 ( 19, 13) [000005] VACXGO-----                         *  STOREIND  long  
N003 ( 15,  6) [000004] --CXG--N---                         +--*  ADD       byref 
N001 ( 14,  5) [000002] H-CXG------                         |  +--*  CALL help byref  CORINFO_HELP_READYTORUN_NONGCSTATIC_BASE
N002 (  1,  1) [000003] -----------                         |  \--*  CNS_INT   long   8 Fseq[FtnHolder]
N004 (  1,  4) [000001] H----------                         \--*  CNS_INT(h) long   0x4000000000420068 ftn

After fgExpandStaticInitForCall (BB02 is split and BB07 is created):

BB07:
N005 (  8, 10) [000005] VA--GO-----                         *  STOREIND  long  
N003 (  4,  3) [000004] ----G--N---                         +--*  ADD       byref 
N001 (  3,  2) [000056] -----------                         |  +--*  LCL_VAR   long   V02 tmp1         
N002 (  1,  1) [000003] -----------                         |  \--*  CNS_INT   long   8 Fseq[FtnHolder]
N004 (  1,  4) [000001] H----------                         \--*  CNS_INT(h) long   0x4000000000420068 ftn

@github-actions github-actions bot locked and limited conversation to collaborators Apr 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-x64 arch-x86 area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants