1. Subtracting integers may result in an overflow or underflow. 2. Right at the 'edge' of underflowing, the result of subtraction may be `INT_MIN`, and the call to `abs()` will also result in `INT_MIN`.
This fix accounts for all of these conditions.
EXAMPLES: 1. can be encountered by comparing 2.0 and -2.0 2. can be encountered by comparing -2.0 and 2.0
NOTE: There are 14 more instances of `compare_float` across several modules. I would like to make sure the logic is sound with this MR, then I can take on the rest.
-- v2: ddraw/tests: Fix overflow bugs in compare_float.
From: Jeff Smith whydoubt@gmail.com
--- dlls/ddraw/tests/ddraw1.c | 27 ++++++++++++--------------- dlls/ddraw/tests/ddraw2.c | 27 ++++++++++++--------------- dlls/ddraw/tests/ddraw4.c | 27 ++++++++++++--------------- dlls/ddraw/tests/ddraw7.c | 27 ++++++++++++--------------- 4 files changed, 48 insertions(+), 60 deletions(-)
diff --git a/dlls/ddraw/tests/ddraw1.c b/dlls/ddraw/tests/ddraw1.c index cde80c58e99..bb90b77d527 100644 --- a/dlls/ddraw/tests/ddraw1.c +++ b/dlls/ddraw/tests/ddraw1.c @@ -50,20 +50,24 @@ struct create_window_thread_param HANDLE thread; };
+static BOOL compare_uint(unsigned int x, unsigned int y, unsigned int max_diff) +{ + unsigned int diff = x > y ? x - y : y - x; + + return diff <= max_diff; +} + static BOOL compare_float(float f, float g, unsigned int ulps) { int x = *(int *)&f; int y = *(int *)&g;
- if (x < 0) - x = INT_MIN - x; - if (y < 0) - y = INT_MIN - y; - - if (abs(x - y) > ulps) - return FALSE; + if (x >= 0) + x = -(INT_MIN + x); + if (y >= 0) + y = -(INT_MIN + y);
- return TRUE; + return compare_uint(x, y, ulps); }
static BOOL compare_vec4(const struct vec4 *vec, float x, float y, float z, float w, unsigned int ulps) @@ -74,13 +78,6 @@ static BOOL compare_vec4(const struct vec4 *vec, float x, float y, float z, floa && compare_float(vec->w, w, ulps); }
-static BOOL compare_uint(unsigned int x, unsigned int y, unsigned int max_diff) -{ - unsigned int diff = x > y ? x - y : y - x; - - return diff <= max_diff; -} - static BOOL compare_color(D3DCOLOR c1, D3DCOLOR c2, BYTE max_diff) { return compare_uint(c1 & 0xff, c2 & 0xff, max_diff) diff --git a/dlls/ddraw/tests/ddraw2.c b/dlls/ddraw/tests/ddraw2.c index 86056014f96..32496631d39 100644 --- a/dlls/ddraw/tests/ddraw2.c +++ b/dlls/ddraw/tests/ddraw2.c @@ -51,20 +51,24 @@ struct create_window_thread_param HANDLE thread; };
+static BOOL compare_uint(unsigned int x, unsigned int y, unsigned int max_diff) +{ + unsigned int diff = x > y ? x - y : y - x; + + return diff <= max_diff; +} + static BOOL compare_float(float f, float g, unsigned int ulps) { int x = *(int *)&f; int y = *(int *)&g;
- if (x < 0) - x = INT_MIN - x; - if (y < 0) - y = INT_MIN - y; - - if (abs(x - y) > ulps) - return FALSE; + if (x >= 0) + x = -(INT_MIN + x); + if (y >= 0) + y = -(INT_MIN + y);
- return TRUE; + return compate_uint(x, y, ulps); }
static BOOL compare_vec4(const struct vec4 *vec, float x, float y, float z, float w, unsigned int ulps) @@ -75,13 +79,6 @@ static BOOL compare_vec4(const struct vec4 *vec, float x, float y, float z, floa && compare_float(vec->w, w, ulps); }
-static BOOL compare_uint(unsigned int x, unsigned int y, unsigned int max_diff) -{ - unsigned int diff = x > y ? x - y : y - x; - - return diff <= max_diff; -} - static BOOL compare_color(D3DCOLOR c1, D3DCOLOR c2, BYTE max_diff) { return compare_uint(c1 & 0xff, c2 & 0xff, max_diff) diff --git a/dlls/ddraw/tests/ddraw4.c b/dlls/ddraw/tests/ddraw4.c index 4458cea5145..adbd111746d 100644 --- a/dlls/ddraw/tests/ddraw4.c +++ b/dlls/ddraw/tests/ddraw4.c @@ -57,20 +57,24 @@ struct create_window_thread_param HANDLE thread; };
+static BOOL compare_uint(unsigned int x, unsigned int y, unsigned int max_diff) +{ + unsigned int diff = x > y ? x - y : y - x; + + return diff <= max_diff; +} + static BOOL compare_float(float f, float g, unsigned int ulps) { int x = *(int *)&f; int y = *(int *)&g;
- if (x < 0) - x = INT_MIN - x; - if (y < 0) - y = INT_MIN - y; - - if (abs(x - y) > ulps) - return FALSE; + if (x >= 0) + x = -(INT_MIN + x); + if (y >= 0) + y = -(INT_MIN + y);
- return TRUE; + return compare_uint(x, y, ulps); }
static BOOL compare_vec4(const struct vec4 *vec, float x, float y, float z, float w, unsigned int ulps) @@ -81,13 +85,6 @@ static BOOL compare_vec4(const struct vec4 *vec, float x, float y, float z, floa && compare_float(vec->w, w, ulps); }
-static BOOL compare_uint(unsigned int x, unsigned int y, unsigned int max_diff) -{ - unsigned int diff = x > y ? x - y : y - x; - - return diff <= max_diff; -} - static BOOL compare_color(D3DCOLOR c1, D3DCOLOR c2, BYTE max_diff) { return compare_uint(c1 & 0xff, c2 & 0xff, max_diff) diff --git a/dlls/ddraw/tests/ddraw7.c b/dlls/ddraw/tests/ddraw7.c index bbb78169aea..e26a6ad7a8e 100644 --- a/dlls/ddraw/tests/ddraw7.c +++ b/dlls/ddraw/tests/ddraw7.c @@ -68,20 +68,24 @@ struct create_window_thread_param HANDLE thread; };
+static BOOL compare_uint(unsigned int x, unsigned int y, unsigned int max_diff) +{ + unsigned int diff = x > y ? x - y : y - x; + + return diff <= max_diff; +} + static BOOL compare_float(float f, float g, unsigned int ulps) { int x = *(int *)&f; int y = *(int *)&g;
- if (x < 0) - x = INT_MIN - x; - if (y < 0) - y = INT_MIN - y; - - if (abs(x - y) > ulps) - return FALSE; + if (x >= 0) + x = -(INT_MIN + x); + if (y >= 0) + y = -(INT_MIN + y);
- return TRUE; + return compare_uint(x, y, ulps); }
static BOOL compare_vec3(struct vec3 *vec, float x, float y, float z, unsigned int ulps) @@ -99,13 +103,6 @@ static BOOL compare_vec4(const struct vec4 *vec, float x, float y, float z, floa && compare_float(vec->w, w, ulps); }
-static BOOL compare_uint(unsigned int x, unsigned int y, unsigned int max_diff) -{ - unsigned int diff = x > y ? x - y : y - x; - - return diff <= max_diff; -} - static BOOL compare_color(D3DCOLOR c1, D3DCOLOR c2, BYTE max_diff) { return compare_uint(c1 & 0xff, c2 & 0xff, max_diff)
Hi,
It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated.
The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=135487
Your paranoid android.
=== build (build log) ===
/home/winetest/tools/testbot/var/wine-exe32/../wine/dlls/ddraw/tests/ddraw2.c:71: undefined reference to `compate_uint' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-exe32/../wine/dlls/ddraw/tests/ddraw2.c:71: undefined reference to `compate_uint' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-exe32/../wine/dlls/ddraw/tests/ddraw2.c:71: undefined reference to `compate_uint' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-exe32/../wine/dlls/ddraw/tests/ddraw2.c:71: undefined reference to `compate_uint' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-exe32/../wine/dlls/ddraw/tests/ddraw2.c:71: undefined reference to `compate_uint' collect2: error: ld returned 1 exit status Task: The exe32 Wine build failed
=== debian11 (build log) ===
/home/winetest/tools/testbot/var/wine-win32/../wine/dlls/ddraw/tests/ddraw2.c:71: undefined reference to `compate_uint' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-win32/../wine/dlls/ddraw/tests/ddraw2.c:71: undefined reference to `compate_uint' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-win32/../wine/dlls/ddraw/tests/ddraw2.c:71: undefined reference to `compate_uint' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-win32/../wine/dlls/ddraw/tests/ddraw2.c:71: undefined reference to `compate_uint' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-win32/../wine/dlls/ddraw/tests/ddraw2.c:71: undefined reference to `compate_uint' /home/winetest/tools/testbot/var/wine-win32/../wine/dlls/ddraw/tests/ddraw2.c:71: undefined reference to `compate_uint' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-win32/../wine/dlls/ddraw/tests/ddraw2.c:71: undefined reference to `compate_uint' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-win32/../wine/dlls/ddraw/tests/ddraw2.c:71: undefined reference to `compate_uint' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-win32/../wine/dlls/ddraw/tests/ddraw2.c:71: undefined reference to `compate_uint' /usr/bin/i686-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-win32/../wine/dlls/ddraw/tests/ddraw2.c:71: undefined reference to `compate_uint' collect2: error: ld returned 1 exit status collect2: error: ld returned 1 exit status Task: The win32 Wine build failed
=== debian11b (build log) ===
/home/winetest/tools/testbot/var/wine-wow64/../wine/dlls/ddraw/tests/ddraw2.c:71: undefined reference to `compate_uint' /usr/bin/x86_64-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-wow64/../wine/dlls/ddraw/tests/ddraw2.c:71: undefined reference to `compate_uint' /usr/bin/x86_64-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-wow64/../wine/dlls/ddraw/tests/ddraw2.c:71: undefined reference to `compate_uint' /usr/bin/x86_64-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-wow64/../wine/dlls/ddraw/tests/ddraw2.c:71: undefined reference to `compate_uint' /usr/bin/x86_64-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-wow64/../wine/dlls/ddraw/tests/ddraw2.c:71: undefined reference to `compate_uint' /home/winetest/tools/testbot/var/wine-wow64/../wine/dlls/ddraw/tests/ddraw2.c:71: undefined reference to `compate_uint' /usr/bin/x86_64-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-wow64/../wine/dlls/ddraw/tests/ddraw2.c:71: undefined reference to `compate_uint' /usr/bin/x86_64-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-wow64/../wine/dlls/ddraw/tests/ddraw2.c:71: undefined reference to `compate_uint' /usr/bin/x86_64-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-wow64/../wine/dlls/ddraw/tests/ddraw2.c:71: undefined reference to `compate_uint' /usr/bin/x86_64-w64-mingw32-ld: /home/winetest/tools/testbot/var/wine-wow64/../wine/dlls/ddraw/tests/ddraw2.c:71: undefined reference to `compate_uint' collect2: error: ld returned 1 exit status collect2: error: ld returned 1 exit status Task: The wow64 Wine build failed
-
Jeff Smith -
Jeffrey Smith (＠whydoubt)
-
Marvin