[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...
25 Nov
2014
25 Nov
'14
11:44 p.m.
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)
3683
days inactive
3683
days old
0 comments
1 participants
participants (1)
-
tkreuzer＠svn.reactos.org