[1.16] ConPTY crash on git log - PGO seems to break main

[DHowett]

I've got a reproduceable crash as of microsoft/terminal@dev/duhowett/selfhost-1.16 (version 1.16.189).

1. cd ~/src/terminal
2. git log --pretty=oneline
3. down
4. up

stack trace

 # Child-SP          RetAddr               Call Site
00 000000c1`f9cff420 00007ff7`4f2ea780     OpenConsole!TextBuffer::_RefreshRowIDs+0xc2 [C:\a\_work\1\s\src\buffer\out\textBuffer.cpp @ 1038] 
01 000000c1`f9cff4d0 00007ff7`4f2ec8e9     OpenConsole!TextBuffer::ScrollRows+0xa8 [C:\a\_work\1\s\src\buffer\out\textBuffer.cpp @ 786] 
02 000000c1`f9cff520 00007ff7`4f2ece52     OpenConsole!Microsoft::Console::VirtualTerminal::AdaptDispatch::_ScrollRectVertically+0x85 [C:\a\_work\1\s\src\terminal\adapter\adaptDispatch.cpp @ 443] 
03 000000c1`f9cff5d0 00007ff7`4f2ece89     OpenConsole!Microsoft::Console::VirtualTerminal::AdaptDispatch::_ScrollMovement+0xae [C:\a\_work\1\s\src\terminal\adapter\adaptDispatch.cpp @ 875] 
04 000000c1`f9cff640 00007ff7`4f2ee604     OpenConsole!Microsoft::Console::VirtualTerminal::AdaptDispatch::ScrollDown+0x9 [C:\a\_work\1\s\src\terminal\adapter\adaptDispatch.cpp @ 898] 
05 000000c1`f9cff670 00007ff7`4f2e3d01     OpenConsole!Microsoft::Console::VirtualTerminal::OutputStateMachineEngine::ActionCsiDispatch+0x534 [C:\a\_work\1\s\src\terminal\parser\OutputStateMachineEngine.cpp @ 545] 
06 (Inline Function) --------`--------     OpenConsole!Microsoft::Console::VirtualTerminal::StateMachine::_ActionCsiDispatch::__l2::<lambda_1>::operator()+0x5d [C:\a\_work\1\s\src\terminal\parser\stateMachine.cpp @ 481] 
07 000000c1`f9cff720 00007ff7`4f2e30f8     OpenConsole!Microsoft::Console::VirtualTerminal::StateMachine::_SafeExecute<`Microsoft::Console::VirtualTerminal::StateMachine::_ActionCsiDispatch'::`2'::<lambda_1> >+0x61 [C:\a\_work\1\s\src\terminal\parser\stateMachine.cpp @ 2040] 
08 (Inline Function) --------`--------     OpenConsole!Microsoft::Console::VirtualTerminal::StateMachine::_SafeExecuteWithLog+0x13 [C:\a\_work\1\s\src\terminal\parser\stateMachine.cpp @ 2055] 
09 (Inline Function) --------`--------     OpenConsole!Microsoft::Console::VirtualTerminal::StateMachine::_ActionCsiDispatch+0x5a [C:\a\_work\1\s\src\terminal\parser\stateMachine.cpp @ 480] 
0a (Inline Function) --------`--------     OpenConsole!Microsoft::Console::VirtualTerminal::StateMachine::_EventCsiEntry+0x13f [C:\a\_work\1\s\src\terminal\parser\stateMachine.cpp @ 1176] 
0b (Inline Function) --------`--------     OpenConsole!Microsoft::Console::VirtualTerminal::StateMachine::ProcessCharacter+0x759 [C:\a\_work\1\s\src\terminal\parser\stateMachine.cpp @ 1743] 
0c 000000c1`f9cff760 00007ff7`4f2e2799     OpenConsole!Microsoft::Console::VirtualTerminal::StateMachine::ProcessString+0x7b8 [C:\a\_work\1\s\src\terminal\parser\stateMachine.cpp @ 1852] 

Debugging notes

0:000> dx _storage
_storage                 : { size=30 }

0:000> dx i
i                : 41 [Type: int]

Why is i greater than _storage.size? It starts at 0 and is incremented once per iteration over _storage.

AV is on line 1038 here:

terminal/src/buffer/out/textBuffer.cpp

Lines 1031 to 1041 in 1faf67f

	void TextBuffer::_RefreshRowIDs(std::optional<til::CoordType> newRowWidth)
	{
	std::unordered_map<til::CoordType, til::CoordType> rowMap;
	til::CoordType i = 0;
	for (auto& it : _storage)
	{
	// Build a map so we can update Unicode Storage
	rowMap.emplace(it.GetId(), i);
	// Update the IDs
	it.SetId(i++);

@lhecker, could this be fallout from #13368?

[DHowett]

rowMap has some interesting guys in it:

0:000> dx rowMap
rowMap                 : { size=0x20 } [Type: std::unordered_map<int,int,std::hash<int>,std::equal_to<int>,std::allocator<std::pair<int const ,int> > >]
    [<Raw View>]     [Type: std::unordered_map<int,int,std::hash<int>,std::equal_to<int>,std::allocator<std::pair<int const ,int> > >]
    [bucket_count]   : 0x40 [Type: unsigned __int64]
    [load_factor]    : 0.500000
    [max_load_factor] : 1.000000 [Type: float]
    [hash_function]  : hash [Type: std::_Compressed_pair<std::hash<int>,std::_Compressed_pair<std::equal_to<int>,float,1>,1>]
    [key_eq]         : equal_to [Type: std::_Compressed_pair<std::equal_to<int>,float,1>]
    [allocator]      : allocator [Type: std::_Compressed_pair<std::allocator<std::_List_node<std::pair<int const ,int>,void *> >,std::_List_val<std::_List_simple_types<std::pair<int const ,int> > >,1>]
    [allocator]      : allocator [Type: std::_Compressed_pair<std::allocator<std::_List_node<std::pair<int const ,int>,void *> >,std::_List_val<std::_List_simple_types<std::pair<int const ,int> > >,1>]
    [0x0]            : 13, 0 [Type: std::pair<int const ,int>]
    [0x1]            : 14, 1 [Type: std::pair<int const ,int>]
    [0x2]            : 15, 2 [Type: std::pair<int const ,int>]
    [0x3]            : 16, 3 [Type: std::pair<int const ,int>]
    [0x4]            : 17, 4 [Type: std::pair<int const ,int>]
    [0x5]            : 18, 5 [Type: std::pair<int const ,int>]
    [0x6]            : 19, 6 [Type: std::pair<int const ,int>]
    [0x7]            : 20, 7 [Type: std::pair<int const ,int>]
    [0x8]            : 21, 8 [Type: std::pair<int const ,int>]
    [0x9]            : 22, 9 [Type: std::pair<int const ,int>]
    [0xa]            : 23, 10 [Type: std::pair<int const ,int>]
    [0xb]            : 24, 11 [Type: std::pair<int const ,int>]
    [0xc]            : 25, 12 [Type: std::pair<int const ,int>]
    [0xd]            : 26, 13 [Type: std::pair<int const ,int>]
    [0xe]            : 27, 14 [Type: std::pair<int const ,int>]
    [0xf]            : 28, 15 [Type: std::pair<int const ,int>]
    [0x10]           : 29, 16 [Type: std::pair<int const ,int>]
    [0x11]           : 0, 17 [Type: std::pair<int const ,int>]
    [0x12]           : 1, 18 [Type: std::pair<int const ,int>]
    [0x13]           : 2, 19 [Type: std::pair<int const ,int>]
    [0x14]           : 32762, 35 [Type: std::pair<int const ,int>]
    [0x15]           : 3, 20 [Type: std::pair<int const ,int>]
    [0x16]           : 4, 21 [Type: std::pair<int const ,int>]
    [0x17]           : 5, 22 [Type: std::pair<int const ,int>]
    [0x18]           : 6, 23 [Type: std::pair<int const ,int>]
    [0x19]           : 7, 24 [Type: std::pair<int const ,int>]
    [0x1a]           : 8, 25 [Type: std::pair<int const ,int>]
    [0x1b]           : 9, 26 [Type: std::pair<int const ,int>]
    [0x1c]           : 10, 27 [Type: std::pair<int const ,int>]
    [0x1d]           : 11, 28 [Type: std::pair<int const ,int>]
    [0x1e]           : 12, 29 [Type: std::pair<int const ,int>]
    [0x1f]           : 561, 39 [Type: std::pair<int const ,int>]

[lhecker]

FYI I rebuilt the branch with PGO disabled and the crash wasn't reproducible anymore.

[DHowett]

Notes from @malxau:

1038 00000001`4001192f 8b87c4010000    mov     eax,dword ptr [rdi+1C4h]
1038 00000001`40011935 8945d7          mov     dword ptr [rbp-29h],eax
1038 00000001`40011938 4c8d4def        lea     r9,[rbp-11h]
^ Load a POINTER to i
1038 00000001`4001193c 4c8d45d7        lea     r8,[rbp-29h]
1038 00000001`40011940 488d55df        lea     rdx,[rbp-21h]
1038 00000001`40011944 488d4df7        lea     rcx,[rbp-9]
1038 00000001`40011948 e823010000      call    OpenConsole!std::_Hash<std::_Umap_traits<int,int,std::_Uhash_compare<int,std::hash<int>,std::equal_to<int> >,std::allocator<std::pair<int const ,int> >,0> >::emplace<int,int &> (00000001`40011a70)
^ Note the final value is a reference
1041 00000001`4001194d 8b4def          mov     ecx,dword ptr [rbp-11h]
^ A strangely convoluted increment that has to reload from stack because it assumes emplace can change it
1041 00000001`40011950 8d4101          lea     eax,[rcx+1]
1041 00000001`40011953 8945ef          mov     dword ptr [rbp-11h],eax

It would be interesting to see whether emplace<int, int> was erroneously merged with emplace<int, int&>.

Also: I wonder if using insert would (1) fix the local issue and (2) not cost extra, because it's not like we're moving integers anyway. It's also our canary in the coalmine here, so it would (3) remove our repro.

[zadjii-msft]

Dupe of #13500?

[DHowett]

Frick, it could be that we're using VS2019 PGO data on VS2022 eh? Or that we're optimizing our exception handler ;)