Auxtools debugger

[willox]

This adds support for a debug server written in auxtools (currently located at https://github.com/willox/auxtools.)

The dependency is similar to extools, where SpacemanDMM's auxtools_types.rs has to be up-to-date with the server_types.rs file located in whichever version of debug server is used. In the future this could change to be some shared dependency, or maybe the debug server could just be moved into SpacemanDMM.

There's a bunch of repeated code in mod.rs where there's match statements where one branch is for the extools client and one branch is for auxtools client. It's a bit iffy, but they all have minor differences and wouldn't be super easy to merge.

I accidentally ran a cargo fmt on the files I was working with at some point, so there's a few formatting changes about. I don't think it's too much to read over.

I haven't edited any documentation yet, so here's how it works:
In your DM project:

// Currently needed for auxtools' error reporting. TG code already has this defined.
/proc/stack_trace(msg)
	CRASH(msg)

/proc/enable_debugging(mode, port)
	CRASH("auxtools not loaded")

/world/New()
	var/debug_server = world.GetConfig("env", "AUXTOOLS_DEBUG_DLL")
	if (debug_server)
		call(debug_server, "auxtools_init")()
		enable_debugging()
	. = ..()

/world/Del()
	var/debug_server = world.GetConfig("env", "AUXTOOLS_DEBUG_DLL")
	if (debug_server)
		call(debug_server, "auxtools_shutdown")()
	. = ..()

In your project's SpacemanDMM.toml

[debugger]
engine = "auxtools"

The extension doesn't have a way to override the DLL being used (and I don't think it should), so if you're testing stuff I suggest you set the env vars to something like below and use the attach mode:

AUXTOOLS_DEBUG_DLL=path_to_your_build
AUXTOOLS_DEBUG_MODE=BLOCK

LMK what needs changing

[willox]

this was changed to avoid a rust bug where symbols are not generated (at least on Windows with MSVC) if a dash is in the name

[SpaceManiac]

is there an upstream ticket?

[willox]

for this specific issue idk, it was a problem before but got 'fixed'.
rust-lang/cargo#8123
perhaps it was only fixed when there was no [[bin]].name override.

[SpaceManiac]

I will probably revert this in the name of not having to modify my deploy scripts, but I suggest filing an up-to-date report upstream

[willox]

That's fine. It's more that I forgot to remove it first, but it was worth pointing out anyway.

[SpaceManiac]

The extension doesn't have a way to override the DLL being used (and I don't think it should)

The extension currently has a hidden setting (not exposed in the UI) to override the extools DLL for testing purposes, would the same work here?

[willox]

Yeah, I haven't added the equivalent for auxtools because I don't see it actually helping me. There was maybe a reason for people to use it before when developing their own extensions onto extools but with auxtools we can have multiple separate libraries loaded at once.

We can add it, but changing that option just seems more complicated than setting the env-var.

[SpaceManiac]

Is there any chance that the /world/Del hook can be stuffed into the .dll itself rather than seeping into DM? What about stack_trace?

we can have multiple separate libraries loaded at once.

Can you explain how this works? Would I be shipping a "just the debugger" DLL and require users to install the base separately, or would I be shipping a base+debugger DLL where the base might or might not be the one "in charge"? What are the failure modes?

I accidentally ran a cargo fmt on the files I was working with at some point, so there's a few formatting changes about.

I'll live, but please avoid this in the future. It does add noise to the diff.

[willox]

Is there any chance that the /world/Del hook can be stuffed into the .dll itself rather than seeping into DM? What about stack_trace?

Doing the cleanup from a hooked proc is a little difficult because the code isn't really in a state where the hooks can be removed. Using the FFI interface was a simple way to get around that. It doesn't just disconnect the debug server, but also gets auxtools into a state where starting another DMB won't fuck up.

Auxtools modules do not unload/reload when restarting the world.

Can you explain how this works? Would I be shipping a "just the debugger" DLL and require users to install the base separately, or would I be shipping a base+debugger DLL where the base might or might not be the one "in charge"? What are the failure modes?

The base is statically linked in. You can just ship the dll and everything will work as long as the SpacemanDMM.toml and DM have the necessary stuff. There's a small performance loss on proc call hooking for each library loaded, but except for when using the debug server I don't expect any code-bases to load more than one at a time.

I didn't mention above, but the call(debug_server, "auxtools_init")() will gracefully return "FAILED (<message here>)" if it fails to load and "SUCCESS" otherwise.

[willox]

Fixed up the indentation you mentioned, there could easily be more.

[Cyberboss]

Based PR

[ZeWaka]

Based

[willox]

Is there any chance that the /world/Del hook can be stuffed into the .dll itself rather than seeping into DM? What about stack_trace?

I put a little thought into stack_trace, but we don't really have a way to trigger a runtime in BYOND without calling some sort of proc as auxtools has no way to manually create a new stack frame. If we just cause a runtime to happen at the end of our proc hook, it'll be treated as the runtime occurring in the caller and will cease execution of the proc that is calling us.

[SpaceManiac]

Still working through my review of auxtools itself.

We can add [a DLL setting], but changing that option just seems more complicated than setting the env-var.

I'll take care of it myself if I find myself needing it.

except for when using the debug server I don't expect any code-bases to load more than one at a time.

Well, I do have to worry about that situation. Is there a risk of problems if whichever of auxtools-tg.dll or debug_server.dll loads first has an older auxtools bundled? Maybe you can point me to the implementation in auxtools.

debug server written in auxtools (currently located at https://github.com/willox/auxtools.)

Do you intend to maintain it going forward, or would it make sense to move it in-tree with SpacemanDMM eventually?

[willox]

Well, I do have to worry about that situation. Is there a risk of problems if whichever of auxtools-tg.dll or debug_server.dll loads first has an older auxtools bundled? Maybe you can point me to the implementation in auxtools.

There's no part of auxtools where two modules communicate with each other. We've just been writing it in a way where they won't interfere with each other to the point where it breaks. When a BYOND C-function is being hooked, we make sure to scan for a reference to it instead of for the function itself. That's basically it.

The side-effects should be pretty simple because of that. Stuff like two modules hooking a single proc will result in only the last module to load ever seeing calls to that proc.

I've enforced a pretty simple rule in auxtools where we are only hooking BYOND functions that we absolutely have to. It's one of the older pieces of code, but if you check out hooks.rs's init function you'll see we only hook Runtime and CallProcById.

The biggest risk of backwards incompatibility is if we have a signature in an old version of auxtools that is of a function's contents and a new version of auxtools hooks that function. It's something I intend to just avoid, though. Less hooks = good.

Do you intend to maintain it going forward, or would it make sense to move it in-tree with SpacemanDMM eventually?

I'd expect to keep it working regardless of where it lives. It'd be easier for it to be in this repository, but at this point I'm more interested in getting auxtools on some servers than spending time moving the debug server.

Edit: There was one 'incompatibility' that hasn't been sorted. There's a #[runtime_handler] proc macro that'll trigger on runtimes that are meant to be caught by auxtools modules loaded before yours. Everything works out atm if the debug server loads first.

[SpaceManiac]

When a BYOND C-function is being hooked, we make sure to scan for a reference to it instead of for the function itself.

Thanks for the explanation. Sounds like it will keep problems to a minimum and now I can at least start to investigate if something does go wrong.

at this point I'm more interested in getting auxtools on some servers than spending time moving the debug server.

Sure, I was meaning more like after things have stabilized enough that /tg/station has migrated.

Everything works out atm if the debug server loads first.

Will we be okay if the debug server loads second, but whatever loads first doesn't have a #[runtime_handler] fn?

[SpaceManiac]

Just a TODO or is there something making this difficult?

[willox]

There's just no commands to add yet. You could add a diassemble command fairly easily, but I have only designed our disassembler to produce the correct length for each opcode/operands. The output is mostly correct, but that was a secondary objective.

The disassembler produces more correct output than extools, but it's not formatted. There's still some issues with GetVar (I've only noticed them in (init) methods so far.)

Example output would be:

Dism for Proc(/world/proc/enable_debugger)
	0-1: DbgFile(spaceman_dmm.dm)
	2-3: DbgLine(32)
	4-6: PushVal("env")
	7-9: PushVal("EXTOOLS_DLL")
	10-15: Call(RuntimeProcField(World, [], GetConfig), 2)
	16-18: SetVar(Local(0))
	19-20: DbgLine(33)
	21-23: GetVar(Local(0))
	24-24: Test
	25-26: Jz(38)
	27-28: DbgLine(34)
	29-31: GetVar(Local(0))
	32-34: PushVal("debug_initialize")
	35-36: CallName(ParamCount(0))
	37-37: Pop
	38-38: End

[SpaceManiac]

The formatting doesn't really matter, but the #disassemble command helped me a lot when working on extools.

It's not a priority for this PR, but I wanted to be sure there wasn't anything fundamental blocking it.

The DAP Disassemble request doesn't matter because VSC doesn't implement it.

[willox]

I'll add it in when I get around to figuring out what's wrong with (init) method SetVar instructions (it wasn't GetVar, but they have similar operands so the bug probably exists in both.)

For reference, I'm seeing default values of var/list/x = list() being decoded to

NewList(0)
SetVar(Field(Src, [shotgun dart, x]))

I haven't looked into it at all.

[willox]

Will we be okay if the debug server loads second, but whatever loads first doesn't have a #[runtime_handler] fn?

Nope. It won't crash or anything, but you will see extra runtimes coming from hooked proc calls when there's a runtime caught by our try/catch system.

The reasoning is that the runtime hook is defined as

extern "C" void runtime_hook(char* pError) {
	const char* pErrorCorrected = (pError != nullptr) ? pError : "<null>";
	if (runtime_contexts.top()) {
#ifdef USE_SJLJ
		longjmp(*current_jmp, 1);
#else
		throw AuxtoolsException(pErrorCorrected);
#endif
		return;
	}

	on_runtime(pErrorCorrected);
	return runtime_original(pError);
}

on_runtime is the part that calls into all the Rust runtime handlers. If your library is the second one to load, runtime_original will actually be pointing to the first library's hook. This means that your library will have called on_runtime before giving the first library a chance to catch and re-throw the runtime to itself.

[SpaceManiac]

Still some distance from production-ready but this PR is itself mergeable