dhewm/dhewm3
0
0
Fork 0
mirror of https://github.com/dhewm/dhewm3.git synced 2025-03-19 09:11:41 +00:00
Code Issues Projects Releases Packages Wiki Activity

Fix script interpreter on 64bit Big Endian platforms

Browse source 
idInterpreter::Push() is used only for int and (reinterpreted) float
values, not pointers (as far as I can tell), so 32bit values on all
relevant platforms.
It stored its value as intptr_t at `&localstack[ localstackUsed ]` - on
64bit platforms intptr_t is 64bit.
Unfortunately, all code reading from the stack just get got a pointer
to `&localstack[ localstackUsed ]` in the type they want to read
(like `int*` or `float*`) and read that. On Little Endian that happens
to work, on 64bit Big Endian it reads the wrong 4 bytes of the intptr_t,
so it doesn't work.

fixes #625, #472
This commit is contained in:
Daniel Gibson 2024-10-31 03:01:27 +01:00
parent 76a9fdcebe
commit 3ad384a0da
2 changed files with 40 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

21
neo/d3xp/script/Script_Interpreter.h
View file

@ -146,7 +146,26 @@ ID_INLINE void idInterpreter::Push( intptr_t value ) {
if ( localstackUsed + sizeof( intptr_t ) > LOCALSTACK_SIZE ) {
Error( "Push: locals stack overflow\n" );
}
*( intptr_t * )&localstack[ localstackUsed ] = value;
// DG: fix for 64bit Big Endian machines
//*( intptr_t * )&localstack[ localstackUsed ] = value;
int val = value;
assert(value == val && "I really assumed that value would always fit into 32bit");
// dhewm3 used to store these values on localstack[] as intptr_t, even though
// they always seem to be int32. Unfortunately, all the code reading localstack[]
// gets the pointer to &localstack[ localstackUsed ] and then interprets that
// as int* or float* or whatever.
// So with 64bit machines it's stored as the wrong size (intptr_t is int64, int is int32),
// and on Big Endian the values are just 0 (or UINT_MAX if value < 0) - on Little Endian
// the first 4 bytes are the ones with actual data so the bug was hidden).
// To fix this, now a regular itn is stored at `&localstack[ localstackUsed ]`,
// as expected by the code using localstack[].
// TODO: once this has been tested more (and the assertion above has never triggered),
// change idInterpreter::Push() to take a regular int argument instead of intptr_t.
int *stackVar = ( int * )&localstack[ localstackUsed ];
*stackVar = val;
// even though a 32bit int is put onto the stack, increase it by sizeof(intptr_t)
// so it remains aligned to multiples of the native pointer size
localstackUsed += sizeof( intptr_t );
}

21
neo/game/script/Script_Interpreter.h
View file

@ -146,7 +146,26 @@ ID_INLINE void idInterpreter::Push( intptr_t value ) {
if ( localstackUsed + sizeof( intptr_t ) > LOCALSTACK_SIZE ) {
Error( "Push: locals stack overflow\n" );
}
*( intptr_t * )&localstack[ localstackUsed ] = value;
// DG: fix for 64bit Big Endian machines
//*( intptr_t * )&localstack[ localstackUsed ] = value;
int val = value;
assert(value == val && "I really assumed that value would always fit into 32bit");
// dhewm3 used to store these values on localstack[] as intptr_t, even though
// they always seem to be int32. Unfortunately, all the code reading localstack[]
// gets the pointer to &localstack[ localstackUsed ] and then interprets that
// as int* or float* or whatever.
// So with 64bit machines it's stored as the wrong size (intptr_t is int64, int is int32),
// and on Big Endian the values are just 0 (or UINT_MAX if value < 0) - on Little Endian
// the first 4 bytes are the ones with actual data so the bug was hidden).
// To fix this, now a regular itn is stored at `&localstack[ localstackUsed ]`,
// as expected by the code using localstack[].
// TODO: once this has been tested more (and the assertion above has never triggered),
// change idInterpreter::Push() to take a regular int argument instead of intptr_t.
int *stackVar = ( int * )&localstack[ localstackUsed ];
*stackVar = val;
// even though a 32bit int is put onto the stack, increase it by sizeof(intptr_t)
// so it remains aligned to multiples of the native pointer size
localstackUsed += sizeof( intptr_t );
}