From 76197a4be981dfad16793f27ce74bf527d5ef110 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 8 Aug 2018 16:00:07 -0400 Subject: [PATCH 1/2] common/color: Get rid of undefined behavior Gets rid of type punning via reinterpret_cast within functions. Instead, we use memcpy to transfer the contents across types. --- src/common/color.h | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/src/common/color.h b/src/common/color.h index 24a445dac..2e56af5a9 100644 --- a/src/common/color.h +++ b/src/common/color.h @@ -4,6 +4,8 @@ #pragma once +#include + #include "common/common_types.h" #include "common/swap.h" #include "common/vector_math.h" @@ -83,7 +85,8 @@ inline const Math::Vec4 DecodeRG8(const u8* bytes) { * @return Result color decoded as Math::Vec4 */ inline const Math::Vec4 DecodeRGB565(const u8* bytes) { - const u16_le pixel = *reinterpret_cast(bytes); + u16_le pixel; + std::memcpy(&pixel, bytes, sizeof(pixel)); return {Convert5To8((pixel >> 11) & 0x1F), Convert6To8((pixel >> 5) & 0x3F), Convert5To8(pixel & 0x1F), 255}; } @@ -94,7 +97,8 @@ inline const Math::Vec4 DecodeRGB565(const u8* bytes) { * @return Result color decoded as Math::Vec4 */ inline const Math::Vec4 DecodeRGB5A1(const u8* bytes) { - const u16_le pixel = *reinterpret_cast(bytes); + u16_le pixel; + std::memcpy(&pixel, bytes, sizeof(pixel)); return {Convert5To8((pixel >> 11) & 0x1F), Convert5To8((pixel >> 6) & 0x1F), Convert5To8((pixel >> 1) & 0x1F), Convert1To8(pixel & 0x1)}; } @@ -105,7 +109,8 @@ inline const Math::Vec4 DecodeRGB5A1(const u8* bytes) { * @return Result color decoded as Math::Vec4 */ inline const Math::Vec4 DecodeRGBA4(const u8* bytes) { - const u16_le pixel = *reinterpret_cast(bytes); + u16_le pixel; + std::memcpy(&pixel, bytes, sizeof(pixel)); return {Convert4To8((pixel >> 12) & 0xF), Convert4To8((pixel >> 8) & 0xF), Convert4To8((pixel >> 4) & 0xF), Convert4To8(pixel & 0xF)}; } @@ -116,7 +121,9 @@ inline const Math::Vec4 DecodeRGBA4(const u8* bytes) { * @return Depth value as an u32 */ inline u32 DecodeD16(const u8* bytes) { - return *reinterpret_cast(bytes); + u16_le data; + std::memcpy(&data, bytes, sizeof(data)); + return data; } /** @@ -175,8 +182,10 @@ inline void EncodeRG8(const Math::Vec4& color, u8* bytes) { * @param bytes Destination pointer to store encoded color */ inline void EncodeRGB565(const Math::Vec4& color, u8* bytes) { - *reinterpret_cast(bytes) = + const u16_le data = (Convert8To5(color.r()) << 11) | (Convert8To6(color.g()) << 5) | Convert8To5(color.b()); + + std::memcpy(bytes, &data, sizeof(data)); } /** @@ -185,9 +194,10 @@ inline void EncodeRGB565(const Math::Vec4& color, u8* bytes) { * @param bytes Destination pointer to store encoded color */ inline void EncodeRGB5A1(const Math::Vec4& color, u8* bytes) { - *reinterpret_cast(bytes) = (Convert8To5(color.r()) << 11) | - (Convert8To5(color.g()) << 6) | - (Convert8To5(color.b()) << 1) | Convert8To1(color.a()); + const u16_le data = (Convert8To5(color.r()) << 11) | (Convert8To5(color.g()) << 6) | + (Convert8To5(color.b()) << 1) | Convert8To1(color.a()); + + std::memcpy(bytes, &data, sizeof(data)); } /** @@ -196,9 +206,10 @@ inline void EncodeRGB5A1(const Math::Vec4& color, u8* bytes) { * @param bytes Destination pointer to store encoded color */ inline void EncodeRGBA4(const Math::Vec4& color, u8* bytes) { - *reinterpret_cast(bytes) = (Convert8To4(color.r()) << 12) | - (Convert8To4(color.g()) << 8) | - (Convert8To4(color.b()) << 4) | Convert8To4(color.a()); + const u16 data = (Convert8To4(color.r()) << 12) | (Convert8To4(color.g()) << 8) | + (Convert8To4(color.b()) << 4) | Convert8To4(color.a()); + + std::memcpy(bytes, &data, sizeof(data)); } /** @@ -207,7 +218,8 @@ inline void EncodeRGBA4(const Math::Vec4& color, u8* bytes) { * @param bytes Pointer where to store the encoded value */ inline void EncodeD16(u32 value, u8* bytes) { - *reinterpret_cast(bytes) = value & 0xFFFF; + const u16_le data = static_cast(value); + std::memcpy(bytes, &data, sizeof(data)); } /** From 5a9c00ea04c9010ead9790054d6034306ebbdc92 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Wed, 8 Aug 2018 16:17:38 -0400 Subject: [PATCH 2/2] common/color: Remove unnecessary const qualifiers on return types These are just superfluous and not necessesary --- src/common/color.h | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/common/color.h b/src/common/color.h index 2e56af5a9..0379040be 100644 --- a/src/common/color.h +++ b/src/common/color.h @@ -57,7 +57,7 @@ constexpr u8 Convert8To6(u8 value) { * @param bytes Pointer to encoded source color * @return Result color decoded as Math::Vec4 */ -inline const Math::Vec4 DecodeRGBA8(const u8* bytes) { +inline Math::Vec4 DecodeRGBA8(const u8* bytes) { return {bytes[3], bytes[2], bytes[1], bytes[0]}; } @@ -66,7 +66,7 @@ inline const Math::Vec4 DecodeRGBA8(const u8* bytes) { * @param bytes Pointer to encoded source color * @return Result color decoded as Math::Vec4 */ -inline const Math::Vec4 DecodeRGB8(const u8* bytes) { +inline Math::Vec4 DecodeRGB8(const u8* bytes) { return {bytes[2], bytes[1], bytes[0], 255}; } @@ -75,7 +75,7 @@ inline const Math::Vec4 DecodeRGB8(const u8* bytes) { * @param bytes Pointer to encoded source color * @return Result color decoded as Math::Vec4 */ -inline const Math::Vec4 DecodeRG8(const u8* bytes) { +inline Math::Vec4 DecodeRG8(const u8* bytes) { return {bytes[1], bytes[0], 0, 255}; } @@ -84,7 +84,7 @@ inline const Math::Vec4 DecodeRG8(const u8* bytes) { * @param bytes Pointer to encoded source color * @return Result color decoded as Math::Vec4 */ -inline const Math::Vec4 DecodeRGB565(const u8* bytes) { +inline Math::Vec4 DecodeRGB565(const u8* bytes) { u16_le pixel; std::memcpy(&pixel, bytes, sizeof(pixel)); return {Convert5To8((pixel >> 11) & 0x1F), Convert6To8((pixel >> 5) & 0x3F), @@ -96,7 +96,7 @@ inline const Math::Vec4 DecodeRGB565(const u8* bytes) { * @param bytes Pointer to encoded source color * @return Result color decoded as Math::Vec4 */ -inline const Math::Vec4 DecodeRGB5A1(const u8* bytes) { +inline Math::Vec4 DecodeRGB5A1(const u8* bytes) { u16_le pixel; std::memcpy(&pixel, bytes, sizeof(pixel)); return {Convert5To8((pixel >> 11) & 0x1F), Convert5To8((pixel >> 6) & 0x1F), @@ -108,7 +108,7 @@ inline const Math::Vec4 DecodeRGB5A1(const u8* bytes) { * @param bytes Pointer to encoded source color * @return Result color decoded as Math::Vec4 */ -inline const Math::Vec4 DecodeRGBA4(const u8* bytes) { +inline Math::Vec4 DecodeRGBA4(const u8* bytes) { u16_le pixel; std::memcpy(&pixel, bytes, sizeof(pixel)); return {Convert4To8((pixel >> 12) & 0xF), Convert4To8((pixel >> 8) & 0xF), @@ -140,7 +140,7 @@ inline u32 DecodeD24(const u8* bytes) { * @param bytes Pointer to encoded source values * @return Resulting values stored as a Math::Vec2 */ -inline const Math::Vec2 DecodeD24S8(const u8* bytes) { +inline Math::Vec2 DecodeD24S8(const u8* bytes) { return {static_cast((bytes[2] << 16) | (bytes[1] << 8) | bytes[0]), bytes[3]}; }