Clamp out-of-range unorm inputs (#651)

Clamp input data for channels specified as UNORM to be between 0 and 65535 in the internal encoding. NaNs also get replaced with zeros. This avoids undefined behavior when later storing values into an integer.

Author: solidpixel

Docs/ChangeLog-5x.md

@@ -18,6 +18,10 @@ The 5.6.0 release is a minor maintenance release.
     LDR image to a DDS container.
   * **Bug fix:** Fixed potential integer overflow when storing very large
     uncompressed images to a `dds` or `.ktx` output image format.
+  * **Bug fix:** Avoid undefined behavior caused by passing floating point
+    values outside of the [0.0, 1.0] range as data for a UNORM color channel.
+  * **Bug fix:** Avoid undefined behavior caused by unaligned access in SSE
+    and AVX2 SIMD library implementations.
 
 <!-- ---------------------------------------------------------------------- -->
 ## 5.5.0

Source/UnitTest/test_encode.cpp

@@ -131,4 +131,171 @@ TEST(compress, data_buffer_exceeded)
 	astcenc_context_free(context);
 }
 
+/** @brief Helper to run a compression on a single 4x4 block */
+static void run_positive_test(
+	astcenc_profile profile,
+	float* input
+) {
+	astcenc_error status;
+	astcenc_config config;
+	astcenc_context* context;
+
+	static const astcenc_swizzle swizzle {
+		ASTCENC_SWZ_R, ASTCENC_SWZ_G, ASTCENC_SWZ_B, ASTCENC_SWZ_A
+	};
+
+	astcenc_config_init(profile, 4, 4, 1, ASTCENC_PRE_MEDIUM, 0, &config);
+	status = astcenc_context_alloc(&config, 1, &context, nullptr);
+	EXPECT_EQ(status, ASTCENC_SUCCESS);
+
+	// Single 16 byte block output data
+	uint8_t data[16];
+
+	astcenc_image image;
+	image.dim_x = 4u;
+	image.dim_y = 4u;
+	image.dim_z = 1u;
+	image.data_type = ASTCENC_TYPE_F32;
+	float* slices = input;
+	image.data = reinterpret_cast<void**>(&slices);
+
+	// Should not error or trip any UBSAN errors
+	status = astcenc_compress_image(context, &image, &swizzle, data, 16, 0);
+	EXPECT_EQ(status, ASTCENC_SUCCESS);
+
+	astcenc_context_free(context);
+}
+
+/** @brief Input data contains negative infinities. */
+TEST(compress, data_neg_inf_ldr)
+{
+	// Cycle though a single bad value in each color channel
+	for(int offset = 0; offset < 4; offset++)
+	{
+		// 4x4 block input data
+		std::vector<float> input(4 * 4 * 4, 0.5f);
+		input[0 + offset] = -std::numeric_limits<float>::infinity();
+
+		// Make data more complex to hit more compressor paths
+		input[4 + offset] = 0.75f;
+		input[8 + offset] = 0.35f;
+		input[12 + offset] = 0.0f;
+
+		run_positive_test(ASTCENC_PRF_LDR, input.data());
+	}
+}
+
+/** @brief Input data contains negative infinities. */
+TEST(compress, data_neg_inf_hdr)
+{
+	// Cycle though a single bad value in each color channel
+	for(int offset = 0; offset < 4; offset++)
+	{
+		// 4x4 block input data
+		std::vector<float> input(4 * 4 * 4, 0.5f);
+		input[0 + offset] = -std::numeric_limits<float>::infinity();
+
+		run_positive_test(ASTCENC_PRF_HDR, input.data());
+	}
+}
+
+/** @brief Input data contains negative infinities. */
+TEST(compress, data_neg_inf_hdr_ldra)
+{
+	// Cycle though a single bad value in each color channel
+	for(int offset = 0; offset < 4; offset++)
+	{
+		// 4x4 block input data
+		std::vector<float> input(4 * 4 * 4, 0.5f);
+		input[0 + offset] = -std::numeric_limits<float>::infinity();
+
+		run_positive_test(ASTCENC_PRF_HDR_RGB_LDR_A, input.data());
+	}
+}
+
+/** @brief Input data contains positive infinities. */
+TEST(compress, data_pos_inf_ldr)
+{
+	// Cycle though a single bad value in each color channel
+	for(int offset = 0; offset < 4; offset++)
+	{
+		// 4x4 block input data
+		std::vector<float> input(4 * 4 * 4, 0.5f);
+		input[0 + offset] = std::numeric_limits<float>::infinity();
+
+		run_positive_test(ASTCENC_PRF_LDR, input.data());
+	}
+}
+
+/** @brief Input data contains positive infinities. */
+TEST(compress, data_pos_inf_hdr)
+{
+	// Cycle though a single bad value in each color channel
+	for(int offset = 0; offset < 4; offset++)
+	{
+		// 4x4 block input data
+		std::vector<float> input(4 * 4 * 4, 0.5f);
+		input[0 + offset] = std::numeric_limits<float>::infinity();
+
+		run_positive_test(ASTCENC_PRF_HDR, input.data());
+	}
+}
+
+/** @brief Input data contains positive infinities. */
+TEST(compress, data_pos_inf_hdr_ldra)
+{
+	// Cycle though a single bad value in each color channel
+	for(int offset = 0; offset < 4; offset++)
+	{
+		// 4x4 block input data
+		std::vector<float> input(4 * 4 * 4, 0.5f);
+		input[0 + offset] = std::numeric_limits<float>::infinity();
+
+		run_positive_test(ASTCENC_PRF_HDR_RGB_LDR_A, input.data());
+	}
+}
+
+
+/** @brief Input data contains NaN. */
+TEST(compress, data_nan_ldr)
+{
+	// Cycle though a single bad value in each color channel
+	for(int offset = 0; offset < 4; offset++)
+	{
+		// 4x4 block input data
+		std::vector<float> input(4 * 4 * 4, 0.5f);
+		input[0 + offset] = std::numeric_limits<float>::quiet_NaN();
+
+		run_positive_test(ASTCENC_PRF_LDR, input.data());
+	}
+}
+
+/** @brief Input data contains NaN. */
+TEST(compress, data_nan_hdr)
+{
+	// Cycle though a single bad value in each color channel
+	for(int offset = 0; offset < 4; offset++)
+	{
+		// 4x4 block input data
+		std::vector<float> input(4 * 4 * 4, 0.5f);
+		input[0 + offset] = std::numeric_limits<float>::quiet_NaN();
+
+		run_positive_test(ASTCENC_PRF_HDR, input.data());
+	}
+}
+
+/** @brief Input data contains NaN. */
+TEST(compress, data_nan_hdr_ldra)
+{
+	// Cycle though a single bad value in each color channel
+	for(int offset = 0; offset < 4; offset++)
+	{
+		// 4x4 block input data
+		std::vector<float> input(4 * 4 * 4, 0.5f);
+		input[0 + offset] = std::numeric_limits<float>::quiet_NaN();
+
+		run_positive_test(ASTCENC_PRF_HDR_RGB_LDR_A, input.data());
+	}
+}
+
 }

Source/astcenc_image.cpp

@@ -121,6 +121,8 @@ static vfloat4 swz_texel(
 /**
  * @brief Encode a texel that is entirely LDR linear.
  *
+ * Out-of-range inputs will be clamped to the valid UNORM range.
+ *
  * @param data       The RGBA data to encode.
  * @param lns_mask   The mask for the HDR channels than need LNS encoding.
  */
@@ -129,20 +131,29 @@ static vfloat4 encode_texel_unorm(
 	vmask4 lns_mask
 ) {
 	(void)lns_mask;
-	return data * 65535.0f;
+	vfloat4 raw_value = data * 65535.0f;
+
+	// Unorm data must be in 0-1 range, so clamp because data can come from an
+	// unconstrained float. This will replace NaNs with zero.
+	return clamp(0.0f, 65535.0f, raw_value);
 }
 
 /**
  * @brief Encode a texel that includes at least some HDR LNS texels.
  *
+ * Out-of-range inputs will be clamped to the valid LNS or UNORM range,
+ * depending on @c lns_mask.
+ *
  * @param data       The RGBA data to encode.
  * @param lns_mask   The mask for the HDR channels than need LNS encoding.
  */
 static vfloat4 encode_texel_lns(
 	vfloat4 data,
 	vmask4 lns_mask
 ) {
-	vfloat4 datav_unorm = data * 65535.0f;
+	vfloat4 datav_unorm = encode_texel_unorm(data, lns_mask);
+
+	// Out-of-range inputs are clamped inside float_to_lns
 	vfloat4 datav_lns = float_to_lns(data);
 	return select(datav_unorm, datav_lns, lns_mask);
 }