Repository: microsoft/terminal (language: cpp)

Issue:
Crash when entering a lot of content in the Japanese Microsoft IME
### Windows Terminal version

OpenConsole 8a34a0e59ac41bc805c308fb82bc7683e583d45d

### Windows build number

10.0.19041.1415

### Other Software

_No response_

### Steps to reproduce

1. Install the Japanese language pack.
2. Open a WSL bash shell in a recent version of OpenConsole.
3. Execute `read -n 10000`.
4. Switch the Japanese IME to Hiragana mode.
5. Hold down the `a` key for a while.
6. You should see a bunch of `あ` glyphs being written out.
7. If you keep the `a` pressed for long enough, it'll eventually crash.

### Expected Behavior

It shouldn't crash.

### Actual Behavior

When run from within the Visual Studio debugger, it fails with the following stack trace:

```
OpenConsole.exe!ConsoleImeInfo::_WriteConversionArea(const std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<OutputCell>>> begin, const std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<OutputCell>>> end, _COORD & pos, const Microsoft::Console::Types::Viewport view, SCREEN_INFORMATION & screenInfo) Line 366	C++
OpenConsole.exe!ConsoleImeInfo::_WriteUndeterminedChars(const std::basic_string_view<wchar_t,std::char_traits<wchar_t>> text, const gsl::span<unsigned char const ,-1> attributes, const gsl::span<unsigned short const ,-1> colorArray) Line 444	C++
OpenConsole.exe!ConsoleImeInfo::WriteCompMessage(const std::basic_string_view<wchar_t,std::char_traits<wchar_t>> text, const gsl::span<unsigned char const ,-1> attributes, const gsl::span<unsigned short const ,-1> colorArray) Line 74	C++
OpenConsole.exe!ImeComposeData(std::basic_string_view<wchar_t,std::char_traits<wchar_t>> text, gsl::span<unsigned char const ,-1> attributes, gsl::span<unsigned short const ,-1> colorArray) Line 141	C++
OpenConsole.exe!CConversionArea::DrawComposition(const std::basic_string_view<wchar_t,std::char_traits<wchar_t>> CompStr, const std::vector<TF_DISPLAYATTRIBUTE,std::allocator<TF_DISPLAYATTRIBUTE>> & DisplayAttributes, const unsigned long CompCursorPos) Line 51	C++
OpenConsole.exe!CEditSessionUpdateCompositionString::_MakeCompositionString(unsigned long ec, ITfRange * FullTextRange, int bInWriteSession, CicCategoryMgr * pCicCatMgr, CicDisplayAttributeMgr * pCicDispAttr) Line 988	C++
OpenConsole.exe!CEditSessionUpdateCompositionString::UpdateCompositionString(unsigned long ec) Line 849	C++
OpenConsole.exe!CEditSessionObject::DoEditSession(unsigned long ec) Line 64	C++
[External Code]	
OpenConsole.exe!ConsoleInputThreadProcWin32(void * __formal) Line 1057	C++
```

But the actual problem is probably deeper than that. If I allow the debugger to continue, I get an additional two stack entries implying the crash to be somewhere in the `OutputCellIterator` constructor.

```
[Inline Frame] OpenConsole.exe!OutputCellIterator::{ctor}(const gsl::span<OutputCell const ,-1>) Line 149	C++
[Inline Frame] OpenConsole.exe!ConversionAreaInfo::WriteText(const std::vector<OutputCell,std::allocator<OutputCell>> &) Line 116	C++
```

As a wild I guess, I suspect it might have something to do with the `lineVec` vector being zero length (or so it appeared at the time of the crash).


Discussion / hints:
I don't know if this is the right solution, but I can prevent the crash if I add a check like this:

```cpp
if (lineEnd <= lineBegin)
{
    return lineEnd;
}
```

before the `lineVec` is constructed here:

https://github.com/microsoft/terminal/blob/91b454ac95b4f8404f2d7c2cb6b13f5c3205001d/src/host/conimeinfo.cpp#L356-L357

It's also worth mentioning that I couldn't reproduce this in my inbox conhost, so it might be a regression.
Ah heck, I tried reproing this Friday and couldn't get a repro. Now, for whatever reason, the internal build I'm on is seemingly refusing to download the Japanese typing package 😑 We might be stuck here for the time being. 

How long is long enough for "If you keep the `a` pressed for long enough, it'll eventually crash."? IIRC I had filled up an entire viewport of output, without it crashing.

I'd be suspicious of when this actually regressed - we haven't touched conhost's IME code in a LONG time... I'd bet this repros quite a way back in the past, or broke because of some other interaction outside `ConsoleImeInfo::_WriteConversionArea`
I also bet that the check would work. We could probably also get rid of the FAILFAST a few lines up, on 346, and just deal with it. That wouldn't create a new conversion area in this weird edge case where we've backed up before the start of the conversion, because the...

We've gotten the line we're working on
We've gotten the last cell of that line
we've determined the last cell was wide, so we want to start 1 column earlier
we now find that the end is before the start, so we can't actually create a new vector with length=-1


That's a weird edge case. The actual width of the buffer might matter here - might need to be an odd/even number thing, might explain why I couldn't trivially repro Friday.
We might also just want to move that failfast down, after the decrement to `lineEnd`. Seems like everything in the IME just uses exceptions for flow control anyways. Er, I suppose it shouldn't be a failfast, it could just be a `THROW_IF` or something. Maybe both accomplish the same thing. 
I just checked now and I can still reproduce the crash. At the time of the crash I can see five full lines and then another four characters on the sixth line with a windows size of 80x24. It seems like it can vary though.
New day, new build, Japanese IME magically fixed.

Weirdly, I got a different stack doing the same thing...


```
 	OpenConsole.exe!OutputCell::TextAttrBehavior(void) Line 76	Unknown
 	OpenConsole.exe!OutputCellIterator::s_GenerateView(class OutputCell const &) Line 527	Unknown
 	OpenConsole.exe!OutputCellIterator::OutputCellIterator(class gsl::span<class OutputCell const ,-1>) Line 151	Unknown
 	OpenConsole.exe!ConversionAreaInfo::WriteText(const std::vector<OutputCell,std::allocator<OutputCell>> & text, const short column) Line 116	C++
 	OpenConsole.exe!ConsoleImeInfo::_WriteConversionArea(const std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<OutputCell>>> begin, const std::_Vector_const_iterator<std::_Vector_val<std::_Simple_types<OutputCell>>> end, _COORD & pos, const Microsoft::Console::Types::Viewport view, SCREEN_INFORMATION & screenInfo) Line 370	C++
>	OpenConsole.exe!ConsoleImeInfo::_WriteUndeterminedChars(const std::basic_string_view<wchar_t,std::char_traits<wchar_t>> text, const gsl::span<unsigned char const ,-1> attributes, const gsl::span<unsigned short const ,-1> colorArray) Line 444	C++
 	OpenConsole.exe!ConsoleImeInfo::WriteCompMessage(const std::basic_string_view<wchar_t,std::char_traits<wchar_t>> text, const gsl::span<unsigned char const ,-1> attributes, const gsl::span<unsigned short const ,-1> colorArray) Line 75	C++
 	OpenConsole.exe!ImeComposeData(std::basic_string_view<wchar_t,std::char_traits<wchar_t>> text, gsl::span<unsigned char const ,-1> attributes, gsl::span<unsigned short const ,-1> colorArray) Line 141	C++
 	OpenConsole.exe!CConversionArea::DrawComposition(const std::basic_string_view<wchar_t,std::char_traits<wchar_t>> CompStr, const std::vector<TF_DISPLAYATTRIBUTE,std::allocator<TF_DISPLAYATTRIBUTE>> & DisplayAttributes, const unsigned long CompCursorPos) Line 51	C++
 	OpenConsole.exe!CEditSessionUpdateCompositionString::_MakeCompositionString(unsigned long ec, ITfRange * FullTextRange, int bInWriteSession, CicCategoryMgr * pCicCatMgr, CicDisplayAttributeMgr * pCicDispAttr) Line 988	C++
 	OpenConsole.exe!CEditSessionUpdateCompositionString::UpdateCompositionString(unsigned long ec) Line 845	C++
 	OpenConsole.exe!CEditSessionUpdateCompositionString::_DoEditSession(unsigned long ec) Line 196	C++
 	OpenConsole.exe!CEditSessionObject::DoEditSession(unsigned long ec) Line 63	C++
 	[External Code]	
 	OpenConsole.exe!ConsoleInputThreadProcWin32(void * __formal) Line 1057	C++
 	[External Code]	
```
I suspect you might be using a debug build, so for me a lot of that stack would be inlined. I think you're still getting the crash in essentially the same place.

Fix patch:
diff --git a/src/host/conimeinfo.cpp b/src/host/conimeinfo.cpp
index 21ad2be20f5..697a176018b 100644
--- a/src/host/conimeinfo.cpp
+++ b/src/host/conimeinfo.cpp
@@ -343,8 +343,6 @@ std::vector<OutputCell>::const_iterator ConsoleImeInfo::_WriteConversionArea(con
 
     // We must attempt to compensate for ending on a leading byte. We can't split a full-width character across lines.
     // As such, if the last item is a leading byte, back the end up by one.
-    FAIL_FAST_IF(lineEnd <= lineBegin); // We should have at least 1 space we can back up.
-
     // Get the last cell in the run and if it's a leading byte, move the end position back one so we don't
     // try to insert it.
     const auto lastCell = lineEnd - 1;
@@ -353,6 +351,13 @@ std::vector<OutputCell>::const_iterator ConsoleImeInfo::_WriteConversionArea(con
         lineEnd--;
     }
 
+    // GH#12730 - if the lineVec would now be empty, just return early. Failing
+    // to do so will later cause a crash trying to construct an empty view.
+    if (lineEnd <= lineBegin)
+    {
+        return lineEnd;
+    }
+
     // Copy out the substring into a vector.
     const std::vector<OutputCell> lineVec(lineBegin, lineEnd);
 


Test patch: