Skip to content

Commit 55d415c

Browse files
olwangclaude
andcommitted
fix: resolve cross-layer review findings (VM soundness, AOT parity, front-end diagnostics, sandbox, spec)
Cross-cutting fixes from a language/VM/JIT/AOT/self-host review. reg-VM soundness & crashes: - scalar_regs elision leak: add permanent scalar_poison_regs so a register reused (via local(name)) across a scalar and a heap binding is never treated as scalar — a heap binding's DeepCopy can no longer be wrongly elided. - Map/Set heap-key alias leak: deep_copy_value now rebuilds keys from a deep-copied value; reclassify MapKeys/SetToList/SortedMapKeys as AliasReturner. - Return clones instead of moving, so a mut-scalar-param that is also the return source survives for mut-writeback (fixes "read uninitialized register" panic). - mut-place call args (mut c.count / mut c.items) are restored into their field/index after the call, matching AOT &mut place semantics. - Default-features build: add missing #[cfg(feature = "native-jit")] in tier.rs. AOT (compiled-backend) parity: - mut class-field args lower through try_write_at (no read-view clone). - Correct bitwise-vs-comparison precedence table; parenthesize computed index-assignment; drop the broken &Managed<T> ABI for retained value-struct read params (value semantics + clone-at-store). Front-end diagnostics: - RS0038 for empty/multi-scalar char literals (decode_char_token made total). - Report malformed arms in match EXPRESSIONS; assignment-check closures nested in list/struct literals; lex unknown chars to a distinct Unknown token. Sandbox: String.repeat/pad_* charge mem_budget before allocating; host_call_budget disarms native tiering. Self-host: bound scan_char's escape skip; check.rss recovers past malformed items; lexer.rss/scan.rss emit the Unknown token for parity. Spec: fix §A.1 keyword/symbol/token lists, §5A assignment rule, and scope the exec-spec Map-order claim (cross-backend iteration order is unspecified). Adds regression tests (elision reuse leak, mut-scalar-param return, mut class-field writeback, RS0038). Verified: lib/differential/static/runtime/soak/jit_cost_model green + self-host corpus 556x3. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 202521c commit 55d415c

27 files changed

Lines changed: 466 additions & 74 deletions

crates/rsscript/src/analyzer/assign.rs

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -250,9 +250,21 @@ impl<'a> AssignChecker<'a> {
250250
self.expr(&entry.value);
251251
}
252252
}
253-
Expr::ObjectLiteral { .. }
254-
| Expr::ArrayLiteral { .. }
255-
| Expr::Ident(..)
253+
// Recurse into literal contents: a closure nested in a struct/list
254+
// literal (`let fs = [|x| { total = x }]`) must still have its body
255+
// assignment-checked, or it could mutate an immutable outer binding
256+
// with no diagnostic.
257+
Expr::ObjectLiteral { fields, .. } => {
258+
for field in fields {
259+
self.expr(&field.value);
260+
}
261+
}
262+
Expr::ArrayLiteral { items, .. } => {
263+
for item in items {
264+
self.expr(item);
265+
}
266+
}
267+
Expr::Ident(..)
256268
| Expr::Number(..)
257269
| Expr::String(..)
258270
| Expr::CharLiteral(..)

crates/rsscript/src/analyzer/syntax_support.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -462,8 +462,20 @@ impl Analyzer<'_> {
462462
self.check_unsupported_syntax_expr(value);
463463
}
464464
Expr::Closure { body, .. } => self.check_unsupported_syntax_block(body),
465-
Expr::Match { value, arms, .. } => {
465+
Expr::Match {
466+
value,
467+
arms,
468+
malformed_arm_spans,
469+
..
470+
} => {
466471
self.check_unsupported_syntax_expr(value);
472+
for span in malformed_arm_spans {
473+
self.unsupported_syntax(
474+
span.clone(),
475+
"malformed match arm",
476+
"Match arms must use `pattern => statement` or `pattern => { ... }`.",
477+
);
478+
}
467479
for arm in arms {
468480
self.check_unsupported_syntax_block(&arm.body);
469481
}

crates/rsscript/src/checks/body/semantics.rs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,28 @@ pub(super) fn check_integer_literal_range(analyzer: &mut Analyzer<'_>, expr: &Hi
379379
}
380380
}
381381

382+
/// A `Char` literal denotes exactly one Unicode scalar. Reject `''` (empty) and
383+
/// `'ab'` (multi-scalar) at the frontend so they never reach lowering (which
384+
/// would otherwise silently truncate to the first scalar) or the VM/compiled
385+
/// backend. Well-formed single-scalar literals are left alone.
386+
pub(super) fn check_char_literal_scalar(analyzer: &mut Analyzer<'_>, expr: &HirExpr) {
387+
let HirExpr::Char { value, span } = expr else {
388+
return;
389+
};
390+
let count = crate::text_util::char_literal_scalar_count(value);
391+
if count != 1 {
392+
analyzer.diagnostics.push(error_cause_manual_fix(
393+
code::CHAR_LITERAL_NOT_SINGLE_SCALAR,
394+
format!("character literal must contain exactly one character, found {count}."),
395+
span.clone(),
396+
"invalid character literal",
397+
"A `Char` is a single Unicode scalar value; `''` is empty and `'ab'` holds more than one.",
398+
"use_single_char_literal",
399+
"Put exactly one character between the single quotes, or use a `String` literal (double quotes) for text.",
400+
));
401+
}
402+
}
403+
382404
pub(super) fn check_bool_condition(analyzer: &mut Analyzer<'_>, expr: &HirExpr, construct: &str) {
383405
let Some(type_name) = hir_expr_type_name(expr) else {
384406
return;
@@ -482,6 +504,22 @@ pub(super) fn check_match_pattern_matches_type(
482504
match pattern {
483505
MatchPattern::Binding { .. } | MatchPattern::Wildcard(_) => {}
484506
MatchPattern::Literal { value, span } => {
507+
if let MatchLiteral::Char(raw) = value {
508+
if crate::text_util::char_literal_scalar_count(raw) != 1 {
509+
analyzer.diagnostics.push(error_cause_manual_fix(
510+
code::CHAR_LITERAL_NOT_SINGLE_SCALAR,
511+
format!(
512+
"character literal must contain exactly one character, found {}.",
513+
crate::text_util::char_literal_scalar_count(raw)
514+
),
515+
span.clone(),
516+
"invalid character literal",
517+
"A `Char` is a single Unicode scalar value; `''` is empty and `'ab'` holds more than one.",
518+
"use_single_char_literal",
519+
"Put exactly one character between the single quotes.",
520+
));
521+
}
522+
}
485523
let literal_type = match value {
486524
MatchLiteral::Int(_) => "Int",
487525
MatchLiteral::String(_) => "String",
@@ -1150,6 +1188,7 @@ pub(super) fn check_expr_semantics_with_context(
11501188
live_after: &HashSet<String>,
11511189
) {
11521190
check_integer_literal_range(analyzer, expr);
1191+
check_char_literal_scalar(analyzer, expr);
11531192
match expr {
11541193
HirExpr::Call {
11551194
callee,

crates/rsscript/src/diagnostic.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ pub mod code {
4141
pub const LOWER_NAME_CONFLICT: &str = "RS0035";
4242
pub const MESSAGE_PAYLOAD_NOT_TRANSFERABLE: &str = "RS0036";
4343
pub const VARIANT_PATTERN_ARITY_MISMATCH: &str = "RS0037";
44+
pub const CHAR_LITERAL_NOT_SINGLE_SCALAR: &str = "RS0038";
4445
pub const FEATURE_VIOLATION: &str = "RS0101";
4546
pub const UNNAMED_ARGUMENT: &str = "RS0201";
4647
pub const MISSING_DATA_EFFECT: &str = "RS0202";

crates/rsscript/src/hir/lower.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1390,6 +1390,7 @@ fn lower_hir_expr(
13901390
scrutinee_effect,
13911391
arms,
13921392
span,
1393+
..
13931394
} => {
13941395
let value_type = infer_hir_expr_type(hir, value, value_types);
13951396
let lowered_value = lower_hir_expr(hir, function_name, value, value_types);

crates/rsscript/src/lexer.rs

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,11 @@ pub enum TokenKind {
1010
MultilineString(String),
1111
Keyword(&'static str),
1212
Symbol(&'static str),
13+
/// A source character outside RSScript's lexical inventory. Kept as a token
14+
/// (rather than silently mapped to a valid operator) so the parser surfaces it
15+
/// as unsupported syntax instead of, e.g., turning a stray `©` into the `?`
16+
/// try operator.
17+
Unknown(char),
1318
Eof,
1419
}
1520

@@ -29,6 +34,7 @@ impl Token {
2934
| TokenKind::InterpolatedString(value)
3035
| TokenKind::MultilineString(value) => value.clone(),
3136
TokenKind::Keyword(value) | TokenKind::Symbol(value) => (*value).to_string(),
37+
TokenKind::Unknown(ch) => ch.to_string(),
3238
TokenKind::Eof => "<eof>".to_string(),
3339
}
3440
}
@@ -337,7 +343,8 @@ impl Lexer<'_> {
337343

338344
fn push_one(&mut self) {
339345
let span = self.span(1);
340-
let symbol = match self.bump().unwrap() {
346+
let ch = self.bump().unwrap();
347+
let symbol = match ch {
341348
':' => ":",
342349
',' => ",",
343350
'.' => ".",
@@ -363,7 +370,16 @@ impl Lexer<'_> {
363370
'!' => "!",
364371
';' => ";",
365372
'#' => "#",
366-
_ => "?",
373+
// Not in the lexical inventory: keep the character verbatim as an
374+
// `Unknown` token so the parser reports it, instead of silently
375+
// aliasing it to the `?` try operator.
376+
_ => {
377+
self.tokens.push(Token {
378+
kind: TokenKind::Unknown(ch),
379+
span,
380+
});
381+
return;
382+
}
367383
};
368384
self.tokens.push(Token {
369385
kind: TokenKind::Symbol(symbol),

crates/rsscript/src/reg_vm/exec.rs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,11 @@ impl RegVm {
101101
self.limits.step_budget.is_none()
102102
&& self.limits.cancel.is_none()
103103
&& self.limits.mem_budget.is_none()
104+
// Native code runs whole-function without routing intrinsic dispatch
105+
// through `charge_host_call`, so an armed `host_call_budget` must also
106+
// force the interpreter/tier-0 path — otherwise the host-call cap is
107+
// silently unenforced once a function tiers up to native.
108+
&& self.limits.host_call_budget.is_none()
104109
}
105110

106111
/// Charge one instruction against the step budget. Always increments the
@@ -767,7 +772,16 @@ impl RegVm {
767772
);
768773
}
769774
RegInstr::Return { src } => {
770-
return Ok(PureStep::Return(self.take_reg(base + *src)));
775+
// Clone rather than move the return value out of its register. A
776+
// `mut` parameter register can be BOTH the return source and a
777+
// `mut_writeback` target (`fn f(i: mut Int) -> Int { return i }`):
778+
// moving would clear the register, so the writeback that runs after
779+
// the frame pops would read an uninitialized slot (panic) or, worse,
780+
// write `Unit` back to the caller. Cloning leaves the register live
781+
// for `apply_mut_writeback`; the leftover copy is dropped when the
782+
// frame window is reused (see `prepare_frame`). The extra clone is a
783+
// scalar/`Rc` bump on the interpreter-tier return path only.
784+
return Ok(PureStep::Return(self.reg(base + *src).clone()));
771785
}
772786
// Anything else is outside the pure subset; the caller handles it.
773787
_ => return Ok(PureStep::NotPure),

crates/rsscript/src/reg_vm/intrinsics/string.rs

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -114,16 +114,21 @@ impl RegVm {
114114
Ok(VmValue::Int(value.len() as i64))
115115
}
116116
RegIntrinsic::StringPadLeft => {
117-
let value = expect_string_ref(intrinsic_arg(&self.stack, base, args, 0)?)?;
118117
let width = expect_int_ref(intrinsic_arg(&self.stack, base, args, 1)?)?;
119-
let fill = expect_string_ref(intrinsic_arg(&self.stack, base, args, 2)?)?;
120-
Ok(VmValue::string(string_pad(value, width, fill, true)))
118+
let value = expect_string_ref(intrinsic_arg(&self.stack, base, args, 0)?)?.to_owned();
119+
let fill = expect_string_ref(intrinsic_arg(&self.stack, base, args, 2)?)?.to_owned();
120+
// Charge the (size-parameterized) result against `mem_budget` BEFORE
121+
// allocating, so `pad_*` cannot allocate an arbitrarily large string
122+
// in one step and bypass the memory ceiling.
123+
self.account_bytes(value.len().max(width.max(0) as usize))?;
124+
Ok(VmValue::string(string_pad(&value, width, &fill, true)))
121125
}
122126
RegIntrinsic::StringPadRight => {
123-
let value = expect_string_ref(intrinsic_arg(&self.stack, base, args, 0)?)?;
124127
let width = expect_int_ref(intrinsic_arg(&self.stack, base, args, 1)?)?;
125-
let fill = expect_string_ref(intrinsic_arg(&self.stack, base, args, 2)?)?;
126-
Ok(VmValue::string(string_pad(value, width, fill, false)))
128+
let value = expect_string_ref(intrinsic_arg(&self.stack, base, args, 0)?)?.to_owned();
129+
let fill = expect_string_ref(intrinsic_arg(&self.stack, base, args, 2)?)?.to_owned();
130+
self.account_bytes(value.len().max(width.max(0) as usize))?;
131+
Ok(VmValue::string(string_pad(&value, width, &fill, false)))
127132
}
128133
RegIntrinsic::StringParseFloat => {
129134
let value = expect_string_ref(intrinsic_arg(&self.stack, base, args, 0)?)?;
@@ -140,8 +145,12 @@ impl RegVm {
140145
})
141146
}
142147
RegIntrinsic::StringRepeat => {
143-
let value = expect_string_ref(intrinsic_arg(&self.stack, base, args, 0)?)?;
144148
let count = nonnegative_count(intrinsic_arg(&self.stack, base, args, 1)?)?;
149+
let value = expect_string_ref(intrinsic_arg(&self.stack, base, args, 0)?)?.to_owned();
150+
// Charge the projected result against `mem_budget` BEFORE allocating,
151+
// so `"x".repeat(2_000_000_000)` trips the memory ceiling instead of
152+
// eagerly allocating ~2 GB in a single intrinsic step.
153+
self.account_bytes(value.len().saturating_mul(count))?;
145154
Ok(VmValue::string(value.repeat(count)))
146155
}
147156
RegIntrinsic::StringReplace => {

0 commit comments

Comments
 (0)