From aa65449f280f66372942ef24c0f740dda9aecfc4 Mon Sep 17 00:00:00 2001 From: Nahuel Rocchetti Date: Thu, 24 Aug 2023 18:58:01 -0300 Subject: [PATCH] Fix stack overflow, test bhav infinite loop --- .../Scripts/OpenTS2/SimAntics/VMStackFrame.cs | 81 +++++++++++------- .../Tests/OpenTS2/SimAntics/SimAnticsTest.cs | 23 ++++- TestAssets/SimAntics/bhav.package | Bin 459 -> 583 bytes 3 files changed, 72 insertions(+), 32 deletions(-) diff --git a/Assets/Scripts/OpenTS2/SimAntics/VMStackFrame.cs b/Assets/Scripts/OpenTS2/SimAntics/VMStackFrame.cs index 39b12ca..a56c340 100644 --- a/Assets/Scripts/OpenTS2/SimAntics/VMStackFrame.cs +++ b/Assets/Scripts/OpenTS2/SimAntics/VMStackFrame.cs @@ -23,6 +23,7 @@ namespace OpenTS2.SimAntics public VMContinueHandler CurrentContinueHandler = null; public short[] Locals; public short[] Arguments; + private static int MaxIterations = 500000; public VMStackFrame(BHAVAsset bhav, VMStack stack) { @@ -34,52 +35,70 @@ namespace OpenTS2.SimAntics public VMExitCode Tick() { + var currentIterations = 0; + var nodeExecuted = false; + VMExitCode result = VMExitCode.Continue; + if (CurrentContinueHandler != null) { - var returnCode = CurrentContinueHandler.Tick(); - if (returnCode == VMExitCode.Continue) - return returnCode; - else - { - return AdvanceNodeAndRunTick(returnCode); - } + result = CurrentContinueHandler.Tick(); + if (result == VMExitCode.Continue) + return result; + // This tells the following code to just transition to the next node, as we already ran this node. + nodeExecuted = true; } - return RunCurrentTick(); + + var currentNode = GetCurrentNode(); + if (!nodeExecuted) + result = ExecuteNode(currentNode); + if (result == VMExitCode.Continue) + return result; + var returnTarget = GetNodeReturnTarget(currentNode, result); + + while (returnTarget != BHAVAsset.Node.ErrorReturnValue && returnTarget != BHAVAsset.Node.TrueReturnValue && returnTarget != BHAVAsset.Node.FalseReturnValue) + { + currentIterations++; + if (currentIterations > MaxIterations && MaxIterations > 0) + { + throw new SimAnticsException($"Thread entered infinite loop! ( >{MaxIterations} primitives )", this); + } + SetCurrentNode(returnTarget); + currentNode = GetCurrentNode(); + result = ExecuteNode(currentNode); + if (result == VMExitCode.Continue) + return result; + returnTarget = GetNodeReturnTarget(currentNode, result); + } + + if (returnTarget == BHAVAsset.Node.ErrorReturnValue) + throw new SimAnticsException("Attempted to transition to error.", this); + + if (returnTarget == BHAVAsset.Node.TrueReturnValue) + return VMExitCode.True; + + return VMExitCode.False; } - VMExitCode AdvanceNodeAndRunTick(VMExitCode exitCode) + ushort GetNodeReturnTarget(BHAVAsset.Node node, VMExitCode exitCode) { - var currentNode = GetCurrentNode(); ushort returnTarget; if (exitCode == VMExitCode.True) - returnTarget = currentNode.TrueTarget; + returnTarget = node.TrueTarget; else - returnTarget = currentNode.FalseTarget; - switch (returnTarget) - { - case BHAVAsset.Node.FalseReturnValue: - return VMExitCode.False; - case BHAVAsset.Node.TrueReturnValue: - return VMExitCode.True; - case BHAVAsset.Node.ErrorReturnValue: - throw new SimAnticsException("Attempted to go to error.", this); - default: - SetCurrentNode(returnTarget); - return RunCurrentTick(); - } + returnTarget = node.FalseTarget; + return returnTarget; } - VMExitCode RunCurrentTick() + VMExitCode ExecuteNode(BHAVAsset.Node node) { - var currentNode = GetCurrentNode(); - if (currentNode != null) + if (node != null) { var context = new VMContext { StackFrame = this, - Node = currentNode + Node = node }; - var opcode = currentNode.OpCode; + var opcode = node.OpCode; var prim = VMPrimitiveRegistry.GetPrimitive(opcode); if (prim != null) { @@ -96,7 +115,7 @@ namespace OpenTS2.SimAntics else { CurrentContinueHandler = null; - return AdvanceNodeAndRunTick(primReturn.Code); + return primReturn.Code; } } else @@ -150,7 +169,7 @@ namespace OpenTS2.SimAntics } } - var argAmount = 0; + int argAmount; switch(format) { diff --git a/Assets/Tests/OpenTS2/SimAntics/SimAnticsTest.cs b/Assets/Tests/OpenTS2/SimAntics/SimAnticsTest.cs index d6cce25..2b9c9a7 100644 --- a/Assets/Tests/OpenTS2/SimAntics/SimAnticsTest.cs +++ b/Assets/Tests/OpenTS2/SimAntics/SimAnticsTest.cs @@ -64,7 +64,6 @@ public class SimAnticsTest vm.Scheduler.ScheduleInterrupt(entity.Stack); vm.Tick(); Assert.That(entity.Temps[0], Is.EqualTo(0)); - } [Test] @@ -73,4 +72,26 @@ public class SimAnticsTest var vmExpressionPrim = VMPrimitiveRegistry.GetPrimitive(0x2); Assert.That(vmExpressionPrim, Is.TypeOf(typeof(VMExpression))); } + + [Test] + public void TestBHAVThrowsOnInfiniteLoop() + { + // VM Entities need to be attached to an OBJD to be aware of private/semiglobal scope. + var testObjectDefinition = new ObjectDefinitionAsset(); + testObjectDefinition.TGI = new ResourceKey(1, _groupID, TypeIDs.OBJD); + + var bhav = VM.GetBHAV(0x1003, _groupID); + + var vm = new VM(); + var entity = new VMEntity(testObjectDefinition); + vm.AddEntity(entity); + + var stackFrame = new VMStackFrame(bhav, entity.Stack); + entity.Stack.Frames.Push(stackFrame); + + Assert.Throws(() => + { + vm.Tick(); + }); + } } diff --git a/TestAssets/SimAntics/bhav.package b/TestAssets/SimAntics/bhav.package index 8e7cf5fe0bf6236bda2526f6688b380c5f29d545..9df05254215613f1384ee62d44cade6746fa1a6c 100644 GIT binary patch delta 98 zcmX@je4J&13MVrI1H*qt1_qCfx_OKf6F7K1^U^Z&GD}hweDd=PCKhVTa5ey?85r1v q8vg%hU;?uK{0H$F7+5*jCr@UykO*`1aQY7f%mNVc*+BIvKqdep8yN-w delta 24 gcmX@ka+-O93MUf-1H)`a1_pzTx_OL~|1%l@088%%H2?qr