fix: native-compile winston (ES5 .call(this) super-chain → 59/59 corpus) (#5368)

* fix(codegen): emit per-module install on native-module value-read property path

Reading a native-module callable export AS A VALUE (`const f = util.inherits`)
and invoking it indirectly dispatches through the per-module NM_DISPATCH_REGISTRY,
populated by `js_nm_install_<module>()`. The PropertyGet on a NativeModuleRef
(`util.inherits` value) lowered to `js_native_module_property_by_name` without
ever emitting the install. The *direct* call form (`util.inherits(a, b)`) is
statically lowered straight to the runtime extern and never touches the registry,
so a module reached ONLY through the value-read path left the registry empty and
the indirect call resolved to `undefined`.

This broke winston's `class Logger extends Transform` (readable-stream):
`require('inherits')(Transform, Duplex)` is an indirect `util.inherits` value-call.
With it skipped, the ES5 parent edges were never registered, so the nested
`Readable.call(this)` guard `if (!(this instanceof Readable))` saw false, returned
a discarded `new Readable()`, and `this._readableState.needReadable = true` threw
on null.

Fix: emit `js_nm_install_<module>()` before the `js_native_module_property_by_name`
call in the NativeModuleRef PropertyGet branch (property_get.rs).

Adds tests/test_native_module_value_read_install_codegen.sh (runtime + IR assertion).

* fix(stream): expose EventEmitter methods on legacy Stream.prototype for .call borrows

readable-stream's `Readable.prototype.on = function (ev, fn) { var res =
Stream.prototype.on.call(this, ev, fn); … }` (with `Stream = require('stream')`)
needs `Stream.prototype.on` to be EventEmitter.prototype.on. In Node
`require('stream')` IS the legacy Stream constructor; Perry models it as a
namespace object, so both `require('stream').prototype` and
`require('stream').Stream.prototype` lacked the EventEmitter methods —
`Stream.prototype.on` was undefined and the `.call` threw
"Function.prototype.call was called on a value that is not a function".

Two parts:
- install_event_emitter_prototype_methods() installs on/once/emit/
  removeListener/removeAllListeners/addListener/etc. on the legacy
  Stream.prototype AND the Readable/Writable/Duplex/Transform/PassThrough
  prototypes, as receiver-from-`this` values (slot-0 capture = TAG_UNDEFINED
  sentinel → this_value reads IMPLICIT_THIS, set by Function.prototype.call/
  apply). Existing instance-bound closures always capture a real object so are
  unaffected.
- get_native_module_constant resolves `require('stream').prototype` to the same
  legacy Stream.prototype the `.Stream` export carries.

Adds tests/test_stream_prototype_eventemitter_borrow.sh.

* fix(runtime): rebind this on inherited defineProperty(Class.prototype) getters

A getter installed via Object.defineProperty(Class.prototype, name, { get }) is
an ordinary method closure whose body reads this from its captured receiver
slot, not IMPLICIT_THIS. The inherited prototype-accessor walk
(inherited_proto_accessor_value) merely set IMPLICIT_THIS and called the closure
with js_closure_call0, so the getter observed the PROTOTYPE it lives on instead
of the instance — this.<ownField> came back undefined.

winston's Object.defineProperty(Logger.prototype, 'transports', { get() {
const { pipes } = this._readableState; … } }), read as this.transports inside
the Logger constructor, then threw 'Cannot convert undefined or null to object'
(this._readableState was undefined on the prototype).

Fix: route inherited_proto_accessor_value through invoke_accessor_getter, which
clones the getter closure with this rebound to the real receiver (and applies
strict/sloppy coercion), matching the own-accessor read path.

Adds tests/test_defineproperty_prototype_getter_this.sh.

---------

Co-authored-by: Ralph Küpper <ralph2@skelpo.com>

Author: proggeramlug

crates/perry-codegen/src/expr/property_get.rs

@@ -611,6 +611,20 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
                 let prop_idx = ctx.strings.intern(property);
                 let prop_bytes_global = format!("@{}", ctx.strings.entry(prop_idx).bytes_global);
                 let prop_len_str = property.len().to_string();
+                // The value read of a native-module callable export (`const f =
+                // util.inherits`) mints a BOUND_METHOD closure that, when invoked
+                // indirectly, dispatches through the per-module `NM_DISPATCH_REGISTRY`
+                // populated by `js_nm_install_<module>()`. The *direct* call form
+                // (`util.inherits(a, b)`) is statically lowered to the runtime extern
+                // and never touches the registry, so a module reached ONLY via this
+                // value-read path would leave the registry empty and the indirect call
+                // would resolve to `undefined` (winston/readable-stream's
+                // `require('inherits')` → `util.inherits` value → `inherits(Sub, Base)`
+                // silently skipped, breaking the ES5 super-chain). Emit the install
+                // here so the value-read path's later dispatch finds the module fn.
+                if let Some(install_sym) = crate::nm_install::nm_install_symbol(module_name) {
+                    ctx.block().call_void(install_sym, &[]);
+                }
                 return Ok(ctx.block().call(
                     DOUBLE,
                     "js_native_module_property_by_name",

crates/perry-runtime/src/node_stream.rs

@@ -148,7 +148,18 @@ fn this_value(closure: *const ClosureHeader) -> f64 {
     // host object value cast to i64; reverse the cast.
     if !closure.is_null() {
         let bits = js_closure_get_capture_ptr(closure, 0) as u64;
-        return f64::from_bits(bits);
+        // A `TAG_UNDEFINED` slot-0 marks a *prototype-method value* (e.g.
+        // `Stream.prototype.on`, installed by `attach_event_emitter_prototype_methods`)
+        // rather than an instance-bound method: it has no fixed receiver and must
+        // read the call-site `this` (set by `Function.prototype.call`/`apply` into
+        // IMPLICIT_THIS), so `Stream.prototype.on.call(streamInstance, ev, fn)`
+        // — readable-stream's `Readable.prototype.on` wrapper — registers the
+        // listener on the instance, not on the prototype. `build_object` always
+        // captures a real object pointer, so existing instance-bound closures
+        // never hit this branch.
+        if bits != crate::value::TAG_UNDEFINED {
+            return f64::from_bits(bits);
+        }
     }
     crate::object::js_implicit_this_get()
 }

crates/perry-runtime/src/node_stream_dispatch.rs

@@ -109,6 +109,42 @@ pub(super) fn install_methods_on_existing_object(
     }
 }
 
+/// Install the EventEmitter prototype methods (`on`/`once`/`emit`/
+/// `removeListener`/`removeAllListeners`/…) on a *prototype* object as named
+/// own properties, each a closure that reads its receiver from the call-site
+/// `this` (IMPLICIT_THIS) rather than a captured instance.
+///
+/// readable-stream's `Readable.prototype.on` is `function (ev, fn) { var res =
+/// Stream.prototype.on.call(this, ev, fn); … }` — the legacy `Stream.prototype`
+/// must therefore expose `.on` (and siblings) as VALUES so the `.call(this)`
+/// borrow dispatches the EventEmitter `on` against the real stream instance.
+/// Before this, `Stream.prototype.on` was `undefined` and the `.call` threw
+/// "Function.prototype.call was called on a value that is not a function".
+///
+/// The closures capture `TAG_UNDEFINED` in slot 0; `this_value` treats that as
+/// "no fixed receiver — read IMPLICIT_THIS", which `Function.prototype.call`/
+/// `.apply` sets to the borrowed `this`.
+pub(crate) fn install_event_emitter_prototype_methods(proto: *mut ObjectHeader) {
+    register_stub_arities();
+    let methods = super::emitter_methods();
+    let mut on_method: Option<f64> = None;
+    for (name, func) in methods {
+        if name == "addListener" {
+            if let Some(val) = on_method {
+                js_object_set_field_by_name(proto, hidden_key(name.as_bytes()), val);
+                continue;
+            }
+        }
+        let closure = js_closure_alloc(func as *const u8, 1);
+        crate::closure::js_closure_set_capture_ptr(closure, 0, crate::value::TAG_UNDEFINED as i64);
+        let val = f64::from_bits(JSValue::pointer(closure as *const u8).bits());
+        if name == "on" {
+            on_method = Some(val);
+        }
+        js_object_set_field_by_name(proto, hidden_key(name.as_bytes()), val);
+    }
+}
+
 pub(super) fn register_stub_arities() {
     let register = |func: *const u8, arity: u32| {
         crate::closure::js_register_closure_arity(func, arity);

crates/perry-runtime/src/object/class_registry.rs

@@ -759,14 +759,20 @@ unsafe fn inherited_proto_accessor_value(
     if acc.get == 0 {
         return Some(JSValue::undefined());
     }
-    let closure = (acc.get & crate::value::POINTER_MASK) as *const crate::closure::ClosureHeader;
-    if closure.is_null() {
-        return Some(JSValue::undefined());
-    }
-    let previous_this = js_implicit_this_set(receiver);
-    let value = crate::closure::js_closure_call0(closure);
-    js_implicit_this_set(previous_this);
-    Some(JSValue::from_bits(value.to_bits()))
+    // Route through `invoke_accessor_getter` rather than a bare
+    // `js_implicit_this_set` + `js_closure_call0`. A getter installed via
+    // `Object.defineProperty(Class.prototype, name, { get })` is an ORDINARY
+    // method closure whose body reads `this` from its captured receiver slot —
+    // not from IMPLICIT_THIS — so merely setting IMPLICIT_THIS left the getter
+    // observing the prototype it lives on instead of the instance (winston's
+    // `get transports()` saw the prototype, whose `this._readableState` is
+    // undefined → "Cannot convert undefined or null to object").
+    // `invoke_accessor_getter` clones the closure with `this` rebound to the
+    // real receiver (and applies strict/sloppy coercion), matching the
+    // own-accessor read path.
+    Some(super::field_get_set::invoke_accessor_getter(
+        acc.get, receiver,
+    ))
 }
 
 unsafe fn resolve_proto_chain_field_inner(

crates/perry-runtime/src/object/native_module.rs

@@ -6445,6 +6445,28 @@ pub(crate) unsafe fn get_native_module_constant(
         return cjs_default_export_value("process");
     }
 
+    // Node's `require('stream')` IS the legacy `Stream` constructor (a function
+    // that also carries `.Readable`/`.Writable`/… statics), so its `.prototype`
+    // is the EventEmitter-derived `Stream.prototype`. Perry models the module as
+    // a namespace OBJECT, so `require('stream').prototype` was `undefined`.
+    // readable-stream's `Readable.prototype.on = function (ev, fn) { var res =
+    // Stream.prototype.on.call(this, ev, fn); … }` (where `Stream =
+    // require('stream')`) then threw "Function.prototype.call was called on a
+    // value that is not a function". Resolve `require('stream').prototype` to the
+    // same legacy `Stream.prototype` the `.Stream` export carries (minted +
+    // cached by `bound_native_callable_export_value("stream", "Stream")`), which
+    // now exposes the EventEmitter prototype methods.
+    if module_name == "stream" && property == "prototype" {
+        let stream_ctor = bound_native_callable_export_value("stream", "Stream");
+        let ctor_ptr = (stream_ctor.to_bits() & crate::value::POINTER_MASK) as usize;
+        if ctor_ptr != 0 {
+            let proto = crate::closure::closure_get_dynamic_prop(ctor_ptr, "prototype");
+            if !JSValue::from_bits(proto.to_bits()).is_undefined() {
+                return Some(proto);
+            }
+        }
+    }
+
     if property == "default" && !is_cjs_default_object && module_name != "process" {
         if let Some(value) = cjs_default_export_value(module_name) {
             return Some(value);

crates/perry-runtime/src/object/native_module_stream.rs

@@ -36,6 +36,10 @@ pub(crate) fn attach_stream_legacy_prototype(constructor_value: f64) {
         b"constructor\0".len() as u32,
     );
     js_object_set_field(proto, 0, JSValue::from_bits(constructor_value.to_bits()));
+    // readable-stream's `Readable.prototype.on` borrows `Stream.prototype.on`
+    // via `.call(this)`; expose the EventEmitter methods on the legacy
+    // `Stream.prototype` as receiver-from-`this` values so the borrow works.
+    crate::node_stream::install_event_emitter_prototype_methods(proto);
     let proto_value = crate::value::js_nanbox_pointer(proto as i64);
     STREAM_EVENT_EMITTER_PROTOTYPES.with(|protos| {
         let mut protos = protos.borrow_mut();
@@ -79,6 +83,11 @@ pub(crate) fn attach_stream_constructor_prototype(constructor_value: f64, name:
         b"constructor\0".len() as u32,
     );
     js_object_set_field(proto, 0, JSValue::from_bits(constructor_value.to_bits()));
+    // `Readable`/`Writable`/`Duplex`/`Transform`/`PassThrough` also chain their
+    // own prototype methods onto `<Ctor>.prototype.<m>.call(this, …)` (e.g.
+    // `Duplex.prototype.on` ↔ readable-stream borrows). Expose the EventEmitter
+    // methods on these prototypes too.
+    crate::node_stream::install_event_emitter_prototype_methods(proto);
     let proto_value = crate::value::js_nanbox_pointer(proto as i64);
     STREAM_EVENT_EMITTER_PROTOTYPES.with(|protos| {
         let mut protos = protos.borrow_mut();

tests/test_defineproperty_prototype_getter_this.sh

@@ -0,0 +1,106 @@
+#!/usr/bin/env bash
+# Regression: a getter installed via `Object.defineProperty(Class.prototype,
+# name, { get })` must run with `this` bound to the INSTANCE, not the prototype
+# object it lives on.
+#
+# Such a getter is an ordinary method closure whose body reads `this` from its
+# captured receiver slot (not IMPLICIT_THIS). The inherited-accessor walk used
+# to merely set IMPLICIT_THIS and call the closure, so the getter observed the
+# prototype — `this.<ownField>` came back undefined. winston's
+# `Object.defineProperty(Logger.prototype, 'transports', { get() { const {
+# pipes } = this._readableState; … } })` (read as `this.transports` inside the
+# Logger constructor) then threw "Cannot convert undefined or null to object".
+#
+# Fix: the inherited prototype-accessor path routes through
+# `invoke_accessor_getter`, which clones the getter closure with `this` rebound
+# to the real receiver (matching the own-accessor read path).
+set -euo pipefail
+
+SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
+REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)"
+PERRY="${PERRY_BIN:-${PERRY:-$REPO_ROOT/target/release/perry}}"
+
+if [[ ! -x "$PERRY" ]]; then
+    PERRY="$REPO_ROOT/target/debug/perry"
+fi
+if [[ ! -x "$PERRY" ]]; then
+    echo "SKIP: perry binary not found (build with cargo build -p perry)"
+    exit 0
+fi
+
+TMPDIR="$(mktemp -d)"
+trap 'rm -rf "$TMPDIR"' EXIT
+
+SRC="$TMPDIR/defineproperty_proto_getter.ts"
+BIN="$TMPDIR/defineproperty_proto_getter"
+
+cat >"$SRC" <<'TS'
+let failures = 0;
+
+// Plain class: getter reads an own data field through `this`.
+class A {
+  _x = 42;
+}
+Object.defineProperty(A.prototype, "px", {
+  get() { return (this as any)._x; },
+});
+const a: any = new A();
+if (a.px !== 42) {
+  console.log("FAIL: defineProperty prototype getter read prototype, not instance (px=" + a.px + ")");
+  failures = failures + 1;
+}
+
+// Getter that DESTRUCTURES a nested own object — the exact winston shape.
+class L {
+  _readableState: any;
+  constructor() { this._readableState = { pipes: null }; }
+}
+Object.defineProperty(L.prototype, "transports", {
+  configurable: false,
+  enumerable: true,
+  get() {
+    const { pipes } = (this as any)._readableState;
+    return !Array.isArray(pipes) ? [pipes].filter(Boolean) : pipes;
+  },
+});
+const l: any = new L();
+let threw = false;
+let result: any = "unset";
+try {
+  result = l.transports;
+} catch (e) {
+  threw = true;
+}
+if (threw) {
+  console.log("FAIL: destructuring getter threw (this bound to prototype, _readableState undefined)");
+  failures = failures + 1;
+} else if (JSON.stringify(result) !== "[]") {
+  console.log("FAIL: destructuring getter wrong result: " + JSON.stringify(result));
+  failures = failures + 1;
+}
+
+if (failures !== 0) {
+  throw new Error("defineProperty prototype getter this-binding regression failed");
+}
+console.log("defineProperty proto getter this ok");
+TS
+
+"$PERRY" compile --no-cache --no-auto-optimize "$SRC" -o "$BIN" >"$TMPDIR/compile.log" 2>&1 || {
+    echo "FAIL: compile failed"
+    sed 's/^/    /' "$TMPDIR/compile.log" | tail -80
+    exit 1
+}
+
+"$BIN" >"$TMPDIR/run.log" 2>&1 || {
+    echo "FAIL: program failed"
+    sed 's/^/    /' "$TMPDIR/run.log" | tail -80
+    exit 1
+}
+
+if ! grep -q "defineProperty proto getter this ok" "$TMPDIR/run.log"; then
+    echo "FAIL: expected success marker"
+    sed 's/^/    /' "$TMPDIR/run.log" | tail -80
+    exit 1
+fi
+
+echo "PASS: defineProperty prototype getter this-binding"

tests/test_native_module_value_read_install_codegen.sh

@@ -0,0 +1,118 @@
+#!/usr/bin/env bash
+# Regression: reading a native-module callable export AS A VALUE
+# (`const f = util.inherits`) and invoking it indirectly must dispatch to the
+# real runtime impl. The indirect call resolves through the per-module
+# NM_DISPATCH_REGISTRY, which is populated by `js_nm_install_<module>()`. The
+# *direct* call form (`util.inherits(a, b)`) is statically lowered straight to
+# the runtime extern and never touches that registry, so a module reached ONLY
+# through the value-read path used to leave the registry empty — the indirect
+# call silently resolved to `undefined`.
+#
+# Concretely: winston's `class Logger extends Transform` (readable-stream)
+# relies on `require('inherits')(Transform, Duplex)` — an indirect
+# `util.inherits` value-call — to wire the ES5 super-chain so the nested
+# `Readable.call(this)` `if (!(this instanceof Readable))` guard takes the
+# in-place branch and sets `this._readableState`. With the install skipped,
+# the guard saw `false`, returned a discarded `new Readable()`, and
+# `this._readableState.needReadable = true` threw on `null`.
+#
+# This test pins both halves:
+#   1. RUNTIME: the value-read `inherits(Sub, Base)` registers the ES5 parent
+#      edge so `new Sub() instanceof Base` is true and base ctor writes persist.
+#   2. CODEGEN: the `PropertyGet { NativeModuleRef("util"), "inherits" }`
+#      value-read emits `call void @js_nm_install_util()`.
+set -euo pipefail
+
+SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
+REPO_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)"
+PERRY="${PERRY_BIN:-${PERRY:-$REPO_ROOT/target/release/perry}}"
+
+if [[ ! -x "$PERRY" ]]; then
+    PERRY="$REPO_ROOT/target/debug/perry"
+fi
+if [[ ! -x "$PERRY" ]]; then
+    echo "SKIP: perry binary not found (build with cargo build -p perry)"
+    exit 0
+fi
+
+TMPDIR="$(mktemp -d)"
+trap 'rm -rf "$TMPDIR"' EXIT
+
+SRC="$TMPDIR/nm_value_read_install.ts"
+BIN="$TMPDIR/nm_value_read_install"
+OBJ="$TMPDIR/nm_value_read_install.o"
+
+cat >"$SRC" <<'TS'
+import * as util from "util";
+
+// Read the native-module callable export AS A VALUE, then invoke indirectly.
+const inh: any = (util as any).inherits;
+
+function Base(this: any) {
+  if (!(this instanceof Base)) return new (Base as any)();
+  this._state = { x: 1 };
+}
+function Sub(this: any) {
+  (Base as any).call(this);
+  this._state.y = 2; // mirrors readable-stream's post-super `this._x` writes
+}
+inh(Sub, Base);
+
+const s: any = new (Sub as any)();
+
+let failures = 0;
+if (!(s instanceof Base)) {
+  console.log("FAIL: value-read util.inherits did not register the ES5 parent edge");
+  failures = failures + 1;
+}
+if (!s._state || s._state.x !== 1 || s._state.y !== 2) {
+  console.log("FAIL: base ctor writes did not persist through the super-chain");
+  failures = failures + 1;
+}
+
+if (failures !== 0) {
+  throw new Error("native-module value-read inherits regression failed");
+}
+console.log("nm value-read inherits ok");
+TS
+
+"$PERRY" compile --no-cache --no-auto-optimize "$SRC" -o "$BIN" >"$TMPDIR/compile.log" 2>&1 || {
+    echo "FAIL: compile failed"
+    sed 's/^/    /' "$TMPDIR/compile.log" | tail -80
+    exit 1
+}
+
+"$BIN" >"$TMPDIR/run.log" 2>&1 || {
+    echo "FAIL: program failed"
+    sed 's/^/    /' "$TMPDIR/run.log" | tail -80
+    exit 1
+}
+
+if ! grep -q "nm value-read inherits ok" "$TMPDIR/run.log"; then
+    echo "FAIL: expected success marker"
+    sed 's/^/    /' "$TMPDIR/run.log" | tail -80
+    exit 1
+fi
+
+(
+    cd "$TMPDIR"
+    "$PERRY" compile --no-cache --no-auto-optimize --trace llvm --no-link \
+        "$SRC" -o "$OBJ" >"$TMPDIR/trace-compile.log" 2>&1
+) || {
+    echo "FAIL: trace compile failed"
+    sed 's/^/    /' "$TMPDIR/trace-compile.log" | tail -80
+    exit 1
+}
+
+TRACE_DIR="$TMPDIR/.perry-trace/llvm"
+if [[ ! -d "$TRACE_DIR" ]]; then
+    echo "FAIL: LLVM trace directory not found"
+    exit 1
+fi
+
+if ! grep -R "call void @js_nm_install_util()" "$TRACE_DIR" >/dev/null; then
+    echo "FAIL: expected js_nm_install_util() call on the native-module value-read path"
+    exit 1
+fi
+
+echo "PASS: native-module value-read install codegen"