-
Notifications
You must be signed in to change notification settings - Fork 1
ROX-36673: Allow instructions with multiple arguments #62
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2,6 +2,9 @@ use std::collections::HashMap; | |||||
|
|
||||||
| #[derive(Debug, Clone, PartialEq)] | ||||||
| pub enum Arg { | ||||||
| /// Null constant | ||||||
| Null {}, | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rust allows empty variants, like
Suggested change
|
||||||
|
|
||||||
| /// Simple constant | ||||||
| Const { text: String }, | ||||||
|
|
||||||
|
|
@@ -14,9 +17,16 @@ pub enum Arg { | |||||
|
|
||||||
| #[derive(Debug, Clone, PartialEq)] | ||||||
| pub enum Instruction { | ||||||
| /// Execute a binary with specified name and arguments | ||||||
| Task { name: Arg, args: Vec<Arg> }, | ||||||
|
|
||||||
| /// Open a file at specified path | ||||||
| Open { path: Arg }, | ||||||
|
|
||||||
| /// Print a debugging message (subject to configured log level) | ||||||
| Debug { text: Arg }, | ||||||
|
|
||||||
| /// Send a message to a server at specified address | ||||||
| Ping { server: Arg }, | ||||||
| } | ||||||
|
|
||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,6 +50,7 @@ struct BuildContext<'a> { | |
| ee: LLVMExecutionEngineRef, | ||
| builder: LLVMBuilderRef, | ||
| module: LLVMModuleRef, | ||
| context: LLVMContextRef, | ||
| module_state: &'a HashMap<String, LLVMValueRef>, | ||
| module_runtime: &'a HashMap<String, (LLVMValueRef, LLVMTypeRef)>, | ||
| } | ||
|
|
@@ -116,23 +117,45 @@ pub unsafe extern "C" fn ping(addr: *const i8) -> u64 { | |
| /// The caller must ensure the pointer is valid and points to a null | ||
| /// terminated C-string. | ||
| #[unsafe(no_mangle)] | ||
| pub unsafe extern "C" fn task(name: *const i8, random: bool) -> u64 { | ||
| pub unsafe extern "C" fn task(name: *const i8, args: *const i8) -> u64 { | ||
| let name = unsafe { CStr::from_ptr(name) }; | ||
| debug!("Task {:?} {:?}", name, random); | ||
| let uniq_arg: String = rand::thread_rng() | ||
| let mut task = Command::new(name.to_str().unwrap()); | ||
|
|
||
| if !args.is_null() { | ||
| let args = unsafe { CStr::from_ptr(args) }; | ||
| debug!("Task {:?} {:?}", name, args); | ||
|
|
||
| task.args(args.to_str().unwrap().split(' ')); | ||
| } else { | ||
| debug!("Task {:?}, null", name); | ||
| } | ||
|
Comment on lines
+124
to
+131
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you change let args = if !args.is_null() {
unsafe { CStr::from_ptr(args) }
} else {
c""
};
let task = Command::new(name.to_str().unwrap())
.args(args.to_str().unwrap().split_whitespace())
.status()
.expect("Failed to execute task");This would work because |
||
|
|
||
| let status = task.status().expect("Failed to execute task"); | ||
|
|
||
| status.code().unwrap_or(0).try_into().unwrap() | ||
| } | ||
|
|
||
| thread_local! { | ||
| static POINTERS: RefCell<Vec<*mut i8>> = const { RefCell::new(vec![]) }; | ||
| } | ||
|
|
||
| /// Return a randomly generated string. | ||
| /// | ||
| /// # Safety | ||
| /// The caller must ensure the pointer is valid and points to a null | ||
| /// terminated C-string. | ||
| #[unsafe(no_mangle)] | ||
| pub unsafe extern "C" fn random_string() -> *const i8 { | ||
| let rand: String = rand::thread_rng() | ||
| .sample_iter(&Alphanumeric) | ||
| .take(7) | ||
| .map(char::from) | ||
| .collect(); | ||
| let _res = Command::new(name.to_str().unwrap()) | ||
| .arg(uniq_arg) | ||
| .output() | ||
| .unwrap(); | ||
| 0 | ||
| } | ||
|
|
||
| thread_local! { | ||
| static POINTERS: RefCell<Vec<*mut i8>> = const { RefCell::new(vec![]) }; | ||
| let result = CString::new(rand).unwrap().into_raw(); | ||
|
|
||
| POINTERS.with(|ps| ps.borrow_mut().push(result)); | ||
| result | ||
| } | ||
|
|
||
| /// Return a randomly generated path. | ||
|
|
@@ -187,8 +210,8 @@ pub static RUNTIME: LazyLock<HashMap<String, RuntimeFunc>> = | |
| "task".to_string(), | ||
| RuntimeFunc { | ||
| func: task as *const () as usize, | ||
| param_count: 1, | ||
| param_types: &[RuntimeType::Pointer], | ||
| param_count: 2, | ||
| param_types: &[RuntimeType::Pointer, RuntimeType::Pointer], | ||
| return_type: RuntimeType::Int, | ||
| }, | ||
| ), | ||
|
|
@@ -229,6 +252,15 @@ pub static RUNTIME: LazyLock<HashMap<String, RuntimeFunc>> = | |
| return_type: RuntimeType::Pointer, | ||
| }, | ||
| ), | ||
| ( | ||
| "random_string".to_string(), | ||
| RuntimeFunc { | ||
| func: random_string as *const () as usize, | ||
| param_count: 0, | ||
| param_types: &[], | ||
| return_type: RuntimeType::Pointer, | ||
| }, | ||
| ), | ||
| // utils | ||
| ( | ||
| "cleanup".to_string(), | ||
|
|
@@ -243,8 +275,12 @@ pub static RUNTIME: LazyLock<HashMap<String, RuntimeFunc>> = | |
| }); | ||
|
|
||
| impl ScriptWorker { | ||
| fn jit_instruction(name: &CStr, arg: Arg, ctx: &BuildContext) { | ||
| let mut arg_ptr = Self::get_arg_value(arg, ctx); | ||
| fn jit_instruction(name: &CStr, args: Vec<Arg>, ctx: &BuildContext) { | ||
| let (args_ref, args_len, args_cap) = args | ||
| .iter() | ||
| .map(|a| Self::get_arg_value(a.clone(), ctx)) | ||
| .collect::<Vec<_>>() | ||
| .into_raw_parts(); | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| let (func, func_type) = ctx | ||
| .module_runtime | ||
|
|
@@ -256,16 +292,30 @@ impl ScriptWorker { | |
| ctx.builder, | ||
| *func_type, | ||
| *func, | ||
| &mut arg_ptr, | ||
| 1, | ||
| args_ref, | ||
| args_len.try_into().unwrap(), | ||
| name.as_ptr() as *const _, | ||
| ); | ||
|
|
||
| let _ = Vec::from_raw_parts(args_ref, args_len, args_cap); | ||
|
Comment on lines
278
to
+300
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since the vector is immediately reconstructed before returning, your probably safe to not disassemble it in the first place, something like this should work: fn jit_instruction(name: &CStr, args: Vec<Arg>, ctx: &BuildContext) {
let mut args = args
.iter()
.map(|a| Self::get_arg_value(a.clone(), ctx))
.collect::<Vec<_>>();
let (func, func_type) = ctx
.module_runtime
.get(name.to_str().expect("Couldn't convert name to string"))
.unwrap();
unsafe {
LLVMBuildCall2(
ctx.builder,
*func_type,
*func,
args.as_mut_ptr(),
args.len() as u32,
name.as_ptr() as *const _,
);
}
} |
||
| } | ||
| } | ||
|
|
||
| fn get_arg_value(arg: Arg, ctx: &BuildContext) -> LLVMValueRef { | ||
| match arg { | ||
| Arg::Null {} => unsafe { | ||
| let td = LLVMGetExecutionEngineTargetData(ctx.ee); | ||
| let iptr = LLVMIntPtrTypeInContext(ctx.context, td); | ||
| LLVMConstNull(iptr) | ||
| }, | ||
| Arg::Const { text } => unsafe { | ||
| // The name of all constants created this way will be "const", | ||
| // which is ugly, but not a problem as LLVM modifies this to | ||
| // make sure uniqueness, i.e. they will be: | ||
| // | ||
| // @const, @const.1, @const.2, ... | ||
| // | ||
| // in the jited code. | ||
| LLVMBuildGlobalString( | ||
| ctx.builder, | ||
| format!("{text}\0").as_ptr() as *const _, | ||
|
|
@@ -285,12 +335,11 @@ impl ScriptWorker { | |
| .get(&name) | ||
| .expect("No dynamic variable in the static runtime"); | ||
|
|
||
| let text = match &args[0] { | ||
| Arg::Const { text } => text, | ||
| unknown => { | ||
| panic!("Unknown dynamic variable argument: {unknown:?}") | ||
| } | ||
| }; | ||
| let (args_ref, args_len, args_cap) = args | ||
| .iter() | ||
| .map(|a| Self::get_arg_value(a.clone(), ctx)) | ||
| .collect::<Vec<_>>() | ||
| .into_raw_parts(); | ||
|
Comment on lines
+338
to
+342
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as my previous comment, we can probably keep the vector here as well. |
||
|
|
||
| unsafe { | ||
| trace!("Add mapping to {:?}", name); | ||
|
|
@@ -305,20 +354,17 @@ impl ScriptWorker { | |
| runtime_func.func as *mut c_void, | ||
| ); | ||
|
|
||
| let mut helper_ptr = LLVMBuildGlobalString( | ||
| ctx.builder, | ||
| format!("{text}\0").as_ptr() as *const _, | ||
| c"const".as_ptr() as *const _, | ||
| ); | ||
|
|
||
| LLVMBuildCall2( | ||
| let call = LLVMBuildCall2( | ||
| ctx.builder, | ||
| *func_type, | ||
| *func, | ||
| &mut helper_ptr, | ||
| 1, | ||
| args_ref, | ||
| args_len.try_into().unwrap(), | ||
| c"{name}".as_ptr() as *const _, | ||
| ) | ||
| ); | ||
|
|
||
| let _ = Vec::from_raw_parts(args_ref, args_len, args_cap); | ||
| call | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -371,6 +417,7 @@ impl ScriptWorker { | |
|
|
||
| // get a type for main function | ||
| let i64t = LLVMInt64TypeInContext(context); | ||
| let boolt = LLVMInt1TypeInContext(context); | ||
| let iptr = LLVMIntPtrTypeInContext(context, td); | ||
|
|
||
| // Insert runtime functions into the module | ||
|
|
@@ -440,6 +487,12 @@ impl ScriptWorker { | |
| ); | ||
| module_state.insert(String::from("stub"), stub_ptr); | ||
|
|
||
| let true_ptr = LLVMConstInt(boolt, 1, 0); | ||
| module_state.insert(String::from("true"), true_ptr); | ||
|
|
||
| let false_ptr = LLVMConstInt(boolt, 0, 0); | ||
| module_state.insert(String::from("false"), false_ptr); | ||
|
erthalion marked this conversation as resolved.
|
||
|
|
||
| let Node::Work { | ||
| ref instructions, .. | ||
| } = node | ||
|
|
@@ -453,6 +506,7 @@ impl ScriptWorker { | |
| ee, | ||
| builder, | ||
| module, | ||
| context, | ||
| module_state: &module_state, | ||
| module_runtime: &module_runtime, | ||
| }; | ||
|
|
@@ -461,23 +515,26 @@ impl ScriptWorker { | |
| for instr in instructions { | ||
| // JIT the instruction and collect it's name | ||
| let name = match instr.clone() { | ||
| Instruction::Task { name, args: _ } => { | ||
| Self::jit_instruction(c"task", name, &ctx); | ||
| Instruction::Task { name, args } => { | ||
| let mut task_args = vec![name]; | ||
| task_args.extend_from_slice(&args); | ||
|
|
||
| Self::jit_instruction(c"task", task_args, &ctx); | ||
|
erthalion marked this conversation as resolved.
|
||
| "task" | ||
| } | ||
|
|
||
| Instruction::Open { path } => { | ||
| Self::jit_instruction(c"open", path, &ctx); | ||
| Self::jit_instruction(c"open", vec![path], &ctx); | ||
| "open" | ||
| } | ||
|
|
||
| Instruction::Ping { server } => { | ||
| Self::jit_instruction(c"ping", server, &ctx); | ||
| Self::jit_instruction(c"ping", vec![server], &ctx); | ||
| "ping" | ||
| } | ||
|
|
||
| Instruction::Debug { text } => { | ||
| Self::jit_instruction(c"debug", text, &ctx); | ||
| Self::jit_instruction(c"debug", vec![text], &ctx); | ||
| "debug" | ||
| } | ||
| }; | ||
|
|
@@ -502,13 +559,7 @@ impl ScriptWorker { | |
| } | ||
|
|
||
| // Final instruction to clear dangling pointers | ||
| Self::jit_instruction( | ||
| c"cleanup", | ||
| Arg::Const { | ||
| text: "".to_string(), | ||
| }, | ||
| &ctx, | ||
| ); | ||
| Self::jit_instruction(c"cleanup", vec![], &ctx); | ||
|
|
||
| let module_func = LLVMGetNamedFunction( | ||
| module, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.