diff --git a/demo/tests.cpp b/demo/tests.cpp index 522733f53..260878a5c 100644 --- a/demo/tests.cpp +++ b/demo/tests.cpp @@ -50,33 +50,31 @@ static const PV kVertexBuffer[] = { }; static const unsigned char kVertexDataV0[] = { - 0xa0, 0x01, 0x3f, 0x00, 0x00, 0x00, 0x58, 0x57, 0x58, 0x01, 0x26, 0x00, 0x00, 0x00, 0x01, - 0x0c, 0x00, 0x00, 0x00, 0x58, 0x01, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, - 0x3f, 0x00, 0x00, 0x00, 0x17, 0x18, 0x17, 0x01, 0x26, 0x00, 0x00, 0x00, 0x01, 0x0c, 0x00, - 0x00, 0x00, 0x17, 0x01, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // clang-format :-/ + 0xa0, 0x01, 0x3f, 0x00, 0x00, 0x00, 0x58, 0x57, 0x58, 0x01, 0x26, 0x00, 0x00, 0x00, 0x01, 0x0c, + 0x00, 0x00, 0x00, 0x58, 0x01, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x3f, 0x00, + 0x00, 0x00, 0x17, 0x18, 0x17, 0x01, 0x26, 0x00, 0x00, 0x00, 0x01, 0x0c, 0x00, 0x00, 0x00, 0x17, + 0x01, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, // clang-format :-/ }; static const unsigned char kVertexDataV1[] = { - 0xae, 0xee, 0xaa, 0xee, 0x00, 0x4b, 0x4b, 0x4b, 0x00, 0x00, 0x4b, 0x00, 0x00, 0x7d, 0x7d, - 0x7d, 0x00, 0x00, 0x7d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x62, 0x00, 0x62, // clang-format :-/ + 0xae, 0xee, 0xaa, 0xee, 0x00, 0x4b, 0x4b, 0x4b, 0x00, 0x00, 0x4b, 0x00, 0x00, 0x7d, 0x7d, 0x7d, + 0x00, 0x00, 0x7d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x62, 0x00, 0x62, // clang-format :-/ }; // This binary blob is a valid v1 encoding of vertex buffer but it used a custom version of // the encoder that exercised all features of the format; because of this it is much larger // and will never be produced by the encoder itself. static const unsigned char kVertexDataV1Custom[] = { - 0xae, 0xd4, 0x94, 0xd4, 0x01, 0x0e, 0x00, 0x58, 0x57, 0x58, 0x02, 0x02, 0x12, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x58, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, - 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x0e, 0x00, 0x7d, 0x7d, 0x7d, 0x01, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x7d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x01, 0x62, // clang-format :-/ + 0xae, 0xd4, 0x94, 0xd4, 0x01, 0x0e, 0x00, 0x58, 0x57, 0x58, 0x02, 0x02, 0x12, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x58, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x0e, 0x00, 0x7d, 0x7d, 0x7d, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x7d, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x62, // clang-format :-/ }; static void decodeIndexV0() diff --git a/src/vertexcodec.cpp b/src/vertexcodec.cpp index 4c70ea1ba..725eecda0 100644 --- a/src/vertexcodec.cpp +++ b/src/vertexcodec.cpp @@ -128,7 +128,8 @@ const size_t kVertexBlockSizeBytes = 8192; const size_t kVertexBlockMaxSize = 256; const size_t kByteGroupSize = 16; const size_t kByteGroupDecodeLimit = 24; -const size_t kTailMinSize = 32; // must be >= kByteGroupDecodeLimit +const size_t kTailMinSizeV0 = 32; +const size_t kTailMinSizeV1 = 24; static const int kBitsV0[4] = {0, 2, 4, 8}; static const int kBitsV1[5] = {0, 1, 2, 4, 8}; @@ -354,7 +355,7 @@ static void encodeDeltas(unsigned char* buffer, const unsigned char* vertex_data case 2: return encodeDeltas1(buffer, vertex_data, vertex_count, vertex_size, last_vertex, k, channel >> 4); default: - assert(!"Unsupported channel encoding"); + assert(!"Unsupported channel encoding"); // unreachable } } @@ -385,15 +386,12 @@ static int estimateRotate(const unsigned char* vertex_data, size_t vertex_count, vertex += vertex_size; } - // ignore trivial groups for performance - if (bitg == 0 || bitg == ~0u) - continue; - for (int j = 0; j < 8; ++j) { unsigned int bitr = rotate(bitg, j); - sizes[j] += estimateBits((unsigned char)(bitr >> 0)) + estimateBits((unsigned char)(bitr >> 8)) + estimateBits((unsigned char)(bitr >> 16)) + estimateBits((unsigned char)(bitr >> 24)); + sizes[j] += estimateBits((unsigned char)(bitr >> 0)) + estimateBits((unsigned char)(bitr >> 8)); + sizes[j] += estimateBits((unsigned char)(bitr >> 16)) + estimateBits((unsigned char)(bitr >> 24)); } } @@ -764,8 +762,7 @@ static const unsigned char* decodeVertexBlock(const unsigned char* data, const u decodeDeltas1(buffer, transposed + k, vertex_count, vertex_size, last_vertex + k, (32 - (channel >> 4)) & 31); break; default: - // invalid channel type - return NULL; + return NULL; // invalid channel type } } @@ -947,7 +944,7 @@ inline const unsigned char* decodeBytesGroupSimd(const unsigned char* data, unsi } default: - SIMD_UNREACHABLE(); + SIMD_UNREACHABLE(); // unreachable } } #endif @@ -1015,7 +1012,7 @@ inline const unsigned char* decodeBytesGroupSimd(const unsigned char* data, unsi } default: - SIMD_UNREACHABLE(); + SIMD_UNREACHABLE(); // unreachable } } #endif @@ -1159,8 +1156,7 @@ inline const unsigned char* decodeBytesGroupSimd(const unsigned char* data, unsi } default: - SIMD_UNREACHABLE(); - return data; + SIMD_UNREACHABLE(); // unreachable } } #endif @@ -1279,7 +1275,7 @@ inline const unsigned char* decodeBytesGroupSimd(const unsigned char* data, unsi } default: - SIMD_UNREACHABLE(); + SIMD_UNREACHABLE(); // unreachable } } #endif @@ -1593,8 +1589,7 @@ static const unsigned char* decodeVertexBlockSimd(const unsigned char* data, con decodeDeltas4Simd<2>(buffer, transposed + k, vertex_count_aligned, vertex_size, last_vertex + k, (32 - (channel >> 4)) & 31); break; default: - // invalid channel type - return NULL; + return NULL; // invalid channel type } } @@ -1681,7 +1676,8 @@ size_t meshopt_encodeVertexBufferLevel(unsigned char* buffer, size_t buffer_size } size_t tail_size = vertex_size + (version == 0 ? 0 : vertex_size / 4); - size_t tail_size_pad = tail_size < kTailMinSize ? kTailMinSize : tail_size; + size_t tail_size_min = version == 0 ? kTailMinSizeV0 : kTailMinSizeV1; + size_t tail_size_pad = tail_size < tail_size_min ? tail_size_min : tail_size; if (size_t(data_end - data) < tail_size_pad) return 0; @@ -1775,7 +1771,9 @@ size_t meshopt_encodeVertexBufferBound(size_t vertex_count, size_t vertex_size) size_t vertex_block_data_size = vertex_block_size; size_t tail_size = vertex_size + (vertex_size / 4); - size_t tail_size_pad = tail_size < kTailMinSize ? kTailMinSize : tail_size; + size_t tail_size_min = kTailMinSizeV0 > kTailMinSizeV1 ? kTailMinSizeV0 : kTailMinSizeV1; + size_t tail_size_pad = tail_size < tail_size_min ? tail_size_min : tail_size; + assert(tail_size_pad >= kByteGroupDecodeLimit); return 1 + vertex_block_count * vertex_size * (vertex_block_control_size + vertex_block_header_size + vertex_block_data_size) + tail_size_pad; } @@ -1828,7 +1826,8 @@ int meshopt_decodeVertexBuffer(void* destination, size_t vertex_count, size_t ve return -1; size_t tail_size = vertex_size + (version == 0 ? 0 : vertex_size / 4); - size_t tail_size_pad = tail_size < kTailMinSize ? kTailMinSize : tail_size; + size_t tail_size_min = version == 0 ? kTailMinSizeV0 : kTailMinSizeV1; + size_t tail_size_pad = tail_size < tail_size_min ? tail_size_min : tail_size; if (size_t(data_end - data) < tail_size_pad) return -2; diff --git a/tools/codecfuzz.cpp b/tools/codecfuzz.cpp index 2e2c89489..41602a292 100644 --- a/tools/codecfuzz.cpp +++ b/tools/codecfuzz.cpp @@ -27,9 +27,14 @@ void fuzzRoundtrip(const uint8_t* data, size_t size, size_t stride, int level) assert(encoded && decoded); size_t res = meshopt_encodeVertexBufferLevel(static_cast(encoded), bound, data, count, stride, level); - assert(res <= bound); + assert(res > 0 && res <= bound); - int rc = meshopt_decodeVertexBuffer(decoded, count, stride, static_cast(encoded), res); + // encode again at the boundary to check for memory safety + // this should produce the same output because encoder is deterministic + size_t rese = meshopt_encodeVertexBufferLevel(static_cast(encoded) + bound - res, res, data, count, stride, level); + assert(rese == res); + + int rc = meshopt_decodeVertexBuffer(decoded, count, stride, static_cast(encoded) + bound - res, res); assert(rc == 0); assert(memcmp(data, decoded, count * stride) == 0); @@ -49,30 +54,24 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) fuzzDecoder(data, size, 4, meshopt_decodeIndexSequence); // decodeVertexBuffer supports any strides divisible by 4 in 4-256 interval - // It's a waste of time to check all of them, so we'll just check a few with different alignment mod 16 + // it's a waste of time to check all of them, so we'll just check a few with different alignment mod 16 fuzzDecoder(data, size, 4, meshopt_decodeVertexBuffer); fuzzDecoder(data, size, 16, meshopt_decodeVertexBuffer); fuzzDecoder(data, size, 24, meshopt_decodeVertexBuffer); fuzzDecoder(data, size, 32, meshopt_decodeVertexBuffer); // encodeVertexBuffer/decodeVertexBuffer should roundtrip for any stride, check a few with different alignment mod 16 - fuzzRoundtrip(data, size, 4, 0); - fuzzRoundtrip(data, size, 16, 0); - fuzzRoundtrip(data, size, 24, 0); - fuzzRoundtrip(data, size, 32, 0); - - // repeat roundtrip testing for new encoding - meshopt_encodeVertexVersion(0xe); - - for (int level = 0; level < 4; ++level) - { - fuzzRoundtrip(data, size, 4, level); - fuzzRoundtrip(data, size, 16, level); - fuzzRoundtrip(data, size, 24, level); - fuzzRoundtrip(data, size, 32, level); - } - - meshopt_encodeVertexVersion(0); + // this also checks memory safety properties of the encoder + // to conserve time, we only check one version/level combination, biased towards version 1 + uint8_t data0 = size > 0 ? data[0] : 0; + int level = data0 % 5; + + meshopt_encodeVertexVersion(level < 4 ? 0xe : 0); + + fuzzRoundtrip(data, size, 4, level); + fuzzRoundtrip(data, size, 16, level); + fuzzRoundtrip(data, size, 24, level); + fuzzRoundtrip(data, size, 32, level); return 0; }