[PATCH v2 0/1] MR3458: ddraw/tests: Fix overflow bugs in compare_float.
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. https://gitlab.winehq.org/wine/wine/-/merge_requests/3458
From: Jeff Smith <whydoubt(a)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) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3458
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
participants (1)
-
Jeff Smith -
Jeffrey Smith (＠whydoubt)
-
Marvin