mbbsemu / MBBSEmu

The MajorBBS Emulation Project is an Open Source, Cross-Platform emulator for easily running The MajorBBS & Worldgroup Modules
https://www.mbbsemu.com
MIT License
128 stars 14 forks source link

[MBMGEMP] Galactic Empire - Crash while playing game #421

Closed up2l8 closed 3 years ago

up2l8 commented 3 years ago

Module Information

Describe the bug Game plays for awhile then crashes. Once it crashes like this the game won't even start (same message).

To Reproduce Steps to reproduce the behavior:

  1. Play Galactic Empire
  2. See error

Expected behavior Game does not crash

Screenshots

Unhandled exception. System.Exception: Unsupported FPU Register: None
   at MBBSEmu.CPU.FpuStatusRegister.GetStackPointer(Register register)
   at MBBSEmu.CPU.CpuCore.GetOperandValueDouble(OpKind opKind, EnumOperandType operandType)
   at MBBSEmu.CPU.CpuCore.Op_Fld()
   at MBBSEmu.CPU.CpuCore.Tick()
   at MBBSEmu.HostProcess.ExecutionUnits.ExecutionUnit.Execute(FarPtr entryPoint, UInt16 channelNumber, Boolean simulateCallFar, Boolean bypassState, Queue`1 initialStackValues, UInt16 initialStackPointer)
   at MBBSEmu.Module.MbbsModule.Execute(FarPtr entryPoint, UInt16 channelNumber, Boolean simulateCallFar, Boolean bypassSetState, Queue`1 initialStackValues, UInt16 initialStackPointer)
   at MBBSEmu.HostProcess.MbbsHost.WorkerThread()
   at System.Threading.ThreadHelper.ThreadStart_Context(Object state)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.ThreadHelper.ThreadStart()

Software Information:

enusbaum commented 3 years ago

I'll need to dive into the x87 Opcode documentation but I suspect based on the signature here that it's a load immediate value into implied ST0 -- which would explain the FPU Register being set to None here.

enusbaum commented 3 years ago

I think I found the issue here.

image

This opcode seems to be calling FLD ST(0).

Looking at the documentation:

image

In this instance, I believe the purpose of this opcode is to duplicate the top of the stack.

The fix here is to enhance GetOperandValueDouble to handle None when a FPU Register is the Source, implying that it is to duplicate the value at ST(0) by pushing the same value to the stack. Essentially making ST(0) and ST(1) the same.

Source: https://www.felixcloutier.com/x86/fld

enusbaum commented 3 years ago

@up2l8 This issue should be addressed with the latest merge.

You can grab the latest automated builds from here: https://www.mbbsemu.com/Downloads/master/030321

Thanks!!

up2l8 commented 3 years ago

I grabbed that version, during startup I noticed a new error: 2021-03-05 01:56:31.0345 Debug MBBSEmu.HostProcess.ExportedModules.Phapi.DosAllocRealSeg (MBMGEMP) Allocating 64 in Real-Mode memory at 2007:0000 2021-03-05 01:56:31.0345 Error MBBSEmu.Memory.MemoryCore.Free Attempted to deallocate memory from an unknown segment 2005:0000 2021-03-05 01:56:31.0345 Debug MBBSEmu.HostProcess.ExportedModules.Phapi.DosAllocRealSeg (MBMGEMP) Allocating 665 in Real-Mode memory at 2008:0000

The module loaded and started to play but quickly crashed with a new error:

2021-03-05 01:57:16.6913 Debug MBBSEmu.Session.Telnet.TelnetSession.OnIacVerbReceived >> Channel 0: IAC WILL BinaryTransmission Unhandled exception. System.Exception: Destination 0000:FCC8 would overlap with source 0000:FCE2 (26 bytes) at MBBSEmu.HostProcess.ExportedModules.Majorbbs.memcpy() at MBBSEmu.HostProcess.ExportedModules.Majorbbs.Invoke(UInt16 ordinal, Boolean offsetsOnly) at MBBSEmu.HostProcess.ExecutionUnits.ExecutionUnit.ExternalFunctionDelegate(UInt16 ordinal, UInt16 functionOrdinal) at MBBSEmu.CPU.CpuCore.Op_Call() at MBBSEmu.CPU.CpuCore.Tick() at MBBSEmu.HostProcess.ExecutionUnits.ExecutionUnit.Execute(FarPtr entryPoint, UInt16 channelNumber, Boolean simulateCallFar, Boolean bypassState, Queue1 initialStackValues, UInt16 initialStackPointer) at MBBSEmu.Module.MbbsModule.Execute(FarPtr entryPoint, UInt16 channelNumber, Boolean simulateCallFar, Boolean bypassSetState, Queue1 initialStackValues, UInt16 initialStackPointer) at MBBSEmu.HostProcess.MbbsHost.WorkerThread() at System.Threading.ThreadHelper.ThreadStart_Context(Object state) at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state) --- End of stack trace from previous location --- at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state) at System.Threading.ThreadHelper.ThreadStart()

Looks like maybe it got further in the stack, at least the error seems to be in the telnet stack instead of the emulation layer?

enusbaum commented 3 years ago

Yeah, looks like we moved passed the opcode issue

@paladine memcpy() all you broh ❤️

enusbaum commented 3 years ago

@paladine It also looks like we need to change the PHAPI real mode allocation to be tracked by free as well, or somehow handle real mode allocation of an entire segment?

enusbaum commented 3 years ago

@up2l8 Please give the latest builds a go:

https://www.mbbsemu.com/Downloads/master/030521

I've tested Galactic Empire locally (3.2e) and it seems to work properly (entering/exiting/shutting down/cleanup). Hopefully this clears it up for you 😃

up2l8 commented 3 years ago

Definitely got past the earlier issues. I ran "help set" in the game and got:

Unhandled exception. System.IO.InvalidDataException: (MBMGEMP) Unhandled Printf Control Character: f at MBBSEmu.HostProcess.ExportedModules.ExportedModuleBase.FormatPrintf(ReadOnlySpan1 stringToParse, UInt16 startingParameterOrdinal, Boolean isVsPrintf) at MBBSEmu.HostProcess.ExportedModules.Majorbbs.prfmsg() at MBBSEmu.HostProcess.ExportedModules.Majorbbs.Invoke(UInt16 ordinal, Boolean offsetsOnly) at MBBSEmu.HostProcess.ExecutionUnits.ExecutionUnit.ExternalFunctionDelegate(UInt16 ordinal, UInt16 functionOrdinal) at MBBSEmu.CPU.CpuCore.Op_Call() at MBBSEmu.CPU.CpuCore.Tick() at MBBSEmu.HostProcess.ExecutionUnits.ExecutionUnit.Execute(FarPtr entryPoint, UInt16 channelNumber, Boolean simulateCallFar, Boolean bypassState, Queue1 initialStackValues, UInt16 initialStackPointer) at MBBSEmu.Module.MbbsModule.Execute(FarPtr entryPoint, UInt16 channelNumber, Boolean simulateCallFar, Boolean bypassSetState, Queue`1 initialStackValues, UInt16 initialStackPointer) at MBBSEmu.HostProcess.MbbsHost.WorkerThread() at System.Threading.ThreadHelper.ThreadStart_Context(Object state) at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state) --- End of stack trace from previous location --- at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state) at System.Threading.ThreadHelper.ThreadStart()

enusbaum commented 3 years ago

Looks like that sequence has an unescaped %

\u001b[0m\u001b[2J***\r\n\u001b[36mThe SET command lets you set user configurable options.\r\n\r\n\u001b[1;32m  set  <option>/? [on/off]\r\n\u001b[36m\r\n  options are;\r\n\r\n  scanhome   -  home the screen before displaying scan\r\n  scanfull   -  full screen scans\r\n  scannames  -  show user names in Range Scans (scanfull must be ON too)\r\n  filter     -  filters out 80% of the battle messages  \r\n\r\n  examples:\r\n  set scanhome on\r\n  set scannames off\r\n  set ?\r\n\u001b[0m

We'll need to enhance the printf function to better handle unescaped sequences. 🤔

paladine commented 3 years ago

You mean the 80% that should in theory be in 80%%?

enusbaum commented 3 years ago

You mean the 80% that should in theory be in 80%%?

Yeah, or in general just skip/echo as-is sequences that aren't valid printf sequences.

up2l8 commented 3 years ago

Is this a bug in the actual GE code?

On Fri, Mar 5, 2021 at 9:01 AM Eric P. Nusbaum notifications@github.com wrote:

You mean the 80% that should in theory be in 80%%?

Yeah, or in general just skip/echo as-is sequences that aren't valid printf sequences.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mbbsemu/MBBSEmu/issues/421#issuecomment-791512145, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABVS2BZMXHENUMSM3WQPIWLTCD54LANCNFSM4YPMV6PQ .

enusbaum commented 3 years ago

Is this a bug in the actual GE code?

No -- it's how MBBEmu is handling an unescaped % character. Technically when a string sequence is processed through printf and it wants to output a single %, it needs to be escaped as %%. It appears the printf implementation in early version of Borland C++ are pretty forgiving and I think if we encounter an invalid sequence, we should just print the invalid sequence, log it, and move on.

up2l8 commented 3 years ago

Makes sense. The joys of dealing with 20+ year-old code huh? That said, updating the message files in https://github.com/bsimser/ge itself doesn't look terrible, but maybe there's more to it.

On Fri, Mar 5, 2021 at 9:20 AM Eric P. Nusbaum notifications@github.com wrote:

Is this a bug in the actual GE code?

No -- it's how MBBEmu is handling an unescaped % character. Technically when a string sequence is processed through printf and it wants to output a single %, it needs to be escaped as %%. It appears the printf implementation in early version of Borland C++ are pretty forgiving and I think if we encounter an invalid sequence, we should just print the invalid sequence, log it, and move on.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mbbsemu/MBBSEmu/issues/421#issuecomment-791524703, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABVS2BYZIUB4B6GFNOVXFFLTCEAFLANCNFSM4YPMV6PQ .

enusbaum commented 3 years ago

@up2l8 The issue auto-closed with the fix, but if you run into another issue we'll open a new one to track it there.

Give the latest automated build a go! 👍

https://www.mbbsemu.com/Downloads/master/030521

up2l8 commented 3 years ago

Looks great so far! Thanks you guys are rock stars!