Fixing a very rare 1-bit overflow in ETC1S SSE optimized colorspace error calculations (which are 32-bit vs. the plain C++ code's 64-bit). Ultimately the output would still be correct and transcodable, but this could cause a compressor assertion check to fail. Found during compressor fuzz testing.

This commit is contained in:
Richard Geldreich
2025-11-18 17:27:51 -05:00
parent 6e54503437
commit 5a1ae5d65d
4 changed files with 280 additions and 95 deletions

View File

@@ -3987,5 +3987,133 @@ namespace basisu
return true;
}
static void rasterize_line(image& dst, int xs, int ys, int xe, int ye, int pred, int inc_dec, int e, int e_inc, int e_no_inc, const color_rgba& color)
{
int start, end, var;
if (pred)
{
start = ys; end = ye; var = xs;
for (int i = start; i <= end; i++)
{
dst.set_clipped(var, i, color);
if (e < 0)
e += e_no_inc;
else
{
var += inc_dec;
e += e_inc;
}
}
}
else
{
start = xs; end = xe; var = ys;
for (int i = start; i <= end; i++)
{
dst.set_clipped(i, var, color);
if (e < 0)
e += e_no_inc;
else
{
var += inc_dec;
e += e_inc;
}
}
}
}
void draw_line(image& dst, int xs, int ys, int xe, int ye, const color_rgba& color)
{
if (xs > xe)
{
std::swap(xs, xe);
std::swap(ys, ye);
}
int dx = xe - xs, dy = ye - ys;
if (!dx)
{
if (ys > ye)
std::swap(ys, ye);
for (int i = ys; i <= ye; i++)
dst.set_clipped(xs, i, color);
}
else if (!dy)
{
for (int i = xs; i < xe; i++)
dst.set_clipped(i, ys, color);
}
else if (dy > 0)
{
if (dy <= dx)
{
int e = 2 * dy - dx, e_no_inc = 2 * dy, e_inc = 2 * (dy - dx);
rasterize_line(dst, xs, ys, xe, ye, 0, 1, e, e_inc, e_no_inc, color);
}
else
{
int e = 2 * dx - dy, e_no_inc = 2 * dx, e_inc = 2 * (dx - dy);
rasterize_line(dst, xs, ys, xe, ye, 1, 1, e, e_inc, e_no_inc, color);
}
}
else
{
dy = -dy;
if (dy <= dx)
{
int e = 2 * dy - dx, e_no_inc = 2 * dy, e_inc = 2 * (dy - dx);
rasterize_line(dst, xs, ys, xe, ye, 0, -1, e, e_inc, e_no_inc, color);
}
else
{
int e = 2 * dx - dy, e_no_inc = (2 * dx), e_inc = 2 * (dx - dy);
rasterize_line(dst, xe, ye, xs, ys, 1, -1, e, e_inc, e_no_inc, color);
}
}
}
// Used for generating random test data
void draw_circle(image& dst, int cx, int cy, int r, const color_rgba& color)
{
assert(r >= 0);
if (r < 0)
return;
int x = r;
int y = 0;
int err = 1 - x;
while (x >= y)
{
dst.set_clipped(cx + x, cy + y, color);
dst.set_clipped(cx + y, cy + x, color);
dst.set_clipped(cx - y, cy + x, color);
dst.set_clipped(cx - x, cy + y, color);
dst.set_clipped(cx - x, cy - y, color);
dst.set_clipped(cx - y, cy - x, color);
dst.set_clipped(cx + y, cy - x, color);
dst.set_clipped(cx + x, cy - y, color);
++y;
if (err < 0)
{
err += 2 * y + 1;
}
else
{
--x;
err += 2 * (y - x) + 1;
}
}
}
void set_image_alpha(image& img, uint32_t a)
{
for (uint32_t y = 0; y < img.get_height(); y++)
for (uint32_t x = 0; x < img.get_width(); x++)
img(x, y).a = (uint8_t)a;
}
} // namespace basisu

View File

@@ -31,9 +31,6 @@
// This module is really just a huge grab bag of classes and helper functions needed by the encoder.
// If BASISU_USE_HIGH_PRECISION_COLOR_DISTANCE is 1, quality in perceptual mode will be slightly greater, but at a large increase in encoding CPU time.
#define BASISU_USE_HIGH_PRECISION_COLOR_DISTANCE (0)
#if BASISU_SUPPORT_SSE
// Declared in basisu_kernels_imp.h, but we can't include that here otherwise it would lead to circular type errors.
extern void update_covar_matrix_16x16_sse41(uint32_t num_vecs, const void* pWeighted_vecs, const void* pOrigin, const uint32_t *pVec_indices, void* pMatrix16x16);
@@ -175,6 +172,20 @@ namespace basisu
inline int bounds_check_incl(int v, int l, int h) { (void)v; (void)l; (void)h; assert(v >= l && v <= h); return v; }
inline uint32_t bounds_check_incl(uint32_t v, uint32_t l, uint32_t h) { (void)v; (void)l; (void)h; assert(v >= l && v <= h); return v; }
inline bool equal_rel_tol(float a, float b, float rel_tol)
{
float diff = std::fabs(a - b);
float max_abs = std::max(std::fabs(a), std::fabs(b));
return diff <= (max_abs * rel_tol);
}
inline bool equal_rel_tol(double a, double b, double rel_tol)
{
double diff = std::fabs(a - b);
double max_abs = std::max(std::fabs(a), std::fabs(b));
return diff <= (max_abs * rel_tol);
}
inline uint32_t clz(uint32_t x)
{
if (!x)
@@ -1072,48 +1083,15 @@ namespace basisu
return color_distance(c0.r, c0.g, c0.b, c1.r, c1.g, c1.b);
}
// TODO: Allow user to control channel weightings.
inline uint32_t color_distance(bool perceptual, const color_rgba &e1, const color_rgba &e2, bool alpha)
// Original library color_distance(), for testing
inline uint32_t color_distance_orig(bool perceptual, const color_rgba& e1, const color_rgba& e2, bool alpha)
{
if (perceptual)
{
#if BASISU_USE_HIGH_PRECISION_COLOR_DISTANCE
const float l1 = e1.r * .2126f + e1.g * .715f + e1.b * .0722f;
const float l2 = e2.r * .2126f + e2.g * .715f + e2.b * .0722f;
const float cr1 = e1.r - l1;
const float cr2 = e2.r - l2;
const float cb1 = e1.b - l1;
const float cb2 = e2.b - l2;
const float dl = l1 - l2;
const float dcr = cr1 - cr2;
const float dcb = cb1 - cb2;
uint32_t d = static_cast<uint32_t>(32.0f*4.0f*dl*dl + 32.0f*2.0f*(.5f / (1.0f - .2126f))*(.5f / (1.0f - .2126f))*dcr*dcr + 32.0f*.25f*(.5f / (1.0f - .0722f))*(.5f / (1.0f - .0722f))*dcb*dcb);
if (alpha)
{
int da = static_cast<int>(e1.a) - static_cast<int>(e2.a);
d += static_cast<uint32_t>(128.0f*da*da);
}
return d;
#elif 1
int dr = e1.r - e2.r;
int dg = e1.g - e2.g;
int db = e1.b - e2.b;
#if 0
int delta_l = dr * 27 + dg * 92 + db * 9;
int delta_cr = dr * 128 - delta_l;
int delta_cb = db * 128 - delta_l;
uint32_t id = ((uint32_t)(delta_l * delta_l) >> 7U) +
((((uint32_t)(delta_cr * delta_cr) >> 7U) * 26U) >> 7U) +
((((uint32_t)(delta_cb * delta_cb) >> 7U) * 3U) >> 7U);
#else
int64_t delta_l = dr * 27 + dg * 92 + db * 9;
int64_t delta_cr = dr * 128 - delta_l;
int64_t delta_cb = db * 128 - delta_l;
@@ -1121,7 +1099,6 @@ namespace basisu
uint32_t id = ((uint32_t)((delta_l * delta_l) >> 7U)) +
((((uint32_t)((delta_cr * delta_cr) >> 7U)) * 26U) >> 7U) +
((((uint32_t)((delta_cb * delta_cb) >> 7U)) * 3U) >> 7U);
#endif
if (alpha)
{
@@ -1131,32 +1108,67 @@ namespace basisu
}
return id;
#else
}
else
{
return color_distance(e1, e2, alpha);
}
}
inline uint32_t color_distance(bool perceptual, const color_rgba &e1, const color_rgba &e2, bool alpha)
{
if (perceptual)
{
int dr = e1.r - e2.r;
int dg = e1.g - e2.g;
int db = e1.b - e2.b;
int64_t delta_l = dr * 27 + dg * 92 + db * 9;
int64_t delta_cr = dr * 128 - delta_l;
int64_t delta_cb = db * 128 - delta_l;
// This calc can't overflow or the SSE variants will overflow too.
int delta_l = dr * 14 + dg * 45 + db * 5;
int delta_cr = dr * 64 - delta_l;
int delta_cb = db * 64 - delta_l;
// not >> 6, so the output is scaled by 7 bits, not 6 (to match the original function which scaled by 7, but had rare overflow issues)
uint32_t id = ((uint32_t)(delta_l * delta_l) >> 5U) +
((((uint32_t)(delta_cr * delta_cr) >> 5U) * 26U) >> 7U) +
((((uint32_t)(delta_cb * delta_cb) >> 5U) * 3U) >> 7U);
int64_t id = ((delta_l * delta_l) * 128) +
((delta_cr * delta_cr) * 26) +
((delta_cb * delta_cb) * 3);
#if defined(DEBUG) || defined(_DEBUG)
// Shouldn't need 64-bit now, but make sure
{
int64_t alt_delta_l = dr * 14 + dg * 45 + db * 5;
int64_t alt_delta_cr = dr * 64 - alt_delta_l;
int64_t alt_delta_cb = db * 64 - alt_delta_l;
int64_t alt_id = ((alt_delta_l * alt_delta_l) >> 5) +
((((alt_delta_cr * alt_delta_cr) >> 5) * 26) >> 7) +
((((alt_delta_cb * alt_delta_cb) >> 5) * 3) >> 7);
assert(alt_id == id);
}
#endif
if (alpha)
{
int64_t da = (e1.a - e2.a);
id += (da * da) * 128;
int da = (e1.a - e2.a) << 7;
// This shouldn't overflow if da is 255 or -255: 29.99 bits after squaring.
uint32_t ea = ((uint32_t)(da * da) >> 7U);
id += ea;
#if defined(DEBUG) || defined(_DEBUG)
// Make sure it can't overflow
assert((((int64_t)da * (int64_t)da) >> 7) == ea);
#endif
}
int d = (id + 8192) >> 14;
return d;
#endif
return id;
}
else
{
return color_distance(e1, e2, alpha);
}
}
static inline uint32_t color_distance_la(const color_rgba& a, const color_rgba& b)
@@ -2906,6 +2918,9 @@ namespace basisu
image &fill_box(uint32_t x, uint32_t y, uint32_t w, uint32_t h, const color_rgba &c)
{
assert((int)w >= 0);
assert((int)h >= 0);
for (uint32_t iy = 0; iy < h; iy++)
for (uint32_t ix = 0; ix < w; ix++)
set_clipped(x + ix, y + iy, c);
@@ -2914,6 +2929,9 @@ namespace basisu
image& fill_box_alpha(uint32_t x, uint32_t y, uint32_t w, uint32_t h, const color_rgba& c)
{
assert((int)w >= 0);
assert((int)h >= 0);
for (uint32_t iy = 0; iy < h; iy++)
for (uint32_t ix = 0; ix < w; ix++)
set_clipped_alpha(x + ix, y + iy, c);
@@ -3197,6 +3215,7 @@ namespace basisu
void debug_text(uint32_t x_ofs, uint32_t y_ofs, uint32_t x_scale, uint32_t y_scale, const color_rgba &fg, const color_rgba *pBG, bool alpha_only, const char* p, ...);
// bilinear filtering
vec4F get_filtered_vec4F(float x, float y) const
{
x -= .5f;
@@ -3244,6 +3263,36 @@ namespace basisu
color_rgba_vec m_pixels;
};
void draw_line(image& dst, int xs, int ys, int xe, int ye, const color_rgba& color);
void draw_circle(image& dst, int cx, int cy, int r, const color_rgba& color);
inline bool is_solid_block(uint32_t n, const color_rgba* pPixels)
{
assert(n);
if (n <= 1)
return true;
const color_rgba c(pPixels[0]);
for (uint32_t i = 1; i < n; i++)
if (c != pPixels[i])
return false;
return true;
}
inline bool is_alpha_block(uint32_t n, const color_rgba* pPixels)
{
assert(n);
for (uint32_t i = 0; i < n; i++)
if (pPixels[i][3] != 255)
return true;
return false;
}
// Float images
typedef basisu::vector<vec4F> vec4F_vec;

View File

@@ -811,7 +811,8 @@ namespace basisu
#if defined(DEBUG) || defined(_DEBUG)
{
// sanity check the returned error
// Ultimate sanity check on the returned error.
// If this check fails, it likely means the SSE code diverged from C++ somehow, or there was an overflow somewhere.
color_rgba block_colors[4];
m_best_solution.m_coords.get_block_colors(block_colors);
@@ -1234,6 +1235,8 @@ namespace basisu
perceptual_distance_rgb_4_N_sse41((int64_t*)&total_error, pSelectors_to_use, block_colors, pSrc_pixels, n, trial_solution.m_error);
else
linear_distance_rgb_4_N_sse41((int64_t*)&total_error, pSelectors_to_use, block_colors, pSrc_pixels, n, trial_solution.m_error);
for (uint32_t i = 0; i < n; i++)
m_temp_selectors[i] = pSelectors_to_use[i];
}
else
{

View File

@@ -17,6 +17,11 @@ using namespace CPPSPMD;
namespace CPPSPMD_NAME(basisu_kernels_namespace)
{
static inline int64_t reduce_add64(const vint &x)
{
return (int64_t)VINT_EXTRACT(x, 0) + (int64_t)VINT_EXTRACT(x, 1) +
(int64_t)VINT_EXTRACT(x, 2) + (int64_t)VINT_EXTRACT(x, 3);
}
struct perceptual_distance_rgb_4_N : spmd_kernel
{
void _call(int64_t* pDistance,
@@ -66,15 +71,15 @@ namespace CPPSPMD_NAME(basisu_kernels_namespace)
vint dg = base_g - g;
vint db = base_b - b;
vint delta_l = dr * 27 + dg * 92 + db * 9;
vint delta_cr = dr * 128 - delta_l;
vint delta_cb = db * 128 - delta_l;
vint delta_l = dr * 14 + dg * 45 + db * 5;
vint delta_cr = dr * 64 - delta_l;
vint delta_cb = db * 64 - delta_l;
vint id = ((delta_l * delta_l) >> 7) +
((((delta_cr * delta_cr) >> 7) * 26) >> 7) +
((((delta_cb * delta_cb) >> 7) * 3) >> 7);
vint id = ((delta_l * delta_l) >> 5) +
((((delta_cr * delta_cr) >> 5) * 26) >> 7) +
((((delta_cb * delta_cb) >> 5) * 3) >> 7);
*pDistance += reduce_add(id);
*pDistance += reduce_add64(id);
if (*pDistance >= early_out_err)
return;
}
@@ -90,13 +95,13 @@ namespace CPPSPMD_NAME(basisu_kernels_namespace)
int dg = base_g - g;
int db = base_b - b;
int delta_l = dr * 27 + dg * 92 + db * 9;
int delta_cr = dr * 128 - delta_l;
int delta_cb = db * 128 - delta_l;
int delta_l = dr * 14 + dg * 45 + db * 5;
int delta_cr = dr * 64 - delta_l;
int delta_cb = db * 64 - delta_l;
int id = ((delta_l * delta_l) >> 7) +
((((delta_cr * delta_cr) >> 7) * 26) >> 7) +
((((delta_cb * delta_cb) >> 7) * 3) >> 7);
int id = ((delta_l * delta_l) >> 5) +
((((delta_cr * delta_cr) >> 5) * 26) >> 7) +
((((delta_cb * delta_cb) >> 5) * 3) >> 7);
*pDistance += id;
if (*pDistance >= early_out_err)
@@ -156,7 +161,7 @@ namespace CPPSPMD_NAME(basisu_kernels_namespace)
vint id = dr * dr + dg * dg + db * db;
*pDistance += reduce_add(id);
*pDistance += reduce_add64(id);
if (*pDistance >= early_out_err)
return;
}
@@ -191,13 +196,13 @@ namespace CPPSPMD_NAME(basisu_kernels_namespace)
vint dg = base_g - g;
vint db = base_b - b;
vint delta_l = dr * 27 + dg * 92 + db * 9;
vint delta_cr = dr * 128 - delta_l;
vint delta_cb = db * 128 - delta_l;
vint delta_l = dr * 14 + dg * 45 + db * 5;
vint delta_cr = dr * 64 - delta_l;
vint delta_cb = db * 64 - delta_l;
vint id = VINT_SHIFT_RIGHT(delta_l * delta_l, 7) +
VINT_SHIFT_RIGHT(VINT_SHIFT_RIGHT(delta_cr * delta_cr, 7) * 26, 7) +
VINT_SHIFT_RIGHT(VINT_SHIFT_RIGHT(delta_cb * delta_cb, 7) * 3, 7);
vint id = VINT_SHIFT_RIGHT(delta_l * delta_l, 5) +
VINT_SHIFT_RIGHT(VINT_SHIFT_RIGHT(delta_cr * delta_cr, 5) * 26, 7) +
VINT_SHIFT_RIGHT(VINT_SHIFT_RIGHT(delta_cb * delta_cb, 5) * 3, 7);
return id;
}
@@ -243,7 +248,7 @@ namespace CPPSPMD_NAME(basisu_kernels_namespace)
__m128i vsels = shuffle_epi8(sels.m_value, shuf);
storeu_si32((void *)(pSelectors + i), vsels);
*pDistance += reduce_add(min_dist);
*pDistance += reduce_add64(min_dist);
if (*pDistance >= early_out_err)
return;
}
@@ -261,13 +266,13 @@ namespace CPPSPMD_NAME(basisu_kernels_namespace)
int dg = base_g - g;
int db = base_b - b;
int delta_l = dr * 27 + dg * 92 + db * 9;
int delta_cr = dr * 128 - delta_l;
int delta_cb = db * 128 - delta_l;
int delta_l = dr * 14 + dg * 45 + db * 5;
int delta_cr = dr * 64 - delta_l;
int delta_cb = db * 64 - delta_l;
int id = ((delta_l * delta_l) >> 7) +
((((delta_cr * delta_cr) >> 7) * 26) >> 7) +
((((delta_cb * delta_cb) >> 7) * 3) >> 7);
int id = ((delta_l * delta_l) >> 5) +
((((delta_cr * delta_cr) >> 5) * 26) >> 7) +
((((delta_cb * delta_cb) >> 5) * 3) >> 7);
if (id < best_err)
{
best_err = id;
@@ -339,7 +344,7 @@ namespace CPPSPMD_NAME(basisu_kernels_namespace)
__m128i vsels = shuffle_epi8(sels.m_value, shuf);
storeu_si32((void *)(pSelectors + i), vsels);
*pDistance += reduce_add(min_dist);
*pDistance += reduce_add64(min_dist);
if (*pDistance >= early_out_err)
return;
}
@@ -384,13 +389,13 @@ namespace CPPSPMD_NAME(basisu_kernels_namespace)
vint dg = base_g - g;
vint db = base_b - b;
vint delta_l = dr * 27 + dg * 92 + db * 9;
vint delta_cr = dr * 128 - delta_l;
vint delta_cb = db * 128 - delta_l;
vint delta_l = dr * 14 + dg * 45 + db * 5;
vint delta_cr = dr * 64 - delta_l;
vint delta_cb = db * 64 - delta_l;
vint id = VINT_SHIFT_RIGHT(delta_l * delta_l, 7) +
VINT_SHIFT_RIGHT(VINT_SHIFT_RIGHT(delta_cr * delta_cr, 7) * 26, 7) +
VINT_SHIFT_RIGHT(VINT_SHIFT_RIGHT(delta_cb * delta_cb, 7) * 3, 7);
vint id = VINT_SHIFT_RIGHT(delta_l * delta_l, 5) +
VINT_SHIFT_RIGHT(VINT_SHIFT_RIGHT(delta_cr * delta_cr, 5) * 26, 7) +
VINT_SHIFT_RIGHT(VINT_SHIFT_RIGHT(delta_cb * delta_cb, 5) * 3, 7);
return id;
}
@@ -428,7 +433,7 @@ namespace CPPSPMD_NAME(basisu_kernels_namespace)
vint min_dist = min(min(min(dist0, dist1), dist2), dist3);
*pDistance += reduce_add(min_dist);
*pDistance += reduce_add64(min_dist);
if (*pDistance > early_out_error)
return;
}
@@ -446,13 +451,13 @@ namespace CPPSPMD_NAME(basisu_kernels_namespace)
int dg = base_g - g;
int db = base_b - b;
int delta_l = dr * 27 + dg * 92 + db * 9;
int delta_cr = dr * 128 - delta_l;
int delta_cb = db * 128 - delta_l;
int delta_l = dr * 14 + dg * 45 + db * 5;
int delta_cr = dr * 64 - delta_l;
int delta_cb = db * 64 - delta_l;
int id = ((delta_l * delta_l) >> 7) +
((((delta_cr * delta_cr) >> 7) * 26) >> 7) +
((((delta_cb * delta_cb) >> 7) * 3) >> 7);
int id = ((delta_l * delta_l) >> 5) +
((((delta_cr * delta_cr) >> 5) * 26) >> 7) +
((((delta_cb * delta_cb) >> 5) * 3) >> 7);
if (id < best_err)
{
@@ -515,7 +520,7 @@ namespace CPPSPMD_NAME(basisu_kernels_namespace)
vint min_dist = min(min(min(dist0, dist1), dist2), dist3);
*pDistance += reduce_add(min_dist);
*pDistance += reduce_add64(min_dist);
if (*pDistance > early_out_error)
return;
}