[ft-devel] [PATCH] Fix invalid function pointer casts

[Andrei Alexeyev]

Hi.

Calling a function through a pointer of incompatible type has undefined 
behavior in C: https://wiki.sei.cmu.edu/confluence/display/c/CC.
+Undefined+Behavior#CC.UndefinedBehavior-ub_26

It happens to work on most platforms and calling conventions, and some code in 
Freetype abuses this fact. Unfortunately, WebAssembly is not one of those 
platforms: https://emscripten.org/docs/porting/guidelines/
function_pointer_issues.html

It can be worked around, but that incurs a performance penalty on the entire 
program. This patch is meant to avoid that.

I'm not very familiar with the codebase; mind that these changes might be 
breaking, so you might not want to merge them as is. In particular, the 
FT_DebugHook_Func return type was changed, which I believe is part of the 
public API. I couldn't think of a better way to fix it. Adding a new 
FT_DebugHook_Func field to FT_FaceRec_ doesn't seem like an option either, 
because that, too, is a public struct.

I suggest enabling the -Wcast-function-type gcc warning by default to avoid 
this problem in the future.

Patch follows:

>From 1431d73da13e9fbf1d9cce12beaa0d37adc77ca4 Mon Sep 17 00:00:00 2001
From: Andrei Alexeyev <address@hidden>
Date: Thu, 28 Feb 2019 08:47:08 +0200
Subject: [PATCH] Fix invalid function pointer casts

This change should allow Freetype to work on WASM/Emscripten without
needing -s EMULATE_FUNCTION_POINTER_CASTS=1
---
 include/freetype/ftmodapi.h |  5 ++++-
 src/autofit/afdummy.c       |  7 ++++---
 src/cid/cidload.c           | 26 ++++++++++++++++----------
 src/sfnt/ttcmap.c           |  2 +-
 4 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/include/freetype/ftmodapi.h b/include/freetype/ftmodapi.h
index 88488bfe8..ef8df223a 100644
--- a/include/freetype/ftmodapi.h
+++ b/include/freetype/ftmodapi.h
@@ -622,8 +622,11 @@ FT_BEGIN_HEADER
    *     structure (which depends on the font module).  For TrueType fonts
    *     it is bytecode interpreter's execution context, `TT_ExecContext`,
    *     which is declared in FreeType's internal header file `tttypes.h`.
+   *
+   * @return:
+   *   The return value is currently ignored.
    */
-  typedef void
+  typedef FT_Error
   (*FT_DebugHook_Func)( void*  arg );
 
 
diff --git a/src/autofit/afdummy.c b/src/autofit/afdummy.c
index 6dd98d907..99fb033eb 100644
--- a/src/autofit/afdummy.c
+++ b/src/autofit/afdummy.c
@@ -38,9 +38,10 @@
 
 
   static FT_Error
-  af_dummy_hints_apply( FT_UInt        glyph_index,
-                        AF_GlyphHints  hints,
-                        FT_Outline*    outline )
+  af_dummy_hints_apply( FT_UInt         glyph_index,
+                        AF_GlyphHints   hints,
+                        FT_Outline*     outline,
+                        AF_StyleMetrics metrics )
   {
     FT_Error  error;
 
diff --git a/src/cid/cidload.c b/src/cid/cidload.c
index e94fb3328..c859f6c62 100644
--- a/src/cid/cidload.c
+++ b/src/cid/cidload.c
@@ -154,10 +154,11 @@
   }
 
 
-  FT_CALLBACK_DEF( FT_Error )
+  FT_CALLBACK_DEF( void )
   cid_parse_font_matrix( CID_Face     face,
                          CID_Parser*  parser )
   {
+    FT_Error      error = FT_Err_Ok;
     CID_FaceDict  dict;
     FT_Face       root = (FT_Face)&face->root;
     FT_Fixed      temp[6];
@@ -179,7 +180,10 @@
       result = cid_parser_to_fixed_array( parser, 6, temp, 3 );
 
       if ( result < 6 )
-        return FT_THROW( Invalid_File_Format );
+      {
+        error = FT_THROW( Invalid_File_Format );
+        goto Exit;
+      }
 
       FT_TRACE4(( " [%f %f %f %f %f %f]\n",
                   (double)temp[0] / 65536 / 1000,
@@ -194,7 +198,8 @@
       if ( temp_scale == 0 )
       {
         FT_ERROR(( "cid_parse_font_matrix: invalid font matrix\n" ));
-        return FT_THROW( Invalid_File_Format );
+        error = FT_THROW( Invalid_File_Format );
+        goto Exit;
       }
 
       /* atypical case */
@@ -228,11 +233,12 @@
       offset->y  = temp[5] >> 16;
     }
 
-    return FT_Err_Ok;
+Exit:
+    return /* error */;
   }
 
 
-  FT_CALLBACK_DEF( FT_Error )
+  FT_CALLBACK_DEF( void )
   parse_fd_array( CID_Face     face,
                   CID_Parser*  parser )
   {
@@ -304,7 +310,7 @@
     }
 
   Exit:
-    return error;
+    return /* error */;
   }
 
 
@@ -312,7 +318,7 @@
   /* and CID_FaceDictRec (both are public header files and can't  */
   /* changed).  We simply copy the value.                         */
 
-  FT_CALLBACK_DEF( FT_Error )
+  FT_CALLBACK_DEF( void )
   parse_expansion_factor( CID_Face     face,
                           CID_Parser*  parser )
   {
@@ -329,7 +335,7 @@
       FT_TRACE4(( "%d\n", dict->expansion_factor ));
     }
 
-    return FT_Err_Ok;
+    return /* FT_Err_Ok */;
   }
 
 
@@ -337,7 +343,7 @@
   /* `FontName' keyword.  FreeType doesn't need it, but it is nice */
   /* to catch it for producing better trace output.                */
 
-  FT_CALLBACK_DEF( FT_Error )
+  FT_CALLBACK_DEF( void )
   parse_font_name( CID_Face     face,
                    CID_Parser*  parser )
   {
@@ -361,7 +367,7 @@
     FT_UNUSED( parser );
 #endif
 
-    return FT_Err_Ok;
+    return /* FT_Err_Ok */;
   }
 
 
diff --git a/src/sfnt/ttcmap.c b/src/sfnt/ttcmap.c
index 8d9737310..9131d0b3e 100644
--- a/src/sfnt/ttcmap.c
+++ b/src/sfnt/ttcmap.c
@@ -3306,7 +3306,7 @@
   }
 
 
-  FT_CALLBACK_DEF( FT_Int )
+  FT_CALLBACK_DEF( FT_Bool )
   tt_cmap14_char_var_isdefault( TT_CMap    cmap,
                                 FT_UInt32  charcode,
                                 FT_UInt32  variantSelector )
-- 
2.20.1

signature.asc
Description: This is a digitally signed message part.