From d56d6dbebb67c525975df738badefa87b63af03a Mon Sep 17 00:00:00 2001 From: Nick Nassiri Date: Sat, 4 Jul 2026 14:03:49 -0700 Subject: [PATCH 1/2] fix(compile): make the fs facade emit IL-verifiable methods (#1246) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Compiling any program that imports `fs` produced 68 IL verification errors in the emitted facade module. The program ran (the JIT tolerates the mismatches), but `--compile … --verify` could not gate any fs-using program. Two shapes, both a value stored into a narrower slot without the bridge the verifier requires: 1. `object` → `string`: a $M_fs helper stages an `any`-typed argument (e.g. `__joinPath(src, names[i])` in cpSync) into a `string` parameter temp, but `EmitConversionForParameter` handled only value-type targets (unbox) and fell through for reference targets, leaving `object` where `string` was expected. Fix: emit `castclass` on string parameter coercion. Restricted to `string` — the one non-object reference slot whose runtime value is a genuine CLR instance; delegates ($TSFunction), collections ($Array), and Task ($Promise) keep their prior object store. 2. `$Promise` → `Task`: the promise primitives return a wrapped `$Promise`, which the callback helpers (`__cbData`/`__cbVoid`) took in a `Promise` → `Task` parameter slot. `ParameterTypeResolver` widens other unsound slots to object but never the non-async `Promise` parameter case — the symmetric gap to the return-slot widening #393 already applies. Fix: widen `Promise` (→ Task/Task) parameter slots to object in `CoerceParamSlotType`. Adds regression tests: the whole-fs-facade verify (the issue repro) plus a minimal plain-TS repro of each class. xUnit 15415/0, Test262 22/0/4skip, TSConf 31/0, --verify clean. --- .../ExpressionEmitterBase.CallHelpers.cs | 20 +++++- Compilation/ILEmitter.Helpers.cs | 20 +++++- Compilation/ParameterTypeResolver.cs | 19 ++++++ .../CompilerTests/ILVerificationTests.cs | 64 +++++++++++++++++++ 4 files changed, 119 insertions(+), 4 deletions(-) diff --git a/Compilation/ExpressionEmitterBase.CallHelpers.cs b/Compilation/ExpressionEmitterBase.CallHelpers.cs index 53f9c2ea..6f9d4497 100644 --- a/Compilation/ExpressionEmitterBase.CallHelpers.cs +++ b/Compilation/ExpressionEmitterBase.CallHelpers.cs @@ -2361,9 +2361,25 @@ protected void EmitConversionForParameter(Expr expr, Type targetType) } // If target is a value type and we have an object, unbox - if (targetType.IsValueType && StackType != StackType.Double && StackType != StackType.Boolean) + if (targetType.IsValueType) { - IL.Emit(OpCodes.Unbox_Any, targetType); + if (StackType != StackType.Double && StackType != StackType.Boolean) + IL.Emit(OpCodes.Unbox_Any, targetType); + return; + } + + // Target is a non-object reference type. EmitExpression can leave a broader `object` on the + // stack — an `any`-typed value, or a GetIndex/GetProperty result — that the callee's typed + // string slot would reject at IL verification even though the JIT tolerates it. Emit the + // downcast the verifier needs (#1246). Only `string` qualifies: it is the one non-object + // reference slot whose runtime value is genuinely a CLR instance of the slot. Other reference + // targets carry a boxed-differently runtime value ($TSFunction for a Delegate, $Array for a + // List, $Promise for a Task — the last widened to `object` at the parameter slot by + // ParameterTypeResolver), so a castclass would throw; they keep their prior object store. + if (Types.IsString(targetType) && StackType != StackType.String) + { + EnsureBoxed(); + IL.Emit(OpCodes.Castclass, targetType); } } diff --git a/Compilation/ILEmitter.Helpers.cs b/Compilation/ILEmitter.Helpers.cs index 2a6018c7..d9657e61 100644 --- a/Compilation/ILEmitter.Helpers.cs +++ b/Compilation/ILEmitter.Helpers.cs @@ -276,9 +276,25 @@ or TypeInfo.Void } // If target is a value type and we have an object, unbox - if (targetType.IsValueType && _stackType != StackType.Double && _stackType != StackType.Boolean) + if (targetType.IsValueType) { - IL.Emit(OpCodes.Unbox_Any, targetType); + if (_stackType != StackType.Double && _stackType != StackType.Boolean) + IL.Emit(OpCodes.Unbox_Any, targetType); + return; + } + + // Target is a non-object reference type. EmitExpression can leave a broader `object` on the + // stack — an `any`-typed value, or a GetIndex/GetProperty result — that the callee's typed + // string slot would reject at IL verification even though the JIT tolerates it. Emit the + // downcast the verifier needs (#1246). Only `string` qualifies: it is the one non-object + // reference slot whose runtime value is genuinely a CLR instance of the slot. Other reference + // targets carry a boxed-differently runtime value ($TSFunction for a Delegate, $Array for a + // List, $Promise for a Task — the last widened to `object` at the parameter slot by + // ParameterTypeResolver), so a castclass would throw; they keep their prior object store. + if (_ctx.Types.IsString(targetType) && _stackType != StackType.String) + { + EmitBoxIfNeeded(expr); + IL.Emit(OpCodes.Castclass, targetType); } } diff --git a/Compilation/ParameterTypeResolver.cs b/Compilation/ParameterTypeResolver.cs index e5d0e3ae..7c321cf1 100644 --- a/Compilation/ParameterTypeResolver.cs +++ b/Compilation/ParameterTypeResolver.cs @@ -163,9 +163,28 @@ private static Type CoerceParamSlotType(Type mapped, TSTypeInfo source, TypeMapp return typeof(object); if (UnionAdmitsUndefined(source)) return typeof(object); + if (IsPromiseTaskSlot(mapped)) + return typeof(object); return mapped; } + /// + /// True when is a CLR Task/Task<T> produced by + /// strictly mapping a Promise<T> annotation. A parameter so typed receives the runtime + /// $Promise object at the call boundary, never a real CLR Task (the promise + /// primitives return $Promise, and no path constructs a bare Task guest value), so + /// the slot must widen to object. Mirrors the identical Promise<T> → object + /// widening already applied to non-async return slots in (#393); + /// its absence on parameters left the fs facade callback helpers emitting a $Promise + /// into a Task<object> slot, which the JIT tolerates but IL verification rejects + /// (#1246). The widening is unconditional — even an async function's Promise<T> param + /// receives a passed-in $Promise; only the enclosing function's return builds a real + /// Task. + /// + private static bool IsPromiseTaskSlot(Type mapped) => + mapped == typeof(Task) || + (mapped.IsGenericType && mapped.GetGenericTypeDefinition() == typeof(Task<>)); + /// /// True when is a union one of whose members is the undefined /// type, so its runtime values include the $Undefined sentinel. diff --git a/SharpTS.Tests/CompilerTests/ILVerificationTests.cs b/SharpTS.Tests/CompilerTests/ILVerificationTests.cs index 6142e32b..2e5727b4 100644 --- a/SharpTS.Tests/CompilerTests/ILVerificationTests.cs +++ b/SharpTS.Tests/CompilerTests/ILVerificationTests.cs @@ -837,6 +837,70 @@ async function main() { Assert.Empty(errors); } + [Fact] + public void FsFacadeImport_PassesILVerification() + { + // #1246: importing `fs` emitted 68 unverifiable methods across the facade. Two shapes, + // both a value stored into a narrower slot without the verifier-required bridge: + // (1) `object` → `string` — a $M_fs helper stages an `any`-typed argument (e.g. + // `__joinPath(src, names[i])` in cpSync) into a string parameter temp with no castclass; + // (2) `$Promise` → `Task` — the promise primitives return a wrapped $Promise which + // the callback helpers (`__cbData`/`__cbVoid`) took in a `Promise` → Task + // parameter slot. Fixed by (1) a castclass on string parameter coercion and (2) widening + // non-async Promise parameter slots to object (symmetric to #393's return-slot fix). + // Verifies the whole facade — every $M_fs_* method is emitted regardless of what is used. + var files = new Dictionary + { + ["main.ts"] = """ + import * as fs from 'fs'; + console.log(fs.statSync('.').isDirectory()); + """ + }; + + var errors = TestHarness.CompileModulesAndVerifyOnly(files, "main.ts"); + + Assert.Empty(errors); + } + + [Fact] + public void ObjectArgumentToStringParameter_PassesILVerification() + { + // #1246 class 1, minimal repro: an `any`-typed value (here a bracket read off an `any` + // array, which the emitter types as `object`) passed to a `string` parameter left the + // object on the stack where the callee's string slot was expected — no castclass. The + // JIT tolerated it; ILVerify reported StackUnexpected {Found object, Expected string}. + var source = """ + function join(a: string, b: string): string { return a + "/" + b; } + const names: any = ["x", "y"]; + console.log(join("d", names[0])); + """; + + var (errors, output) = TestHarness.CompileVerifyAndRun(source); + + Assert.Empty(errors); + Assert.Equal("d/x\n", output); + } + + [Fact] + public void PromiseValueToPromiseParameter_PassesILVerification() + { + // #1246 class 2, minimal repro: a runtime $Promise (from Promise.resolve / new Promise — + // not a real CLR Task, unlike an async function's result) passed to a `Promise` + // parameter, whose slot mapped to Task. Storing the $Promise into the Task + // slot ran fine but failed ILVerify {Found $Promise, Expected Task}. The parameter + // slot is now widened to object. + var source = """ + function consume(p: Promise, cb: any): void { p.then((v: any) => cb(v)); } + consume(Promise.resolve(5), (v: any) => console.log(v)); + consume(new Promise((res: any) => res(7)), (v: any) => console.log(v)); + """; + + var (errors, output) = TestHarness.CompileVerifyAndRun(source); + + Assert.Empty(errors); + Assert.Equal("5\n7\n", output); + } + [Fact] public void ClassGetPropertyWithTypedGetter_PassesILVerification() { From 8114f8795cb91270e68f9070645f62c3afce8769 Mon Sep 17 00:00:00 2001 From: Nick Nassiri Date: Sat, 4 Jul 2026 14:31:06 -0700 Subject: [PATCH 2/2] fix(process): isolate uptime() script-start baseline per thread (flaky CI) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Process_Uptime_IncreasesOverTime(mode: Interpreted) intermittently failed on CI with `up2 >= up1` == false — uptime() ran backwards. Root cause is independent of this branch's IL-compiler changes (it's the interpreter path): `uptime()` returns `now - (_scriptStartTimestamp ?? _startTimestamp)`, and `_scriptStartTimestamp` was a *shared* mutable static that the in-process test runner toggles per run (SetScriptArguments sets a recent timestamp, ClearScriptArguments nulls it). Because xUnit parallelizes collections across one CLR process, a concurrent script's SetScriptArguments could land between another script's two uptime() reads: the first read used the null → process-start baseline (large elapsed), the second the just-set recent baseline (~0 elapsed), so uptime appeared to decrease. The earlier Stopwatch switch fixed clock monotonicity but not this baseline race. Fix: make `_scriptStartTimestamp` [ThreadStatic] so each script gets a stable per-thread baseline — same precedent as the ThreadArgv/ThreadEnv fields directly above it. Not a production bug: a real single-script process sets the baseline once on its own thread and has no concurrent script to clear it. Full suite green under the CI filter (Category!=LiveNetwork&Category!=LoadSensitive): 15408/0. --- Runtime/BuiltIns/ProcessBuiltIns.cs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/Runtime/BuiltIns/ProcessBuiltIns.cs b/Runtime/BuiltIns/ProcessBuiltIns.cs index 789415ad..dbdeffbf 100644 --- a/Runtime/BuiltIns/ProcessBuiltIns.cs +++ b/Runtime/BuiltIns/ProcessBuiltIns.cs @@ -59,7 +59,22 @@ public static partial class ProcessBuiltIns // Script start as a monotonic Stopwatch timestamp — reset each time a script // begins execution. When set, uptime() reports time since script start rather // than process start. - private static long? _scriptStartTimestamp; + // + // [ThreadStatic] because the in-process test runner shares this one CLR process + // across many concurrently-running scripts (xUnit parallelizes test collections): + // SetScriptArguments/ClearScriptArguments toggle this baseline (a recent timestamp + // ↔ null) on each run, and a *shared* field let one script's SetScriptArguments land + // between another script's two uptime() reads — the first read using the null → + // process-start baseline (large elapsed), the second the just-set recent baseline + // (near-zero elapsed) — so uptime() ran backwards and intermittently failed + // Process_Uptime_IncreasesOverTime (`up2 >= up1`). Per-thread isolation gives each + // script a stable baseline (same precedent as ThreadArgv/ThreadEnv above); the + // Stopwatch switch earlier fixed clock monotonicity but not this baseline race. In a + // real single-script process SetScriptArguments and the synchronous run share one + // thread, so script-relative uptime is preserved; an uptime() read on an async + // continuation thread falls back to the (process-start) baseline, differing only by + // the milliseconds between process and script start. + [ThreadStatic] private static long? _scriptStartTimestamp; /// /// Gets a member of the process object by name. Resolves process-specific