Skip to content

Commit 2909417

Browse files
Haofeiclaude
andcommitted
fix(rss): resolve 10 audited compiler bugs (RSS-1…RSS-10)
Verified every item in BUGS.md against a fresh build and fixed all ten correctness/soundness bugs. Each fix is confirmed across eval, run, and run --release; the full cargo test suite stays green and the rss self-test suite shows no new failures. - RSS-1 prefix !/~ now bind tighter than binary ops (parser) - RSS-2 release AOT traps on integer overflow; Math.pow uses checked_pow - RSS-3 clone-coerce borrowed RHS on assignment and let aliases (lowerer) - RSS-4 infer Option<T>/Result<…> for enum-variant constructors (hir) - RSS-5 root turbofish ctor name so named-field path is taken (lowerer) - RSS-6 shifts get their own precedence tier between comparison/additive - RSS-7 reject malformed number literals (1.2.3, 5.) in the lexer - RSS-8 require matching numeric roots in arithmetic/relational checks - RSS-9 VM compares floats with IEEE == (matches AOT), not bitwise - RSS-10 generic ctor return type carries its params so they substitute RSS-11 (LOW, perf-only) is confirmed valid but deferred; rationale and re-audit notes are recorded in BUGS.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent ff9b7d1 commit 2909417

9 files changed

Lines changed: 204 additions & 52 deletions

File tree

BUGS.md

Lines changed: 40 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,20 @@ Found by a white-box audit of the compiler on **2026-06-10**, reproduced against
44
release binary built from commit `f5fab92` (`target/release/rss`). Every item below was
55
reproduced; the HIGH items were additionally re-verified by hand.
66

7+
> **Status (2026-06-10, re-audit + fix):** all 11 items were re-verified against a fresh
8+
> build. **RSS-1 … RSS-10 are fixed** and confirmed across `eval`, `run`, and
9+
> `run --release`, with the full `cargo test` suite still green. **RSS-11** (LOW,
10+
> compile-time perf only) is confirmed valid but **deferred** — see its entry.
11+
>
12+
> Repro notes found during the re-audit:
13+
> - **RSS-5 / RSS-10:** the original single-line struct bodies (`{ first: T second: T }`)
14+
> additionally tripped a field-separator quirk that masked the documented symptom; with
15+
> idiomatic newline-separated fields both bugs reproduce exactly as described and are fixed.
16+
> - Separately discovered (not one of the 11, left open): matching on a borrowed
17+
> `read Option<T>`/`Result<…>` param and using the bound payload by value
18+
> (`match o { Some(s) => return s }`) lowers to `&T` and fails rustc E0308. Pre-existing
19+
> at `f5fab92`; independent of the RSS-4 fix.
20+
721
Run recipe used for all repros:
822

923
```sh
@@ -24,7 +38,7 @@ precedence trees, or borrow/clone coercion on assignment — which is where ever
2438

2539
## HIGH
2640

27-
### RSS-1 — Prefix `!` and `~` bind looser than every binary operator (silent wrong result)
41+
### RSS-1 — [FIXED]Prefix `!` and `~` bind looser than every binary operator (silent wrong result)
2842
- **Class:** wrong result (no error; both backends agree on the wrong parse)
2943
- **Source:** `crates/rsscript/src/syntax/parser.rs:2464` (`parse_unary_expr` is tried before
3044
`parse_binary_expr`), `:2960-2968` (`unary_operand_range` extends the operand to the rest of
@@ -47,7 +61,7 @@ fn main() -> Unit {
4761
parens gives the right answer, proving the mis-grouping.
4862
- **Fix:** give prefix `!`/`~` precedence above all binary operators.
4963

50-
### RSS-2 — Integer overflow silently wraps in release AOT but traps everywhere else
64+
### RSS-2 — [FIXED]Integer overflow silently wraps in release AOT but traps everywhere else
5165
- **Class:** soundness / cross-backend divergence
5266
- **Source:** `crates/rsscript/src/rust_lower/lowerer.rs` Add/Sub/Mul emit bare `+`/`-`/`*`; the
5367
generated release profile sets no `overflow-checks = true`. Sibling: `crates/runtime/src/math.rs`
@@ -71,7 +85,7 @@ fn main() -> Unit {
7185
- **Fix:** lower `+ - *` to `wrapping_*`/`checked_*` (as `<<` already uses `wrapping_shl`), or set
7286
`overflow-checks = true` in the generated release profile; route `Math.pow` through `checked_pow`.
7387

74-
### RSS-3 — `read`-param / borrowed RHS not cloned on assignment → non-compiling Rust (E0308)
88+
### RSS-3 — [FIXED]`read`-param / borrowed RHS not cloned on assignment → non-compiling Rust (E0308)
7589
- **Class:** miscompile (valid program won't compile under AOT)
7690
- **Source:** `crates/rsscript/src/rust_lower/lowerer.rs:1865-1869` (`Stmt::Assign` emits
7791
`target = lower_expr(value)` with no clone coercion; the `let`-init path at `:1601-1613` *does*
@@ -99,7 +113,7 @@ fn main() -> Unit {
99113
- **Fix:** apply the same `.clone()` coercion in `Stmt::Assign` lowering; also clone when the RHS is
100114
a read-view alias (`:3922`).
101115

102-
### RSS-4 — Untyped `Some(...)` / `Ok(...)` local bypasses argument type checking (false-accept)
116+
### RSS-4 — [FIXED]Untyped `Some(...)` / `Ok(...)` local bypasses argument type checking (false-accept)
103117
- **Class:** soundness (false-accept → backend E0308)
104118
- **Source:** `crates/rsscript/src/hir.rs:2431-2459` (`infer_hir_expr_type``None` for
105119
`CallResolution::EnumVariant`), `:1422-1446` (`Stmt::Let` records `type_name=None`),
@@ -124,7 +138,7 @@ fn main() -> Unit {
124138
- **Fix:** infer `Option<Int>` / `Result<Int,_>` for enum-variant constructors so the local carries
125139
a type.
126140

127-
### RSS-5 — Turbofish struct constructor miscompiles to a positional tuple-struct call (E0423)
141+
### RSS-5 — [FIXED]Turbofish struct constructor miscompiles to a positional tuple-struct call (E0423)
128142
- **Class:** miscompile
129143
- **Source:** `crates/rsscript/src/rust_lower/lowerer.rs:2342` (named-field path looked up via the
130144
raw callee string), fall-through at `:2646`; `type_kinds` keyed by bare name at `:58`.
@@ -149,33 +163,33 @@ fn main() -> Unit {
149163

150164
## MEDIUM
151165

152-
### RSS-6 — `<<` / `>>` share the comparison precedence tier
166+
### RSS-6 — [FIXED]`<<` / `>>` share the comparison precedence tier
153167
- **Source:** `crates/rsscript/src/syntax/parser.rs:2883-2891` (`ShiftLeft`/`ShiftRight` grouped
154168
with `Equal`/`Less`/`Greater` in `find_top_level_operator`).
155169
- `4 == 1 << 2` parses as `(4 == 1) << 2` (shift on a bool). In C/Rust shift binds tighter than
156170
comparison, so the correct parse is `4 == (1 << 2)` = `4 == 4` = true. `check` passes; both
157171
backends then fail (`no method named wrapping_shl for type bool`, rustc E0599). False-reject.
158172
- **Fix:** move `<<`/`>>` to a tier between additive and relational.
159173

160-
### RSS-7 — Lexer accepts malformed multi-dot number literals (`1.2.3`, `5.`)
174+
### RSS-7 — [FIXED]Lexer accepts malformed multi-dot number literals (`1.2.3`, `5.`)
161175
- **Source:** `crates/rsscript/src/lexer.rs:114-134` (`lex_number` consumes any run of digit-or-`.`).
162176
- `1.2.3` and `5.` tokenize as one Float; `check` reports `ok`; the lowerer emits the verbatim
163177
token `let x = 1.2.3;` → rustc E0610. False-accept that defers to a confusing backend error.
164178
- **Fix:** reject more than one `.` (and trailing `.`) in `lex_number`.
165179

166-
### RSS-8 — Mixed numeric operands skip operand-type checking
180+
### RSS-8 — [FIXED]Mixed numeric operands skip operand-type checking
167181
- **Source:** `crates/rsscript/src/checks/forbidden.rs:377-381` (empty arm), `:341-358`.
168182
- `Float + Int` and `Float < Int` are not type-checked (the `==` path correctly rejects), so they
169183
pass `check` then fail the backend with E0277/E0308.
170184
- **Fix:** require matching numeric roots in the arithmetic/relational arms.
171185

172-
### RSS-9 — reg_vm compares floats bitwise, diverging from AOT's IEEE `==`
186+
### RSS-9 — [FIXED]reg_vm compares floats bitwise, diverging from AOT's IEEE `==`
173187
- **Source:** `crates/rsscript/src/.../vm_value.rs:308` (Float `PartialEq` via `to_bits`).
174188
- `NaN == NaN` → true and `0.0 == -0.0` → false in the interpreter; AOT uses IEEE `==` and gives
175189
the opposite. A real interp-vs-compiled divergence.
176190
- **Fix:** use IEEE `==` in the Float equality arm.
177191

178-
### RSS-10 — Generic struct constructor drops its type argument
192+
### RSS-10 — [FIXED]Generic struct constructor drops its type argument
179193
- **Source:** `crates/rsscript/src/hir.rs:3391`, `:2483` (`constructor_sig_from_type` returns the
180194
bare `"Wrap"`).
181195
- `let w: Wrap<Int> = Wrap(item: 7)` is wrongly rejected (RS0207) because the constructor's return
@@ -186,9 +200,19 @@ fn main() -> Unit {
186200

187201
## LOW
188202

189-
### RSS-11 — ~O(n³) `check` blowup on deeply nested generics (compile-time DoS surface)
190-
- **Source:** `crates/rsscript/src/checks/calls.rs:2052-2142` (repeated string re-parsing of type
191-
names).
192-
- Nested generics (`List<…<Int>>`) make `rss check` super-linear (depth 500 ≈ 9–21s, depth 2000
193-
times out). Not a correctness fault.
194-
- **Fix:** parse type strings once / memoize substitutions.
203+
### RSS-11 — [VALID · DEFERRED]~O(n³) `check` blowup on deeply nested generics (compile-time DoS surface)
204+
- **Source:** `crates/rsscript/src/checks/calls.rs:2052-2142` (`collect_type_param_substitutions`
205+
and `substitute_type_params` re-parse the full type string at every nesting level).
206+
- Nested generics passed to a **generic** function (e.g. `fn id<T>(x: read T) -> T` called with
207+
`List<…<Int>>`) make `rss check` super-linear. Re-measured on this machine: depth 100 ≈ 21 ms,
208+
300 ≈ 74 ms, 500 ≈ 283 ms, 800 ≈ 1.1 s, 2000 ≈ 16.8 s. Confirms the ~cubic shape. (The original
209+
repro used a *non-generic* `id`, which never enters the substitution recursion and stays flat —
210+
the trigger requires a generic callee.) Not a correctness fault.
211+
- **Fix:** parse type strings into a tree once / memoize substitutions.
212+
- **Status — deferred (intentional):** the asymptotic fix means replacing the string-based type
213+
representation in the generic-substitution path with a parse-once tree — a sizeable refactor of
214+
correctness-critical, heavily-relied-upon generics code. The trigger is adversarial, self-authored
215+
source (the compiler only processes local source, so there is no remote DoS vector), real code
216+
never nests generics more than a handful of levels, and the item is rated LOW / non-correctness.
217+
The risk of regressing generics resolution was judged disproportionate to the benefit, so the fix
218+
is left for a dedicated change rather than bundled with the RSS-1…RSS-10 correctness fixes.

crates/rsscript/src/checks/forbidden.rs

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,17 @@ fn check_builtin_operator_operand_types(
354354
&right_type,
355355
"numeric operands",
356356
);
357+
} else if type_root_name(&left_type) != type_root_name(&right_type) {
358+
// Both numeric but different roots (e.g. `Float < Int`): the backend
359+
// rejects the mixed comparison, so reject it here too.
360+
operator_type_mismatch_diagnostic(
361+
analyzer,
362+
span.clone(),
363+
operator_label(op),
364+
&left_type,
365+
&right_type,
366+
"matching operand types",
367+
);
357368
}
358369
}
359370
BinaryOp::LogicalAnd | BinaryOp::LogicalOr => {
@@ -378,7 +389,30 @@ fn check_builtin_operator_operand_types(
378389
| BinaryOp::Subtract
379390
| BinaryOp::Multiply
380391
| BinaryOp::Divide
381-
| BinaryOp::Modulo => {}
392+
| BinaryOp::Modulo => {
393+
// Non-numeric operands are already rejected by the operator-overload
394+
// check; here catch mixed numeric roots (e.g. `Float + Int`), which
395+
// otherwise pass `check` and then fail the backend with E0277/E0308.
396+
let (Some(left_type), Some(right_type)) = (
397+
inferred_operand_type(analyzer, left).map(str::to_string),
398+
inferred_operand_type(analyzer, right).map(str::to_string),
399+
) else {
400+
return;
401+
};
402+
if is_numeric_type(&left_type)
403+
&& is_numeric_type(&right_type)
404+
&& type_root_name(&left_type) != type_root_name(&right_type)
405+
{
406+
operator_type_mismatch_diagnostic(
407+
analyzer,
408+
span.clone(),
409+
operator_label(op),
410+
&left_type,
411+
&right_type,
412+
"matching operand types",
413+
);
414+
}
415+
}
382416
BinaryOp::BitAnd
383417
| BinaryOp::BitOr
384418
| BinaryOp::BitXor

crates/rsscript/src/hir.rs

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2402,6 +2402,31 @@ fn hir_binding_kind(kind: LetKind) -> HirBindingKind {
24022402
}
24032403
}
24042404

2405+
/// Infer the type of a built-in `Option`/`Result` variant constructor call so an
2406+
/// untyped local (`let o = Some(5)`) carries a type and downstream argument checks
2407+
/// are not silently skipped. The variant's known payload position is filled from the
2408+
/// argument; the other generic position (e.g. the error type of `Ok`) is left as a
2409+
/// single-uppercase placeholder so `unresolved_generic_type` skips it rather than
2410+
/// reporting a spurious mismatch.
2411+
fn infer_enum_variant_type(
2412+
hir: &Hir,
2413+
variant: &str,
2414+
args: &[crate::syntax::ast::CallArg],
2415+
value_types: &HashMap<String, String>,
2416+
) -> Option<String> {
2417+
let payload_type = |args: &[crate::syntax::ast::CallArg]| {
2418+
args.first()
2419+
.and_then(|arg| infer_hir_expr_type(hir, &arg.value, value_types))
2420+
};
2421+
match variant {
2422+
"Some" => Some(format!("Option<{}>", payload_type(args)?)),
2423+
"Ok" => Some(format!("Result<{}, E>", payload_type(args)?)),
2424+
"Err" => Some(format!("Result<T, {}>", payload_type(args)?)),
2425+
// `None` and bare `Option`/`Result` carry no inferable payload type.
2426+
_ => None,
2427+
}
2428+
}
2429+
24052430
pub(crate) fn infer_hir_expr_type(
24062431
hir: &Hir,
24072432
expr: &Expr,
@@ -2454,7 +2479,9 @@ pub(crate) fn infer_hir_expr_type(
24542479
.map(str::to_string),
24552480
Callee::Qualified { .. } | Callee::ReceiverCall { .. } => None,
24562481
},
2457-
CallResolution::EnumVariant => None,
2482+
CallResolution::EnumVariant => {
2483+
infer_enum_variant_type(hir, callee_name(callee), args, value_types)
2484+
}
24582485
}
24592486
}
24602487
Expr::Field { base, name, .. } => {
@@ -3388,7 +3415,16 @@ fn constructor_sig_from_type(type_info: &TypeInfo, is_builtin: bool) -> Function
33883415
type_name: field.type_name.clone(),
33893416
})
33903417
.collect(),
3391-
return_type: Some(type_info.name.clone()),
3418+
// A generic struct's constructor returns the type *applied to its params*
3419+
// (`Wrap<T>`), not the bare name (`Wrap`). Carrying the params lets
3420+
// `infer_signature_return_type` substitute them from the arguments
3421+
// (`Wrap(item: 7)` -> `Wrap<Int>`); a bare name leaves nothing to
3422+
// substitute and spuriously rejects `let w: Wrap<Int> = Wrap(item: 7)`.
3423+
return_type: Some(if type_info.type_params.is_empty() {
3424+
type_info.name.clone()
3425+
} else {
3426+
format!("{}<{}>", type_info.name, type_info.type_params.join(", "))
3427+
}),
33923428
returns_fresh: type_info.kind == HirTypeKind::Struct,
33933429
effects: Vec::new(),
33943430
retained_params: HashSet::new(),

crates/rsscript/src/lexer.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,12 +115,21 @@ impl Lexer<'_> {
115115
let start_line = self.line;
116116
let start_column = self.column;
117117
let start_index = self.index;
118-
while self
119-
.peek()
120-
.is_some_and(|ch| ch.is_ascii_digit() || ch == '.')
121-
{
118+
// Integer part.
119+
while self.peek().is_some_and(|ch| ch.is_ascii_digit()) {
122120
self.bump();
123121
}
122+
// At most one fractional part, and only when a digit follows the `.`.
123+
// This rejects malformed literals like `5.` (trailing dot) and `1.2.3`
124+
// (multiple dots), which previously lexed as a single Float token and
125+
// lowered to invalid Rust. The remaining `.`/digits are lexed separately
126+
// (as a field-access `.` or a new number), so the parser reports them.
127+
if self.peek() == Some('.') && self.peek_next().is_some_and(|ch| ch.is_ascii_digit()) {
128+
self.bump();
129+
while self.peek().is_some_and(|ch| ch.is_ascii_digit()) {
130+
self.bump();
131+
}
132+
}
124133
let value = self.chars[start_index..self.index].iter().collect();
125134
self.tokens.push(Token {
126135
kind: TokenKind::Number(value),

0 commit comments

Comments
 (0)