Fix skyrim IP not being assigned per op

modified hook to make the IP correct

src/DarkId.Papyrus.DebugServer/BreakpointManager.cpp

@@ -67,7 +67,7 @@ namespace DarkId::Papyrus::DebugServer
 	void BreakpointManager::ClearBreakpoints() {
 		m_breakpoints.clear();
 	}
-	bool BreakpointManager::GetExecutionIsAtValidBreakpoint(RE::BSScript::Internal::CodeTasklet* tasklet)
+	bool BreakpointManager::GetExecutionIsAtValidBreakpoint(RE::BSScript::Internal::CodeTasklet* tasklet, uint32_t actualIP)
 	{
 		auto & func = tasklet->topFrame->owningFunction;
 
@@ -83,9 +83,11 @@ namespace DarkId::Papyrus::DebugServer
 			if (!breakpointLines.empty())
 			{
 				uint32_t currentLine;
-				bool success = func->TranslateIPToLineNumber(tasklet->topFrame->STACK_FRAME_IP, currentLine);
-				auto found = breakpointLines.find(currentLine);
-				return success && found != breakpointLines.end();
+				bool success = func->TranslateIPToLineNumber(actualIP, currentLine);
+				if (success && breakpointLines.find(currentLine) != breakpointLines.end()) {
+					return true;
+				}
+				return false;
 			}
 		}
 

src/DarkId.Papyrus.DebugServer/BreakpointManager.h

@@ -23,6 +23,6 @@ namespace DarkId::Papyrus::DebugServer
 
 		dap::ResponseOrError<dap::SetBreakpointsResponse> SetBreakpoints(const dap::Source& src, const std::vector<dap::SourceBreakpoint>& srcBreakpoints);
 		void ClearBreakpoints();
-		bool GetExecutionIsAtValidBreakpoint(RE::BSScript::Internal::CodeTasklet* tasklet);
+		bool GetExecutionIsAtValidBreakpoint(RE::BSScript::Internal::CodeTasklet* tasklet, uint32_t actualIP);
 	};
 }

src/DarkId.Papyrus.DebugServer/DebugExecutionManager.cpp

@@ -5,7 +5,7 @@ namespace DarkId::Papyrus::DebugServer
 {
 	using namespace RE::BSScript::Internal;
 
-	void DebugExecutionManager::HandleInstruction(CodeTasklet* tasklet, CodeTasklet::OpCode opCode)
+	void DebugExecutionManager::HandleInstruction(CodeTasklet* tasklet, uint32_t actualIP)
 	{
 		std::lock_guard<std::mutex> lock(m_instructionMutex);
 
@@ -24,7 +24,7 @@ namespace DarkId::Papyrus::DebugServer
 		{
 			pauseReason = "paused";
 		}
-		else if (m_state != DebuggerState::kPaused && m_breakpointManager->GetExecutionIsAtValidBreakpoint(tasklet))
+		else if (m_state != DebuggerState::kPaused && m_breakpointManager->GetExecutionIsAtValidBreakpoint(tasklet, actualIP))
 		{
 			pauseReason = "breakpoint";
 		}

src/DarkId.Papyrus.DebugServer/DebugExecutionManager.h

@@ -48,7 +48,7 @@ namespace DarkId::Papyrus::DebugServer
 		}
 
 		void Close();
-		void HandleInstruction(CodeTasklet* tasklet, CodeTasklet::OpCode opCode);
+		void HandleInstruction(CodeTasklet* tasklet, uint32_t actualIP);
 		void Open(std::shared_ptr<dap::Session> ses);
 		bool Continue();
 		bool Pause();

src/DarkId.Papyrus.DebugServer/PapyrusDebugger.cpp

@@ -259,9 +259,9 @@ namespace DarkId::Papyrus::DebugServer
 		});
 	}
 
-	void PapyrusDebugger::InstructionExecution(CodeTasklet* tasklet, CodeTasklet::OpCode opcode) const
+	void PapyrusDebugger::InstructionExecution(CodeTasklet* tasklet, uint32_t actualIP) const
 	{
-		m_executionManager->HandleInstruction(tasklet, opcode);
+		m_executionManager->HandleInstruction(tasklet, actualIP);
 	}
 
 	void PapyrusDebugger::CheckSourceLoaded(const std::string &scriptName) const{

src/DarkId.Papyrus.DebugServer/PapyrusDebugger.h

@@ -100,7 +100,7 @@ namespace DarkId::Papyrus::DebugServer
 		void EventLogged(const RE::BSScript::LogEvent* logEvent) const;
 		void StackCreated(RE::BSTSmartPointer<RE::BSScript::Stack>& stack);
 		void StackCleanedUp(uint32_t stackId);
-		void InstructionExecution(CodeTasklet* tasklet, CodeTasklet::OpCode opCode) const;
+		void InstructionExecution(CodeTasklet* tasklet, uint32_t actualIP) const;
 		void CheckSourceLoaded(const std::string &scriptName) const;
 };
 }

src/DarkId.Papyrus.DebugServer/RuntimeEvents.cpp

@@ -33,7 +33,7 @@ namespace DarkId::Papyrus::DebugServer
 			return g_##NAME##Event.remove(handle); \
 		} \
 
-		EVENT_WRAPPER_IMPL(InstructionExecution, void(RE::BSScript::Internal::CodeTasklet*, RE::BSScript::Internal::CodeTasklet::OpCode))
+		EVENT_WRAPPER_IMPL(InstructionExecution, void(RE::BSScript::Internal::CodeTasklet*, uint32_t actualIP))
 		EVENT_WRAPPER_IMPL(CreateStack, void(RE::BSTSmartPointer<RE::BSScript::Stack>&))
 		EVENT_WRAPPER_IMPL(CleanupStack, void(uint32_t))
 		// EVENT_WRAPPER_IMPL(InitScript, void(RE::TESInitScriptEvent*))
@@ -69,14 +69,47 @@ namespace DarkId::Papyrus::DebugServer
 			#endif
 
 		};
+		struct CallPatch : Xbyak::CodeGenerator
+		{
+		protected:
+			void saveVolatiles() {
+				push(rax);	// save volatile registers
+				push(rcx);
+				push(rdx);
+				push(r8);
+				push(r9);
+				push(r10);
+				push(r11);
+				push(r11);
+			}
+			void loadVolatiles() {
+				pop(r11); // load the saved volatile registers
+				pop(r11);
+				pop(r10);
+				pop(r9);
+				pop(r8);
+				pop(rdx);
+				pop(rcx);
+				pop(rax);
+			}
+		};
 
 #if SKYRIM
+		struct UnknownInstructionData {
+			uint32_t unk00;
+			uint32_t pad04;
+			uint32_t unk08;
+			uint32_t pad0C;
+		};
+		static_assert(sizeof(UnknownInstructionData) == 0x10);
 
-		void InstructionExecute_Hook(RE::BSScript::Internal::CodeTasklet* a_tasklet, RE::BSScript::Internal::CodeTasklet::OpCode a_opCode)
+		void InstructionExecute_Hook(RE::BSScript::Internal::CodeTasklet* a_tasklet, uint32_t currentIP)
 		{
 			if (a_tasklet->topFrame)
 			{
-				g_InstructionExecutionEvent(a_tasklet, a_opCode);
+				// assign the correct IP
+				a_tasklet->topFrame->STACK_FRAME_IP = currentIP;
+				g_InstructionExecutionEvent(a_tasklet, currentIP);
 			}
 		}
 
@@ -102,11 +135,92 @@ namespace DarkId::Papyrus::DebugServer
 		}
 
 		namespace Internal
-		{
-			void CommitHooks()
-			{
-				std::size_t BASE_LOAD_ADDR = 0;
+     	{
+			struct InstructionExecuteHook {
+				
+				struct Patch : CallPatch
 				{
+
+					Patch(std::uintptr_t a_callAddr, std::uintptr_t a_retAddr, std::uintptr_t a_ifFreezeLabelAddr)
+					{
+						Xbyak::Label callLbl;
+						Xbyak::Label retLbl;
+						Xbyak::Label isFrozen;
+						Xbyak::Label ifFreezeLabel;
+						Xbyak::Label ifgeInstructionDataBitCountLabel;
+						/**
+						* The main issue we are trying to solve is that the InstructionPointer on the top stack frame 
+						* doesn't get updated until the tasklet actually finishes executing
+						* 
+						* It doesn't do this until it either:
+						* - reaches the end of the instruction bitstream
+						* - the stack is about to freeze
+						* - the max ops per tasklet have been executed (100)
+						* 
+						* The actual IP is set in edx before checking for the first two conditions, and this will be used to set the topFrame's IP if the tasklet exits
+						* So we need to install our branch right after that to be able to load it into our hook
+						* We install into the `jz` instruction since it's a 6-byte long jump.
+						* 
+						* Here's our hook target, near the start of the main loop:
+						* ```
+                        *  lea     edx, [rax+rcx*8]								      # at this point, edx holds the actual current IP
+						*  cmp     dword ptr [rax+6Ch], 1                             # check if this->stack->freeze state is 1 (frozen)
+						*  jz      if_frozen_label                                    # jumps if above comparison is true               <-- branch installed here
+						*  cmp     edx, [<rsi/rdi>+40h]                               # compare the current IP to this->InstructionDataBitCount ("(CodeTasklet) this" is rsi in AE, rdi in SE)
+						*  jb      short if_less_than_InstructionDataBitCount_label   # jump if the current IP is less than this->InstructionDataBitCount
+						* ```
+						*
+						* Since we hook right in the middle of the checking of the first two conditions, we want to check those before calling our hook,
+						* we don't want to block the codetasklet thread if it's about to exit.
+						* the current ops count and max ops comparison happens at the end of the loop, so we don't have to check that
+						*/
+						cmp(dword[rax + 0x6C], 1); // check to see if stack->freeze state is 1 (frozen)
+						jz(isFrozen); // we overwrite this instruction, so we have to jump to our saved address
+						if (REL::Module::IsAE()) {
+							cmp(edx, dword[rsi + 0x40]); // (CodeTasklet)this is rsi in AE
+						} else {
+							cmp(edx, dword[rdi + 0x40]); // (CodeTasklet)this is rdi in SE
+						}
+						// originally a `jb` that skips the main switch case; we just want this to return if the above comparison is true
+						// we didn't overwrite that jump or the above comparison, so we can just return to the return address
+						jge(ifgeInstructionDataBitCountLabel);
+
+						saveVolatiles();	// save volatile registers
+
+						if (REL::Module::IsAE()) {
+							mov(rcx, rsi);	// first param: BSScript::Internal::CodeTasklet*
+							// second param: edx is already what we want, the current instruction pointer
+						}
+						else {
+							mov(rcx, rdi);	// first param: rcx = rdi == BSScript::Internal::CodeTasklet*
+											// second param: edx is already what we want, the current instruction pointer
+						}
+						sub(rsp, 0x20); // pad the stack with the amount of parameters that we're going to be using (up to 4 64-bit values)
+						call(ptr[rip + callLbl]);	// make call
+						add(rsp, 0x20); // put it back
+
+						loadVolatiles();
+
+						L(ifgeInstructionDataBitCountLabel);
+						jmp(ptr[rip + retLbl]);	// resume execution
+
+						L(callLbl);
+						dq(a_callAddr);
+
+						L(retLbl);
+						dq(a_retAddr);
+
+						L(isFrozen);
+						jmp(ptr[rip + ifFreezeLabel]);
+
+						L(ifFreezeLabel);
+						dq(a_ifFreezeLabelAddr);
+
+					}
+
+				};
+				static inline void Install()
+				{					
 					// InstructionExecute
 					// 1.5.97:  0x141278110: BSScript__Internal__CodeTasklet::VMProcess_141278110
 					// 1.6.640: 0x14139C860: BSScript__Internal__CodeTasklet::sub_14139C860
@@ -115,99 +229,60 @@ namespace DarkId::Papyrus::DebugServer
 					// 1_5_97   CAVE_END   = 0x176
 					// 1_6_640  CAVE_END   = 0x153
 					// Cave start and cave end indicate the beginning and end of the instructions 
-					// that set the operand value that gets switched on in InstructionExecute
+					// We install near the beginning of the loop
+					// The installation target is the `jz` instruction that jumps if the freeze state is 1 
 					// CAVE_SIZE = 6 
 					auto vmprocess_reloc = RELOCATION_ID(98520, 105176);
 
 					//TODO: Find VR offsets, using SE offsets as placeholders
-					auto cave_start_var_offset = REL::VariantOffset(0x170, 0x14C, 0x170);
-					auto cave_end_var_offset = REL::VariantOffset(0x176, 0x153, 0x176);
+					auto cave_start_var_offset = REL::VariantOffset(0xD6, 0xCA, 0xD6);
+					auto cave_end_var_offset = REL::Offset(cave_start_var_offset.offset()+6);
 
 					REL::Relocation<std::uintptr_t> cave_start_reloc{ vmprocess_reloc, cave_start_var_offset };
 					REL::Relocation<std::uintptr_t> cave_end_reloc{ vmprocess_reloc, cave_end_var_offset };
 					std::size_t CAVE_START = cave_start_var_offset.offset();
 					std::size_t CAVE_END = cave_end_var_offset.offset();
 					std::size_t CAVE_SIZE = CAVE_END - CAVE_START;
-					struct Patch : Xbyak::CodeGenerator
-					{
-						Patch(std::uintptr_t a_callAddr, std::uintptr_t a_retAddr)
-						{
-							Xbyak::Label callLbl;
-							Xbyak::Label retLbl;
 
-							push(rax);	// save volatile registers
-							push(rcx);
-							push(rdx);
-							push(r8);
-							push(r9);
-							push(r10);
-							push(r11);
-							push(r11);
-							if (REL::Module::IsAE()) {
-								mov(rcx, rsi);	// rsi == BSScript::Internal::CodeTasklet*
-							} else {
-								mov(rcx, rdi);	// rdi == BSScript::Internal::CodeTasklet*
-							}
-							mov(r8d, edx);	// edx == BSScript::Internal::CodeTasklet::OpCode
-							xor_(rdx, rdx);
-							mov(edx, r8d);
-
-							sub(rsp, 0x20);	// pad the stack
-							call(ptr[rip + callLbl]);	// make call
-							add(rsp, 0x20);
-
-							pop(r11);
-							pop(r11);
-							pop(r10);
-							pop(r9);
-							pop(r8);
-							pop(rdx);
-							pop(rcx);
-							pop(rax);
-							if (REL::Module::IsAE()) {
-								// total bytes of instructions below = 7
-								mov(r10, r9);	// execute overridden ops
-								and_(r10d, 0x3F); 
-							} else {
-								// total bytes of instructions below = 6
-								mov(rax, r8);	// execute overridden ops
-								and_(eax, 0x3F);
-							}
-							jmp(ptr[rip + retLbl]);	// resume execution
-
-							L(callLbl);
-							dq(a_callAddr);
-
-							L(retLbl);
-							dq(a_retAddr);
-						}
-					};
 					assert(CAVE_SIZE >= 6);
-					auto patch = Patch(XSE::stl::unrestricted_cast<std::uintptr_t>(InstructionExecute_Hook), cave_end_reloc.address());
+					// we need to read what the offset is in the `jz` instruction;
+					// jz instruction is opcode (`0F 84`) followed by a four-byte offset
+					auto if_freeze_label_offset_loc_offset = REL::Offset(cave_start_var_offset.offset() + 2);
+					REL::Relocation<std::uintptr_t> if_freeze_label_offset_loc_addr{ vmprocess_reloc, if_freeze_label_offset_loc_offset };
+					uint32_t * ptr_to_offset = (uint32_t *)if_freeze_label_offset_loc_addr.address();
+					uint32_t offset_val = *ptr_to_offset;
+					auto if_freeze_label_offset = REL::Offset(offset_val + cave_end_var_offset.offset()); // the offset relative to the address AFTER the jz instruction
+					REL::Relocation<std::uintptr_t> if_freeze_label_address{ vmprocess_reloc, if_freeze_label_offset };
+
+					auto patch = Patch(XSE::stl::unrestricted_cast<std::uintptr_t>(InstructionExecute_Hook), cave_end_reloc.address(), if_freeze_label_address.address());
 					auto& trampoline = SKSE::GetTrampoline();
 					SKSE::AllocTrampoline(patch.getSize() + 14);
 					auto result = trampoline.allocate(patch);
 					trampoline.write_branch<6>(cave_start_reloc.address(), (std::uintptr_t)result);
-					// A write_branch<6> writes a 6 byte branch to the address we want; if there's more than 6 bytes we have to skip over, we have to nop it out
-					// Maybe?? I don't think this affects anything since we jump to the CAVE_END regardless, so may not be needed.
-					if (CAVE_SIZE > 6){
-						REL::safe_fill(cave_start_reloc.address() + 6, REL::NOP, CAVE_SIZE-6);
-					}
-					BASE_LOAD_ADDR = vmprocess_reloc.address() - vmprocess_reloc.offset();
+					auto BASE_LOAD_ADDR = vmprocess_reloc.address() - vmprocess_reloc.offset();
 					logger::info("Base for executable is: 0x{:X}", BASE_LOAD_ADDR);
 					logger::info("InstructionExecute address: 0x{:X}", vmprocess_reloc.address());
 					logger::info("InstructionExecute relocation offset: 0x{:X}", vmprocess_reloc.offset());
 
 					logger::info("InstructionExecuteHook hooked at address 0x{:X}", cave_start_reloc.address());
 					logger::info("InstructionExecuteHook hooked at offset 0x{:X}", cave_start_reloc.offset());
+					logger::info("InstructionExecuteHook if_freeze_label_offset at address 0x{:X}", if_freeze_label_offset.address());
+					logger::info("InstructionExecuteHook if_freeze_label_offset at offset 0x{:X}", if_freeze_label_offset.offset());
 					logger::info("InstructionExecuteHook:CAVE_START is 0x{:X}", CAVE_START);
 					logger::info("InstructionExecuteHook:CAVE_END is 0x{:X}", CAVE_END);
 					logger::info("InstructionExecuteHook:CAVE_SIZE is 0x{:X}", CAVE_SIZE);
 
 					std::size_t RESULT_ADDR = (std::uintptr_t)result;
 					logger::info("InstructionExecuteHook patch allocation address: 0x{:X}", RESULT_ADDR);
 					logger::info("InstructionExecuteHook patch allocation offset: 0x{:X}", RESULT_ADDR - BASE_LOAD_ADDR);
+
 				}
+			};
+
+
+			void CommitHooks()
+			{
+				InstructionExecuteHook::Install();
 
 				{
 					// CreateStack
@@ -248,7 +323,6 @@ namespace DarkId::Papyrus::DebugServer
 
 					std::size_t RESULT_ADDR = (std::uintptr_t)result;
 					logger::info("CreateStackHook patch allocation address: 0x{:X}", RESULT_ADDR);
-					logger::info("CreateStackHook patch allocation offset: 0x{:X}", RESULT_ADDR - BASE_LOAD_ADDR);
 				}
 
 
@@ -301,7 +375,6 @@ namespace DarkId::Papyrus::DebugServer
 
 					std::size_t RESULT_ADDR = (std::uintptr_t)result;
 					logger::info("CleanupStackHook patch allocation address: 0x{:X}", RESULT_ADDR);
-					logger::info("CleanupStackHook patch allocation offset: 0x{:X}", RESULT_ADDR - BASE_LOAD_ADDR);
 
 
 				}
@@ -321,7 +394,8 @@ namespace DarkId::Papyrus::DebugServer
 		{
 			if (tasklet->topFrame)
 			{
-				g_InstructionExecutionEvent(tasklet, opCode);
+				// We don't need to set the instruction pointer because Fallout 4 assigns the IP every time an opcode is executed
+				g_InstructionExecutionEvent(tasklet, tasklet->topFrame->STACK_FRAME_IP);
 			}
 		}
 		// TODO: There's a second CreateStack() @ 1427422C0, do we need to hook that?

src/DarkId.Papyrus.DebugServer/RuntimeEvents.h

@@ -14,7 +14,7 @@ namespace DarkId::Papyrus::DebugServer
 {
 	namespace RuntimeEvents
 	{
-		EVENT_DECLARATION(InstructionExecution, void(RE::BSScript::Internal::CodeTasklet*, RE::BSScript::Internal::CodeTasklet::OpCode))
+		EVENT_DECLARATION(InstructionExecution, void(RE::BSScript::Internal::CodeTasklet*, uint32_t actualIP))
 		EVENT_DECLARATION(CreateStack, void(RE::BSTSmartPointer<RE::BSScript::Stack>&))
 		EVENT_DECLARATION(CleanupStack, void(uint32_t))
 		// EVENT_DECLARATION(InitScript, void(RE::TESInitScriptEvent*))