Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reintroduce QVM support #1182

Open
wants to merge 28 commits into
base: master
Choose a base branch
from
Open

Conversation

Daggolin
Copy link
Contributor

@Daggolin Daggolin commented Nov 2, 2023

Prologue

The original jka release used native binaries, but the vanilla jamp and jampded engine binaries were still capable of loading QVM mods and both, the 2003 SDK and the 2013 release of the code could be used to compile QVMs for jampgame, cgame and ui. It is unknown whether anyone tried to make use of that before 2013, but there seems to be no publicly known mods that made use of QVMs, thus OpenJK removed support for QVMs early on. However, the interest in QVM mods has increased over time for better mod-compatibility accross different platforms and there is currently at least one server running jampded with a QVM based mod. For those reasons this pull request readds QVM support by porting the QVM code from jk2mv (which ported parts of it from ioq3) and adjusting the module APIs to support QVMs.

Issues when using QVMs on vanilla jamp/jampded

  • The string pointers written into the glconfig_t instances inside of cgame and ui would point to engine memory, making them invalid
  • The notetrack callback for the roff system would pass a pointer to engine memory into the module, making it invalid (this issue already existed in jk2, but vanilla jk2 modules didn't properly support roff, leading to a very low chance of anyone actually having encountered the bug on there)
  • A few of the NAV VM_Calls would pass vec3_t (pointer to float) into the game module, possibly affecting NPC pathfinding

Pull Request

QVM related changes

  • Ports vm_arm.cpp, vm_interpreted.cpp, vm_local.h and vm_x86.c from mvdevs/jk2mv (which in turn got some of its code from ioq3 and multiple contributors)
  • Ports various QVM related functions from mvdevs/jk2mv
  • Reintroduces the cvars vm_game, vm_cgame and vm_ui
  • Reintroduces the commands vminfo and vmprofile
  • Maps G2 pointers to integer handles for the module APIs when using QVMs, because the types within the QVM are 32 bit, making 64 bit pointers incompatible
  • Replaces sharedEntity_t in the engine with sharedEntityMapper_t. As the QVM types are 32 bit the sharedEntity_t struct doesn't match when running the engine as 64 bit build. To work around this the engine stores the correct addresses for the members of sharedEntity_t in sharedEntityMapper_t when the game module passes the shared entities into SV_LocateGameData.
  • Solves the vanilla jamp/jampded issues by allocating additional memory for the QVM on load. This additional memory is used to write data into and then pass a pointer to that data into the module, which is valid, because it's within the datamask for the module.
  • Replaces the exceptions in qcommon.cpp with longjmp, because throwing exceptions within a VM_Call was unreliable

Other changes

  • Fixes undefined behavior in SV_ClipMoveToEntities, which becmae obvious when replacing sharedEntity_t with sharedEntityMapper_t
  • The vm_legacy cvar is no longer limited to _DEBUG builds to make potenital API tests with users easier, because it doesn't require extra builds
  • Adds com_dedicatedForceErrorsToFatal cvar to be able to disable the ERR_DROP to ERR_FATAL rewrite for Com_Error calls on dedicated servers
  • Blocks additional file extensions for write attempts from VMs (dll, exe, bat, cmd, dylib, so, qvm, pk3).
  • Resolves filenames for the extension check to cover symlinks and potential bypasses of the checks
  • Blocks a few characters in filenames specified by VMs (<>:"|?*)

…vs/jk2mv@7dc8400.

Does not compile. Files unmodified to make diffs easier.
Move some vm related structs.
Merge some vm related structs.
Adjust a few places in vm_* files.
Port a few functions from jk2mv's vm.cpp file (same revision as the vm_* files).
Add vm_game, vm_cgame and vm_ui cvars to set the prefered vm mode:
	- 0: native
	- 1: interpret (qvm)
	- 2: compile (qvm)

Can load QVMs on 64 bit, but it's not stable, yet. Loading the ui.qvm shows the menu, but crashes when entering player customization. Loading jampgame.qvm crashes when loading a map. These crashes are most likely related to the QVM being 32 bit and addresses getting passed in and out that don't fit. Also shared structs may be offset.
This issue occured when the game module called the syscall G_NAV_GETNEARESTNODE. The module would receive a different value than the engine intended to return and that caused issues when the incorrect return value was used as input for other functions, causing issues and eventually crashes during later syscalls. This did not affect interpreted QVMs. Once pinpointed the source of the issue being DoSyscall a diff with ioq3 provided a fix. This fix comes from ioq3@1ce8ba0cdb8d8f2825177877a16d8525100280dd.
Add g2handles to map g2 instances (64 bit address) to integers that the QVM can handle and adjust legacy uiapi to make use of them for QVMs.
Convert glconfig_t in uiapi for QVMs, because the structs don't match.
…de a destination itself and expects to be given a pointer.

Make use of QVM extra memory in CL_GetGlconfig for uiapi.
…tibility.

Still missing conversion for structs like sharedEntity_t.
As the QVM is always 32 bit, even if the engine is 64 bit the structs don't match due to pointers inside of them. The sharedEntityMapper_t struct has the same members as the sharedEntity_t struct, but all of them are pointers to the types sharedEntity_t uses. When the game module calls the locate gamedata syscall the pointers of the mapper struct are set. The mapper should be usable with native libraries and QVM.
The vanilla and OpenJK game module use "-1" as magic value for the SV_Trace passEntityNum instead of ENTITYNUM_NONE (1023). This lead to SV_ClipMoveToEntities trying to access entity "-1" and to read its ownerNum. When switching from sharedEntity_t to sharedEntityMapper_t for the QVM support the code got adjusted to no longer read out of bounds values, potentially leaving passOwnerNum unset/undefined (as opposed to be read from undefined memory). This commit fixes this by assuming a default value and refactoring some code to be simpler.
As most (maybe all) modules that support the OpenJK API also support the Legacy API give users the option to use the legacy system without requiring a debug build.
… comes from within the QVM results in a crash.
At some point OpenJK forced all Com_Error calls for dedicated servers to be ERR_FATAL, leading to the process terminating. This can be helpful for some simple forms of automatic restart scripts, but the original behavior of dropping to the interactive console can be useful during mod development or when testing various settings. To avoid issues with restart scripts depending on the ERR_FATAL rewrite the cvar defaults to enabled, but can be disabled to restore the original behavior.
- Use GetFullPathNameA, GetFinalPathNameByHandleA and realpath to actually resolve paths when validating filenames
- Use ERR_DROP instead of ERR_FATAL when a VM tries to access a forbidden extension
- Check for more forbidden extensions
- Prevent the VM from accessing files with unwanted characters in their names (just block the access and print a warning, don't drop)
@Daggolin Daggolin requested a review from a team as a code owner November 2, 2023 22:27
@Daggolin Daggolin force-pushed the qvm_support branch 2 times, most recently from c41979d to d3f43ad Compare November 3, 2023 02:04
Existing comments in the function suggested it would already push and pop ebx (non-volatile), but really performed the actions on ecx (volatile).
…tCallDoSyscall, because they are non-volatile.
…4 bit addresses from QVMs when dereferencing.

Remove ConvertedEntity function, because the sharedEntityMapper read macros handle the required changes on demand now.
…es that SV_EntityMapperReadData was not aware of.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant