Skip to content

Commit

Permalink
Fix some issues found by ASan/UBSan
Browse files Browse the repository at this point in the history
  • Loading branch information
dpogue committed Apr 27, 2024
1 parent 6fdfdd2 commit a0f4370
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 54 deletions.
24 changes: 3 additions & 21 deletions Sources/Plasma/CoreLib/hsStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,8 @@ uint32_t hsRAMStream::Read(uint32_t byteCount, void * buffer)

uint32_t hsRAMStream::Write(uint32_t byteCount, const void* buffer)
{
hsAssert(fVector.data(), "Trying to write to a null RAM buffer");

size_t spaceUntilEof = fVector.size() - fPosition;
if (byteCount <= spaceUntilEof) {
memcpy(fVector.data() + fPosition, buffer, byteCount);
Expand Down Expand Up @@ -957,26 +959,6 @@ bool hsQueueStream::AtEnd()
// hsBufferedStream
///////////////////////////////////////////////////////////////////////////////

inline void FastByteCopy(void* dest, const void* src, uint32_t bytes)
{
// Don't use memcpy if the read is 4 bytes or less, it's faster to just do a
// direct copy
switch (bytes)
{
case 4:
*((uint32_t*)dest) = *((const uint32_t*)src);
break;
case 2:
*((uint16_t*)dest) = *((const uint16_t*)src);
break;
case 1:
*((uint8_t*)dest) = *((const uint8_t*)src);
break;
default:
memcpy(dest, src, bytes);
}
}

//#define LOG_BUFFERED

hsBufferedStream::hsBufferedStream()
Expand Down Expand Up @@ -1081,7 +1063,7 @@ uint32_t hsBufferedStream::Read(uint32_t bytes, void* buffer)
uint32_t bytesInBuffer = fBufferLen - bufferPos;
uint32_t cachedReadSize = bytesInBuffer < bytes ? bytesInBuffer : bytes;

FastByteCopy(buffer, &fBuffer[bufferPos], cachedReadSize);
memcpy(buffer, &fBuffer[bufferPos], cachedReadSize);

fPosition += cachedReadSize;
numReadBytes += cachedReadSize;
Expand Down
47 changes: 18 additions & 29 deletions Sources/Plasma/NucleusLib/pnUUID/pnUUID_Unix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -50,41 +50,30 @@ static_assert(sizeof(plUUID) == sizeof(uuid_t), "plUUID and uuid_t types differ

struct plUUIDHelper
{
typedef struct _GUID {
uint32_t Data1;
uint16_t Data2;
uint16_t Data3;
uint8_t Data4[8];
} GUID;

static inline void CopyToPlasma( plUUID * dst, const uuid_t & src )
{
hsAssert( sizeof(uuid_t)==sizeof(dst->fData), "sizeof(uuid_t)!=sizeof(plUUID)" );
memcpy( (void*)dst->fData, (const void *)src, sizeof( plUUID ) );

/*
GUIDs are always internally encoded in little endian,
while the uuid_t structure is big endian.
*/
GUID* guid = reinterpret_cast<GUID*>(dst->fData);
guid->Data1 = hsSwapEndian32(guid->Data1);
guid->Data2 = hsSwapEndian16(guid->Data2);
guid->Data3 = hsSwapEndian16(guid->Data3);
hsAssert(sizeof(uuid_t) == sizeof(dst->fData), "sizeof(uuid_t) != sizeof(plUUID)");
memcpy((void*)dst->fData, (const void*)src, sizeof(plUUID));

// plUUIDs need to always be encoded in little endian, but the uuid_t struct is big endian
// This isn't memory-aligned, so casting to pointers is technically UB
std::swap(dst->fData[0], dst->fData[3]);
std::swap(dst->fData[1], dst->fData[2]);
std::swap(dst->fData[4], dst->fData[5]);
std::swap(dst->fData[6], dst->fData[7]);
}

static inline void CopyToNative( uuid_t & dst, const plUUID * src )
{
hsAssert( sizeof(uuid_t)==sizeof(src->fData), "sizeof(uuid_t)!=sizeof(plUUID)" );
memcpy( (void*)dst, (const void *)src->fData, sizeof( plUUID ) );

/*
GUIDs are always internally encoded in little endian,
while the uuid_t structure is big endian.
*/
GUID* guid = reinterpret_cast<GUID*>(dst);
guid->Data1 = hsSwapEndian32(guid->Data1);
guid->Data2 = hsSwapEndian16(guid->Data2);
guid->Data3 = hsSwapEndian16(guid->Data3);
hsAssert(sizeof(uuid_t) == sizeof(src->fData), "sizeof(uuid_t) != sizeof(plUUID)");
memcpy((void*)dst, (const void*)src->fData, sizeof(plUUID));

// uuid_t struct is big endian, but plUUIDs are always encoded in little endian
// This isn't memory-aligned, so casting to pointers is technically UB
std::swap(dst[0], dst[3]);
std::swap(dst[1], dst[2]);
std::swap(dst[4], dst[5]);
std::swap(dst[6], dst[7]);
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ void plBufferedFileReader::Close()
{
//plProfile_DelMem( SndBufferedMem, fBufferSize );

delete fBuffer;
delete[] fBuffer;
fBuffer = nullptr;
fBufferSize = 0;
fCursor = 0;
Expand Down
6 changes: 3 additions & 3 deletions Sources/Plasma/PubUtilLib/plPipeline/plTransitionMgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,10 @@ You can contact Cyan Worlds, Inc. by email legal@cyan.com
//// Constructor/Destructor //////////////////////////////////////////////////

plTransitionMgr::plTransitionMgr()
: fEffectPlate(), fCurrentEffect(kIdle), fRegisteredForTime(),
fHoldAtEnd(), fPlaying(), fNoSoundFade(), fEffectLength(), fCurrOpacity(),
fOpacDelta(), fLastTime()
{
fEffectPlate = nullptr;
fCurrentEffect = kIdle;
fPlaying = false;
}

void plTransitionMgr::Init()
Expand Down

0 comments on commit a0f4370

Please sign in to comment.