From 61c8fa44822650972e3fc7570dee4dd69bc4fa3a Mon Sep 17 00:00:00 2001
From: Yee Cheng Chin <ychin.git@gmail.com>
Date: Sun, 3 Nov 2019 18:47:04 -0800
Subject: [PATCH] Fix potential CoreText font rendering infinite recursion

The function `recurseDraw` is actually a misnomer. It uses recursion in
a way that is unnecessary as it's a simple while loop that keeps drawing
as many characters as possible until all are drawn. The recursion is
just a convenience to invoke the CoreText rendering code. If the API for
`CTFontGetGlyphsForCharacters` works as expected, in theory we shouldn't
get infinite recursion since `lookupFont` calls the same function, but
just for safety and to avoid potential subtle interactions, just kill
the recursion and directly draw the texts in the loop to make it easier
to reason through.

Fix #983.
---
 src/MacVim/MMCoreTextView.m | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/src/MacVim/MMCoreTextView.m b/src/MacVim/MMCoreTextView.m
index 7ce71a4bb6..99d02814f8 100644
--- a/src/MacVim/MMCoreTextView.m
+++ b/src/MacVim/MMCoreTextView.m
@@ -1710,7 +1710,7 @@ - (void)batchDrawData:(NSData *)data
 
     static CTFontRef
 lookupFont(NSMutableArray *fontCache, const unichar *chars, UniCharCount count,
-           CTFontRef currFontRef)
+           CTFontRef currFontRef, CGGlyph *glyphsOut)
 {
     CGGlyph glyphs[count];
 
@@ -1720,7 +1720,10 @@ - (void)batchDrawData:(NSData *)data
         NSFont *font = [fontCache objectAtIndex:i];
 
         if (CTFontGetGlyphsForCharacters((CTFontRef)font, chars, glyphs, count))
+        {
+            memcpy(glyphsOut, glyphs, count * sizeof(CGGlyph));
             return (CTFontRef)[font retain];
+        }
     }
 
     // Ask Core Text for a font (can be *very* slow, which is why we cache
@@ -1737,6 +1740,7 @@ - (void)batchDrawData:(NSData *)data
     if (newFontRef)
         [fontCache addObject:(NSFont *)newFontRef];
 
+    memcpy(glyphsOut, glyphs, count * sizeof(CGGlyph));
     return newFontRef;
 }
 
@@ -1840,6 +1844,8 @@ - (void)batchDrawData:(NSData *)data
             UniCharCount length, CGContextRef context, CTFontRef fontRef,
             NSMutableArray *fontCache, BOOL isComposing, BOOL useLigatures)
 {
+    // Note: This function is misnamed. It does not actually use recursion and
+    // will be renamed in future.
     if (CTFontGetGlyphsForCharacters(fontRef, chars, glyphs, length)) {
         // All chars were mapped to glyphs, so draw all at once and return.
         length = isComposing || useLigatures
@@ -1897,7 +1903,7 @@ - (void)batchDrawData:(NSData *)data
             CTFontRef fallback = nil;
             while (fallback == nil && attemptedCount > 0) {
                 fallback = lookupFont(fontCache, chars, attemptedCount,
-                                      fontRef);
+                                      fontRef, glyphs);
                 if (!fallback)
                     attemptedCount /= 2;
             }
@@ -1905,9 +1911,13 @@ - (void)batchDrawData:(NSData *)data
             if (!fallback)
                 return;
 
-            recurseDraw(chars, glyphs, positions, attemptedCount, context,
-                        fallback, fontCache, isComposing, useLigatures);
+            UniCharCount actualAttemptLength = isComposing || useLigatures
+                ? composeGlyphsForChars(chars, glyphs, positions, attemptedCount,
+                                    fallback, isComposing, useLigatures)
+                : gatherGlyphs(glyphs, attemptedCount);
+            CTFontDrawGlyphs(fallback, glyphs, positions, actualAttemptLength, context);
 
+            // TODO: This doesn't take into account surrogate pairs for 'p'. Clean this up.
             // If only a portion of the invalid range was rendered above,
             // the remaining range needs to be attempted by subsequent
             // iterations of the draw loop.