Skip to content

Commit

Permalink
Fix "t->c->value.argSize == func->parmTotal" Assertion in Scripts, #303
Browse files Browse the repository at this point in the history
If a "class" (object) in a Script has multiple member function
prototypes, and one function implementation later calls another before
that is implemented, there was an assertion when the script was parsed
(at map start), because the size of function arguments at the call-site
didn't match what the function expected - because the function hadn't
calculated that size yet, that only happened once its own
implementation was parsed.
Now it's calculated (and stored) when the prototype/declaration is
parsed to prevent this issue, which seems to be kinda common with Mods,
esp. Script-only mods, as the release game DLLs had Assertions disabled.
  • Loading branch information
DanielGibson committed Aug 2, 2020
1 parent 8b66bad commit 4902bc3
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 40 deletions.
53 changes: 33 additions & 20 deletions neo/d3xp/script/Script_Compiler.cpp
Expand Up @@ -2144,34 +2144,47 @@ void idCompiler::ParseFunctionDef( idTypeDef *returnType, const char *name ) {
}
}

// check if this is a prototype or declaration
if ( !CheckToken( "{" ) ) {
// it's just a prototype, so get the ; and move on
ExpectToken( ";" );
return;
}
// DG: make sure parmSize gets calculated when parsing prototype (not just when parsing
// implementation) so calling this function/method before implementation has been parsed
// works without getting Assertions in IdInterpreter::Execute() and ::LeaveFunction()
// ("st->c->value.argSize == func->parmTotal", "localstackUsed == localstackBase", see #303)

// calculate stack space used by parms
numParms = type->NumParameters();
func->parmSize.SetNum( numParms );
for( i = 0; i < numParms; i++ ) {
parmType = type->GetParmType( i );
if ( parmType->Inherits( &type_object ) ) {
func->parmSize[ i ] = type_object.Size();
} else {
func->parmSize[ i ] = parmType->Size();
if( numParms != func->parmSize.Num() ) { // DG: make sure not to do this twice

// if it hasn't been parsed yet, parmSize.Num() should be 0..
assert( func->parmSize.Num() == 0 && "function had different number of arguments before?!" );

func->parmSize.SetNum( numParms );
for( i = 0; i < numParms; i++ ) {
parmType = type->GetParmType( i );
if ( parmType->Inherits( &type_object ) ) {
func->parmSize[ i ] = type_object.Size();
} else {
func->parmSize[ i ] = parmType->Size();
}
func->parmTotal += func->parmSize[ i ];
}
func->parmTotal += func->parmSize[ i ];
}

// define the parms
for( i = 0; i < numParms; i++ ) {
if ( gameLocal.program.GetDef( type->GetParmType( i ), type->GetParmName( i ), def ) ) {
Error( "'%s' defined more than once in function parameters", type->GetParmName( i ) );
// define the parms
for( i = 0; i < numParms; i++ ) {
if ( gameLocal.program.GetDef( type->GetParmType( i ), type->GetParmName( i ), def ) ) {
Error( "'%s' defined more than once in function parameters", type->GetParmName( i ) );
}
gameLocal.program.AllocDef( type->GetParmType( i ), type->GetParmName( i ), def, false );
}
gameLocal.program.AllocDef( type->GetParmType( i ), type->GetParmName( i ), def, false );
}

// DG: moved this down here so parmSize also gets calculated when parsing prototype
// check if this is a prototype or declaration
if ( !CheckToken( "{" ) ) {
// it's just a prototype, so get the ; and move on
ExpectToken( ";" );
return;
}
// DG end

oldscope = scope;
scope = def;

Expand Down
53 changes: 33 additions & 20 deletions neo/game/script/Script_Compiler.cpp
Expand Up @@ -2144,34 +2144,47 @@ void idCompiler::ParseFunctionDef( idTypeDef *returnType, const char *name ) {
}
}

// check if this is a prototype or declaration
if ( !CheckToken( "{" ) ) {
// it's just a prototype, so get the ; and move on
ExpectToken( ";" );
return;
}
// DG: make sure parmSize gets calculated when parsing prototype (not just when parsing
// implementation) so calling this function/method before implementation has been parsed
// works without getting Assertions in IdInterpreter::Execute() and ::LeaveFunction()
// ("st->c->value.argSize == func->parmTotal", "localstackUsed == localstackBase", see #303)

// calculate stack space used by parms
numParms = type->NumParameters();
func->parmSize.SetNum( numParms );
for( i = 0; i < numParms; i++ ) {
parmType = type->GetParmType( i );
if ( parmType->Inherits( &type_object ) ) {
func->parmSize[ i ] = type_object.Size();
} else {
func->parmSize[ i ] = parmType->Size();
if( numParms != func->parmSize.Num() ) { // DG: make sure not to do this twice

// if it hasn't been parsed yet, parmSize.Num() should be 0..
assert( func->parmSize.Num() == 0 && "function had different number of arguments before?!" );

func->parmSize.SetNum( numParms );
for( i = 0; i < numParms; i++ ) {
parmType = type->GetParmType( i );
if ( parmType->Inherits( &type_object ) ) {
func->parmSize[ i ] = type_object.Size();
} else {
func->parmSize[ i ] = parmType->Size();
}
func->parmTotal += func->parmSize[ i ];
}
func->parmTotal += func->parmSize[ i ];
}

// define the parms
for( i = 0; i < numParms; i++ ) {
if ( gameLocal.program.GetDef( type->GetParmType( i ), type->GetParmName( i ), def ) ) {
Error( "'%s' defined more than once in function parameters", type->GetParmName( i ) );
// define the parms
for( i = 0; i < numParms; i++ ) {
if ( gameLocal.program.GetDef( type->GetParmType( i ), type->GetParmName( i ), def ) ) {
Error( "'%s' defined more than once in function parameters", type->GetParmName( i ) );
}
gameLocal.program.AllocDef( type->GetParmType( i ), type->GetParmName( i ), def, false );
}
gameLocal.program.AllocDef( type->GetParmType( i ), type->GetParmName( i ), def, false );
}

// DG: moved this down here so parmSize also gets calculated when parsing prototype
// check if this is a prototype or declaration
if ( !CheckToken( "{" ) ) {
// it's just a prototype, so get the ; and move on
ExpectToken( ";" );
return;
}
// DG end

oldscope = scope;
scope = def;

Expand Down

0 comments on commit 4902bc3

Please sign in to comment.