From ad679187d3f98e91872bf1be928eb33f243a9608 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Sat, 27 Jul 2024 19:27:13 +0100 Subject: [PATCH 1/2] gsk, testsuite: Avoid undefined behaviour in float_to_half_one() If, for example, e == 0, it is undefined behaviour to compute an expression involving an out-of-range shift by (125 - e), even if the result is in fact irrelevant because it's going to be multiplied by 0. This was already fixed for the memorytexture test in commit 5d1b839 "testsuite: Fix another ubsan warning", so use the implementation from that test everywhere. It's in the header as an inline function to keep the linking of the relevant tests simple: its only caller in production code is fp16.c, so there will be no duplication outside the test suite. Detected by running a subset of the test suite with -Dsanitize=address,undefined on x86_64. Signed-off-by: Simon McVittie --- gsk/gl/fp16.c | 9 ------ gsk/gl/fp16private.h | 20 ++++++++++++ testsuite/gdk/memorytexture.c | 58 ++++++++++++----------------------- testsuite/gsk/scaling.c | 41 ++++++++++--------------- 4 files changed, 57 insertions(+), 71 deletions(-) diff --git a/gsk/gl/fp16.c b/gsk/gl/fp16.c index 0f767e9334..3f67f339ec 100644 --- a/gsk/gl/fp16.c +++ b/gsk/gl/fp16.c @@ -45,15 +45,6 @@ half_to_float_one (const guint16 x) return as_float((x&0x8000)<<16 | (e!=0)*((e+112)<<23|m) | ((e==0)&(m!=0))*((v-37)<<23|((m<<(150-v))&0x007FE000))); } -static inline guint16 -float_to_half_one (const float x) -{ - const guint b = as_uint(x)+0x00001000; // round-to-nearest-even - const guint e = (b&0x7F800000)>>23; // exponent - const guint m = b&0x007FFFFF; // mantissa - return (b&0x80000000)>>16 | (e>112)*((((e-112)<<10)&0x7C00)|m>>13) | ((e<113)&(e>101))*((((0x007FF000+m)>>(125-e))+1)>>1) | (e>143)*0x7FFF; // sign : normalized : denormalized : saturate -} - void float_to_half4_c (const float f[4], guint16 h[4]) diff --git a/gsk/gl/fp16private.h b/gsk/gl/fp16private.h index faeeaa2d81..29e87897d4 100644 --- a/gsk/gl/fp16private.h +++ b/gsk/gl/fp16private.h @@ -28,6 +28,26 @@ G_BEGIN_DECLS #define FP16_ONE ((guint16)15360) #define FP16_MINUS_ONE ((guint16)48128) +static inline guint16 +float_to_half_one (const float x) +{ + const guint b = *(guint*)&x+0x00001000; // round-to-nearest-even + const guint e = (b&0x7F800000)>>23; // exponent + const guint m = b&0x007FFFFF; // mantissa + guint n0 = 0; + guint n1 = 0; + guint n2 = 0; + + if (e > 112) + n0 = (((e - 112) << 10) & 0x7C00) | m >> 13; + if (e < 113 && e > 101) + n1 = (((0x007FF000 + m) >> (125- e)) + 1) >> 1; + if (e > 143) + n2 = 0x7FFF; + + return (b & 0x80000000) >> 16 | n0 | n1 | n2; // sign : normalized : denormalized : saturate +} + void float_to_half4 (const float f[4], guint16 h[4]); diff --git a/testsuite/gdk/memorytexture.c b/testsuite/gdk/memorytexture.c index 4f7a3379a9..989f633d5a 100644 --- a/testsuite/gdk/memorytexture.c +++ b/testsuite/gdk/memorytexture.c @@ -2,6 +2,8 @@ #include +#include "gsk/gl/fp16private.h" + #define N 10 static GdkGLContext *gl_context = NULL; @@ -59,7 +61,7 @@ as_float (const guint x) // IEEE-754 16-bit floating-point format (without infinity): 1-5-10 // static inline float -half_to_float (const guint16 x) +half_to_float_one (const guint16 x) { const guint e = (x&0x7C00)>>10; // exponent const guint m = (x&0x03FF)<<13; // mantissa @@ -67,26 +69,6 @@ half_to_float (const guint16 x) return as_float((x&0x8000)<<16 | (e!=0)*((e+112)<<23|m) | ((e==0)&(m!=0))*((v-37)<<23|((m<<(150-v))&0x007FE000))); } -static inline guint16 -float_to_half (const float x) -{ - const guint b = *(guint*)&x+0x00001000; // round-to-nearest-even - const guint e = (b&0x7F800000)>>23; // exponent - const guint m = b&0x007FFFFF; // mantissa - guint n0 = 0; - guint n1 = 0; - guint n2 = 0; - - if (e > 112) - n0 = (((e - 112) << 10) & 0x7C00) | m >> 13; - if (e < 113 && e > 101) - n1 = (((0x007FF000 + m) >> (125- e)) + 1) >> 1; - if (e > 143) - n2 = 0x7FFF; - - return (b & 0x80000000) >> 16 | n0 | n1 | n2; // sign : normalized : denormalized : saturate -} - static gsize gdk_memory_format_bytes_per_pixel (GdkMemoryFormat format) { @@ -432,7 +414,7 @@ gdk_memory_format_pixel_print (GdkMemoryFormat format, case GDK_MEMORY_R16G16B16_FLOAT: { const guint16 *data16 = (const guint16 *) data; - g_string_append_printf (string, "%f %f %f", half_to_float (data16[0]), half_to_float (data16[1]), half_to_float (data16[2])); + g_string_append_printf (string, "%f %f %f", half_to_float_one (data16[0]), half_to_float_one (data16[1]), half_to_float_one (data16[2])); } break; @@ -440,13 +422,13 @@ gdk_memory_format_pixel_print (GdkMemoryFormat format, case GDK_MEMORY_R16G16B16A16_FLOAT_PREMULTIPLIED: { const guint16 *data16 = (const guint16 *) data; - g_string_append_printf (string, "%f %f %f %f", half_to_float (data16[0]), half_to_float (data16[1]), half_to_float (data16[2]), half_to_float (data16[3])); + g_string_append_printf (string, "%f %f %f %f", half_to_float_one (data16[0]), half_to_float_one (data16[1]), half_to_float_one (data16[2]), half_to_float_one (data16[3])); } break; case GDK_MEMORY_A16_FLOAT: { const guint16 *data16 = (const guint16 *) data; - g_string_append_printf (string, "%f", half_to_float (data16[0])); + g_string_append_printf (string, "%f", half_to_float_one (data16[0])); } break; @@ -537,8 +519,8 @@ gdk_memory_format_pixel_equal (GdkMemoryFormat format, guint i; for (i = 0; i < gdk_memory_format_bytes_per_pixel (format) / sizeof (guint16); i++) { - float f1 = half_to_float (((guint16 *) pixel1)[i]); - float f2 = half_to_float (((guint16 *) pixel2)[i]); + float f1 = half_to_float_one (((guint16 *) pixel1)[i]); + float f2 = half_to_float_one (((guint16 *) pixel2)[i]); if (!G_APPROX_VALUE (f1, f2, accurate ? 1./65535 : 1./255)) return FALSE; } @@ -774,9 +756,9 @@ texture_builder_set_pixel (TextureBuilder *builder, case GDK_MEMORY_R16G16B16_FLOAT: { guint16 pixels[3] = { - float_to_half (color->red * color->alpha), - float_to_half (color->green * color->alpha), - float_to_half (color->blue * color->alpha) + float_to_half_one (color->red * color->alpha), + float_to_half_one (color->green * color->alpha), + float_to_half_one (color->blue * color->alpha) }; memcpy (data, pixels, 3 * sizeof (guint16)); } @@ -784,10 +766,10 @@ texture_builder_set_pixel (TextureBuilder *builder, case GDK_MEMORY_R16G16B16A16_FLOAT_PREMULTIPLIED: { guint16 pixels[4] = { - float_to_half (color->red * color->alpha), - float_to_half (color->green * color->alpha), - float_to_half (color->blue * color->alpha), - float_to_half (color->alpha) + float_to_half_one (color->red * color->alpha), + float_to_half_one (color->green * color->alpha), + float_to_half_one (color->blue * color->alpha), + float_to_half_one (color->alpha) }; memcpy (data, pixels, 4 * sizeof (guint16)); } @@ -795,10 +777,10 @@ texture_builder_set_pixel (TextureBuilder *builder, case GDK_MEMORY_R16G16B16A16_FLOAT: { guint16 pixels[4] = { - float_to_half (color->red), - float_to_half (color->green), - float_to_half (color->blue), - float_to_half (color->alpha) + float_to_half_one (color->red), + float_to_half_one (color->green), + float_to_half_one (color->blue), + float_to_half_one (color->alpha) }; memcpy (data, pixels, 4 * sizeof (guint16)); } @@ -889,7 +871,7 @@ texture_builder_set_pixel (TextureBuilder *builder, break; case GDK_MEMORY_A16_FLOAT: { - guint16 pixel = float_to_half (color->alpha); + guint16 pixel = float_to_half_one (color->alpha); memcpy (data, &pixel, sizeof (guint16)); } break; diff --git a/testsuite/gsk/scaling.c b/testsuite/gsk/scaling.c index e247ee9c94..3ad226050f 100644 --- a/testsuite/gsk/scaling.c +++ b/testsuite/gsk/scaling.c @@ -2,6 +2,8 @@ #define N 10 +#include "gsk/gl/fp16private.h" + struct { const char *name; GskRenderer * (*create_func) (void); @@ -63,7 +65,7 @@ as_float (const guint x) // IEEE-754 16-bit floating-point format (without infinity): 1-5-10 // static inline float -half_to_float (const guint16 x) +half_to_float_one (const guint16 x) { const guint e = (x&0x7C00)>>10; // exponent const guint m = (x&0x03FF)<<13; // mantissa @@ -71,15 +73,6 @@ half_to_float (const guint16 x) return as_float((x&0x8000)<<16 | (e!=0)*((e+112)<<23|m) | ((e==0)&(m!=0))*((v-37)<<23|((m<<(150-v))&0x007FE000))); } -static inline guint16 -float_to_half (const float x) -{ - const guint b = *(guint*)&x+0x00001000; // round-to-nearest-even - const guint e = (b&0x7F800000)>>23; // exponent - const guint m = b&0x007FFFFF; // mantissa - return (b&0x80000000)>>16 | (e>112)*((((e-112)<<10)&0x7C00)|m>>13) | ((e<113)&(e>101))*((((0x007FF000+m)>>(125-e))+1)>>1) | (e>143)*0x7FFF; // sign : normalized : denormalized : saturate -} - static gsize gdk_memory_format_bytes_per_pixel (GdkMemoryFormat format) { @@ -350,8 +343,8 @@ gdk_memory_format_pixel_equal (GdkMemoryFormat format, guint i; for (i = 0; i < gdk_memory_format_bytes_per_pixel (format) / sizeof (guint16); i++) { - float f1 = half_to_float (((guint16 *) pixel1)[i]); - float f2 = half_to_float (((guint16 *) pixel2)[i]); + float f1 = half_to_float_one (((guint16 *) pixel1)[i]); + float f2 = half_to_float_one (((guint16 *) pixel2)[i]); if (!G_APPROX_VALUE (f1, f2, accurate ? 1./65535 : 1./255)) return FALSE; } @@ -567,9 +560,9 @@ texture_builder_set_pixel (TextureBuilder *builder, case GDK_MEMORY_R16G16B16_FLOAT: { guint16 pixels[3] = { - float_to_half (color->red * color->alpha), - float_to_half (color->green * color->alpha), - float_to_half (color->blue * color->alpha) + float_to_half_one (color->red * color->alpha), + float_to_half_one (color->green * color->alpha), + float_to_half_one (color->blue * color->alpha) }; memcpy (data, pixels, 3 * sizeof (guint16)); } @@ -577,10 +570,10 @@ texture_builder_set_pixel (TextureBuilder *builder, case GDK_MEMORY_R16G16B16A16_FLOAT_PREMULTIPLIED: { guint16 pixels[4] = { - float_to_half (color->red * color->alpha), - float_to_half (color->green * color->alpha), - float_to_half (color->blue * color->alpha), - float_to_half (color->alpha) + float_to_half_one (color->red * color->alpha), + float_to_half_one (color->green * color->alpha), + float_to_half_one (color->blue * color->alpha), + float_to_half_one (color->alpha) }; memcpy (data, pixels, 4 * sizeof (guint16)); } @@ -588,10 +581,10 @@ texture_builder_set_pixel (TextureBuilder *builder, case GDK_MEMORY_R16G16B16A16_FLOAT: { guint16 pixels[4] = { - float_to_half (color->red), - float_to_half (color->green), - float_to_half (color->blue), - float_to_half (color->alpha) + float_to_half_one (color->red), + float_to_half_one (color->green), + float_to_half_one (color->blue), + float_to_half_one (color->alpha) }; memcpy (data, pixels, 4 * sizeof (guint16)); } @@ -682,7 +675,7 @@ texture_builder_set_pixel (TextureBuilder *builder, break; case GDK_MEMORY_A16_FLOAT: { - guint16 pixel = float_to_half (color->alpha); + guint16 pixel = float_to_half_one (color->alpha); memcpy (data, &pixel, sizeof (guint16)); } break; From 660c6c8d6fb42792aa2f8c2ccdbf88a15a7b5ede Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Sat, 27 Jul 2024 20:15:50 +0100 Subject: [PATCH 2/2] gsk, testsuite: Avoid undefined behaviour in half_to_float_one() Similar to the previous commit, to avoid undefined behaviour we need to avoid evaluating out-of-bounds shifts, even if their result is going to ignored by being multiplied by 0 later. Detected by running a subset of the test suite with -Dsanitize=address,undefined on x86_64. Signed-off-by: Simon McVittie --- gsk/gl/fp16.c | 23 ----------------------- gsk/gl/fp16private.h | 31 +++++++++++++++++++++++++++++++ testsuite/gdk/memorytexture.c | 23 ----------------------- testsuite/gsk/scaling.c | 23 ----------------------- 4 files changed, 31 insertions(+), 69 deletions(-) diff --git a/gsk/gl/fp16.c b/gsk/gl/fp16.c index 3f67f339ec..cc227a3ac1 100644 --- a/gsk/gl/fp16.c +++ b/gsk/gl/fp16.c @@ -22,29 +22,6 @@ #include "fp16private.h" -static inline guint -as_uint (const float x) -{ - return *(guint*)&x; -} - -static inline float -as_float (const guint x) -{ - return *(float*)&x; -} - -// IEEE-754 16-bit floating-point format (without infinity): 1-5-10 - -static inline float -half_to_float_one (const guint16 x) -{ - const guint e = (x&0x7C00)>>10; // exponent - const guint m = (x&0x03FF)<<13; // mantissa - const guint v = as_uint((float)m)>>23; - return as_float((x&0x8000)<<16 | (e!=0)*((e+112)<<23|m) | ((e==0)&(m!=0))*((v-37)<<23|((m<<(150-v))&0x007FE000))); -} - void float_to_half4_c (const float f[4], guint16 h[4]) diff --git a/gsk/gl/fp16private.h b/gsk/gl/fp16private.h index 29e87897d4..c9ebdccfd2 100644 --- a/gsk/gl/fp16private.h +++ b/gsk/gl/fp16private.h @@ -28,6 +28,37 @@ G_BEGIN_DECLS #define FP16_ONE ((guint16)15360) #define FP16_MINUS_ONE ((guint16)48128) +static inline guint +as_uint (const float x) +{ + return *(guint*)&x; +} + +static inline float +as_float (const guint x) +{ + return *(float*)&x; +} + +// IEEE-754 16-bit floating-point format (without infinity): 1-5-10 +static inline float +half_to_float_one (const guint16 x) +{ + const guint e = (x & 0x7C00) >> 10; // exponent + const guint m = (x & 0x03FF) << 13; // mantissa + const guint v = as_uint ((float) m) >> 23; + guint normalized = 0; + guint denormalized = 0; + + if (e != 0) + normalized = (e + 112) << 23 | m; + + if (e == 0 && m != 0) + denormalized = (v - 37) << 23 | ((m << (150 - v)) & 0x007FE000); + + return as_float ((x & 0x8000u) << 16 | normalized | denormalized); +} + static inline guint16 float_to_half_one (const float x) { diff --git a/testsuite/gdk/memorytexture.c b/testsuite/gdk/memorytexture.c index 989f633d5a..2fc0e14a60 100644 --- a/testsuite/gdk/memorytexture.c +++ b/testsuite/gdk/memorytexture.c @@ -46,29 +46,6 @@ struct _TextureBuilder gsize offset; }; -static inline guint -as_uint (const float x) -{ - return *(guint*)&x; -} - -static inline float -as_float (const guint x) -{ - return *(float*)&x; -} - -// IEEE-754 16-bit floating-point format (without infinity): 1-5-10 -// -static inline float -half_to_float_one (const guint16 x) -{ - const guint e = (x&0x7C00)>>10; // exponent - const guint m = (x&0x03FF)<<13; // mantissa - const guint v = as_uint((float)m)>>23; - return as_float((x&0x8000)<<16 | (e!=0)*((e+112)<<23|m) | ((e==0)&(m!=0))*((v-37)<<23|((m<<(150-v))&0x007FE000))); -} - static gsize gdk_memory_format_bytes_per_pixel (GdkMemoryFormat format) { diff --git a/testsuite/gsk/scaling.c b/testsuite/gsk/scaling.c index 3ad226050f..1f3c5e2b98 100644 --- a/testsuite/gsk/scaling.c +++ b/testsuite/gsk/scaling.c @@ -50,29 +50,6 @@ struct _TextureBuilder gsize offset; }; -static inline guint -as_uint (const float x) -{ - return *(guint*)&x; -} - -static inline float -as_float (const guint x) -{ - return *(float*)&x; -} - -// IEEE-754 16-bit floating-point format (without infinity): 1-5-10 -// -static inline float -half_to_float_one (const guint16 x) -{ - const guint e = (x&0x7C00)>>10; // exponent - const guint m = (x&0x03FF)<<13; // mantissa - const guint v = as_uint((float)m)>>23; - return as_float((x&0x8000)<<16 | (e!=0)*((e+112)<<23|m) | ((e==0)&(m!=0))*((v-37)<<23|((m<<(150-v))&0x007FE000))); -} - static gsize gdk_memory_format_bytes_per_pixel (GdkMemoryFormat format) {