Skip to content

Commit 47152c6

Browse files
committed
fix(jit): guard inlined callable schemas
1 parent 289cc3c commit 47152c6

6 files changed

Lines changed: 220 additions & 4 deletions

File tree

src/vm/jit/inline.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ pub(crate) enum InlineRejectReason {
1616
HostTarget,
1717
CapturedCallable,
1818
ArityMismatch,
19+
SchemaUnproven,
1920
Recursive,
2021
NestedScriptCall,
2122
YieldingCall,

src/vm/jit/ir.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,10 @@ pub(crate) enum SsaInstKind {
105105
CloneTagged {
106106
input: SsaValueId,
107107
},
108+
ValueIsType {
109+
input: SsaValueId,
110+
tag: ValueType,
111+
},
108112
UnboxInt {
109113
input: SsaValueId,
110114
},
@@ -394,6 +398,7 @@ impl SsaInstKind {
394398
Self::HostCall { args, .. } => args.clone(),
395399

396400
Self::CloneTagged { input }
401+
| Self::ValueIsType { input, .. }
397402
| Self::UnboxInt { input }
398403
| Self::UnboxFloat { input }
399404
| Self::UnboxBool { input }
@@ -1121,6 +1126,9 @@ fn render_inst_kind(kind: &SsaInstKind) -> String {
11211126
match kind {
11221127
SsaInstKind::Constant(value) => format!("const {value:?}"),
11231128
SsaInstKind::CloneTagged { input } => format!("clone_tagged {input}"),
1129+
SsaInstKind::ValueIsType { input, tag } => {
1130+
format!("value_is_type {input}, {tag:?}")
1131+
}
11241132
SsaInstKind::UnboxInt { input } => format!("unbox_int {input}"),
11251133
SsaInstKind::UnboxFloat { input } => format!("unbox_float {input}"),
11261134
SsaInstKind::UnboxBool { input } => format!("unbox_bool {input}"),

src/vm/jit/native/lower.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1274,6 +1274,7 @@ fn ssa_trace_supported(ssa: &SsaTrace) -> bool {
12741274
inst.kind,
12751275
SsaInstKind::Constant(_)
12761276
| SsaInstKind::CloneTagged { .. }
1277+
| SsaInstKind::ValueIsType { .. }
12771278
| SsaInstKind::UnboxHeapPtr { .. }
12781279
| SsaInstKind::UnboxInt { .. }
12791280
| SsaInstKind::UnboxFloat { .. }
@@ -2090,6 +2091,29 @@ fn lower_ssa_inst(
20902091
)?;
20912092
out
20922093
}
2094+
SsaInstKind::ValueIsType { input, tag } => {
2095+
let input = *values.get(input).ok_or_else(|| {
2096+
VmError::JitNative("SSA type predicate input missing".to_string())
2097+
})?;
2098+
let expected_tag = match tag {
2099+
ValueType::Null => layout.value.null_tag,
2100+
ValueType::Int => layout.value.int_tag,
2101+
ValueType::Float => layout.value.float_tag,
2102+
ValueType::Bool => layout.value.bool_tag,
2103+
ValueType::String => layout.value.string_tag,
2104+
ValueType::Bytes => layout.value.bytes_tag,
2105+
ValueType::Array => layout.value.array_tag,
2106+
ValueType::Map => layout.value.map_tag,
2107+
ValueType::Callable | ValueType::Unknown => {
2108+
return Err(VmError::JitNative(format!(
2109+
"unsupported SSA type predicate tag {tag:?}"
2110+
)));
2111+
}
2112+
};
2113+
let actual_tag = ssa_load_tag_i32(b, layout.value, input);
2114+
b.ins()
2115+
.icmp_imm(IntCC::Equal, actual_tag, i64::from(expected_tag))
2116+
}
20932117
SsaInstKind::UnboxInt { input } => {
20942118
let input = *values
20952119
.get(input)

src/vm/jit/recorder.rs

Lines changed: 132 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,15 @@ use std::{fmt, sync::Arc};
22

33
use crate::CallableValue;
44
use crate::builtins::BuiltinFunction;
5+
use crate::compiler::TypeSchema;
56
use crate::vm::{OpCode, Program, Value, ValueType, checked_int_div};
67

78
use super::JitTraceTerminal;
89
use super::deopt::materialize_ssa_values;
9-
use super::inline::{InlineCandidate, classify_static_inline_candidate};
10+
use super::inline::{InlineCandidate, InlineRejectReason, classify_static_inline_candidate};
1011
use super::ir::{
1112
SsaBranchTarget, SsaInstKind, SsaMaterialization, SsaTerminator, SsaTrace, SsaTraceBuilder,
12-
SsaValue, SsaValueRepr, VirtualFrameSnapshot,
13+
SsaValue, SsaValueId, SsaValueRepr, VirtualFrameSnapshot,
1314
};
1415

1516
#[derive(Clone, Debug, PartialEq)]
@@ -263,6 +264,99 @@ struct SymbolicValue {
263264
info: ValueInfo,
264265
}
265266

267+
fn inline_schema_guard_type(schema: &TypeSchema) -> Option<Option<ValueType>> {
268+
match schema {
269+
TypeSchema::Unknown | TypeSchema::GenericParam(_) => Some(None),
270+
TypeSchema::Int => Some(Some(ValueType::Int)),
271+
TypeSchema::Float => Some(Some(ValueType::Float)),
272+
TypeSchema::Bool => Some(Some(ValueType::Bool)),
273+
TypeSchema::String => Some(Some(ValueType::String)),
274+
TypeSchema::Bytes => Some(Some(ValueType::Bytes)),
275+
TypeSchema::Named(_, _) | TypeSchema::Map(_) | TypeSchema::Object(_) => {
276+
Some(Some(ValueType::Map))
277+
}
278+
TypeSchema::Array(_) | TypeSchema::ArrayTuple(_) | TypeSchema::ArrayTupleRest { .. } => {
279+
Some(Some(ValueType::Array))
280+
}
281+
TypeSchema::Null
282+
| TypeSchema::Number
283+
| TypeSchema::Optional(_)
284+
| TypeSchema::Callable { .. } => None,
285+
}
286+
}
287+
288+
fn inline_argument_schemas_supported(
289+
arguments: &[SymbolicValue],
290+
schema: Option<&TypeSchema>,
291+
) -> bool {
292+
let Some(TypeSchema::Callable { params, .. }) = schema else {
293+
return schema.is_none();
294+
};
295+
params.len() == arguments.len()
296+
&& params.iter().zip(arguments).all(|(schema, argument)| {
297+
let Some(guard_type) = inline_schema_guard_type(schema) else {
298+
return false;
299+
};
300+
match (guard_type, argument.info.repr) {
301+
(None, _) | (Some(_), SsaValueRepr::Tagged) => true,
302+
(Some(ValueType::Int), SsaValueRepr::I64)
303+
| (Some(ValueType::Float), SsaValueRepr::F64)
304+
| (Some(ValueType::Bool), SsaValueRepr::Bool) => true,
305+
(Some(expected), SsaValueRepr::HeapPtr(actual)) => expected == actual,
306+
_ => false,
307+
}
308+
})
309+
}
310+
311+
fn append_inline_argument_schema_guards(
312+
builder: &mut SsaTraceBuilder,
313+
block: super::ir::SsaBlockId,
314+
ip: usize,
315+
arguments: &[SymbolicValue],
316+
schema: Option<&TypeSchema>,
317+
) -> Result<Option<SsaValueId>, TraceRecordError> {
318+
let Some(TypeSchema::Callable { params, .. }) = schema else {
319+
return Ok(None);
320+
};
321+
let mut guard = None;
322+
for (schema, argument) in params.iter().zip(arguments) {
323+
let Some(Some(expected)) = inline_schema_guard_type(schema) else {
324+
continue;
325+
};
326+
if argument.info.repr != SsaValueRepr::Tagged {
327+
continue;
328+
}
329+
let predicate = builder
330+
.append_value_inst(
331+
block,
332+
ip,
333+
SsaValueRepr::Bool,
334+
SsaInstKind::ValueIsType {
335+
input: argument.value.id,
336+
tag: expected,
337+
},
338+
)
339+
.map_err(|err| TraceRecordError::InvalidIr(err.to_string()))?;
340+
guard = Some(if let Some(previous) = guard {
341+
builder
342+
.append_value_inst(
343+
block,
344+
ip,
345+
SsaValueRepr::Bool,
346+
SsaInstKind::BoolAnd {
347+
lhs: previous,
348+
rhs: predicate.id,
349+
},
350+
)
351+
.map_err(|err| TraceRecordError::InvalidIr(err.to_string()))?
352+
.id
353+
} else {
354+
predicate.id
355+
});
356+
}
357+
Ok(guard)
358+
}
359+
266360
#[derive(Clone, Debug, PartialEq)]
267361
struct SymbolicFrame {
268362
stack: Vec<SymbolicValue>,
@@ -1241,12 +1335,30 @@ pub(crate) fn record_trace_with_local_count(
12411335
callable.info.source_local,
12421336
argc,
12431337
max_trace_len.saturating_sub(cursor.recorded_ops),
1244-
);
1338+
)
1339+
.and_then(|candidate| {
1340+
let prototype = &program.callable_prototypes[candidate.prototype_id as usize];
1341+
let argument_start = frame.stack.len() - usize::from(argc);
1342+
inline_argument_schemas_supported(
1343+
&frame.stack[argument_start..],
1344+
prototype.schema.as_ref(),
1345+
)
1346+
.then_some(candidate)
1347+
.ok_or(InlineRejectReason::SchemaUnproven)
1348+
});
12451349
let inline_reject_reason = candidate.as_ref().err().copied();
12461350
if inline_frame.is_none()
12471351
&& let Ok(candidate) = candidate
12481352
{
12491353
let prototype = &program.callable_prototypes[candidate.prototype_id as usize];
1354+
let argument_start = frame.stack.len() - usize::from(argc);
1355+
let schema_guard = append_inline_argument_schema_guards(
1356+
&mut builder,
1357+
current_block,
1358+
ip,
1359+
&frame.stack[argument_start..],
1360+
prototype.schema.as_ref(),
1361+
)?;
12501362
let expected_callable = builder
12511363
.append_value_inst(
12521364
current_block,
@@ -1270,6 +1382,22 @@ pub(crate) fn record_trace_with_local_count(
12701382
},
12711383
)
12721384
.map_err(|err| TraceRecordError::InvalidIr(err.to_string()))?;
1385+
let inline_guard = if let Some(schema_guard) = schema_guard {
1386+
builder
1387+
.append_value_inst(
1388+
current_block,
1389+
ip,
1390+
SsaValueRepr::Bool,
1391+
SsaInstKind::BoolAnd {
1392+
lhs: schema_guard,
1393+
rhs: callable_matches.id,
1394+
},
1395+
)
1396+
.map_err(|err| TraceRecordError::InvalidIr(err.to_string()))?
1397+
.id
1398+
} else {
1399+
callable_matches.id
1400+
};
12731401
let identity_exit =
12741402
add_symbolic_exit(&mut builder, ip, &frame, inline_frame.as_ref());
12751403
let (guarded_block, guarded_frame, guard_args) = continue_with_inline_frame(
@@ -1282,7 +1410,7 @@ pub(crate) fn record_trace_with_local_count(
12821410
.set_terminator(
12831411
current_block,
12841412
SsaTerminator::BranchBool {
1285-
condition: callable_matches.id,
1413+
condition: inline_guard,
12861414
if_true: SsaBranchTarget::Block {
12871415
target: guarded_block,
12881416
args: guard_args,

src/vm/jit/region.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,7 @@ fn remap_inst_inputs(
212212
}
213213
}
214214
SsaInstKind::CloneTagged { input }
215+
| SsaInstKind::ValueIsType { input, .. }
215216
| SsaInstKind::UnboxInt { input }
216217
| SsaInstKind::UnboxFloat { input }
217218
| SsaInstKind::UnboxBool { input }

tests/jit/jit_tests.rs

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6498,6 +6498,60 @@ fn trace_jit_guards_static_inline_callable_identity() {
64986498
assert!(vm.jit_native_exec_count() > 0, "{}", vm.dump_jit_info());
64996499
}
65006500

6501+
#[test]
6502+
fn trace_jit_preserves_inline_callable_argument_schema_checks() {
6503+
if !native_jit_supported() {
6504+
return;
6505+
}
6506+
let source = r#"
6507+
fn ignore(value: int) -> int { 1 }
6508+
let mut i = 0;
6509+
let value: int = 7;
6510+
while i < 100 {
6511+
i = i + ignore(value);
6512+
}
6513+
i;
6514+
"#;
6515+
let compiled = compile_source(source).expect("callable schema source should compile");
6516+
let value_slot = compiled
6517+
.program
6518+
.debug
6519+
.as_ref()
6520+
.expect("debug metadata")
6521+
.locals
6522+
.iter()
6523+
.find(|local| local.name == "value")
6524+
.expect("value local")
6525+
.index;
6526+
let mut vm = Vm::new(compiled.program.with_local_count(compiled.locals));
6527+
vm.set_fuel_check_interval(1).expect("fuel interval");
6528+
vm.set_fuel(1);
6529+
loop {
6530+
assert_eq!(
6531+
vm.run().expect("initialization step should run"),
6532+
VmStatus::Yielded
6533+
);
6534+
if vm.locals().get(value_slot as usize) == Some(&Value::Int(7)) {
6535+
break;
6536+
}
6537+
vm.recharge_fuel(1).expect("fuel recharge");
6538+
}
6539+
vm.set_local(value_slot, Value::Bool(true))
6540+
.expect("dynamic value replacement should succeed");
6541+
vm.clear_fuel();
6542+
vm.set_jit_config(JitConfig {
6543+
enabled: true,
6544+
hot_loop_threshold: 1,
6545+
max_trace_len: 512,
6546+
});
6547+
6548+
let error = vm.run().expect_err("invalid callable argument must fail");
6549+
assert!(
6550+
matches!(error, vm::VmError::TypeMismatch("callable argument schema")),
6551+
"unexpected error: {error:?}"
6552+
);
6553+
}
6554+
65016555
#[test]
65026556
fn trace_jit_inlines_array_swap_leaf() {
65036557
if !native_jit_supported() {

0 commit comments

Comments
 (0)