From a60bc20585bc88b5451fb954fe74cfbeb52f1c55 Mon Sep 17 00:00:00 2001 From: Filip Gawin Date: Thu, 21 Dec 2017 21:37:58 +0100 Subject: [PATCH] Fix misaligned memory(UB) X86 is able to deal with misaligned memory, but it can hurt perf. Other arch like for example mips is not able to digest it. So in order of portability we should get rid of this UB. --- rwengine/src/script/SCMFile.hpp | 3 +- rwlib/CMakeLists.txt | 1 + rwlib/source/loaders/LoaderDFF.cpp | 61 ++++++++++++++----------- rwlib/source/loaders/RWBinaryStream.hpp | 10 ++-- rwlib/source/rw/bit_cast.cpp | 16 +++++++ 5 files changed, 60 insertions(+), 31 deletions(-) create mode 100644 rwlib/source/rw/bit_cast.cpp diff --git a/rwengine/src/script/SCMFile.hpp b/rwengine/src/script/SCMFile.hpp index db59cad4..d3122c93 100644 --- a/rwengine/src/script/SCMFile.hpp +++ b/rwengine/src/script/SCMFile.hpp @@ -6,6 +6,7 @@ #include #include "script/ScriptTypes.hpp" +#include "rw/bit_cast.cpp" /** * @brief Handles in-memory SCM file data including section offsets. @@ -33,7 +34,7 @@ public: template T read(unsigned int offset) const { - return *(T*)(_data + offset); + return bit_cast(*(_data + offset)); } uint32_t getMainSize() const { diff --git a/rwlib/CMakeLists.txt b/rwlib/CMakeLists.txt index 8e3c4fa7..29963f55 100644 --- a/rwlib/CMakeLists.txt +++ b/rwlib/CMakeLists.txt @@ -16,6 +16,7 @@ SET(RWLIB_SOURCES source/gl/TextureData.cpp source/rw/abort.cpp + source/rw/bit_cast.cpp source/rw/filesystem.hpp source/rw/forward.hpp source/rw/types.hpp diff --git a/rwlib/source/loaders/LoaderDFF.cpp b/rwlib/source/loaders/LoaderDFF.cpp index b7fba6ca..963d6288 100644 --- a/rwlib/source/loaders/LoaderDFF.cpp +++ b/rwlib/source/loaders/LoaderDFF.cpp @@ -124,7 +124,7 @@ LoaderDFF::GeometryList LoaderDFF::readGeometryList(const RWBStream &stream) { char *headerPtr = listStream.getCursor(); - unsigned int numGeometries = *(std::uint32_t *)headerPtr; + unsigned int numGeometries = bit_cast(*headerPtr); headerPtr += sizeof(std::uint32_t); std::vector geometrylist; @@ -156,19 +156,20 @@ GeometryPtr LoaderDFF::readGeometry(const RWBStream &stream) { char *headerPtr = geomStream.getCursor(); - geom->flags = *(std::uint16_t *)headerPtr; + geom->flags = bit_cast(*headerPtr); headerPtr += sizeof(std::uint16_t); - /*unsigned short numUVs = *(std::uint8_t*)headerPtr;*/ + /*unsigned short numUVs = bit_cast(*headerPtr);*/ headerPtr += sizeof(std::uint8_t); - /*unsigned short moreFlags = *(std::uint8_t*)headerPtr;*/ + /*unsigned short moreFlags = bit_cast(*headerPtr);*/ headerPtr += sizeof(std::uint8_t); - unsigned int numTris = *(std::uint32_t *)headerPtr; + unsigned int numTris = bit_cast(*headerPtr); headerPtr += sizeof(std::uint32_t); - unsigned int numVerts = *(std::uint32_t *)headerPtr; + unsigned int numVerts = bit_cast(*headerPtr); headerPtr += sizeof(std::uint32_t); - /*unsigned int numFrames = *(std::uint32_t*)headerPtr;*/ + + /*unsigned int numFrames = bit_cast(*headerPtr);*/ headerPtr += sizeof(std::uint32_t); std::vector verts; @@ -182,7 +183,7 @@ GeometryPtr LoaderDFF::readGeometry(const RWBStream &stream) { if ((geom->flags & 8) == 8) { for (size_t v = 0; v < numVerts; ++v) { - verts[v].colour = *(glm::u8vec4 *)headerPtr; + verts[v].colour = bit_cast(*headerPtr); headerPtr += sizeof(glm::u8vec4); } } else { @@ -193,27 +194,28 @@ GeometryPtr LoaderDFF::readGeometry(const RWBStream &stream) { if ((geom->flags & 4) == 4 || (geom->flags & 128) == 128) { for (size_t v = 0; v < numVerts; ++v) { - verts[v].texcoord = *(glm::vec2 *)headerPtr; + verts[v].texcoord = bit_cast(*headerPtr); headerPtr += sizeof(glm::vec2); } } // Grab indicies data to generate normals (if applicable). - RW::BSGeometryTriangle *triangles = (RW::BSGeometryTriangle *)headerPtr; + auto triangles = std::make_unique(numTris); + memcpy(triangles.get(), headerPtr, sizeof(RW::BSGeometryTriangle) * numTris); headerPtr += sizeof(RW::BSGeometryTriangle) * numTris; - geom->geometryBounds = *(RW::BSGeometryBounds *)headerPtr; + geom->geometryBounds = bit_cast(*headerPtr); geom->geometryBounds.radius = std::abs(geom->geometryBounds.radius); headerPtr += sizeof(RW::BSGeometryBounds); for (size_t v = 0; v < numVerts; ++v) { - verts[v].position = *(glm::vec3 *)headerPtr; + verts[v].position = bit_cast(*headerPtr); headerPtr += sizeof(glm::vec3); } if ((geom->flags & 16) == 16) { for (size_t v = 0; v < numVerts; ++v) { - verts[v].normal = *(glm::vec3 *)headerPtr; + verts[v].normal = bit_cast(*headerPtr); headerPtr += sizeof(glm::vec3); } } else { @@ -276,7 +278,7 @@ void LoaderDFF::readMaterialList(GeometryPtr &geom, const RWBStream &stream) { throw DFFLoaderException("MaterialList missing struct chunk"); } - unsigned int numMaterials = *(std::uint32_t *)listStream.getCursor(); + unsigned int numMaterials = bit_cast(*listStream.getCursor()); geom->materials.reserve(numMaterials); @@ -306,18 +308,21 @@ void LoaderDFF::readMaterial(GeometryPtr &geom, const RWBStream &stream) { // Unkown matData += sizeof(std::uint32_t); - material.colour = *(glm::u8vec4 *)matData; + material.colour = bit_cast(*matData); + matData += sizeof(std::uint32_t); // Unkown matData += sizeof(std::uint32_t); - /*bool usesTexture = *(std::uint32_t*)matData;*/ + /*bool usesTexture = bit_cast(*matData);*/ matData += sizeof(std::uint32_t); - material.ambientIntensity = *(float *)matData; + material.ambientIntensity = bit_cast(*matData); matData += sizeof(float); - /*float specular = *(float*)matData;*/ + + /*float specular = bit_cast(*matData);*/ matData += sizeof(float); - material.diffuseIntensity = *(float *)matData; + + material.diffuseIntensity = bit_cast(*matData); matData += sizeof(float); material.flags = 0; @@ -381,10 +386,10 @@ void LoaderDFF::readGeometryExtension(GeometryPtr &geom, void LoaderDFF::readBinMeshPLG(GeometryPtr &geom, const RWBStream &stream) { auto data = stream.getCursor(); - geom->facetype = static_cast(*(std::uint32_t *)data); + geom->facetype = static_cast(bit_cast(*data)); data += sizeof(std::uint32_t); - unsigned int numSplits = *(std::uint32_t *)data; + unsigned int numSplits = bit_cast(*data); data += sizeof(std::uint32_t); // Number of triangles. @@ -396,9 +401,9 @@ void LoaderDFF::readBinMeshPLG(GeometryPtr &geom, const RWBStream &stream) { for (size_t s = 0; s < numSplits; ++s) { SubGeometry sg; - sg.numIndices = *(std::uint32_t *)data; + sg.numIndices = bit_cast(*data); data += sizeof(std::uint32_t); - sg.material = *(std::uint32_t *)data; + sg.material = bit_cast(*data); data += sizeof(std::uint32_t); sg.start = start; start += sg.numIndices; @@ -423,11 +428,13 @@ AtomicPtr LoaderDFF::readAtomic(FrameList &framelist, } auto data = atomicStream.getCursor(); - auto frame = *(std::uint32_t *)data; + std::uint32_t frame = bit_cast(*data); data += sizeof(std::uint32_t); - auto geometry = *(std::uint32_t *)data; + + std::uint32_t geometry = bit_cast(*data); data += sizeof(std::uint32_t); - auto flags = *(std::uint32_t *) data; + + std::uint32_t flags = bit_cast(*data); // Verify the atomic's particulars RW_CHECK(frame < framelist.size(), "atomic frame " << frame @@ -465,7 +472,7 @@ ClumpPtr LoaderDFF::loadFromMemory(FileHandle file) { } // There is only one value in the struct section. - auto numAtomics = *(std::uint32_t *)rootStream.getCursor(); + std::uint32_t numAtomics = bit_cast(*rootStream.getCursor()); RW_UNUSED(numAtomics); GeometryList geometrylist; diff --git a/rwlib/source/loaders/RWBinaryStream.hpp b/rwlib/source/loaders/RWBinaryStream.hpp index 6a2890ec..92b69633 100644 --- a/rwlib/source/loaders/RWBinaryStream.hpp +++ b/rwlib/source/loaders/RWBinaryStream.hpp @@ -2,10 +2,12 @@ #define _LIBRW_RWBINARYSTREAM_HPP_ #include #include +#include #include #include "rw/defines.hpp" +#include "rw/bit_cast.cpp" /** * @brief Class for working with RenderWare binary streams. @@ -39,11 +41,13 @@ public: // _nextChunk is initally = to _data, making this a non-op _dataCur = _nextChunk; - ChunkID id = *(ChunkID*)(_dataCur); + ChunkID id = bit_cast(*_dataCur); _dataCur += sizeof(ChunkID); - _currChunkSz = *(std::uint32_t*)(_dataCur); + + _currChunkSz = bit_cast(*_dataCur); _dataCur += sizeof(std::uint32_t); - _chunkVersion = *(std::uint32_t*)(_dataCur); + + _chunkVersion = bit_cast(*_dataCur); _dataCur += sizeof(std::uint32_t); _nextChunk = _dataCur + _currChunkSz; diff --git a/rwlib/source/rw/bit_cast.cpp b/rwlib/source/rw/bit_cast.cpp new file mode 100644 index 00000000..77112900 --- /dev/null +++ b/rwlib/source/rw/bit_cast.cpp @@ -0,0 +1,16 @@ +#ifndef _LIBRW_BIT_CAST_CPP_ +#define _LIBRW_BIT_CAST_CPP_ + +//Based on https://gist.github.com/socantre/3472964 +#include // memcpy +#include // is_trivially_copyable +#include "rw/defines.hpp" // RW_ASSERT + +template +inline Dest bit_cast(Source const &source) { + Dest dest = Dest{}; + std::memcpy(&dest, &source, sizeof(Dest)); + return dest; +} + +#endif