I think I have identified a bug during stack unwinding following an exception. A crash occurs when determining live objects when a live value object has been declared immediately following block-ending brace “}”.

The root cause may have existed for a while (I'm not sure), but I only noticed a crash following revision 2832 where a stack variable was checked with IsVarInScope before being marked as a live object within the asCContext::DetermineLiveObjects function. IsVarInScope returns false, and I think that's erroneous.

I am including at the bottom a test you can use to reproduce the bug. I'm working on a fix myself, but I thought it might be best to share with you early rather than after I develop a patch.

The problem appears to arise from an assumption made during the counting of asBLOCK_END options within the IsVarInScope function.

asUINT declaredAt = func->scriptData->variables[varIndex]->declaredAtProgramPos;

// If the program position is after the variable declaration it is necessary
// determine if the program position is still inside the statement block where
// the variable was declared.
for( int n = 0; n < (int)func->scriptData->objVariableInfo.GetLength(); n++ )
{
	if( func->scriptData->objVariableInfo[n].programPos >= declaredAt )
	{
		// If the current block ends between the declaredAt and current
		// program position, then we know the variable is no longer visible
		int level = 0;
		for( ; n < (int)func->scriptData->objVariableInfo.GetLength(); n++ )
		{
			if( func->scriptData->objVariableInfo[n].programPos > pos )
				break;

			if( func->scriptData->objVariableInfo[n].option == asBLOCK_BEGIN ) level++;
			if( func->scriptData->objVariableInfo[n].option == asBLOCK_END && --level < 0 )
				return false;
		}

		break;
	}
}

The programPos member of objVariableInfo is being used to identify variable information between the declaration of a specified stack variable and prior to the current code execution position. Beginnings and endings of blocks are encountered and tracked until the end of a block is encountered that hasn't been opened. The idea is that this block end indicates that the specified variable has already gone out of scope, and hence been destroyed, prior to the current code execution.

Similar logic is used by the caller, DetermineLiveObjects, to track object initialization and destruction. Below is a screenshot of my watch window debugging the attached test case. I will walk through what I believe causes the crash using it as an example.

case asOBJ_INIT: // Object was created
	{
		// Which variable is this? Use IsVarInScope to get the correct variable in case there are multiple variables sharing the same offset
		asUINT var = asUINT(-1);
		for (asUINT v = 0; v < func->scriptData->variables.GetLength(); v++)
			if (func->scriptData->variables[v]->stackOffset == func->scriptData->objVariableInfo[n].variableOffset &&
				IsVarInScope(v, stackLevel) )
			{
				var = v;
				break;
			}
		asASSERT(var != asUINT(-1));
		liveObjects[var] += 1;
	}
	break;

Below is the debug output of the main function from the test case. Variable “a” is at stack position 3.

double main()

Temps: 2, 6

Variables: 
 001: int a
 004: String a
 002: int 
 006: double 


- 5,3 -
            {
               VarDecl  0
    0   6 *    SetV4    v1, 0x1          (i:1, f:1.4013e-45)
- 7,2 -
            }
            1:
               VarDecl  1
    2   6 *    PGA      0x5690e380          (str:H)
    5   8 *    PSF      v4
    6  10 *    CALLSYS  20           (String::String(const String&inout))
- 8,2 -
               ObjInfo  v4, 1
    8   6 *    PGA      0x5690dfc0          (str:x)
   11   8 *    PshC4    0x2a          (i:42, f:5.88545e-44)
   13   9 *    PSF      v4
   14  11 *    CALLSYS  84           (void String::SetAt(int, const String&inout))
- 9,2 -
   16   6 *    SetV8    v6, 0x0          (i:0, f:0)
            {
   19   6 *    PSF      v4
   20   8 *    CALLSYS  21           (String::~String())
               ObjInfo  v4, 0
            }
   22   6 *    CpyVtoR8 v6
            0:
   23   6 *    RET      0

An exception is generated at program position 7 (during the call to the “exception” method). The loop in DetermineLiveObjects identifies the final entry in the objVariableInfo array, because programPos (11) is greater than the current program position (7). Then the nested loop begins with the prior entry in that array (index position 2), which has the option asOBJ_INIT. This is the initialization information for variable “a”, having variableOffset equal to 3. Searching through the variables array finds the correct variable “a” comparing to stackOffset. Is this variable still in scope? Based on the test code and the watch variables, I think it should be. asOBJ_UNINIT occurs later than the current position, but IsVarInScope fails to correctly identify that fact.

The asSScriptVariable object for variable “a” has declaredAtProgramPos equal to 2. This is consistent with the debug output above. Position 2 follows VarDecl 1. The object is initialized at position 5 (after ObjInfo v3, 1) and destroyed at position 11 (after ObjInfo v3, 0).

IsVarInScope searches for asBLOCK_BEGIN and asBLOCK_END if the programPos member in the objVariableInfo array is greater than or equal to the declaredAt variable, but not greater than the pos variable. For this example, pos is 7 and declaredAt is 2. In the image of my watch window, you should be able to see that there is an asBLOCK_END variable info at program position 2. This immediately causes InVarInScope to return false. DetermineLiveObjects fails to find the object it expects to, and so it crashes modifying the liveObjects array at index position -1.

I believe IsVarInScope is beginning its search for terminating blocks too early. Perhaps the proper fix is to change the comparison in IsVarInScope (currently line 5133) to greater than, rather than greater than or equal to. Maybe the fix is to change the program position of the asBLOCK_END option to 1, rather than have it share a position with the declaration. Maybe variable declaration needs to be explicitly present in the objVariableInfo array. I'm not sure what the proper fix would be here. Please advise.

Note that in my attached test I have set the engine property asEP_BUILD_WITHOUT_LINE_CUES to true. This is a subtle bug, and the injection of bytecode between the ending of the scope block and declaration will avoid the crash. Line cues add such a marker. Disabling the optimizer also consistently adds bytecode around object initialization.

If I identify a patch I'm happy with, I'll post it here. Otherwise I'll await your fix, suggestion, comments, or questions.

Regards,

Jason

----------------------------------------------------

class ClassValue
{
public:
    ClassValue() {}
    ~ClassValue() {}

    void exception()
    {
        asGetActiveContext()->SetException("Crash.");
    }
};

void Constructor(void* mem)
{
    new(mem) ClassValue();
}

void Destructor(void* mem)
{
    ((ClassValue*)mem)->~ClassValue();
}

bool Test()
{
    bool fail = false;
    int r;

    {
        asIScriptEngine* engine = asCreateScriptEngine(ANGELSCRIPT_VERSION);
    
        engine->RegisterObjectType("ClassValue", sizeof(ClassValue),
            asOBJ_VALUE | asOBJ_APP_CLASS_CD);
        engine->SetEngineProperty(asEP_BUILD_WITHOUT_LINE_CUES, true);
        
        engine->RegisterObjectBehaviour("ClassValue", asBEHAVE_CONSTRUCT,
            "void f()", asFUNCTION(Constructor), asCALL_CDECL_OBJLAST);
        engine->RegisterObjectBehaviour("ClassValue", asBEHAVE_DESTRUCT,
            "void f()", asFUNCTION(Destructor), asCALL_CDECL_OBJLAST);
        engine->RegisterObjectMethod("ClassValue", "void exception()",
            asMETHOD(ClassValue, exception), asCALL_THISCALL);

        asIScriptModule* mod = engine->GetModule("test", asGM_ALWAYS_CREATE);
        mod->AddScriptSection("test",
            R"(
            void main() {
                if (true)
                {
                    int dummy = 1;
                }
                ClassValue a;
                a.exception();
            }
            )");
        r = mod->Build();
        if (r < 0)
            TEST_FAILED;
        
        asIScriptContext* ctx = engine->CreateContext();
        ctx->Prepare(mod->GetFunctionByName("main"));
        r = ctx->Execute();
        if (r != asEXECUTION_EXCEPTION)
            TEST_FAILED;
        ctx->Release();
        engine->ShutDownAndRelease();
    }

    return fail;
}

I've investigated this a little further and found that the asOBJ_VARDECL option is included if there is a “try/catch” block, but omitted if there is not one. It looks like the appropriate fix might be found somewhere around line 1550 of as_bytecode.cpp (asCByteCode::ExtractObjectVariableInfo). However, it might not be possible to detect exceptions generated by the application.

I have a provisional fix that works in my implementation. I don't know if this has any undesirable side effects though, so I hesitate to suggest it as an official fix.

First, always include the asOBJ_VARDECL option in the objVariableInfo array, indicated here by commenting out the condition.

		else if( instr->op == asBC_VarDecl )
		{
			// Record the position for debug info
			outFunc->scriptData->variables[instr->wArg[0]]->declaredAtProgramPos = pos;
			
			// Record declaration of object variables for try/catch handling
			// This is used for identifying if handles and objects on the heap should be cleared upon catching an exception
			// Only extract this info if there is a try/catch block in the function, so we don't use up unnecessary space
			//if( outFunc->scriptData->tryCatchInfo.GetLength() && outFunc->scriptData->variables[instr->wArg[0]]->type.GetTypeInfo() )
			{
				asSObjectVariableInfo info;
				info.programPos     = pos;
				info.variableOffset = outFunc->scriptData->variables[instr->wArg[0]]->stackOffset;
				info.option         = asOBJ_VARDECL;
				outFunc->scriptData->objVariableInfo.PushLast(info);
			}
		}

Change the lookup in IsVarInScope to begin at the location of asOBJ_VARDECL rather than simply at a matching program position.

	asUINT declaredAt = func->scriptData->variables[varIndex]->declaredAtProgramPos;
	int stackOffset = func->scriptData->variables[varIndex]->stackOffset;

	// If the program position is after the variable declaration it is necessary
	// determine if the program position is still inside the statement block where
	// the variable was declared.
	for( int n = 0; n < (int)func->scriptData->objVariableInfo.GetLength(); n++ )
	{
		if( func->scriptData->objVariableInfo[n].programPos == declaredAt
				&& func->scriptData->objVariableInfo[n].option == asOBJ_VARDECL
				&& func->scriptData->objVariableInfo[n].variableOffset == stackOffset)
		{
			// If the current block ends between the declaredAt and current
			// program position, then we know the variable is no longer visible
			int level = 0;
			for( ; n < (int)func->scriptData->objVariableInfo.GetLength(); n++ )
			{
				if( func->scriptData->objVariableInfo[n].programPos > pos )
					break;

				if( func->scriptData->objVariableInfo[n].option == asBLOCK_BEGIN ) level++;
				if( func->scriptData->objVariableInfo[n].option == asBLOCK_END && --level < 0 )
					return false;
			}

			break;
		}
	}

This works for me an appears to pass all the project tests, except for the ones expecting certain bytecode. There may be a more elegant solution, but I leave that in your more than capable hands.

Regards,

Jason

Thanks for letting me know about this, and especially thanks for taking the time to come up with a test case that reproduce the problem, as well as investigate a possible solution. This will really save time for me.

I'll do my own investigation given this input and have it fixed as soon as possible.

Regards,
Andreas

I've fixed this now in revision 2871.

I ended up doing a slightly different solution that avoids keeping more asOBJ_VARDECL in memory (and storing to disk on bytecode serialization), but your analysis was perfect.