convert to byond-scan and fix a lot of shit

Cargo.toml

@@ -1,19 +1,20 @@
 [workspace]
-members = ["auxcov", "auxtools", "auxtools-impl", "debug_server", "instruction_hooking", "tests/auxtest", "tests/byond_get", "tests/test_runner"]
+members = ["auxcov", "auxtools", "auxtools-impl", "debug_server", "instruction_hooking", "tests/auxtest", "tests/byond_get", "tests/debug_test", "tests/test_runner"]
 resolver = "2"
 
 [workspace.package]
 authors = ["Sophie Wallace <sophie@wiox.me>", "Lucy <lucy@absolucy.moe>"]
 edition = "2021"
 license = "MIT"
 repository = "https://github.com/wilox/auxtools"
-rust-version = "1.76"
+rust-version = "1.88"
 version = "0.1.0"
 
 [workspace.dependencies]
 lazy_static = "1"
 retour = { version = "=0.4.0-alpha.4", default-features = false }
 dmasm = { git = "https://github.com/willox/dmasm" }
+byond-scan = { git = "https://github.com/Absolucy/byond-scan" }
 
 [workspace.lints.rust]
 static_mut_refs = "allow"

README.md

@@ -13,6 +13,14 @@ Debug Server - Working with SpaceManiac's [SpacemanDMM](https://github.com/Space
 
 *Dissassembly*  - When currently stopped at a breakpoint, enter the `#dis` command into the Debug Console in VSC to see the DM bytecode for the current proc. You can also provide it any arbitrary proc path like `#dis /mob/proc/Life`.
 
+## Supported BYOND versions
+
+516.1659 through 516.1688 on Windows, and 516.1664 through 516.1688 on Linux. Those are the builds the patterns that find BYOND's functions were checked against. A newer build is still tried, and fails at init with the name of whatever stopped matching. An older build, or another major version, is refused at init with an error that names the build.
+
+The patterns are run by [byond-scan](https://github.com/Absolucy/byond-scan) and live in `auxtools/src/symbols/`. They are copies of the ones in the byond-re repo's `byond_catalog`, which checks each against every local build, so a change to one has to land in both.
+
+What was run on Linux (516.1687): the `auxtest` suite, and `debug_test`, which sets breakpoints, evaluates expressions and re-attaches through a stand-in for the debugger. The code coverage tool's report was checked on Windows and Linux 1687 against a world with a known set of lines. Stepping and breakpoints with a real debugger attached were not tested on either platform.
+
 ## Dependencies
 
 *These instructions were taken directly from tgstation's [rust-g](https://github.com/tgstation/rust-g) documentation.*
@@ -67,7 +75,7 @@ Linux:
 ```sh
 export PKG_CONFIG_ALLOW_CROSS=1
 cargo build --release --target i686-unknown-linux-gnu
-# output: target/i686-unknown-linux-gnu/release/libauxtools.so
+# output: target/i686-unknown-linux-gnu/release/libauxtools.so,libdebug_server.so,libauxcov.so
 ```
 
 Windows:

auxcov/src/codecov.rs

@@ -170,7 +170,8 @@ impl Tracker {
 	fn finalize(&mut self) -> Result<(), Vec<Error>> {
 		let mut errors_option = None;
 		for context in &self.contexts {
-			let result = context.finalize(); // dropping the results because what can ya do?
+			let result = context.finalize(); // dropping the results because
+											 // what can ya do?
 			if let Err(error) = result {
 				match &mut errors_option {
 					None => {
@@ -195,8 +196,9 @@ impl Tracker {
 
 impl Drop for Tracker {
 	fn drop(&mut self) {
-		let _result = self.finalize(); // dropping the result here because what can ya
-		                         // do?
+		let _result = self.finalize(); // dropping the result here because what
+		                               // can ya
+		                               // do?
 	}
 }
 

auxcov/src/lib.rs

@@ -2,8 +2,6 @@
 
 mod codecov;
 
-use std::any::{Any, TypeId};
-
 use auxtools::*;
 use codecov::Tracker;
 use instruction_hooking::INSTRUCTION_HOOKS;
@@ -15,14 +13,13 @@ where
 	unsafe {
 		let hooks = INSTRUCTION_HOOKS.get_mut();
 
-		let tracker_tid = TypeId::of::<Tracker>();
-		let tracker_option = hooks.iter_mut().find(|hook| (*hook).as_ref().type_id() == tracker_tid);
+		// `type_id()` on the boxed hook itself would give the id of the trait
+		// object, never `Tracker`'s, so ask the hook for its concrete
+		// value as `Any` first.
+		let tracker_option = hooks.iter_mut().find_map(|hook| hook.as_mut().as_any().downcast_mut::<Tracker>());
 
 		match tracker_option {
-			Some(existing_hook) => {
-				let mut_hook = existing_hook.as_mut();
-				let any_hook = mut_hook.as_any();
-				let existing_tracker = any_hook.downcast_mut::<Tracker>().unwrap();
+			Some(existing_tracker) => {
 				f(existing_tracker);
 			}
 			None => {

auxtools-impl/Cargo.toml

@@ -15,7 +15,6 @@ doctest = false
 [dependencies]
 proc-macro2 = "1.0"
 quote = "1.0"
-hex = "0.4"
 syn = { version = "2", features = ["full"] }
 
 [lints]

auxtools-impl/src/lib.rs

@@ -2,60 +2,13 @@ mod versioned_struct;
 
 use proc_macro::TokenStream;
 use quote::quote;
-use syn::{parse_macro_input, spanned::Spanned, Lit};
-
-fn from_signature(s: String) -> Vec<Option<u8>> {
-	s.trim()
-		.split(' ')
-		.map(|byte| {
-			let byte = byte.trim();
-			match byte.len() {
-				2 => {
-					if byte == "??" {
-						None
-					} else {
-						hex::decode(byte).map(|decoded_byte| decoded_byte[0]).ok()
-					}
-				}
-				_ => None
-			}
-		})
-		.collect()
-}
+use syn::{spanned::Spanned, Lit};
 
 #[proc_macro_attribute]
 pub fn versioned(attr: TokenStream, item: TokenStream) -> TokenStream {
 	versioned_struct::versioned(attr.into(), item.into()).into()
 }
 
-#[proc_macro]
-pub fn convert_signature(input: TokenStream) -> TokenStream {
-	let string = parse_macro_input!(input as Lit);
-	let string = match string {
-		Lit::Str(lit) => lit.value(),
-		_ => panic!("not string input")
-	};
-
-	let streams: Vec<proc_macro2::TokenStream> = from_signature(string)
-		.into_iter()
-		.map(|x| match x {
-			Some(byte) => {
-				quote! {
-					Some(#byte as u8)
-				}
-			}
-			None => {
-				quote! { None }
-			}
-		})
-		.collect();
-
-	let result = quote! {
-		&[ #( #streams, )* ]
-	};
-	result.into()
-}
-
 fn extract_args(a: &syn::FnArg) -> &syn::PatType {
 	match a {
 		syn::FnArg::Typed(p) => p,
@@ -185,14 +138,11 @@ pub fn pin_dll(attr: TokenStream) -> TokenStream {
 ///
 /// Here we define a hook that multiplies a number passed to it by two.
 /// It can now be used to hook procs, for example
-/// `hooks::hook("/proc/double_up", double_up);`
+/// `Proc::find("/proc/double_up").unwrap().hook(double_up);`
 /// ```ignore
 /// #[hook]
 /// fn double_up(num: Value) {
-///     if let Some(num) = num.as_number() {
-///         Value::from(num * 2.0);
-///     }
-///     Value::NULL
+///     Ok(Value::from(num.as_number()? * 2.0))
 /// }
 /// ```
 ///
@@ -201,9 +151,9 @@ pub fn pin_dll(attr: TokenStream) -> TokenStream {
 /// ```ignore
 /// #[hook("/mob/proc/on_honked")]
 /// fn on_honked(honker: Value) {
-///     src.call("gib", &[]);
-///     honker.call("laugh", &[]);
-///     Value::NULL
+///     src.call("gib", &[])?;
+///     honker.call("laugh", &[])?;
+///     Ok(Value::NULL)
 /// }
 /// ```
 #[proc_macro_attribute]

auxtools/Cargo.toml

@@ -16,20 +16,15 @@ cc = "1.0"
 
 [dependencies]
 auxtools-impl = { path = "../auxtools-impl", version = "0.1.0", package = "auxtools-impl" }
-once_cell = "1"
+byond-scan = { workspace = true }
 inventory = "0.3"
-lazy_static = { workspace = true }
-dashmap = "6"
 ahash = "0.8"
 fxhash = "0.2"
 ctor = "0.2"
 retour = { workspace = true }
 
 [target.'cfg(windows)'.dependencies]
-winapi = { version = "0.3.9", features = ["winuser", "libloaderapi", "psapi", "processthreadsapi"] }
-
-[target.'cfg(unix)'.dependencies]
-libc = "0.2"
+winapi = { version = "0.3.9", features = ["libloaderapi"] }
 
 [lints]
 workspace = true

auxtools/build.rs

@@ -1,4 +1,8 @@
 fn main() {
+	println!("cargo:rerun-if-changed=src/hooks.cpp");
+	println!("cargo:rerun-if-changed=src/hooks.h");
+	println!("cargo:rerun-if-changed=src/raw_types/funcs.cpp");
+
 	cc::Build::new()
 		.include("src/")
 		.file("src/hooks.cpp")

auxtools/src/byond_ffi.rs

@@ -74,9 +74,9 @@ pub fn byond_return(value: Option<Vec<u8>>) -> *const c_char {
 ///
 /// # Examples
 /// ```ignore
-/// // byond_ffi_fn! { my_proc(_input) {
-/// //     Some("Hello, BYOND!".to_owned())
-/// // }
+/// byond_ffi_fn! { my_proc(_input) {
+///     Some("Hello, BYOND!".to_owned())
+/// } }
 /// ```
 #[macro_export]
 macro_rules! byond_ffi_fn {

auxtools/src/bytecode_manager.rs

@@ -59,7 +59,7 @@ pub fn shutdown() {
 		let proc = Proc::from_id(id).unwrap();
 
 		unsafe {
-			raw_types::misc::set_bytecode(*(*proc.entry).metadata.bytecode(), ptr, len);
+			raw_types::misc::set_bytecode(*(*proc.entry()).metadata.bytecode(), ptr, len);
 		}
 	}
 
@@ -104,6 +104,6 @@ pub fn set_bytecode(proc: &Proc, mut bytecode: Vec<u32>) {
 	let len = u16::try_from(len).unwrap();
 
 	unsafe {
-		raw_types::misc::set_bytecode(*(*proc.entry).metadata.bytecode(), ptr, len);
+		raw_types::misc::set_bytecode(*(*proc.entry()).metadata.bytecode(), ptr, len);
 	}
 }

auxtools/src/debug.rs

@@ -36,8 +36,8 @@ impl StackFrame {
 		let src = Value::from_raw((*instance).src);
 		let dot = Value::from_raw(*(*context).dot());
 
-		// Make sure to handle arguments/locals with no names (when there are more
-		// values than names)
+		// Make sure to handle arguments/locals with no names (when there are
+		// more values than names)
 		let args = (0..(*instance).args_count())
 			.map(|i| {
 				let name = param_names.get(i as usize).cloned();

auxtools/src/hooks.cpp

@@ -3,18 +3,38 @@
 
 // The type of the func defined in Byond
 using Runtime_Ptr = void(*)(char *pError);
-using CallProcById_Ptr = Value(LINUX_REGPARM3 *)(Value, uint32_t, uint32_t, uint32_t, Value, Value*, uint32_t, uint32_t, uint32_t);
+// Plain cdecl on both platforms. This carried regparm(3) on Linux, which was
+// never right: `call_proc_by_id` reads all twelve of its dwords off the stack, the
+// first being the hidden buffer the Value return is written through, and ends
+// `retn 4` to pop that dword itself. Declaring regparm(3) made GCC pass the buffer
+// in eax and `usr` in edx:ecx, which shifted every argument the trampoline read
+// (the proc id it looked up was really the output pointer, so no #[hook] ever
+// matched) and left nothing on the stack for `retn 4` to pop, so the stack drifted
+// until an SSE store faulted somewhere inside BYOND. Read byte-identical on
+// 516.1669 and 516.1687.
+//
+// The last two arguments are a completion callback and a value to hand it. BYOND
+// pushes the result's two dwords and then the value, which is the same thing as
+// passing a Value. Plain cdecl on both platforms.
+using CallProc_Callback = void(*)(Value, uint32_t);
+using CallProcById_Ptr = Value(*)(Value, uint32_t, uint32_t, uint32_t, Value, Value*, uint32_t, CallProc_Callback, uint32_t);
 
 // The type of the hook defined in hooks.rs
 using CallProcById_Hook_Ptr = Value(*)(Value, uint32_t, uint32_t, uint32_t, Value, Value*, uint32_t, uint32_t, uint32_t);
 
+// The bit of `proc_type` that says the caller is not going to look at the result
+static const uint32_t PROC_TYPE_DISCARD_RESULT = 0x04;
+
 extern "C" {
 	// The ptr everybody else sees
 	Runtime_Ptr runtime_byond = nullptr;
 
 	// The original function - set by rust after hooking
 	Runtime_Ptr runtime_original = nullptr;
 	CallProcById_Ptr call_proc_by_id_original = nullptr;
+
+	// Lives in funcs.cpp
+	extern void (*dec_ref_count_byond)(Value);
 }
 
 // If the top of this stack is true, we replace byond's runtime exceptions with our own
@@ -42,33 +62,53 @@ extern "C" uint8_t call_proc_by_id_hook(
 	Value usr,
 	uint32_t proc_type,
 	uint32_t proc_id,
-	uint32_t unk_0,
+	uint32_t override_depth,
 	Value src,
 	Value* args,
-	uint8_t args_count,
-	uint32_t unk_1,
-	uint32_t unk_2);
+	uint32_t args_count,
+	CallProc_Callback callback,
+	uint32_t callback_value);
 
-// A little function to handle the odd calling convention on Linux and pass-through to our rust hook
+// Stands in for `call_proc_by_id` itself, so its shape has to match that function
+// exactly (see the note on `CallProcById_Ptr` above). Passes through to our rust hook.
 // Used on Windows too
-extern "C" Value LINUX_REGPARM3 call_proc_by_id_hook_trampoline(
+extern "C" Value call_proc_by_id_hook_trampoline(
 	Value usr,
 	uint32_t proc_type,
 	uint32_t proc_id,
-	uint32_t unk_0,
+	uint32_t override_depth,
 	Value src,
 	Value* args,
-	uint8_t args_count,
-	uint32_t unk_1,
-	uint32_t unk_2
+	uint32_t args_count,
+	CallProc_Callback callback,
+	uint32_t callback_value
 ) {
 	Value ret;
 
-	if (call_proc_by_id_hook(&ret, usr, proc_type, proc_id, unk_0, src, args, args_count, unk_1, unk_2)) {
-		clean(ret);
-		return ret;
-	} else {
-		return call_proc_by_id_original(usr, proc_type, proc_id, unk_0, src, args, args_count, unk_1, unk_2);
+	// A shim running with `RuntimeContext(true)` wants the errors its own BYOND
+	// function raises, not the ones raised inside a DM proc that function calls.
+	// Those have to take BYOND's normal path so they get reported and reach the
+	// callee's own `try`. Every DM proc call passes through here, so this is the
+	// one place that can hand the called proc a clean context.
+	RuntimeContext runtime_scope(false);
+
+	if (!call_proc_by_id_hook(&ret, usr, proc_type, proc_id, override_depth, src, args, args_count, callback, callback_value)) {
+		return call_proc_by_id_original(usr, proc_type, proc_id, override_depth, src, args, args_count, callback, callback_value);
+	}
+	clean(ret);
+
+	// The hook ran instead of `exec_proc`, so it owes the caller what `exec_proc`
+	// does when a proc ends. A caller that passed a callback gets the result
+	// through it, and one that set the discard bit doesn't want it at all. Both of
+	// them get null back and never release anything, so the result's reference is
+	// ours to drop. Done here and not in rust so a callback that throws doesn't
+	// have to unwind through a rust frame.
+	if (callback != nullptr) {
+		callback(ret, callback_value);
+	}
+	if (callback != nullptr || (proc_type & PROC_TYPE_DISCARD_RESULT)) {
+		dec_ref_count_byond(ret);
+		ret = Value{0, 0};
 	}
-	//return call_proc_by_id_hook(usr, proc_type, proc_id, unk_0, src, args, args_count, unk_1, unk_2);
+	return ret;
 }