Merge pull request #273 from scratchcpp/fix_self_calling_broadcast_crash

Fix #256: Fix self calling broadcast crash

Author: adazem009

src/engine/virtualmachine.cpp

@@ -164,18 +164,26 @@ void VirtualMachine::replaceReturnValue(const Value &v, unsigned int offset)
 /*! Continues running the script from last position (the first instruction is skipped). */
 void VirtualMachine::run()
 {
+    impl->running = true;
+
     unsigned int *ret = impl->run(impl->pos);
     assert(ret);
+
     if (impl->savePos)
         impl->pos = ret;
+
+    impl->running = false;
 }
 
 /*! Jumps back to the initial position. */
 void VirtualMachine::reset()
 {
     assert(impl->bytecode);
     impl->pos = impl->bytecode;
-    impl->regCount = 0;
+    impl->atEnd = false;
+
+    if (!impl->running) // Registers will be freed when the script stops running
+        impl->regCount = 0;
 }
 
 /*! Moves back to the last vm::OP_CHECKPOINT instruction in the bytecode. */

src/engine/virtualmachine_p.h

@@ -43,6 +43,7 @@ struct VirtualMachinePrivate
         Script *script = nullptr;
         unsigned int *pos = nullptr;
         unsigned int *checkpoint = nullptr;
+        bool running = false;
         bool atEnd = false;
         std::vector<Loop> loops;
         std::vector<unsigned int *> callTree;

test/engine/engine_test.cpp

@@ -614,3 +614,11 @@ TEST(EngineTest, NoCrashAfterStop)
     ASSERT_TRUE(p.load());
     p.run();
 }
+
+TEST(EngineTest, NoCrashOnBroadcastSelfCall)
+{
+    // Regtest for #256
+    Project p("regtest_projects/256_broadcast_self_call_crash.sb3");
+    ASSERT_TRUE(p.load());
+    p.run();
+}

test/regtest_projects/256_broadcast_self_call_crash.sb3

@@ -1327,6 +1327,21 @@ unsigned int testFunction5(VirtualMachine *vm)
     return 0;
 }
 
+unsigned int testFunction6(VirtualMachine *vm)
+{
+    vm->addReturnValue(0);
+    vm->stop(true, false, false);
+    return 1;
+}
+
+unsigned int testFunction7(VirtualMachine *vm)
+{
+    vm->addReturnValue(0);
+    vm->stop(true, false, false);
+    vm->reset();
+    return 1;
+}
+
 TEST(VirtualMachineTest, OP_EXEC)
 {
     static unsigned int bytecode[] = {
@@ -1518,18 +1533,41 @@ TEST(VirtualMachineTest, OP_WARP)
 
 TEST(VirtualMachineTest, Reset)
 {
-    static unsigned int bytecode[] = { OP_START, OP_NULL, OP_EXEC, 0, OP_HALT };
-    static BlockFunc functions[] = { &testFunction3 };
+    static unsigned int bytecode1[] = { OP_START, OP_NULL, OP_EXEC, 0, OP_HALT };
+    static unsigned int bytecode2[] = { OP_START, OP_NULL, OP_EXEC, 1, OP_HALT };
+    static BlockFunc functions[] = { &testFunction6, &testFunction7 };
 
     VirtualMachine vm;
-    vm.setBytecode(bytecode);
+    vm.setBytecode(bytecode1);
     vm.setFunctions(functions);
+
     vm.run();
     ASSERT_FALSE(vm.atEnd());
+    ASSERT_EQ(vm.registerCount(), 1);
     vm.reset();
+    ASSERT_EQ(vm.registerCount(), 0);
     ASSERT_FALSE(vm.atEnd());
     vm.run();
     ASSERT_FALSE(vm.atEnd());
     vm.run();
     ASSERT_TRUE(vm.atEnd());
+
+    vm.reset();
+    ASSERT_FALSE(vm.atEnd());
+
+    vm.setBytecode(bytecode2);
+
+    vm.run();
+    ASSERT_FALSE(vm.atEnd());
+    ASSERT_EQ(vm.registerCount(), 1);
+    vm.reset();
+    ASSERT_EQ(vm.registerCount(), 0);
+    ASSERT_FALSE(vm.atEnd());
+    vm.run();
+    ASSERT_FALSE(vm.atEnd());
+    vm.run();
+    ASSERT_TRUE(vm.atEnd());
+
+    vm.reset();
+    ASSERT_FALSE(vm.atEnd());
 }