[PDF Ink Signatures] Patch libtess2 to avoid a CFI error

Change libtess2 code that calls through a function pointer to avoid
casting. This lets the compiler do better type checking and fire
-Wincompatible-function-pointer-types when it detects function pointer
calls where the types do not line up. Then line up the types to make the
compiler happy. At which point CFI (and UBSAN) are happy too.

With all the build tools in a happier state, fully re-enable
pdf_unittests test cases that call into libtess2, as they were disabled
for CFI builds.

Bug: 377971422
Change-Id: Id9f215043b7546d57cf6e6206a645035c905df07
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6014941
Reviewed-by: Andy Phan <andyphan@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1382188}

pdf/pdf_ink_module_unittest.cc

@@ -10,7 +10,6 @@
 #include <string_view>
 #include <vector>
 
-#include "base/cfi_buildflags.h"
 #include "base/check_op.h"
 #include "base/containers/contains.h"
 #include "base/containers/span.h"
@@ -1739,13 +1738,7 @@ TEST_F(PdfInkModuleUndoRedoTest, UndoRedoOnTwoPages) {
               ElementsAre(kPage0Matcher, kPage1Matcher));
 }
 
-// TODO(crbug.com/377704081): Enable test for CFI.
-#if BUILDFLAG(CFI_ICALL_CHECK)
-#define MAYBE_UndoRedoEraseLoadedV2Shapes DISABLED_UndoRedoEraseLoadedV2Shapes
-#else
-#define MAYBE_UndoRedoEraseLoadedV2Shapes UndoRedoEraseLoadedV2Shapes
-#endif
-TEST_F(PdfInkModuleUndoRedoTest, MAYBE_UndoRedoEraseLoadedV2Shapes) {
+TEST_F(PdfInkModuleUndoRedoTest, UndoRedoEraseLoadedV2Shapes) {
   constexpr int kPageIndex = 0;
   constexpr InkModeledShapeId kShapeId0(0);
   constexpr InkModeledShapeId kShapeId1(1);

pdf/pdfium/pdfium_engine_unittest.cc

@@ -15,7 +15,6 @@
 #include <optional>
 #include <utility>
 
-#include "base/cfi_buildflags.h"
 #include "base/files/file_path.h"
 #include "base/functional/callback.h"
 #include "base/hash/md5.h"
@@ -2039,13 +2038,7 @@ TEST_P(PDFiumEngineInkTest, CannotSelectTextInAnnotationMode) {
   EXPECT_THAT(engine->GetSelectedText(), IsEmpty());
 }
 
-// TODO(crbug.com/377704081): Enable test for CFI.
-#if BUILDFLAG(CFI_ICALL_CHECK)
-#define MAYBE_LoadV2InkPathsForPage DISABLED_LoadV2InkPathsForPage
-#else
-#define MAYBE_LoadV2InkPathsForPage LoadV2InkPathsForPage
-#endif
-TEST_P(PDFiumEngineInkTest, MAYBE_LoadV2InkPathsForPage) {
+TEST_P(PDFiumEngineInkTest, LoadV2InkPathsForPage) {
   NiceMock<MockTestClient> client;
   std::unique_ptr<PDFiumEngine> engine =
       InitializeEngine(&client, FILE_PATH_LITERAL("ink_v2.pdf"));
@@ -2171,15 +2164,7 @@ TEST_P(PDFiumEngineInkDrawTest, StrokeData) {
             2);
 }
 
-// TODO(crbug.com/377704081): Enable test for CFI.
-#if BUILDFLAG(CFI_ICALL_CHECK)
-#define MAYBE_LoadedV2InkPathsAndUpdateShapeActive \
-  DISABLED_LoadedV2InkPathsAndUpdateShapeActive
-#else
-#define MAYBE_LoadedV2InkPathsAndUpdateShapeActive \
-  LoadedV2InkPathsAndUpdateShapeActive
-#endif
-TEST_P(PDFiumEngineInkDrawTest, MAYBE_LoadedV2InkPathsAndUpdateShapeActive) {
+TEST_P(PDFiumEngineInkDrawTest, LoadedV2InkPathsAndUpdateShapeActive) {
   NiceMock<MockTestClient> client;
   std::unique_ptr<PDFiumEngine> engine =
       InitializeEngine(&client, FILE_PATH_LITERAL("ink_v2.pdf"));

pdf/pdfium/pdfium_ink_reader_unittest.cc

@@ -7,7 +7,6 @@
 #include <memory>
 #include <vector>
 
-#include "base/cfi_buildflags.h"
 #include "pdf/pdf_ink_constants.h"
 #include "pdf/pdfium/pdfium_engine.h"
 #include "pdf/pdfium/pdfium_page.h"
@@ -24,13 +23,7 @@ namespace chrome_pdf {
 
 using PDFiumInkReaderTest = PDFiumTestBase;
 
-// TODO(crbug.com/377704081): Enable test for CFI.
-#if BUILDFLAG(CFI_ICALL_CHECK)
-#define MAYBE_Basic DISABLED_Basic
-#else
-#define MAYBE_Basic Basic
-#endif
-TEST_P(PDFiumInkReaderTest, MAYBE_Basic) {
+TEST_P(PDFiumInkReaderTest, Basic) {
   TestClient client;
   std::unique_ptr<PDFiumEngine> engine =
       InitializeEngine(&client, FILE_PATH_LITERAL("ink_v2.pdf"));

pdf/pdfium/pdfium_ink_writer_unittest.cc

@@ -11,7 +11,6 @@
 #include <utility>
 #include <vector>
 
-#include "base/cfi_buildflags.h"
 #include "base/files/file_path.h"
 #include "base/time/time.h"
 #include "pdf/pdf_ink_brush.h"
@@ -84,13 +83,7 @@ std::unique_ptr<PdfInkBrush> CreateTestBrush() {
 
 using PDFiumInkWriterTest = PDFiumTestBase;
 
-// TODO(crbug.com/377704081): Enable test for CFI.
-#if BUILDFLAG(CFI_ICALL_CHECK)
-#define MAYBE_BasicWriteAndRead DISABLED_BasicWriteAndRead
-#else
-#define MAYBE_BasicWriteAndRead BasicWriteAndRead
-#endif
-TEST_P(PDFiumInkWriterTest, MAYBE_BasicWriteAndRead) {
+TEST_P(PDFiumInkWriterTest, BasicWriteAndRead) {
   TestClient client;
   std::unique_ptr<PDFiumEngine> engine =
       InitializeEngine(&client, FILE_PATH_LITERAL("blank.pdf"));

third_party/libtess2/README.chromium

@@ -13,4 +13,4 @@ Game and tools oriented refactored version of GLU tesselator.
 
 Local Modifications:
 
-None
+patches/0000-leq-cfi.patch: Fix CFI (and UBSAN) error with function pointers.

third_party/libtess2/patches/0000-leq-cfi.patch

@@ -0,0 +1,93 @@
+diff --git a/third_party/libtess2/src/Source/dict.c b/third_party/libtess2/src/Source/dict.c
+index 650adda21d848..579ab09b170dc 100644
+--- a/third_party/libtess2/src/Source/dict.c
++++ b/third_party/libtess2/src/Source/dict.c
+@@ -35,7 +35,7 @@
+ #include "dict.h"
+ 
+ /* really tessDictListNewDict */
+-Dict *dictNewDict( TESSalloc* alloc, void *frame, int (*leq)(void *frame, DictKey key1, DictKey key2) )
++Dict *dictNewDict( TESSalloc* alloc, void *frame, int (*leq)(TESStesselator *frame, ActiveRegion *key1, ActiveRegion *key2) )
+ {
+ 	Dict *dict = (Dict *)alloc->memalloc( alloc->userData, sizeof( Dict ));
+ 	DictNode *head;
+@@ -68,7 +68,7 @@ void dictDeleteDict( TESSalloc* alloc, Dict *dict )
+ }
+ 
+ /* really tessDictListInsertBefore */
+-DictNode *dictInsertBefore( Dict *dict, DictNode *node, DictKey key )
++DictNode *dictInsertBefore( Dict *dict, DictNode *node, ActiveRegion *key )
+ {
+ 	DictNode *newNode;
+ 
+@@ -97,7 +97,7 @@ void dictDelete( Dict *dict, DictNode *node ) /*ARGSUSED*/
+ }
+ 
+ /* really tessDictListSearch */
+-DictNode *dictSearch( Dict *dict, DictKey key )
++DictNode *dictSearch( Dict *dict, ActiveRegion *key )
+ {
+ 	DictNode *node = &dict->head;
+ 
+diff --git a/third_party/libtess2/src/Source/dict.h b/third_party/libtess2/src/Source/dict.h
+index 4cf322657b7d7..e72aabf16e191 100644
+--- a/third_party/libtess2/src/Source/dict.h
++++ b/third_party/libtess2/src/Source/dict.h
+@@ -32,11 +32,13 @@
+ #ifndef DICT_LIST_H
+ #define DICT_LIST_H
+ 
+-typedef void *DictKey;
++#include "../Include/tesselator.h"
++#include "mesh.h"
++
+ typedef struct Dict Dict;
+ typedef struct DictNode DictNode;
+ 
+-Dict *dictNewDict( TESSalloc* alloc, void *frame, int (*leq)(void *frame, DictKey key1, DictKey key2) );
++Dict *dictNewDict( TESSalloc* alloc, void *frame, int (*leq)(TESStesselator *frame, ActiveRegion *key1, ActiveRegion *key2) );
+ 
+ void dictDeleteDict( TESSalloc* alloc, Dict *dict );
+ 
+@@ -44,8 +46,8 @@ void dictDeleteDict( TESSalloc* alloc, Dict *dict );
+ * to the given key.  If there is no such key, returns a node whose
+ * key is NULL.  Similarly, Succ(Max(d)) has a NULL key, etc.
+ */
+-DictNode *dictSearch( Dict *dict, DictKey key );
+-DictNode *dictInsertBefore( Dict *dict, DictNode *node, DictKey key );
++DictNode *dictSearch( Dict *dict, ActiveRegion *key );
++DictNode *dictInsertBefore( Dict *dict, DictNode *node, ActiveRegion *key );
+ void dictDelete( Dict *dict, DictNode *node );
+ 
+ #define dictKey(n)	((n)->key)
+@@ -59,7 +61,7 @@ void dictDelete( Dict *dict, DictNode *node );
+ /*** Private data structures ***/
+ 
+ struct DictNode {
+-	DictKey	key;
++	ActiveRegion *key;
+ 	DictNode *next;
+ 	DictNode *prev;
+ };
+@@ -68,7 +70,7 @@ struct Dict {
+ 	DictNode head;
+ 	void *frame;
+ 	struct BucketAlloc *nodePool;
+-	int (*leq)(void *frame, DictKey key1, DictKey key2);
++	int (*leq)(TESStesselator *frame, ActiveRegion *key1, ActiveRegion *key2);
+ };
+ 
+ #endif
+diff --git a/third_party/libtess2/src/Source/sweep.c b/third_party/libtess2/src/Source/sweep.c
+index 32a56bf406040..8bcd839f73cc4 100644
+--- a/third_party/libtess2/src/Source/sweep.c
++++ b/third_party/libtess2/src/Source/sweep.c
+@@ -1115,7 +1115,7 @@ static void InitEdgeDict( TESStesselator *tess )
+ 	TESSreal w, h;
+ 	TESSreal smin, smax, tmin, tmax;
+ 
+-	tess->dict = dictNewDict( &tess->alloc, tess, (int (*)(void *, DictKey, DictKey)) EdgeLeq );
++	tess->dict = dictNewDict( &tess->alloc, tess, EdgeLeq );
+ 	if (tess->dict == NULL) longjmp(tess->env,1);
+ 
+ 	/* If the bbox is empty, ensure that sentinels are not coincident by slightly enlarging it. */

third_party/libtess2/src/Source/dict.c

@@ -35,7 +35,7 @@
 #include "dict.h"
 
 /* really tessDictListNewDict */
-Dict *dictNewDict( TESSalloc* alloc, void *frame, int (*leq)(void *frame, DictKey key1, DictKey key2) )
+Dict *dictNewDict( TESSalloc* alloc, void *frame, int (*leq)(TESStesselator *frame, ActiveRegion *key1, ActiveRegion *key2) )
 {
 	Dict *dict = (Dict *)alloc->memalloc( alloc->userData, sizeof( Dict ));
 	DictNode *head;
@@ -68,7 +68,7 @@ void dictDeleteDict( TESSalloc* alloc, Dict *dict )
 }
 
 /* really tessDictListInsertBefore */
-DictNode *dictInsertBefore( Dict *dict, DictNode *node, DictKey key )
+DictNode *dictInsertBefore( Dict *dict, DictNode *node, ActiveRegion *key )
 {
 	DictNode *newNode;
 
@@ -97,7 +97,7 @@ void dictDelete( Dict *dict, DictNode *node ) /*ARGSUSED*/
 }
 
 /* really tessDictListSearch */
-DictNode *dictSearch( Dict *dict, DictKey key )
+DictNode *dictSearch( Dict *dict, ActiveRegion *key )
 {
 	DictNode *node = &dict->head;
 

third_party/libtess2/src/Source/dict.h

@@ -32,11 +32,13 @@
 #ifndef DICT_LIST_H
 #define DICT_LIST_H
 
-typedef void *DictKey;
+#include "../Include/tesselator.h"
+#include "mesh.h"
+
 typedef struct Dict Dict;
 typedef struct DictNode DictNode;
 
-Dict *dictNewDict( TESSalloc* alloc, void *frame, int (*leq)(void *frame, DictKey key1, DictKey key2) );
+Dict *dictNewDict( TESSalloc* alloc, void *frame, int (*leq)(TESStesselator *frame, ActiveRegion *key1, ActiveRegion *key2) );
 
 void dictDeleteDict( TESSalloc* alloc, Dict *dict );
 
@@ -44,8 +46,8 @@ void dictDeleteDict( TESSalloc* alloc, Dict *dict );
 * to the given key.  If there is no such key, returns a node whose
 * key is NULL.  Similarly, Succ(Max(d)) has a NULL key, etc.
 */
-DictNode *dictSearch( Dict *dict, DictKey key );
-DictNode *dictInsertBefore( Dict *dict, DictNode *node, DictKey key );
+DictNode *dictSearch( Dict *dict, ActiveRegion *key );
+DictNode *dictInsertBefore( Dict *dict, DictNode *node, ActiveRegion *key );
 void dictDelete( Dict *dict, DictNode *node );
 
 #define dictKey(n)	((n)->key)
@@ -59,7 +61,7 @@ void dictDelete( Dict *dict, DictNode *node );
 /*** Private data structures ***/
 
 struct DictNode {
-	DictKey	key;
+	ActiveRegion *key;
 	DictNode *next;
 	DictNode *prev;
 };
@@ -68,7 +70,7 @@ struct Dict {
 	DictNode head;
 	void *frame;
 	struct BucketAlloc *nodePool;
-	int (*leq)(void *frame, DictKey key1, DictKey key2);
+	int (*leq)(TESStesselator *frame, ActiveRegion *key1, ActiveRegion *key2);
 };
 
 #endif

third_party/libtess2/src/Source/sweep.c

@@ -1115,7 +1115,7 @@ static void InitEdgeDict( TESStesselator *tess )
 	TESSreal w, h;
 	TESSreal smin, smax, tmin, tmax;
 
-	tess->dict = dictNewDict( &tess->alloc, tess, (int (*)(void *, DictKey, DictKey)) EdgeLeq );
+	tess->dict = dictNewDict( &tess->alloc, tess, EdgeLeq );
 	if (tess->dict == NULL) longjmp(tess->env,1);
 
 	/* If the bbox is empty, ensure that sentinels are not coincident by slightly enlarging it. */