diff --git a/libs/matdbg/src/DebugServer.cpp b/libs/matdbg/src/DebugServer.cpp index e7d9df32cc..99645b438f 100644 --- a/libs/matdbg/src/DebugServer.cpp +++ b/libs/matdbg/src/DebugServer.cpp @@ -441,7 +441,8 @@ public: const static size_t kEditCmdLength = kEditCmd.size(); if (0 == strncmp(data, kEditCmd.c_str(), kEditCmdLength)) { - std::istringstream str(data + kEditCmdLength); + std::string command(data + kEditCmdLength, size - kEditCmdLength); + std::istringstream str(command); uint32_t matid; int api; int shaderIndex; @@ -546,8 +547,7 @@ DebugServer::addMaterial(const CString& name, const void* data, size_t size, voi } const uint32_t seed = 42; - auto words = (const uint32_t*) data; - MaterialKey key = utils::hash::murmur3(words, size / 4, seed); + const MaterialKey key = utils::hash::murmurSlow((const uint8_t*) data, size, seed); // Retain a copy of the package to permit queries after the client application has // freed up the original material package. diff --git a/libs/matdbg/src/ShaderReplacer.cpp b/libs/matdbg/src/ShaderReplacer.cpp index 32e67c73f3..5512848acf 100644 --- a/libs/matdbg/src/ShaderReplacer.cpp +++ b/libs/matdbg/src/ShaderReplacer.cpp @@ -324,15 +324,18 @@ void ShaderIndex::addShaderRecords(const uint8_t* chunkContent, size_t size) { stream.read((char*) &record.stage, sizeof(ShaderRecord::stage)); stream.read((char*) &record.offset, sizeof(ShaderRecord::offset)); - const uint8_t* linePtr = chunkContent + record.offset; - record.stringLength = *((uint32_t*) linePtr); - linePtr += sizeof(uint32_t); + const auto previousPosition = stream.tellg(); + stream.seekg(record.offset); + { + stream.read((char*) &record.stringLength, sizeof(ShaderRecord::stringLength)); - const uint32_t lineCount = *((uint32_t*) linePtr); - record.lineIndices.resize(lineCount); - linePtr += sizeof(uint32_t); + uint32_t lineCount; + stream.read((char*) &lineCount, sizeof(lineCount)); - memcpy(record.lineIndices.data(), linePtr, lineCount * sizeof(uint16_t)); + record.lineIndices.resize(lineCount); + stream.read((char*) record.lineIndices.data(), lineCount * sizeof(uint16_t)); + } + stream.seekg(previousPosition); } } diff --git a/libs/utils/CMakeLists.txt b/libs/utils/CMakeLists.txt index 587911ea4e..dc3c054e9a 100644 --- a/libs/utils/CMakeLists.txt +++ b/libs/utils/CMakeLists.txt @@ -141,6 +141,7 @@ set(TEST_SRCS test/test_CyclicBarrier.cpp test/test_Entity.cpp test/test_FixedCapacityVector.cpp + test/test_Hash.cpp test/test_JobSystem.cpp test/test_RangeMap.cpp test/test_StructureOfArrays.cpp diff --git a/libs/utils/include/utils/Hash.h b/libs/utils/include/utils/Hash.h index ee6e87f641..8d0bd9ff16 100644 --- a/libs/utils/include/utils/Hash.h +++ b/libs/utils/include/utils/Hash.h @@ -25,6 +25,7 @@ namespace utils { namespace hash { +// Hash function that takes an arbitrary swath of word-aligned data. inline uint32_t murmur3(const uint32_t* key, size_t wordCount, uint32_t seed) noexcept { uint32_t h = seed; size_t i = wordCount; @@ -46,6 +47,36 @@ inline uint32_t murmur3(const uint32_t* key, size_t wordCount, uint32_t seed) no return h; } +// The hash yields the same result for a given byte sequence regardless of alignment. +inline uint32_t murmurSlow(const uint8_t* key, size_t byteCount, uint32_t seed) noexcept { + const size_t wordCount = (byteCount + 3) / 4; + const uint8_t* const last = key + byteCount; + + // The remainder is identical to murmur3() except an inner loop safely "reads" an entire word. + uint32_t h = seed; + size_t i = wordCount; + do { + uint32_t k = 0; + for (int i = 0; i < 4 && key < last; ++i, ++key) { + k >>= 8; + k |= uint32_t(*key) << 24; + } + k *= 0xcc9e2d51u; + k = (k << 15u) | (k >> 17u); + k *= 0x1b873593u; + h ^= k; + h = (h << 13u) | (h >> 19u); + h = (h * 5u) + 0xe6546b64u; + } while (--i); + h ^= wordCount; + h ^= h >> 16u; + h *= 0x85ebca6bu; + h ^= h >> 13u; + h *= 0xc2b2ae35u; + h ^= h >> 16u; + return h; +} + template struct MurmurHashFn { uint32_t operator()(const T& key) const noexcept { diff --git a/libs/utils/test/test_Hash.cpp b/libs/utils/test/test_Hash.cpp new file mode 100644 index 0000000000..f9615ddc2d --- /dev/null +++ b/libs/utils/test/test_Hash.cpp @@ -0,0 +1,52 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include + +using namespace utils; + +TEST(HashTest, murmur) { + const uint32_t seed = 1701; + const uint32_t words[3] = { 0x12345678, 0x00c0ffee, 0xbadf000d}; + const uint32_t result1 = hash::murmur3(words, 1, seed); + const uint32_t result2 = hash::murmur3(words, 3, seed); + const uint32_t result3 = hash::murmur3(&words[2], 1, seed); + + EXPECT_NE(result1, result2); + EXPECT_NE(result2, result3); + + // Ensure that the byte-based hash function gives consistent results, + // regardless of alignment and surrounding bytes. + + const uint8_t bytes_a0[] = { 0x78, 0x56, 0x34, 0x12, 0xff, 0xaa, 0xbb }; + const uint8_t bytes_a1[] = { 0xcc, 0x78, 0x56, 0x34, 0x12, 0xdd, 0xee }; + const uint8_t bytes_a2[] = { 0xff, 0xaa, 0x78, 0x56, 0x34, 0x12, 0xbb }; + const uint8_t bytes_a3[] = { 0xcc, 0xdd, 0xee, 0x78, 0x56, 0x34, 0x12 }; + + const uint32_t result4 = hash::murmurSlow(&bytes_a0[0], 4, seed); + const uint32_t result5 = hash::murmurSlow(&bytes_a1[1], 4, seed); + const uint32_t result6 = hash::murmurSlow(&bytes_a2[2], 4, seed); + const uint32_t result7 = hash::murmurSlow(&bytes_a3[3], 4, seed); + const uint32_t result8 = hash::murmurSlow(&bytes_a0[0], 5, seed); + + EXPECT_EQ(result1, result4); // <== passes only on little endian machines. + EXPECT_EQ(result4, result5); + EXPECT_EQ(result4, result6); + EXPECT_EQ(result4, result7); + EXPECT_NE(result4, result8); +}