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)