Fix review findings: JIT call soundness, CompiledId affinity, Result generic params, File.open effects

- vm-jit: NativeModule::call now rejects an args slice whose length != the compiled
  function's n_params (the generated entry block reads exactly n_params words from
  args_ptr without bounding by n_args, so a short slice was an out-of-bounds read in
  a 'safe' API) — #1, the highest-severity finding. CompiledId now carries a
  per-module id so an id from another NativeModule is rejected instead of indexing
  the wrong table (#2). Tests: call_rejects_wrong_arg_count, call_rejects_id_from_another_module.
- lowerer: builtin_generic_type_params mapped Result to [K,V] (Map's params); split
  so Result uses [T,E] — fixes silently-failing generic substitution for
  Result.map/map_error/and_then (#7). Unit-test guard added.
- stdlib/fs/file.rssi: File.open/open_read/open_write now carry effects(native) so
  host file-resource creation is review-visible in the signature, consistent with
  the handle reads/writes (#6).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: Haofei

crates/rsscript/src/rust_lower/lowerer.rs

@@ -5438,7 +5438,8 @@ fn builtin_generic_type_params(root: &str) -> Option<Vec<&'static str>> {
         | "Stream" | "Pipeline" => Some(vec!["T"]),
         "FalliblePipeline" => Some(vec!["T", "E"]),
         "Capability" => Some(vec!["P"]),
-        "Map" | "Result" => Some(vec!["K", "V"]),
+        "Map" => Some(vec!["K", "V"]),
+        "Result" => Some(vec!["T", "E"]),
         _ => None,
     }
 }
@@ -5572,3 +5573,21 @@ fn lower_const_value(expr: &Expr) -> String {
 fn is_try_wrapped(expr: &Expr) -> bool {
     matches!(expr, Expr::Try { .. })
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn builtin_generic_type_params_use_each_type_s_own_param_names() {
+        // Regression: `Result` was mapped to `["K", "V"]` (Map's params), so the
+        // namespace/type-argument substitution path never bound `Result`'s real
+        // `T`/`E` params — weakening generic substitution for `Result.map` /
+        // `map_error` / `and_then`. Each type must use its own declared param names.
+        assert_eq!(builtin_generic_type_params("Map"), Some(vec!["K", "V"]));
+        assert_eq!(builtin_generic_type_params("Result"), Some(vec!["T", "E"]));
+        assert_eq!(builtin_generic_type_params("List"), Some(vec!["T"]));
+        assert_eq!(builtin_generic_type_params("Option"), Some(vec!["T"]));
+        assert_eq!(builtin_generic_type_params("NotAGeneric"), None);
+    }
+}

crates/vm-jit/src/lib.rs

@@ -308,14 +308,31 @@ fn declare_import(module: &mut JITModule, name: &str, n_args: usize) -> Result<F
         .map_err(|e| err("declare import", e))
 }
 
+/// A compiled function plus the metadata `call` needs to invoke it safely: the
+/// param count, so `call` can reject an argument slice of the wrong length (the
+/// generated entry block reads exactly `n_params` words from `args_ptr` and does
+/// not bound-check against `n_args`).
+struct CompiledFunc {
+    f: CompiledAbi,
+    n_params: usize,
+}
+
+/// Process-wide source of per-module identities, so a [`CompiledId`] minted by one
+/// [`NativeModule`] is rejected by another (it would otherwise index a different
+/// module's function table). Monotonic; wraparound is not a practical concern.
+static NEXT_MODULE_ID: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0);
+
 /// Owns the JIT-compiled machine code. Compiled functions live as long as the
 /// module, so callers keep this alive and invoke by [`CompiledId`].
 pub struct NativeModule {
     module: JITModule,
     ctx: Context,
     fbctx: FunctionBuilderContext,
-    funcs: Vec<CompiledAbi>,
+    funcs: Vec<CompiledFunc>,
     counter: u32,
+    /// Identity stamped into every [`CompiledId`] this module mints (see
+    /// [`NEXT_MODULE_ID`]).
+    id: u64,
     /// Declared host-helper imports (see [`HostHelpers`]).
     imports: HostFuncs,
 }
@@ -329,9 +346,14 @@ struct HostFuncs {
     list_get_int: FuncId,
 }
 
-/// Handle to a function compiled into a [`NativeModule`].
+/// Handle to a function compiled into a [`NativeModule`]. Carries the minting
+/// module's identity so it can't be used against a different module (which would
+/// index the wrong function table).
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
-pub struct CompiledId(usize);
+pub struct CompiledId {
+    module_id: u64,
+    index: usize,
+}
 
 impl NativeModule {
     pub fn new(helpers: HostHelpers) -> Result<Self, JitError> {
@@ -374,6 +396,7 @@ impl NativeModule {
             fbctx: FunctionBuilderContext::new(),
             funcs: Vec::new(),
             counter: 0,
+            id: NEXT_MODULE_ID.fetch_add(1, std::sync::atomic::Ordering::Relaxed),
             imports,
         })
     }
@@ -421,8 +444,14 @@ impl NativeModule {
         // SAFETY: `code` points at the machine code we just emitted with exactly
         // the `CompiledAbi` signature declared above.
         let f: CompiledAbi = unsafe { std::mem::transmute::<*const u8, CompiledAbi>(code) };
-        let handle = CompiledId(self.funcs.len());
-        self.funcs.push(f);
+        let handle = CompiledId {
+            module_id: self.id,
+            index: self.funcs.len(),
+        };
+        self.funcs.push(CompiledFunc {
+            f,
+            n_params: function.n_params as usize,
+        });
         Ok(handle)
     }
 
@@ -437,7 +466,19 @@ impl NativeModule {
     /// only `unsafe` is the indirect call through a pointer this module emitted with
     /// the matching ABI, with every pointer it passes derived from owned locals.
     pub fn call(&self, id: CompiledId, args: &[i64]) -> Option<i64> {
-        let f = self.funcs[id.0];
+        // Reject an id from a different module and an out-of-range index: either
+        // would invoke the wrong (or no) function. Falling back is always safe.
+        if id.module_id != self.id {
+            return None;
+        }
+        let func = self.funcs.get(id.index)?;
+        // The generated entry block reads exactly `n_params` words from `args_ptr`
+        // without consulting `n_args`, so an args slice shorter than `n_params`
+        // would read out of bounds. Reject any length mismatch and fall back.
+        if args.len() != func.n_params {
+            return None;
+        }
+        let f = func.f;
         let mut out: i64 = 0;
         BAIL_FLAG.with(|bail| {
             bail.set(0);
@@ -1384,4 +1425,45 @@ mod tests {
         ))
         .expect("well-formed heap read should validate");
     }
+
+    // fn(a, b) { return a + b } — a 2-param function for the call-guard tests.
+    fn two_param_add() -> JitFunction {
+        f(
+            2,
+            3,
+            vec![
+                JitInstr::Add {
+                    dst: 2,
+                    lhs: 0,
+                    rhs: 1,
+                },
+                JitInstr::Return { src: 2 },
+            ],
+        )
+    }
+
+    #[test]
+    fn call_rejects_wrong_arg_count() {
+        // The generated entry block reads exactly `n_params` words from `args_ptr`,
+        // so a short slice must be rejected by `call` (otherwise: out-of-bounds
+        // read). Both too-few and too-many fall back rather than misread memory.
+        let mut m = module();
+        let id = m.compile(&two_param_add()).unwrap();
+        assert_eq!(m.call(id, &[2, 3]), Some(5));
+        assert_eq!(m.call(id, &[2]), None); // too few — must not read past the slice
+        assert_eq!(m.call(id, &[]), None);
+        assert_eq!(m.call(id, &[2, 3, 4]), None); // too many
+    }
+
+    #[test]
+    fn call_rejects_id_from_another_module() {
+        // A `CompiledId` minted by one module indexes that module's table; using it
+        // against another module must be rejected, not silently mis-dispatched.
+        let mut m1 = module();
+        let mut m2 = module();
+        let id1 = m1.compile(&two_param_add()).unwrap();
+        let _id2 = m2.compile(&two_param_add()).unwrap();
+        assert_eq!(m1.call(id1, &[2, 3]), Some(5));
+        assert_eq!(m2.call(id1, &[2, 3]), None); // foreign id → fallback, no panic
+    }
 }

stdlib/fs/file.rssi

@@ -3,10 +3,13 @@ features: native
 resource File
 
 pub fn File.open(path: read Path) -> Result<File, FileError>
+    effects(native)
 
 pub fn File.open_read(path: read Path) -> Result<File, FileError>
+    effects(native)
 
 pub fn File.open_write(path: read Path) -> Result<File, FileError>
+    effects(native)
 
 pub fn File.exists(path: read Path) -> Bool
     effects(native)