MacVim crashes when displaying a certain character

[douglasdrumond]

From @GoogleCodeExporter on March 16, 2015 9:26

What steps will reproduce the problem?
MacVim crashes when displaying the following character:
𻯩
I apologize for putting the character here like this, but I couldn't find out 
what kind of character this is.

What is the expected output? What do you see instead?
MacVim shouldn't crash, ever

What version of MacVim and OS X are you using (see "MacVim->About MacVim"
and  "Apple Menu->About This Mac" menu items, e.g. "Snapshot 40, 10.5.6
Intel")?
MacVim 73, OSX 10.10.1

Additional notes:
Terminal vim does not crash when displaying the same character.

Original issue reported on code.google.com by floris.v...@gmail.com on 12 Jan 2015 at 11:34

Copied from original issue: #525

[jpetrie]

Copying and pasting the character in the above report, even though it looks like its rendering incorrectly, will reproduce this. Examining the crash in Console.app suggests some kind of stack overflow due to drawRecurse going on and on forever.

Trapping the crash in the debugger shows that the character appears to be U+d8ae, which is not a valid Unicode character.

[jpetrie]

It actually looks like this invalid character is a surrogate pair (which still ends up referring to a unassigned supplementary plane). U+d8ae and U+dfe9. This is relevant, as the length of the chars array passed to recurseDraw (as determined by CFStringGetLength in batchDrawData) is 2.

What ends up happening is that CTFontGetGlyphsForCharacters cannot convert the 2-character string to glyphs; both glyph array entries are 0. That causes recurseDraw to enter the "skip past consecutive non-drawable characters" loop, where it tries skip past two characters and calls lookupFont to find a fallback that can render the invalid characters.

On the very first call to lookupFont the cache will be empty, so it will simply ask CoreText for a new font. However, when it does so it asks for a font based only on the first character in the string (this makes sense in a way, since the recursive nature of the draw loop should handle subsequent invalid characters exactly the same way). This ends up producing a "LastResort" font that can render that first character (and indeed, CTFontGetGlyphsForCharacters at that point for that single-character string returns true).

However, the recursive call to recurseDraw tries to draw the entire 2-character string again, now that a "fallback" font has been added to the cache. That fallback font cannot convert the 2-character string to glyphs, and so the process repeats and the code calls itself forever.

The code, as written, seems to intend to abort rendering if a fallback font can't be found. By passing a length to lookupFont and then using that when requesting a fallback, lookupFont can return nil if the fallback still can't turn the string into glyphs and thus avoid the endless recursion.

It is also possible to restructure the recurseDraw loop a little to have it try to covert the invalid string one character at a time. This results in the display of some very obvious "invalid character" in MacVim.

Both approaches cause column counting weirdness, although they avoid the crash. The rendering (or the space left by the non-rendered) character will count as four columns, although visually will display as two. This seems undesirable.

I've been able to achieve both of the above options with local code changes. A third idea, which I haven't prototyped yet, would be to replace the non-rendering characters with U+FFFD, the "replacement character" code point. This would hopefully render better.

Are there any strong feelings for choosing one of these solutions over the other? If not I'll probably opt for the simplest of the three after I prototype the last one and send a PR for that.

(EDIT: For what it's worth, regular vim in the Terminal will display this character as two empty boxes and will also exhibit the misaligned column issue. On Windows, gvim replaces the character with two ?? and functions more or less normally.)

[jpetrie]

Fixed by e878fe5.

[moll]

I believe this issue has returned as seen in #463.