[ros-diffs] [tkreuzer] 65490: [WIN32K] - Implement FLOATOBJ_bConvertToLong inline function that converts a FLOATOBJ to a long or returns FALSE if the value would overflow a LONG - Remove underscore prefixes fr...

Author: tkreuzer
Date: Tue Nov 25 23:44:59 2014
New Revision: 65490

URL: http://svn.reactos.org/svn/reactos?rev=65490&view=rev
Log:
[WIN32K]
- Implement FLOATOBJ_bConvertToLong inline function that converts a FLOATOBJ to a long or
returns FALSE if the value would overflow a LONG
- Remove underscore prefixes from inline FLOATOBJ functions and use it only on those that
already exist as non-inline versions.
- Remove duplicated FLOATOBJ defines for non-x86
- Fail on integer overflow in XFORMOBJ_bXformFixPoints to avoid creating bogus
coordinates.

Modified:
    trunk/reactos/win32ss/gdi/eng/floatobj.h
    trunk/reactos/win32ss/gdi/ntgdi/freetype.c
    trunk/reactos/win32ss/gdi/ntgdi/xformobj.c

Modified: trunk/reactos/win32ss/gdi/eng/floatobj.h
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/gdi/eng/floatobj.h…
==============================================================================
--- trunk/reactos/win32ss/gdi/eng/floatobj.h	[iso-8859-1] (original)
+++ trunk/reactos/win32ss/gdi/eng/floatobj.h	[iso-8859-1] Tue Nov 25 23:44:59 2014
@@ -1,4 +1,8 @@
 #pragma once
+
+C_ASSERT(sizeof(FIX) == sizeof(LONG));
+#define FIX2LONG(x) (((x) + 8) >> 4)
+#define LONG2FIX(x) ((x) << 4)
 
 #if defined(_M_IX86)
 
@@ -10,6 +14,7 @@
     EFLOAT_S *pef2 = (EFLOAT_S*)pf2;
     return (pef1->lMant == pef2->lMant && pef1->lExp == pef2->lExp);
 }
+#define FLOATOBJ_Equal _FLOATOBJ_Equal
 
 FORCEINLINE
 LONG
@@ -18,10 +23,36 @@
     EFLOAT_S *pef = (EFLOAT_S*)pf;
     return pef->lMant >> (32 - pef->lExp);
 }
+#define FLOATOBJ_GetLong _FLOATOBJ_GetLong
+
+/*!
+ * \brief Converts a FLOATOBJ into a LONG by truncating the value to integer
+ *
+ * \param pf - Pointer to a FLOATOBJ containing the value to convert
+ *
+ * \param pl - Pointer to a variable that receives the result
+ *
+ * \return TRUE if the function succeeded, FALSE if the result would overflow
+ *         a LONG.
+ */
+FORCEINLINE
+BOOL
+FASTCALL
+FLOATOBJ_bConvertToLong(FLOATOBJ *pf, PLONG pl)
+{
+    EFLOAT_S *pef = (EFLOAT_S*)pf;
+    LONG lShift = 32 - pef->lExp;
+    if (lShift < 0)
+    {
+        return FALSE;
+    }
+    *pl = pef->lMant >> lShift;
+    return TRUE;
+}
 
 FORCEINLINE
 LONG
-_FLOATOBJ_GetFix(FLOATOBJ *pf)
+FLOATOBJ_GetFix(FLOATOBJ *pf)
 {
     EFLOAT_S *pef = (EFLOAT_S*)pf;
     LONG Shift = (28 - pef->lExp);
@@ -30,7 +61,7 @@
 
 FORCEINLINE
 BOOL
-_FLOATOBJ_IsLong(FLOATOBJ *pf)
+FLOATOBJ_IsLong(FLOATOBJ *pf)
 {
     EFLOAT_S *pef = (EFLOAT_S*)pf;
     ULONG ulShift = pef->lExp;
@@ -42,7 +73,7 @@
 
 FORCEINLINE
 BOOL
-_FLOATOBJ_Equal0(FLOATOBJ *pf)
+FLOATOBJ_Equal0(FLOATOBJ *pf)
 {
     EFLOAT_S *pef = (EFLOAT_S*)pf;
     return (pef->lMant == 0 && pef->lExp == 0);
@@ -50,7 +81,7 @@
 
 FORCEINLINE
 BOOL
-_FLOATOBJ_Equal1(FLOATOBJ *pf)
+FLOATOBJ_Equal1(FLOATOBJ *pf)
 {
     EFLOAT_S *pef = (EFLOAT_S*)pf;
     return (pef->lMant == 0x40000000 && pef->lExp == 2);
@@ -70,12 +101,11 @@
 
 #else
 
-#define _FLOATOBJ_Equal(pf,pf1) (*(pf) == *(pf1))
-#define _FLOATOBJ_GetLong(pf) ((LONG)*(pf))
-#define _FLOATOBJ_IsLong(pf) ((FLOAT)((LONG)*(pf)) == *(pf))
-#define _FLOATOBJ_Equal0(pf) (*(pf) == 0.)
-#define _FLOATOBJ_Equal1(pf) (*(pf) == 1.)
-#define _FLOATOBJ_GetFix(pf) ((LONG)(*(pf) * 16.))
+#define FLOATOBJ_bConvertToLong(pf, pl) (*pl = (LONG)*pf, TRUE)
+#define FLOATOBJ_IsLong(pf) ((FLOAT)((LONG)*(pf)) == *(pf))
+#define FLOATOBJ_Equal0(pf) (*(pf) == 0.)
+#define FLOATOBJ_Equal1(pf) (*(pf) == 1.)
+#define FLOATOBJ_GetFix(pf) ((LONG)(*(pf) * 16.))
 
 #define FLOATOBJ_0 0.
 #define FLOATOBJ_1 1.

Modified: trunk/reactos/win32ss/gdi/ntgdi/freetype.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/gdi/ntgdi/freetype…
==============================================================================
--- trunk/reactos/win32ss/gdi/ntgdi/freetype.c	[iso-8859-1] (original)
+++ trunk/reactos/win32ss/gdi/ntgdi/freetype.c	[iso-8859-1] Tue Nov 25 23:44:59 2014
@@ -1105,7 +1105,7 @@
         }
         if (fs.fsCsb[0] == 0)
         { /* Let's see if we can find any interesting cmaps */
-            for (i = 0; i < FontGDI->face->num_charmaps; i++)
+            for (i = 0; i < (UINT)FontGDI->face->num_charmaps; i++)
             {
                 switch (FontGDI->face->charmaps[i]->encoding)
                 {
@@ -1501,7 +1501,7 @@
 {
     TTPOLYGONHEADER *pph;
     TTPOLYCURVE *ppc;
-    unsigned int needed = 0, point = 0, contour, first_pt;
+    int needed = 0, point = 0, contour, first_pt;
     unsigned int pph_start, cpfx;
     DWORD type;
 
@@ -2073,7 +2073,7 @@
             INT x;
             while (h--)
             {
-                for (x = 0; x < pitch; x++)
+                for (x = 0; (UINT)x < pitch; x++)
                 {
                     if (x < ft_face->glyph->bitmap.width)
                         dst[x] = (src[x / 8] & (1 << ( (7 - (x % 8))))) ? 0xff
: 0;
@@ -2346,7 +2346,8 @@
     DWORD dwFlags)
 {
     PDC_ATTR pdcattr;
-    UINT Ret = DEFAULT_CHARSET, i;
+    UINT Ret = DEFAULT_CHARSET;
+    INT i;
     HFONT hFont;
     PTEXTOBJ TextObj;
     PFONTGDI FontGdi;
@@ -3752,7 +3753,7 @@
         {
             // FIXME this should probably be a matrix transform with TextTop as well.
             Scale = pdcattr->mxWorldToDevice.efM11;
-            if (_FLOATOBJ_Equal0(&Scale))
+            if (FLOATOBJ_Equal0(&Scale))
                 FLOATOBJ_Set1(&Scale);
 
             FLOATOBJ_MulLong(&Scale, Dx[i<<DxShift] << 6); // do the
shift before multiplying to preserve precision
@@ -4029,7 +4030,7 @@
     face = FontGDI->face;
     if (face->charmap == NULL)
     {
-        for (i = 0; i < face->num_charmaps; i++)
+        for (i = 0; i < (UINT)face->num_charmaps; i++)
         {
             charmap = face->charmaps[i];
             if (charmap->encoding != 0)
@@ -4227,7 +4228,7 @@
     face = FontGDI->face;
     if (face->charmap == NULL)
     {
-        for (i = 0; i < face->num_charmaps; i++)
+        for (i = 0; i < (UINT)face->num_charmaps; i++)
         {
             charmap = face->charmaps[i];
             if (charmap->encoding != 0)

Modified: trunk/reactos/win32ss/gdi/ntgdi/xformobj.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/gdi/ntgdi/xformobj…
==============================================================================
--- trunk/reactos/win32ss/gdi/ntgdi/xformobj.c	[iso-8859-1] (original)
+++ trunk/reactos/win32ss/gdi/ntgdi/xformobj.c	[iso-8859-1] Tue Nov 25 23:44:59 2014
@@ -12,16 +12,8 @@
 #define NDEBUG
 #include <debug.h>
 
-C_ASSERT(sizeof(FIX) == sizeof(LONG));
-#define FIX2LONG(x) (((x) + 8) >> 4)
-#define LONG2FIX(x) ((x) << 4)
-
-#define FLOATOBJ_Equal _FLOATOBJ_Equal
-#define FLOATOBJ_GetLong _FLOATOBJ_GetLong
-#define FLOATOBJ_GetFix _FLOATOBJ_GetFix
-#define FLOATOBJ_IsLong _FLOATOBJ_IsLong
-#define FLOATOBJ_Equal0 _FLOATOBJ_Equal0
-#define FLOATOBJ_Equal1 _FLOATOBJ_Equal1
+#define DOES_VALUE_OVERFLOW_LONG(x) \
+    (((__int64)((long)(x))) != (x))
 
 /** Inline helper functions ***************************************************/
 
@@ -297,76 +289,153 @@
     return XFORMOBJ_UpdateAccel(pxoDst);
 }
 
+
+/*!
+ * \brief Transforms fix-point coordinates in an array of POINTL structures using
+ *        the transformation matrix from the XFORMOBJ.
+ *
+ * \param pxo - Pointer to the XFORMOBJ
+ *
+ * \param cPoints - Number of coordinates to transform
+ *
+ * \param pptIn - Pointer to an array of POINTL structures containing the
+ *        source coordinates.
+ *
+ * \param pptOut - Pointer to an array of POINTL structures, receiving the
+ *        transformed coordinates. Can be the same as pptIn.
+ *
+ * \return TRUE if the operation was successful, FALSE if any of the calculations
+ *         caused an integer overflow.
+ *
+ * \note If the function returns FALSE, it might still have written to the
+ *       output buffer. If pptIn and pptOut are equal, the source coordinates
+ *       might have been partly overwritten!
+ */
 static
-VOID
+BOOL
 NTAPI
-XFORMOBJ_vXformFixPoints(
-    IN XFORMOBJ  *pxo,
-    IN ULONG  cPoints,
-    IN PPOINTL  pptIn,
-    OUT PPOINTL  pptOut)
+XFORMOBJ_bXformFixPoints(
+    _In_ XFORMOBJ *pxo,
+    _In_ ULONG cPoints,
+    _In_reads_(cPoints) PPOINTL pptIn,
+    _Out_writes_(cPoints) PPOINTL pptOut)
 {
     PMATRIX pmx;
     INT i;
     FLOATOBJ fo1, fo2;
     FLONG flAccel;
+    LONG lM11, lM12, lM21, lM22, lTemp;
+    register LONGLONG llx, lly;
 
     pmx = XFORMOBJ_pmx(pxo);
     flAccel = pmx->flAccel;
 
     if ((flAccel & (XFORM_SCALE|XFORM_UNITY)) == (XFORM_SCALE|XFORM_UNITY))
     {
-        /* Identity transformation, nothing todo */
+        /* Identity transformation, nothing to do */
     }
     else if (flAccel & XFORM_INTEGER)
     {
         if (flAccel & XFORM_UNITY)
         {
-            /* 1-scale integer transform */
-            LONG lM12 = FLOATOBJ_GetLong(&pmx->efM12);
-            LONG lM21 = FLOATOBJ_GetLong(&pmx->efM21);
+            /* 1-scale integer transform, get the off-diagonal elements */
+            if (!FLOATOBJ_bConvertToLong(&pmx->efM12, &lM12) ||
+                !FLOATOBJ_bConvertToLong(&pmx->efM21, &lM21))
+            {
+                NT_ASSERT(FALSE);
+                return FALSE;
+            }
 
             i = cPoints - 1;
             do
             {
-                LONG x = pptIn[i].x + pptIn[i].y * lM21;
-                LONG y = pptIn[i].y + pptIn[i].x * lM12;
-                pptOut[i].y = y;
-                pptOut[i].x = x;
+                /* Calculate x in 64 bit and check for overflow */
+                llx = Int32x32To64(pptIn[i].y, lM21) + pptIn[i].x;
+                if (DOES_VALUE_OVERFLOW_LONG(llx))
+                {
+                    return FALSE;
+                }
+
+                /* Calculate y in 64 bit and check for overflow */
+                lly = Int32x32To64(pptIn[i].x, lM12) + pptIn[i].y;
+                if (DOES_VALUE_OVERFLOW_LONG(lly))
+                {
+                    return FALSE;
+                }
+
+                /* Write back the results */
+                pptOut[i].x = (LONG)llx;
+                pptOut[i].y = (LONG)lly;
             }
             while (--i >= 0);
         }
         else if (flAccel & XFORM_SCALE)
         {
-            /* Diagonal integer transform */
-            LONG lM11 = FLOATOBJ_GetLong(&pmx->efM11);
-            LONG lM22 = FLOATOBJ_GetLong(&pmx->efM22);
+            /* Diagonal integer transform, get the diagonal elements */
+            if (!FLOATOBJ_bConvertToLong(&pmx->efM11, &lM11) ||
+                !FLOATOBJ_bConvertToLong(&pmx->efM22, &lM22))
+            {
+                NT_ASSERT(FALSE);
+                return FALSE;
+            }
 
             i = cPoints - 1;
             do
             {
-                pptOut[i].x = pptIn[i].x * lM11;
-                pptOut[i].y = pptIn[i].y * lM22;
+                /* Calculate x in 64 bit and check for overflow */
+                llx = Int32x32To64(pptIn[i].x, lM11);
+                if (DOES_VALUE_OVERFLOW_LONG(llx))
+                {
+                    return FALSE;
+                }
+
+                /* Calculate y in 64 bit and check for overflow */
+                lly = Int32x32To64(pptIn[i].y, lM22);
+                if (DOES_VALUE_OVERFLOW_LONG(lly))
+                {
+                    return FALSE;
+                }
+
+                /* Write back the results */
+                pptOut[i].x = (LONG)llx;
+                pptOut[i].y = (LONG)lly;
             }
             while (--i >= 0);
         }
         else
         {
             /* Full integer transform */
-            LONG lM11 = FLOATOBJ_GetLong(&pmx->efM11);
-            LONG lM12 = FLOATOBJ_GetLong(&pmx->efM12);
-            LONG lM21 = FLOATOBJ_GetLong(&pmx->efM21);
-            LONG lM22 = FLOATOBJ_GetLong(&pmx->efM22);
+            if (!FLOATOBJ_bConvertToLong(&pmx->efM11, &lM11) ||
+                !FLOATOBJ_bConvertToLong(&pmx->efM12, &lM12) ||
+                !FLOATOBJ_bConvertToLong(&pmx->efM21, &lM21) ||
+                !FLOATOBJ_bConvertToLong(&pmx->efM22, &lM22))
+            {
+                NT_ASSERT(FALSE);
+                return FALSE;
+            }
 
             i = cPoints - 1;
             do
             {
-                LONG x;
-                x  = pptIn[i].x * lM11;
-                x += pptIn[i].y * lM21;
-                pptOut[i].y  = pptIn[i].y * lM22;
-                pptOut[i].y += pptIn[i].x * lM12;
-                pptOut[i].x = x;
+                /* Calculate x in 64 bit and check for overflow */
+                llx  = Int32x32To64(pptIn[i].x, lM11);
+                llx += Int32x32To64(pptIn[i].y, lM21);
+                if (DOES_VALUE_OVERFLOW_LONG(llx))
+                {
+                    return FALSE;
+                }
+
+                /* Calculate y in 64 bit and check for overflow */
+                lly  = Int32x32To64(pptIn[i].y, lM22);
+                lly += Int32x32To64(pptIn[i].x, lM12);
+                if (DOES_VALUE_OVERFLOW_LONG(lly))
+                {
+                    return FALSE;
+                }
+
+                /* Write back the results */
+                pptOut[i].x = (LONG)llx;
+                pptOut[i].y = (LONG)lly;
             }
             while (--i >= 0);
         }
@@ -377,12 +446,35 @@
         i = cPoints - 1;
         do
         {
+            /* Calculate x in 64 bit and check for overflow */
             fo1 = pmx->efM21;
             FLOATOBJ_MulLong(&fo1, pptIn[i].y);
+            if (!FLOATOBJ_bConvertToLong(&fo1, &lTemp))
+            {
+                return FALSE;
+            }
+            llx = (LONGLONG)pptIn[i].x + lTemp;
+            if (DOES_VALUE_OVERFLOW_LONG(llx))
+            {
+                return FALSE;
+            }
+
+            /* Calculate y in 64 bit and check for overflow */
             fo2 = pmx->efM12;
             FLOATOBJ_MulLong(&fo2, pptIn[i].x);
-            pptOut[i].x = pptIn[i].x + FLOATOBJ_GetLong(&fo1);
-            pptOut[i].y = pptIn[i].y + FLOATOBJ_GetLong(&fo2);
+            if (!FLOATOBJ_bConvertToLong(&fo2, &lTemp))
+            {
+                return FALSE;
+            }
+            lly = (LONGLONG)pptIn[i].y + lTemp;
+            if (DOES_VALUE_OVERFLOW_LONG(lly))
+            {
+                return FALSE;
+            }
+
+            /* Write back the results */
+            pptOut[i].x = (LONG)llx;
+            pptOut[i].y = (LONG)lly;
         }
         while (--i >= 0);
     }
@@ -394,10 +486,17 @@
         {
             fo1 = pmx->efM11;
             FLOATOBJ_MulLong(&fo1, pptIn[i].x);
-            pptOut[i].x = FLOATOBJ_GetLong(&fo1);
+            if (!FLOATOBJ_bConvertToLong(&fo1, &pptOut[i].x))
+            {
+                return FALSE;
+            }
+
             fo2 = pmx->efM22;
             FLOATOBJ_MulLong(&fo2, pptIn[i].y);
-            pptOut[i].y = FLOATOBJ_GetLong(&fo2);
+            if (!FLOATOBJ_bConvertToLong(&fo2, &pptOut[i].y))
+            {
+                return FALSE;
+            }
         }
         while (--i >= 0);
     }
@@ -407,10 +506,21 @@
         i = cPoints - 1;
         do
         {
+            /* Calculate x as FLOATOBJ */
             MulAddLong(&fo1, &pmx->efM11, pptIn[i].x, &pmx->efM21,
pptIn[i].y);
+
+            /* Calculate y as FLOATOBJ */
             MulAddLong(&fo2, &pmx->efM12, pptIn[i].x, &pmx->efM22,
pptIn[i].y);
-            pptOut[i].x = FLOATOBJ_GetLong(&fo1);
-            pptOut[i].y = FLOATOBJ_GetLong(&fo2);
+
+            if (!FLOATOBJ_bConvertToLong(&fo1, &pptOut[i].x))
+            {
+                return FALSE;
+            }
+
+            if (!FLOATOBJ_bConvertToLong(&fo2, &pptOut[i].y))
+            {
+                return FALSE;
+            }
         }
         while (--i >= 0);
     }
@@ -421,11 +531,24 @@
         i = cPoints - 1;
         do
         {
-            pptOut[i].x += pmx->fxDx;
-            pptOut[i].y += pmx->fxDy;
+            llx = (LONGLONG)pptOut[i].x + pmx->fxDx;
+            if (DOES_VALUE_OVERFLOW_LONG(llx))
+            {
+                return FALSE;
+            }
+            pptOut[i].x = (LONG)llx;
+
+            lly = (LONGLONG)pptOut[i].y + pmx->fxDy;
+            if (DOES_VALUE_OVERFLOW_LONG(lly))
+            {
+                return FALSE;
+            }
+            pptOut[i].y = (LONG)lly;
         }
         while (--i >= 0);
     }
+
+    return TRUE;
 }
 
 /** Public functions **********************************************************/
@@ -534,7 +657,10 @@
     }
 
     /* Do the actual fixpoint transformation */
-    XFORMOBJ_vXformFixPoints(pxo, cPoints, pvIn, pvOut);
+    if (!XFORMOBJ_bXformFixPoints(pxo, cPoints, pvIn, pvOut))
+    {
+        return FALSE;
+    }
 
     /* Convert POINTFIX to POINTL? */
     if (iMode == XF_INV_FXTOL || iMode == XF_INV_LTOL || iMode == XF_LTOL)