diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b2536de8c..2f0e6fda74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ Releases are listed newest first. - Fixed `--web --max-requests` crash-loop accounting (issue #516): workers that exit after a planned request-quota recycle now use a dedicated status that the master excludes from startup-death streaks, so sustained traffic with a low quota keeps respawning workers instead of shutting down the server; genuine setup failures, other exit codes, and signal deaths still feed the guard. - Fixed nullable chained array reads (issue #525): consuming a value from a `?array` receiver now releases the one-shot hidden owned Mixed temporary created by nullable access, on both null and non-null paths, without invalidating the extracted result or double-releasing repeated reads. - Fixed owned boxed Mixed temporaries leaking after string conversion (issue #527): explicit `(string)` casts plus implicit concatenation and interpolation now release detached Mixed sources, including by-value `foreach` element reads and non-null unions represented as Mixed; borrowed, persistent, moved, and non-heap values remain release no-ops. +- Fixed boxed Mixed values leaking when nested loops reinitialize a local whose storage widens after lowering (issue #534): EIR now records a deferred `ReleaseLocalSlot` before the overwrite, prunes it for final scalar or ref-bound slots, and lowers it using the final slot representation. Cleanup is flow-sensitive across conditional ref-cell promotion, aliases, by-reference `foreach` and pointer paths, and widened parameters, preventing both missed releases and double frees with the EIR optimizer enabled or disabled. ## [0.26.1] - Expanded flow-sensitive type narrowing for PHP's common guard patterns: `int|false` and other false-sentinel unions now preserve the literal `false` subtype and narrow to their success type after a divergent `=== false` guard, without incorrectly removing a full `bool` member; `=== null` and `is_null()` guards narrow nullable values; and stable object properties can be narrowed through `instanceof`, ternaries, and throw guards. Property facts are invalidated after writes or receiver rebindings and are not retained across property hooks or `__get`, whose repeated reads may differ. diff --git a/docs/internals/builtins/pointer/ptr_get.md b/docs/internals/builtins/pointer/ptr_get.md index 524f0f1a0c..2898d85b6d 100644 --- a/docs/internals/builtins/pointer/ptr_get.md +++ b/docs/internals/builtins/pointer/ptr_get.md @@ -10,7 +10,7 @@ sidebar: ## Where it lives - **Signature**: [`src/builtins/pointers/ptr_get.rs`](https://github.com/illegalstudio/elephc/blob/main/src/builtins/pointers/ptr_get.rs) -- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:109](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L109) (`lower_ptr_get`) +- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:104](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L104) (`lower_ptr_get`) - **Function symbol**: `lower_ptr_get()` diff --git a/docs/internals/builtins/pointer/ptr_is_null.md b/docs/internals/builtins/pointer/ptr_is_null.md index 40c5d408ec..b801842f99 100644 --- a/docs/internals/builtins/pointer/ptr_is_null.md +++ b/docs/internals/builtins/pointer/ptr_is_null.md @@ -10,7 +10,7 @@ sidebar: ## Where it lives - **Signature**: [`src/builtins/pointers/ptr_is_null.rs`](https://github.com/illegalstudio/elephc/blob/main/src/builtins/pointers/ptr_is_null.rs) -- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:56](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L56) (`lower_ptr_is_null`) +- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:51](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L51) (`lower_ptr_is_null`) - **Function symbol**: `lower_ptr_is_null()` diff --git a/docs/internals/builtins/pointer/ptr_null.md b/docs/internals/builtins/pointer/ptr_null.md index 32487821eb..b385527c97 100644 --- a/docs/internals/builtins/pointer/ptr_null.md +++ b/docs/internals/builtins/pointer/ptr_null.md @@ -10,7 +10,7 @@ sidebar: ## Where it lives - **Signature**: [`src/builtins/pointers/ptr_null.rs`](https://github.com/illegalstudio/elephc/blob/main/src/builtins/pointers/ptr_null.rs) -- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:49](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L49) (`lower_ptr_null`) +- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:44](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L44) (`lower_ptr_null`) - **Function symbol**: `lower_ptr_null()` diff --git a/docs/internals/builtins/pointer/ptr_offset.md b/docs/internals/builtins/pointer/ptr_offset.md index 6c5bf3dad2..25d1551a57 100644 --- a/docs/internals/builtins/pointer/ptr_offset.md +++ b/docs/internals/builtins/pointer/ptr_offset.md @@ -10,7 +10,7 @@ sidebar: ## Where it lives - **Signature**: [`src/builtins/pointers/ptr_offset.rs`](https://github.com/illegalstudio/elephc/blob/main/src/builtins/pointers/ptr_offset.rs) -- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:86](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L86) (`lower_ptr_offset`) +- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:81](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L81) (`lower_ptr_offset`) - **Function symbol**: `lower_ptr_offset()` diff --git a/docs/internals/builtins/pointer/ptr_read16.md b/docs/internals/builtins/pointer/ptr_read16.md index a38f194c5b..c598423749 100644 --- a/docs/internals/builtins/pointer/ptr_read16.md +++ b/docs/internals/builtins/pointer/ptr_read16.md @@ -10,7 +10,7 @@ sidebar: ## Where it lives - **Signature**: [`src/builtins/pointers/ptr_read16.rs`](https://github.com/illegalstudio/elephc/blob/main/src/builtins/pointers/ptr_read16.rs) -- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:124](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L124) (`lower_ptr_read16`) +- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:119](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L119) (`lower_ptr_read16`) - **Function symbol**: `lower_ptr_read16()` diff --git a/docs/internals/builtins/pointer/ptr_read32.md b/docs/internals/builtins/pointer/ptr_read32.md index 26aead378c..d7897c9488 100644 --- a/docs/internals/builtins/pointer/ptr_read32.md +++ b/docs/internals/builtins/pointer/ptr_read32.md @@ -10,7 +10,7 @@ sidebar: ## Where it lives - **Signature**: [`src/builtins/pointers/ptr_read32.rs`](https://github.com/illegalstudio/elephc/blob/main/src/builtins/pointers/ptr_read32.rs) -- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:129](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L129) (`lower_ptr_read32`) +- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:124](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L124) (`lower_ptr_read32`) - **Function symbol**: `lower_ptr_read32()` diff --git a/docs/internals/builtins/pointer/ptr_read8.md b/docs/internals/builtins/pointer/ptr_read8.md index ce1c57bcb1..2543678d23 100644 --- a/docs/internals/builtins/pointer/ptr_read8.md +++ b/docs/internals/builtins/pointer/ptr_read8.md @@ -10,7 +10,7 @@ sidebar: ## Where it lives - **Signature**: [`src/builtins/pointers/ptr_read8.rs`](https://github.com/illegalstudio/elephc/blob/main/src/builtins/pointers/ptr_read8.rs) -- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:119](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L119) (`lower_ptr_read8`) +- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:114](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L114) (`lower_ptr_read8`) - **Function symbol**: `lower_ptr_read8()` diff --git a/docs/internals/builtins/pointer/ptr_read_string.md b/docs/internals/builtins/pointer/ptr_read_string.md index 82f33512ed..e13b6148b1 100644 --- a/docs/internals/builtins/pointer/ptr_read_string.md +++ b/docs/internals/builtins/pointer/ptr_read_string.md @@ -10,7 +10,7 @@ sidebar: ## Where it lives - **Signature**: [`src/builtins/pointers/ptr_read_string.rs`](https://github.com/illegalstudio/elephc/blob/main/src/builtins/pointers/ptr_read_string.rs) -- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:134](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L134) (`lower_ptr_read_string`) +- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:129](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L129) (`lower_ptr_read_string`) - **Function symbol**: `lower_ptr_read_string()` diff --git a/docs/internals/builtins/pointer/ptr_set.md b/docs/internals/builtins/pointer/ptr_set.md index 186aaf5267..232da81ce1 100644 --- a/docs/internals/builtins/pointer/ptr_set.md +++ b/docs/internals/builtins/pointer/ptr_set.md @@ -10,7 +10,7 @@ sidebar: ## Where it lives - **Signature**: [`src/builtins/pointers/ptr_set.rs`](https://github.com/illegalstudio/elephc/blob/main/src/builtins/pointers/ptr_set.rs) -- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:114](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L114) (`lower_ptr_set`) +- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:109](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L109) (`lower_ptr_set`) - **Function symbol**: `lower_ptr_set()` diff --git a/docs/internals/builtins/pointer/ptr_sizeof.md b/docs/internals/builtins/pointer/ptr_sizeof.md index d0616caf8f..33f5c10568 100644 --- a/docs/internals/builtins/pointer/ptr_sizeof.md +++ b/docs/internals/builtins/pointer/ptr_sizeof.md @@ -10,7 +10,7 @@ sidebar: ## Where it lives - **Signature**: [`src/builtins/pointers/ptr_sizeof.rs`](https://github.com/illegalstudio/elephc/blob/main/src/builtins/pointers/ptr_sizeof.rs) -- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:75](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L75) (`lower_ptr_sizeof`) +- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:70](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L70) (`lower_ptr_sizeof`) - **Function symbol**: `lower_ptr_sizeof()` diff --git a/docs/internals/builtins/pointer/ptr_write16.md b/docs/internals/builtins/pointer/ptr_write16.md index 4ab6dcd112..3dd0f646b5 100644 --- a/docs/internals/builtins/pointer/ptr_write16.md +++ b/docs/internals/builtins/pointer/ptr_write16.md @@ -10,7 +10,7 @@ sidebar: ## Where it lives - **Signature**: [`src/builtins/pointers/ptr_write16.rs`](https://github.com/illegalstudio/elephc/blob/main/src/builtins/pointers/ptr_write16.rs) -- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:161](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L161) (`lower_ptr_write16`) +- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:156](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L156) (`lower_ptr_write16`) - **Function symbol**: `lower_ptr_write16()` diff --git a/docs/internals/builtins/pointer/ptr_write32.md b/docs/internals/builtins/pointer/ptr_write32.md index 7a3ac6eb84..6d910a479d 100644 --- a/docs/internals/builtins/pointer/ptr_write32.md +++ b/docs/internals/builtins/pointer/ptr_write32.md @@ -10,7 +10,7 @@ sidebar: ## Where it lives - **Signature**: [`src/builtins/pointers/ptr_write32.rs`](https://github.com/illegalstudio/elephc/blob/main/src/builtins/pointers/ptr_write32.rs) -- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:166](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L166) (`lower_ptr_write32`) +- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:161](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L161) (`lower_ptr_write32`) - **Function symbol**: `lower_ptr_write32()` diff --git a/docs/internals/builtins/pointer/ptr_write8.md b/docs/internals/builtins/pointer/ptr_write8.md index 0c285cff20..c6ea3e0fb8 100644 --- a/docs/internals/builtins/pointer/ptr_write8.md +++ b/docs/internals/builtins/pointer/ptr_write8.md @@ -10,7 +10,7 @@ sidebar: ## Where it lives - **Signature**: [`src/builtins/pointers/ptr_write8.rs`](https://github.com/illegalstudio/elephc/blob/main/src/builtins/pointers/ptr_write8.rs) -- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:156](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L156) (`lower_ptr_write8`) +- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:151](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L151) (`lower_ptr_write8`) - **Function symbol**: `lower_ptr_write8()` diff --git a/docs/internals/builtins/pointer/ptr_write_string.md b/docs/internals/builtins/pointer/ptr_write_string.md index 1d8277df2a..786e4fed41 100644 --- a/docs/internals/builtins/pointer/ptr_write_string.md +++ b/docs/internals/builtins/pointer/ptr_write_string.md @@ -10,7 +10,7 @@ sidebar: ## Where it lives - **Signature**: [`src/builtins/pointers/ptr_write_string.rs`](https://github.com/illegalstudio/elephc/blob/main/src/builtins/pointers/ptr_write_string.rs) -- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:171](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L171) (`lower_ptr_write_string`) +- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:166](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L166) (`lower_ptr_write_string`) - **Function symbol**: `lower_ptr_write_string()` diff --git a/docs/internals/builtins/pointer/zval_free.md b/docs/internals/builtins/pointer/zval_free.md index ffe045d625..6cb4ef2880 100644 --- a/docs/internals/builtins/pointer/zval_free.md +++ b/docs/internals/builtins/pointer/zval_free.md @@ -10,7 +10,7 @@ sidebar: ## Where it lives - **Signature**: [`src/builtins/pointers/zval_free.rs`](https://github.com/illegalstudio/elephc/blob/main/src/builtins/pointers/zval_free.rs) -- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:606](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L606) (`lower_zval_free`) +- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:598](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L598) (`lower_zval_free`) - **Function symbol**: `lower_zval_free()` diff --git a/docs/internals/builtins/pointer/zval_pack.md b/docs/internals/builtins/pointer/zval_pack.md index e903a83d31..bcf080e234 100644 --- a/docs/internals/builtins/pointer/zval_pack.md +++ b/docs/internals/builtins/pointer/zval_pack.md @@ -10,7 +10,7 @@ sidebar: ## Where it lives - **Signature**: [`src/builtins/pointers/zval_pack.rs`](https://github.com/illegalstudio/elephc/blob/main/src/builtins/pointers/zval_pack.rs) -- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:541](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L541) (`lower_zval_pack`) +- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:533](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L533) (`lower_zval_pack`) - **Function symbol**: `lower_zval_pack()` diff --git a/docs/internals/builtins/pointer/zval_type.md b/docs/internals/builtins/pointer/zval_type.md index 2dedc2723e..51f41e395b 100644 --- a/docs/internals/builtins/pointer/zval_type.md +++ b/docs/internals/builtins/pointer/zval_type.md @@ -10,7 +10,7 @@ sidebar: ## Where it lives - **Signature**: [`src/builtins/pointers/zval_type.rs`](https://github.com/illegalstudio/elephc/blob/main/src/builtins/pointers/zval_type.rs) -- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:597](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L597) (`lower_zval_type`) +- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:589](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L589) (`lower_zval_type`) - **Function symbol**: `lower_zval_type()` diff --git a/docs/internals/builtins/pointer/zval_unpack.md b/docs/internals/builtins/pointer/zval_unpack.md index 867ef4b5a3..eed019e4d6 100644 --- a/docs/internals/builtins/pointer/zval_unpack.md +++ b/docs/internals/builtins/pointer/zval_unpack.md @@ -10,7 +10,7 @@ sidebar: ## Where it lives - **Signature**: [`src/builtins/pointers/zval_unpack.rs`](https://github.com/illegalstudio/elephc/blob/main/src/builtins/pointers/zval_unpack.rs) -- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:588](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L588) (`lower_zval_unpack`) +- **Lowering**: [`src/codegen/lower_inst/builtins/pointers.rs`:580](https://github.com/illegalstudio/elephc/blob/main/src/codegen/lower_inst/builtins/pointers.rs#L580) (`lower_zval_unpack`) - **Function symbol**: `lower_zval_unpack()` diff --git a/docs/internals/memory-model.md b/docs/internals/memory-model.md index f3fa075f31..b43528ae88 100644 --- a/docs/internals/memory-model.md +++ b/docs/internals/memory-model.md @@ -278,7 +278,7 @@ When one of these checks trips, the program exits with a fatal heap-debug error ### When memory is freed -- **Variable reassignment**: when a heap-backed local/global/static slot is overwritten, codegen releases the previous owner through the appropriate runtime path (`__rt_heap_free_safe` for persisted strings, `__rt_decref_*` for refcounted arrays / hashes / objects) +- **Variable reassignment**: when a heap-backed local/global/static slot is overwritten, codegen releases the previous owner through the appropriate runtime path (`__rt_heap_free_safe` for persisted strings, `__rt_decref_*` for refcounted arrays / hashes / objects). When a store inside a loop is lowered before a later store has widened the slot to boxed storage (e.g. an inner `for` counter re-initialized by the outer body but widened Int→Mixed by its `++` update), lowering emits a deferred `release_local_slot` and the backend decides against the slot's final widened storage type, so the previous iteration's box is still released - **`unset()`**: releases the current heap-backed value before nulling the slot - **Targeted cycle collection**: when decref reaches a container/object graph that may only be keeping itself alive, `__rt_gc_collect_cycles` counts heap-only incoming edges, marks externally reachable blocks, and deep-frees the remaining unreachable array/hash/object island - **Generator frame release**: Generator frames are object-kind heap blocks, but their custom Mixed slots and active `yield from` delegate are released by a Generator-specific branch in object deep-free diff --git a/scripts/docs/builtin_registry.json b/scripts/docs/builtin_registry.json index f708adbdf2..f4faf982b8 100644 --- a/scripts/docs/builtin_registry.json +++ b/scripts/docs/builtin_registry.json @@ -19578,7 +19578,7 @@ "checker_line": null, "codegen_file": "src/codegen/lower_inst/builtins/pointers.rs", "codegen_function": "lower_ptr_get", - "codegen_line": 109, + "codegen_line": 104, "notes": [ "Lowers `ptr_get(pointer)` by reading one machine word through a checked pointer." ], @@ -19636,7 +19636,7 @@ "checker_line": null, "codegen_file": "src/codegen/lower_inst/builtins/pointers.rs", "codegen_function": "lower_ptr_is_null", - "codegen_line": 56, + "codegen_line": 51, "notes": [ "Lowers `ptr_is_null(pointer)` by comparing the raw pointer address to zero." ], @@ -19687,7 +19687,7 @@ "checker_line": null, "codegen_file": "src/codegen/lower_inst/builtins/pointers.rs", "codegen_function": "lower_ptr_null", - "codegen_line": 49, + "codegen_line": 44, "notes": [ "Lowers `ptr_null()` by materializing the raw null pointer sentinel." ], @@ -19743,7 +19743,7 @@ "checker_line": null, "codegen_file": "src/codegen/lower_inst/builtins/pointers.rs", "codegen_function": "lower_ptr_offset", - "codegen_line": 86, + "codegen_line": 81, "notes": [ "Lowers `ptr_offset(pointer, offset)` by adding a byte offset to a raw address." ], @@ -19808,7 +19808,7 @@ "checker_line": null, "codegen_file": "src/codegen/lower_inst/builtins/pointers.rs", "codegen_function": "lower_ptr_read16", - "codegen_line": 124, + "codegen_line": 119, "notes": [ "Lowers `ptr_read16(pointer)` by reading one unsigned 16-bit word through a checked pointer." ], @@ -19868,7 +19868,7 @@ "checker_line": null, "codegen_file": "src/codegen/lower_inst/builtins/pointers.rs", "codegen_function": "lower_ptr_read32", - "codegen_line": 129, + "codegen_line": 124, "notes": [ "Lowers `ptr_read32(pointer)` by reading one unsigned 32-bit word through a checked pointer." ], @@ -19928,7 +19928,7 @@ "checker_line": null, "codegen_file": "src/codegen/lower_inst/builtins/pointers.rs", "codegen_function": "lower_ptr_read8", - "codegen_line": 119, + "codegen_line": 114, "notes": [ "Lowers `ptr_read8(pointer)` by reading one unsigned byte through a checked pointer." ], @@ -19992,7 +19992,7 @@ "checker_line": null, "codegen_file": "src/codegen/lower_inst/builtins/pointers.rs", "codegen_function": "lower_ptr_read_string", - "codegen_line": 134, + "codegen_line": 129, "notes": [ "Lowers `ptr_read_string(pointer, length)` by copying raw bytes into an owned PHP string." ], @@ -20065,7 +20065,7 @@ "checker_line": null, "codegen_file": "src/codegen/lower_inst/builtins/pointers.rs", "codegen_function": "lower_ptr_set", - "codegen_line": 114, + "codegen_line": 109, "notes": [ "Lowers `ptr_set(pointer, value)` by writing one machine word through a checked pointer." ], @@ -20130,7 +20130,7 @@ "checker_line": null, "codegen_file": "src/codegen/lower_inst/builtins/pointers.rs", "codegen_function": "lower_ptr_sizeof", - "codegen_line": 75, + "codegen_line": 70, "notes": [ "Lowers `ptr_sizeof(\"type\")` by materializing the checked static byte size." ], @@ -20194,7 +20194,7 @@ "checker_line": null, "codegen_file": "src/codegen/lower_inst/builtins/pointers.rs", "codegen_function": "lower_ptr_write16", - "codegen_line": 161, + "codegen_line": 156, "notes": [ "Lowers `ptr_write16(pointer, value)` by writing one 16-bit word through a checked pointer." ], @@ -20267,7 +20267,7 @@ "checker_line": null, "codegen_file": "src/codegen/lower_inst/builtins/pointers.rs", "codegen_function": "lower_ptr_write32", - "codegen_line": 166, + "codegen_line": 161, "notes": [ "Lowers `ptr_write32(pointer, value)` by writing one 32-bit word through a checked pointer." ], @@ -20340,7 +20340,7 @@ "checker_line": null, "codegen_file": "src/codegen/lower_inst/builtins/pointers.rs", "codegen_function": "lower_ptr_write8", - "codegen_line": 156, + "codegen_line": 151, "notes": [ "Lowers `ptr_write8(pointer, value)` by writing one byte through a checked pointer." ], @@ -20411,7 +20411,7 @@ "checker_line": null, "codegen_file": "src/codegen/lower_inst/builtins/pointers.rs", "codegen_function": "lower_ptr_write_string", - "codegen_line": 171, + "codegen_line": 166, "notes": [ "Lowers `ptr_write_string(pointer, string)` by copying PHP string bytes into raw memory." ], @@ -29824,7 +29824,7 @@ "checker_line": null, "codegen_file": "src/codegen/lower_inst/builtins/pointers.rs", "codegen_function": "lower_zval_free", - "codegen_line": 606, + "codegen_line": 598, "notes": [ "Lowers `zval_free(zval_ptr)` by releasing the zval block and owned children." ], @@ -29868,7 +29868,7 @@ "checker_line": null, "codegen_file": "src/codegen/lower_inst/builtins/pointers.rs", "codegen_function": "lower_zval_pack", - "codegen_line": 541, + "codegen_line": 533, "notes": [ "Lowers `zval_pack(value)` by boxing the operand as a Mixed cell and invoking", "`__rt_zval_pack`, which returns a pointer to a freshly allocated 16-byte zval.", @@ -29921,7 +29921,7 @@ "checker_line": null, "codegen_file": "src/codegen/lower_inst/builtins/pointers.rs", "codegen_function": "lower_zval_type", - "codegen_line": 597, + "codegen_line": 589, "notes": [ "Lowers `zval_type(zval_ptr)` by returning the PHP `IS_*` type byte." ], @@ -29966,7 +29966,7 @@ "checker_line": null, "codegen_file": "src/codegen/lower_inst/builtins/pointers.rs", "codegen_function": "lower_zval_unpack", - "codegen_line": 588, + "codegen_line": 580, "notes": [ "Lowers `zval_unpack(zval_ptr)` by invoking `__rt_zval_unpack`." ], diff --git a/src/codegen/context.rs b/src/codegen/context.rs index 44e821727d..098d29ff0b 100644 --- a/src/codegen/context.rs +++ b/src/codegen/context.rs @@ -16,14 +16,26 @@ use crate::codegen::{abi, emit_box_current_owned_value_as_mixed, emit_box_curren use crate::codegen::data_section::DataSection; use crate::codegen::emit::Emitter; use crate::codegen::platform::Arch; -use crate::ir::{BlockId, DataId, Function, LocalKind, LocalSlotId, Module, Op, Ownership, ValueDef, ValueId}; +use crate::ir::{ + BlockId, DataId, Function, InstId, LocalKind, LocalSlotId, Module, Op, Ownership, ValueDef, + ValueId, +}; use crate::ir_passes::Allocation; use crate::types::PhpType; use super::frame::FrameLayout; +use super::local_analysis::LocalSlotAnalysis; use super::value_placement::ValuePlacement; use super::{CodegenIrError, Result}; +/// Runtime representation known for one local slot at the current EIR instruction. +#[derive(Clone, Copy, PartialEq, Eq)] +enum LocalSlotRepresentation { + Raw, + RefCell, + Dynamic, +} + /// Mutable backend state for one EIR function. pub(crate) struct FunctionContext<'a> { pub(super) module: &'a Module, @@ -34,7 +46,10 @@ pub(crate) struct FunctionContext<'a> { pub(super) allocation: Allocation, pub(super) callee_saved_offsets: Vec<(&'static str, usize)>, local_offsets: HashMap, - promoted_ref_cells: HashSet, + ref_cell_state_offsets: HashMap, + local_analysis: LocalSlotAnalysis, + current_inst: Option, + current_inst_promoted_ref_cells: HashSet, try_handler_offsets: HashMap, pub(super) frame_size: usize, pub(super) concat_base_offset: usize, @@ -69,7 +84,10 @@ impl<'a> FunctionContext<'a> { allocation: layout.allocation, callee_saved_offsets: layout.callee_saved_offsets, local_offsets: layout.local_offsets, - promoted_ref_cells: HashSet::new(), + ref_cell_state_offsets: layout.ref_cell_state_offsets, + local_analysis: layout.local_analysis, + current_inst: None, + current_inst_promoted_ref_cells: HashSet::new(), try_handler_offsets: layout.try_handler_offsets, frame_size: layout.frame_size, concat_base_offset: layout.concat_base_offset, @@ -95,6 +113,65 @@ impl<'a> FunctionContext<'a> { label } + /// Emits an unconditional target-aware branch to one local assembly label. + pub(super) fn emit_branch(&mut self, label: &str) { + match self.emitter.target.arch { + Arch::AArch64 => { + self.emitter + .instruction(&format!("b {}", label)); // join the dynamic local-representation paths + } + Arch::X86_64 => { + self.emitter + .instruction(&format!("jmp {}", label)); // join the dynamic local-representation paths + } + } + } + + /// Materializes the address of a local's current raw value or aliased ref-cell storage. + pub(super) fn materialize_local_storage_address( + &mut self, + slot: LocalSlotId, + destination: &str, + ) -> Result<()> { + let offset = self.local_offset(slot)?; + match self.local_slot_representation(slot) { + LocalSlotRepresentation::Raw => { + abi::emit_frame_slot_address(self.emitter, destination, offset); + } + LocalSlotRepresentation::RefCell => { + abi::load_at_offset(self.emitter, destination, offset); + } + LocalSlotRepresentation::Dynamic => { + let state_offset = self.dynamic_ref_cell_state_offset(slot)?; + let ref_cell = self.next_label("dynamic_local_address_ref_cell"); + let done = self.next_label("dynamic_local_address_done"); + let result_reg = abi::int_result_reg(self.emitter); + let state_reg = if destination == result_reg { + abi::secondary_scratch_reg(self.emitter) + } else { + result_reg + }; + abi::load_at_offset(self.emitter, state_reg, state_offset); + match self.emitter.target.arch { + Arch::AArch64 => { + self.emitter.instruction(&format!("cbnz {}, {}", state_reg, ref_cell)); // select the aliased storage address after runtime promotion + } + Arch::X86_64 => { + self.emitter.instruction(&format!("test {}, {}", state_reg, state_reg)); // test the slot's runtime representation flag + self.emitter + .instruction(&format!("jne {}", ref_cell)); // select the aliased storage address after runtime promotion + } + } + abi::emit_frame_slot_address(self.emitter, destination, offset); + self.emit_branch(&done); + self.emitter.label(&ref_cell); + abi::load_at_offset(self.emitter, destination, offset); + self.emitter.label(&done); + } + } + Ok(()) + } + /// Returns the assembly label for a non-entry EIR block. pub(super) fn block_label(&self, block_name: &str, raw: u32) -> String { format!("_eir_{}_{}_{}", label_fragment(&self.function.name), label_fragment(block_name), raw) @@ -195,24 +272,100 @@ impl<'a> FunctionContext<'a> { .map(|local| local.id) } - /// Marks a local slot as storing a heap reference cell pointer instead of its raw value. + /// Returns whether this slot receives at least one ordinary EIR local store. + pub(super) fn local_slot_has_store(&self, slot: LocalSlotId) -> bool { + self.local_analysis.has_store(slot) + } + + /// Returns whether this slot is represented as a ref-cell pointer anywhere in the function. + pub(super) fn local_slot_ever_stores_ref_cell_pointer(&self, slot: LocalSlotId) -> bool { + self.local_analysis.ever_stores_ref_cell_pointer(slot) + } + + /// Returns whether this deferred release may execute while the slot stores a ref-cell pointer. + pub(super) fn release_local_slot_may_observe_ref_cell(&self, inst: InstId) -> bool { + self.local_analysis.release_may_observe_ref_cell(inst) + } + + /// Returns whether this by-value parameter slot is owned by the callee frame. + pub(super) fn owns_parameter_slot(&self, slot: LocalSlotId) -> bool { + self.local_analysis.owns_parameter_slot(slot) + } + + /// Selects the EIR instruction whose CFG-local representation facts codegen must use. + pub(super) fn begin_instruction(&mut self, inst: InstId) { + self.current_inst = Some(inst); + self.current_inst_promoted_ref_cells.clear(); + } + + /// Returns the frame flag that records whether this slot currently stores a cell pointer. + pub(super) fn ref_cell_state_offset(&self, slot: LocalSlotId) -> Option { + self.ref_cell_state_offsets.get(&slot).copied() + } + + /// Returns the required runtime representation flag offset for one dynamic local slot. + fn dynamic_ref_cell_state_offset(&self, slot: LocalSlotId) -> Result { + self.ref_cell_state_offset(slot).ok_or_else(|| { + CodegenIrError::invalid_module(format!( + "dynamic ref-cell slot {} has no representation flag", + slot.as_raw() + )) + }) + } + + /// Returns whether this slot needs runtime raw-value/ref-cell discrimination at cleanup. + pub(super) fn has_dynamic_ref_cell_state(&self, slot: LocalSlotId) -> bool { + self.local_analysis.has_dynamic_ref_cell_state(slot) + } + + /// Records at runtime that a path has changed this local slot to ref-cell representation. pub(super) fn mark_promoted_ref_cell(&mut self, slot: LocalSlotId) { - self.promoted_ref_cells.insert(slot); + self.current_inst_promoted_ref_cells.insert(slot); + if let Some(offset) = self.ref_cell_state_offset(slot) { + abi::emit_load_int_immediate(self.emitter, abi::int_result_reg(self.emitter), 1); + abi::store_at_offset(self.emitter, abi::int_result_reg(self.emitter), offset); + } } - /// Marks a local slot as storing its raw value again after an `unset()` unbind. + /// Records at runtime that `unset()` restored this local slot to raw representation. pub(super) fn unmark_promoted_ref_cell(&mut self, slot: LocalSlotId) { - self.promoted_ref_cells.remove(&slot); + self.current_inst_promoted_ref_cells.remove(&slot); + if let Some(offset) = self.ref_cell_state_offset(slot) { + abi::emit_store_zero_to_local_slot(self.emitter, offset); + } } - /// Returns true when a local slot has been promoted to a heap reference cell. - pub(super) fn is_promoted_ref_cell(&self, slot: LocalSlotId) -> bool { - self.promoted_ref_cells.contains(&slot) + /// Returns true when this instruction may observe a heap reference-cell pointer in the slot. + pub(super) fn local_stores_ref_cell_pointer(&self, slot: LocalSlotId) -> bool { + self.local_slot_representation(slot) != LocalSlotRepresentation::Raw } - /// Returns true when a local slot stores a heap reference-cell pointer. - pub(super) fn local_stores_ref_cell_pointer(&self, slot: LocalSlotId) -> bool { - self.is_by_ref_param_slot(slot) || self.is_promoted_ref_cell(slot) + /// Returns whether this instruction needs a runtime raw/ref-cell branch for the slot. + pub(super) fn local_ref_cell_representation_is_dynamic(&self, slot: LocalSlotId) -> bool { + self.local_slot_representation(slot) == LocalSlotRepresentation::Dynamic + } + + /// Returns whether every path reaching this instruction stores a ref-cell pointer. + pub(super) fn local_ref_cell_representation_is_definite(&self, slot: LocalSlotId) -> bool { + self.local_slot_representation(slot) == LocalSlotRepresentation::RefCell + } + + /// Classifies the slot as raw, definitely ref-cell, or path-dependent at this instruction. + fn local_slot_representation(&self, slot: LocalSlotId) -> LocalSlotRepresentation { + if self.is_by_ref_param_slot(slot) || self.current_inst_promoted_ref_cells.contains(&slot) { + return LocalSlotRepresentation::RefCell; + } + let may_observe_ref_cell = self.current_inst.is_some_and(|inst| { + self.local_analysis.inst_may_observe_ref_cell(inst, slot) + }); + if !may_observe_ref_cell { + return LocalSlotRepresentation::Raw; + } + if self.ref_cell_state_offset(slot).is_some() { + LocalSlotRepresentation::Dynamic + } else { + LocalSlotRepresentation::RefCell + } } /// Returns true when the local slot is the storage slot for a by-reference parameter. @@ -280,9 +433,38 @@ impl<'a> FunctionContext<'a> { /// Loads a local slot into the target's canonical result register(s). pub(super) fn load_local_to_result(&mut self, slot: LocalSlotId) -> Result { - if self.local_stores_ref_cell_pointer(slot) { - return self.load_ref_cell_local_to_result(slot); + let ty = self.local_php_type(slot)?; + match self.local_slot_representation(slot) { + LocalSlotRepresentation::Raw => self.load_raw_local_to_result(slot), + LocalSlotRepresentation::RefCell => self.load_ref_cell_local_to_result(slot), + LocalSlotRepresentation::Dynamic => { + let state_offset = self.dynamic_ref_cell_state_offset(slot)?; + let ref_cell = self.next_label("dynamic_local_load_ref_cell"); + let done = self.next_label("dynamic_local_load_done"); + let state_reg = abi::secondary_scratch_reg(self.emitter); + abi::load_at_offset(self.emitter, state_reg, state_offset); + match self.emitter.target.arch { + Arch::AArch64 => { + self.emitter.instruction(&format!("cbnz {}, {}", state_reg, ref_cell)); // select ref-cell loading after a runtime promotion + } + Arch::X86_64 => { + self.emitter.instruction(&format!("test {}, {}", state_reg, state_reg)); // test the slot's runtime representation flag + self.emitter + .instruction(&format!("jne {}", ref_cell)); // select ref-cell loading after a runtime promotion + } + } + self.load_raw_local_to_result(slot)?; + self.emit_branch(&done); + self.emitter.label(&ref_cell); + self.load_ref_cell_local_to_result(slot)?; + self.emitter.label(&done); + Ok(ty) + } } + } + + /// Loads a local slot using its raw frame representation without consulting ref-cell state. + pub(super) fn load_raw_local_to_result(&mut self, slot: LocalSlotId) -> Result { let ty = self.local_php_type(slot)?; let offset = self.local_offset(slot)?; abi::emit_load(self.emitter, &ty.codegen_repr(), offset); @@ -361,9 +543,41 @@ impl<'a> FunctionContext<'a> { /// Stores an SSA value into an addressable local slot. pub(super) fn store_value_to_local(&mut self, slot: LocalSlotId, value: ValueId) -> Result<()> { - if self.local_stores_ref_cell_pointer(slot) { - return self.store_value_to_ref_cell_local(slot, value); + match self.local_slot_representation(slot) { + LocalSlotRepresentation::Raw => self.store_value_to_raw_local(slot, value), + LocalSlotRepresentation::RefCell => self.store_value_to_ref_cell_local(slot, value), + LocalSlotRepresentation::Dynamic => { + let state_offset = self.dynamic_ref_cell_state_offset(slot)?; + let ref_cell = self.next_label("dynamic_local_store_ref_cell"); + let done = self.next_label("dynamic_local_store_done"); + let state_reg = abi::secondary_scratch_reg(self.emitter); + abi::load_at_offset(self.emitter, state_reg, state_offset); + match self.emitter.target.arch { + Arch::AArch64 => { + self.emitter.instruction(&format!("cbnz {}, {}", state_reg, ref_cell)); // select ref-cell storage after a runtime promotion + } + Arch::X86_64 => { + self.emitter.instruction(&format!("test {}, {}", state_reg, state_reg)); // test the slot's runtime representation flag + self.emitter + .instruction(&format!("jne {}", ref_cell)); // select ref-cell storage after a runtime promotion + } + } + self.store_value_to_raw_local(slot, value)?; + self.emit_branch(&done); + self.emitter.label(&ref_cell); + self.store_value_to_ref_cell_local(slot, value)?; + self.emitter.label(&done); + Ok(()) + } } + } + + /// Stores an SSA value into a slot known to contain its raw frame representation. + pub(super) fn store_value_to_raw_local( + &mut self, + slot: LocalSlotId, + value: ValueId, + ) -> Result<()> { let source_ty = self.load_value_to_result(value)?; let target_ty = self.local_php_type(slot)?; if target_ty == PhpType::Mixed && source_ty != PhpType::Mixed { @@ -409,12 +623,40 @@ impl<'a> FunctionContext<'a> { /// Stores the current result register(s) directly into an addressable local slot. pub(super) fn store_current_result_to_local(&mut self, slot: LocalSlotId) -> Result<()> { let target_ty = self.local_php_type(slot)?; - if self.local_stores_ref_cell_pointer(slot) { - return self.store_current_result_to_ref_cell_local(slot, &target_ty); + match self.local_slot_representation(slot) { + LocalSlotRepresentation::Raw => { + let offset = self.local_offset(slot)?; + self.store_current_result_at_offset(&target_ty, offset); + Ok(()) + } + LocalSlotRepresentation::RefCell => { + self.store_current_result_to_ref_cell_local(slot, &target_ty) + } + LocalSlotRepresentation::Dynamic => { + let state_offset = self.dynamic_ref_cell_state_offset(slot)?; + let ref_cell = self.next_label("dynamic_current_store_ref_cell"); + let done = self.next_label("dynamic_current_store_done"); + let state_reg = abi::secondary_scratch_reg(self.emitter); + abi::load_at_offset(self.emitter, state_reg, state_offset); + match self.emitter.target.arch { + Arch::AArch64 => { + self.emitter.instruction(&format!("cbnz {}, {}", state_reg, ref_cell)); // select ref-cell storage after a runtime promotion + } + Arch::X86_64 => { + self.emitter.instruction(&format!("test {}, {}", state_reg, state_reg)); // test the slot's runtime representation flag + self.emitter + .instruction(&format!("jne {}", ref_cell)); // select ref-cell storage after a runtime promotion + } + } + let offset = self.local_offset(slot)?; + self.store_current_result_at_offset(&target_ty, offset); + self.emit_branch(&done); + self.emitter.label(&ref_cell); + self.store_current_result_to_ref_cell_local(slot, &target_ty)?; + self.emitter.label(&done); + Ok(()) + } } - let offset = self.local_offset(slot)?; - self.store_current_result_at_offset(&target_ty, offset); - Ok(()) } /// After an in-place hash/array mutation whose runtime helper returns the diff --git a/src/codegen/frame.rs b/src/codegen/frame.rs index 9fb0aec814..66bfe0f522 100644 --- a/src/codegen/frame.rs +++ b/src/codegen/frame.rs @@ -14,6 +14,7 @@ use std::collections::{HashMap, HashSet}; use crate::codegen::abi; +use crate::codegen::emit::Emitter; use crate::codegen::platform::{Arch, Target}; use crate::codegen::{ emit_box_current_value_as_mixed, emit_write_current_string_stderr, emit_write_literal_stderr, @@ -25,6 +26,7 @@ use crate::names::ir_global_symbol; use crate::types::PhpType; use super::context::FunctionContext; +use super::local_analysis::LocalSlotAnalysis; use super::value_placement::{self, ValuePlacement}; const FRAME_FOOTER_BYTES: usize = 16; @@ -42,11 +44,13 @@ pub(super) const WEB_HANDLER_SYMBOL: &str = "_elephc_web_handler"; pub(super) struct FrameLayout { pub(super) value_placement: ValuePlacement, pub(super) local_offsets: HashMap, + pub(super) ref_cell_state_offsets: HashMap, pub(super) try_handler_offsets: HashMap, pub(super) concat_base_offset: usize, pub(super) frame_size: usize, pub(super) allocation: Allocation, pub(super) callee_saved_offsets: Vec<(&'static str, usize)>, + pub(super) local_analysis: LocalSlotAnalysis, } /// Computes the register allocation and fixed stack slots for a function. @@ -68,6 +72,7 @@ pub(super) fn layout_for_function( }; let value_placement = value_placement::allocate(function); + let local_analysis = LocalSlotAnalysis::new(function); let mut local_offsets = HashMap::new(); let mut offset = value_placement.total_slot_bytes; for local in &function.locals { @@ -79,6 +84,13 @@ pub(super) fn layout_for_function( offset += bytes; local_offsets.insert(local.id, offset); } + let mut ref_cell_state_offsets = HashMap::new(); + let mut dynamic_ref_cell_slots = local_analysis.dynamic_ref_cell_slots().collect::>(); + dynamic_ref_cell_slots.sort_by_key(|slot| slot.as_raw()); + for slot in dynamic_ref_cell_slots { + offset += 8; + ref_cell_state_offsets.insert(slot, offset); + } let mut try_handler_offsets = HashMap::new(); for token in try_handler_tokens(function) { offset += TRY_HANDLER_SLOT_SIZE; @@ -95,11 +107,13 @@ pub(super) fn layout_for_function( FrameLayout { value_placement, local_offsets, + ref_cell_state_offsets, try_handler_offsets, concat_base_offset, frame_size, allocation, callee_saved_offsets, + local_analysis, } } @@ -163,6 +177,7 @@ pub(super) fn emit_main_prologue(ctx: &mut FunctionContext<'_>) { abi::emit_enable_heap_debug_flag(ctx.emitter); } zero_initialize_main_cleanup_locals(ctx); + zero_initialize_ref_cell_state_slots(ctx); zero_initialize_ref_cell_owner_locals(ctx); zero_initialize_eval_context_locals(ctx); zero_initialize_eval_scope_locals(ctx); @@ -185,6 +200,9 @@ pub(super) fn emit_function_prologue_with_label( abi::emit_frame_prologue(ctx.emitter, ctx.frame_size); capture_concat_base(ctx); emit_callee_saved_saves(ctx); + + // Save every incoming argument before any parameter post-processing can call + // a runtime helper and clobber caller-saved argument registers. let mut incoming_args = abi::IncomingArgCursor::for_target(ctx.emitter.target, 0); for (index, param) in ctx.function.params.iter().enumerate() { let slot = LocalSlotId::from_raw(index as u32); @@ -197,23 +215,45 @@ pub(super) fn emit_function_prologue_with_label( param.by_ref, &mut incoming_args, ); + } + + // Once all ABI inputs are safe in their frame slots, materialize any widened + // Mixed boxes and retain the parameters whose slots become callee-owned. + for (index, param) in ctx.function.params.iter().enumerate() { + let slot = LocalSlotId::from_raw(index as u32); + let offset = ctx.local_offset(slot)?; let local_ty = ctx.local_php_type(slot)?; - if !param.by_ref + let converted_to_owned_mixed = !param.by_ref && local_ty.codegen_repr() == PhpType::Mixed - && param.php_type.codegen_repr() != PhpType::Mixed - { + && param.php_type.codegen_repr() != PhpType::Mixed; + if converted_to_owned_mixed { abi::emit_load(ctx.emitter, ¶m.php_type.codegen_repr(), offset); emit_box_current_value_as_mixed(ctx.emitter, ¶m.php_type.codegen_repr()); abi::emit_store(ctx.emitter, &PhpType::Mixed, offset); } + if ctx.owns_parameter_slot(slot) && !converted_to_owned_mixed { + retain_owned_parameter_local(ctx.emitter, offset, &local_ty); + } } zero_initialize_function_cleanup_locals(ctx); + zero_initialize_ref_cell_state_slots(ctx); zero_initialize_ref_cell_owner_locals(ctx); zero_initialize_eval_context_locals(ctx); zero_initialize_eval_scope_locals(ctx); Ok(()) } +/// Retains a mutable by-value parameter so its frame slot has one callee-owned reference. +fn retain_owned_parameter_local(emitter: &mut Emitter, offset: usize, ty: &PhpType) { + abi::emit_load(emitter, ty, offset); + if !matches!(ty, PhpType::Str) { + abi::emit_incref_if_refcounted(emitter, ty); + } + // `emit_store` performs the string persist itself; other refcounted values + // were retained explicitly above and are stored without another incref. + abi::emit_store(emitter, ty, offset); +} + /// Captures the caller-visible concat-buffer offset as this frame's reset base. fn capture_concat_base(ctx: &mut FunctionContext<'_>) { let scratch = abi::temp_int_reg(ctx.emitter.target); @@ -347,6 +387,7 @@ pub(super) fn emit_web_handler_prologue(ctx: &mut FunctionContext<'_>) { capture_concat_base(ctx); emit_callee_saved_saves(ctx); zero_initialize_main_cleanup_locals(ctx); + zero_initialize_ref_cell_state_slots(ctx); zero_initialize_ref_cell_owner_locals(ctx); zero_initialize_eval_context_locals(ctx); zero_initialize_eval_scope_locals(ctx); @@ -428,14 +469,9 @@ fn zero_initialize_main_cleanup_locals(ctx: &mut FunctionContext<'_>) { /// Releases owned main locals that still hold refcounted storage at process exit. fn emit_main_local_epilogue_cleanup(ctx: &mut FunctionContext<'_>) { emit_ref_cell_owner_epilogue_cleanup(ctx); - for (name, _, ty, offset) in main_cleanup_locals(ctx) { + for (name, slot, ty, offset) in main_cleanup_locals(ctx) { ctx.emitter.comment(&format!("epilogue cleanup ${}", name)); - match ty { - PhpType::Str => emit_main_string_cleanup(ctx, offset), - PhpType::Callable => emit_main_refcounted_cleanup(ctx, offset, &ty), - other if other.is_refcounted() => emit_main_refcounted_cleanup(ctx, offset, &other), - _ => {} - } + emit_owned_local_cleanup(ctx, slot, offset, &ty); } emit_eval_scope_epilogue_cleanup(ctx); emit_eval_context_epilogue_cleanup(ctx); @@ -454,7 +490,10 @@ fn main_cleanup_locals(ctx: &FunctionContext<'_>) -> Vec<(String, LocalSlotId, P .locals .iter() .filter(|local| local_kind_needs_epilogue_cleanup(local.kind)) - .filter(|local| !promoted_ref_cell_local_slots(ctx.function).contains(&local.id)) + .filter(|local| { + !ctx.local_slot_ever_stores_ref_cell_pointer(local.id) + || ctx.has_dynamic_ref_cell_state(local.id) + }) .filter(|local| { local .name @@ -462,7 +501,7 @@ fn main_cleanup_locals(ctx: &FunctionContext<'_>) -> Vec<(String, LocalSlotId, P .is_none_or(|name| !param_names.contains(name)) }) .filter(|local| { - local_slot_has_store(ctx.function, local.id) || function_has_eval_scope(ctx.function) + ctx.local_slot_has_store(local.id) || function_has_eval_scope(ctx.function) }) .filter_map(|local| { let ty = local.php_type.codegen_repr(); @@ -488,6 +527,21 @@ fn zero_initialize_ref_cell_owner_locals(ctx: &mut FunctionContext<'_>) { } } +/// Zero-initializes runtime flags for slots that may later store ref-cell pointers. +fn zero_initialize_ref_cell_state_slots(ctx: &mut FunctionContext<'_>) { + let mut offsets = ctx + .function + .locals + .iter() + .filter_map(|local| ctx.ref_cell_state_offset(local.id)) + .collect::>(); + offsets.sort_unstable(); + offsets.dedup(); + for offset in offsets { + abi::emit_store_zero_to_local_slot(ctx.emitter, offset); + } +} + /// Releases hidden ref-cell owner slots that still hold fallback cells at exit. fn emit_ref_cell_owner_epilogue_cleanup(ctx: &mut FunctionContext<'_>) { let owners = ref_cell_owner_locals(ctx); @@ -658,65 +712,13 @@ fn function_has_eval_scope(function: &Function) -> bool { .any(|local| matches!(local.kind, LocalKind::EvalScope | LocalKind::EvalGlobalScope)) } -/// Returns true when a local slot is written by an explicit EIR `StoreLocal`. -fn local_slot_has_store(function: &Function, slot: LocalSlotId) -> bool { - function.instructions.iter().any(|inst| { - inst.op == Op::StoreLocal - && matches!(inst.immediate, Some(Immediate::LocalSlot(candidate)) if candidate == slot) - }) -} - -/// Returns PHP-visible locals whose slot is rewritten to a ref-cell pointer. -fn promoted_ref_cell_local_slots(function: &Function) -> HashSet { - let mut slots = function - .instructions - .iter() - .filter_map(|inst| match inst.immediate { - Some(Immediate::LocalSlotPair { first, .. }) if inst.op == Op::PromoteLocalRefCell => { - Some(first) - } - Some(Immediate::LocalSlotPair { first, .. }) if inst.op == Op::AliasLocalRefCell => { - Some(first) - } - _ => None, - }) - .collect::>(); - slots.extend(closure_ref_capture_local_slots(function)); - slots -} - -/// Returns local slots whose value is captured by reference into a closure descriptor. -fn closure_ref_capture_local_slots(function: &Function) -> HashSet { - function - .instructions - .iter() - .filter(|inst| inst.op == Op::ClosureCapture) - .filter(|inst| inst.immediate == Some(Immediate::I64(1))) - .filter_map(|inst| inst.operands.first().copied()) - .filter_map(|value| loaded_local_slot(function, value)) - .collect() -} - -/// Resolves a lowered local read value back to its source slot. -fn loaded_local_slot(function: &Function, value: ValueId) -> Option { - let value = function.value(value)?; - let ValueDef::Instruction { inst, .. } = value.def else { - return None; - }; - let inst = function.instruction(inst)?; - match (inst.op, inst.immediate.as_ref()) { - (Op::LoadLocal | Op::LoadRefCell, Some(Immediate::LocalSlot(slot))) => Some(*slot), - _ => None, - } -} - /// Releases a string local through the validating heap-free helper. /// /// `__rt_heap_free_safe` skips non-heap pointers (null for uninitialized locals, /// .rodata, out-of-range) and frees plausible live heap blocks, so it safely handles /// the zero-length owned strings that `__rt_str_persist` now allocates. The previous /// `cbz len` guard skipped them and leaked every owned empty string at scope exit. -fn emit_main_string_cleanup(ctx: &mut FunctionContext<'_>, offset: usize) { +pub(super) fn emit_main_string_cleanup(ctx: &mut FunctionContext<'_>, offset: usize) { let (ptr_reg, _) = abi::string_result_regs(ctx.emitter); let result_reg = abi::int_result_reg(ctx.emitter); abi::load_at_offset(ctx.emitter, ptr_reg, offset); @@ -737,7 +739,7 @@ fn emit_main_string_cleanup(ctx: &mut FunctionContext<'_>, offset: usize) { } /// Releases a refcounted local when the slot contains a non-null heap pointer. -fn emit_main_refcounted_cleanup(ctx: &mut FunctionContext<'_>, offset: usize, ty: &PhpType) { +pub(super) fn emit_main_refcounted_cleanup(ctx: &mut FunctionContext<'_>, offset: usize, ty: &PhpType) { let result_reg = abi::int_result_reg(ctx.emitter); let done = ctx.next_label("main_refcounted_cleanup_done"); abi::load_at_offset(ctx.emitter, result_reg, offset); @@ -758,7 +760,10 @@ fn emit_main_refcounted_cleanup(ctx: &mut FunctionContext<'_>, offset: usize, ty /// Zero-initializes function locals that may be released by the shared epilogue. fn zero_initialize_function_cleanup_locals(ctx: &mut FunctionContext<'_>) { - for (_, _, ty, offset) in function_cleanup_locals(ctx, None) { + for (_, slot, ty, offset) in function_cleanup_locals(ctx, None) { + if local_slot_is_parameter(ctx.function, slot) { + continue; + } match ty { PhpType::Str => { abi::emit_store_zero_to_local_slot(ctx.emitter, offset); @@ -793,14 +798,9 @@ fn emit_function_local_epilogue_cleanup( push_return_value(ctx, &return_ty); } emit_ref_cell_owner_epilogue_cleanup_for(ctx, ref_cell_owners); - for (name, _, ty, offset) in cleanup_locals { + for (name, slot, ty, offset) in cleanup_locals { ctx.emitter.comment(&format!("epilogue cleanup ${}", name)); - match ty { - PhpType::Str => emit_main_string_cleanup(ctx, offset), - PhpType::Callable => emit_main_refcounted_cleanup(ctx, offset, &ty), - other if other.is_refcounted() => emit_main_refcounted_cleanup(ctx, offset, &other), - _ => {} - } + emit_owned_local_cleanup(ctx, slot, offset, &ty); } for (name, offset) in eval_scopes { ctx.emitter.comment(&format!("epilogue cleanup {}", name)); @@ -824,27 +824,24 @@ fn function_cleanup_locals( ctx: &FunctionContext<'_>, skip_return_slot: Option, ) -> Vec<(String, LocalSlotId, PhpType, usize)> { - let param_names = ctx - .function - .params - .iter() - .map(|param| param.name.as_str()) - .collect::>(); let mut locals = ctx .function .locals .iter() .filter(|local| local_kind_needs_epilogue_cleanup(local.kind)) - .filter(|local| !promoted_ref_cell_local_slots(ctx.function).contains(&local.id)) .filter(|local| { - local - .name - .as_deref() - .is_none_or(|name| !param_names.contains(name)) + !ctx.local_slot_ever_stores_ref_cell_pointer(local.id) + || ctx.has_dynamic_ref_cell_state(local.id) + }) + .filter(|local| { + !local_slot_is_parameter(ctx.function, local.id) + || ctx.owns_parameter_slot(local.id) }) .filter(|local| Some(local.id) != skip_return_slot) .filter(|local| { - local_slot_has_store(ctx.function, local.id) || function_has_eval_scope(ctx.function) + ctx.local_slot_has_store(local.id) + || ctx.owns_parameter_slot(local.id) + || function_has_eval_scope(ctx.function) }) .filter_map(|local| { let ty = local.php_type.codegen_repr(); @@ -863,6 +860,46 @@ fn function_cleanup_locals( locals } +/// Returns whether a local slot is populated from the function's incoming parameter ABI. +fn local_slot_is_parameter(function: &Function, slot: LocalSlotId) -> bool { + function.params.get(slot.as_raw() as usize).is_some() +} + +/// Releases one owned raw local unless its runtime slot currently holds a ref-cell pointer. +pub(super) fn emit_owned_local_cleanup( + ctx: &mut FunctionContext<'_>, + slot: LocalSlotId, + offset: usize, + ty: &PhpType, +) { + let done = ctx + .ref_cell_state_offset(slot) + .map(|_| ctx.next_label("raw_local_cleanup_done")); + if let (Some(state_offset), Some(done)) = (ctx.ref_cell_state_offset(slot), done.as_ref()) { + let state_reg = abi::int_result_reg(ctx.emitter); + abi::load_at_offset(ctx.emitter, state_reg, state_offset); + match ctx.emitter.target.arch { + Arch::AArch64 => { + ctx.emitter.instruction(&format!("cbnz {}, {}", state_reg, done)); // skip raw cleanup while this slot stores a ref-cell pointer + } + Arch::X86_64 => { + ctx.emitter.instruction(&format!("test {}, {}", state_reg, state_reg)); // test whether this slot currently stores a ref-cell pointer + ctx.emitter + .instruction(&format!("jne {}", done)); // skip raw cleanup for the ref-cell representation + } + } + } + match ty { + PhpType::Str => emit_main_string_cleanup(ctx, offset), + PhpType::Callable => emit_main_refcounted_cleanup(ctx, offset, ty), + other if other.is_refcounted() => emit_main_refcounted_cleanup(ctx, offset, other), + _ => {} + } + if let Some(done) = done { + ctx.emitter.label(&done); + } +} + /// Returns whether a local kind can own values through ordinary `StoreLocal`. fn local_kind_needs_epilogue_cleanup(kind: LocalKind) -> bool { matches!( @@ -1190,3 +1227,6 @@ fn superglobal_storage_needed(ctx: &FunctionContext<'_>, name: &str) -> bool { fn argv_array_type() -> PhpType { PhpType::Array(Box::new(PhpType::Str)) } + +#[cfg(test)] +mod tests; diff --git a/src/codegen/frame/tests.rs b/src/codegen/frame/tests.rs new file mode 100644 index 0000000000..4b42beda19 --- /dev/null +++ b/src/codegen/frame/tests.rs @@ -0,0 +1,130 @@ +//! Purpose: +//! Unit tests for callable-frame parameter setup and ownership retention. +//! +//! Called from: +//! - `cargo test` through Rust's test harness. +//! +//! Key details: +//! - Fixtures inspect both supported ABIs so runtime calls never precede later argument saves. + +use super::*; +use crate::codegen::generate_user_asm_from_ir; +use crate::codegen::platform::{Arch, Platform, Target}; +use crate::ir::{Builder, FunctionParam, IrType, Module, Terminator}; + +/// Verifies AArch64 saves a later Mixed argument before retaining an earlier string. +#[test] +fn aarch64_prologue_saves_all_parameters_before_runtime_calls() { + let asm = owned_string_then_mixed_prologue_asm(Target::new( + Platform::Linux, + Arch::AArch64, + )); + let later_param = asm + .find("param $value from x2") + .expect("AArch64 fixture should receive the later Mixed parameter in x2"); + let persist = asm + .find("bl __rt_str_persist") + .expect("owned string parameter should be persisted"); + + assert!(later_param < persist, "later parameter was saved after retain:\n{asm}"); + assert!(asm[later_param..persist].contains("x2, [x29"), "{asm}"); +} + +/// Verifies x86_64 saves a later Mixed argument before retaining an earlier string. +#[test] +fn x86_64_prologue_saves_all_parameters_before_runtime_calls() { + let asm = owned_string_then_mixed_prologue_asm(Target::new( + Platform::Linux, + Arch::X86_64, + )); + let later_param = asm + .find("param $value from rdx") + .expect("x86_64 fixture should receive the later Mixed parameter in rdx"); + let persist = asm + .find("call __rt_str_persist") + .expect("owned string parameter should be persisted"); + + assert!(later_param < persist, "later parameter was saved after retain:\n{asm}"); + assert!(asm[later_param..persist].contains("rdx"), "{asm}"); +} + +/// Verifies the string retention helper emits one persist call on each supported ABI shape. +#[test] +fn owned_string_parameter_is_persisted_once() { + for (target, call) in [ + ( + Target::new(Platform::Linux, Arch::AArch64), + "bl __rt_str_persist", + ), + ( + Target::new(Platform::Linux, Arch::X86_64), + "call __rt_str_persist", + ), + ] { + let mut emitter = Emitter::new(target); + retain_owned_parameter_local(&mut emitter, 16, &PhpType::Str); + let asm = emitter.output(); + + assert_eq!(asm.matches(call).count(), 1, "{asm}"); + } +} + +/// Builds a callable with an owned string parameter followed by a borrowed Mixed parameter. +fn owned_string_then_mixed_prologue_asm(target: Target) -> String { + let mut module = Module::new(target); + let mut function = Function::new( + "prologue_parameter_fixture".to_string(), + IrType::Void, + PhpType::Void, + ); + function.params.push(FunctionParam { + name: "label".to_string(), + ir_type: IrType::Str, + php_type: PhpType::Str, + by_ref: false, + variadic: false, + }); + function.params.push(FunctionParam { + name: "value".to_string(), + ir_type: IrType::Heap(crate::ir::IrHeapKind::Mixed), + php_type: PhpType::Mixed, + by_ref: false, + variadic: false, + }); + let label_slot = function.add_local( + Some("label".to_string()), + IrType::Str, + PhpType::Str, + LocalKind::PhpLocal, + ); + function.add_local( + Some("value".to_string()), + IrType::Heap(crate::ir::IrHeapKind::Mixed), + PhpType::Mixed, + LocalKind::PhpLocal, + ); + { + let mut builder = Builder::new(&mut function); + let entry = builder.create_named_block("entry", Vec::new()); + builder.set_entry(entry); + builder.position_at_end(entry); + let label = builder.emit_load_local(label_slot, IrType::Str, PhpType::Str); + builder.emit_store_local(label_slot, label); + builder.terminate(Terminator::Return { value: None }); + } + module.add_function(function); + + let mut main = Function::new("main".to_string(), IrType::Void, PhpType::Void); + main.flags.is_main = true; + { + let mut builder = Builder::new(&mut main); + let entry = builder.create_named_block("entry", Vec::new()); + builder.set_entry(entry); + builder.position_at_end(entry); + builder.terminate(Terminator::Return { value: None }); + } + module.add_function(main); + + generate_user_asm_from_ir(&module, false, false) + .expect("parameter-prologue fixture should lower") +} diff --git a/src/codegen/local_analysis.rs b/src/codegen/local_analysis.rs new file mode 100644 index 0000000000..13df504681 --- /dev/null +++ b/src/codegen/local_analysis.rs @@ -0,0 +1,593 @@ +//! Purpose: +//! Precomputes local-slot facts consumed repeatedly by EIR assembly lowering. +//! Tracks explicit stores, ref-cell representation changes, and owned parameter slots. +//! +//! Called from: +//! - `crate::codegen::context::FunctionContext::new()`. +//! +//! Key details: +//! - Ref-cell state is a forward may-analysis, so later promotions never affect earlier ops. +//! - `UnsetLocal` returns a promoted slot to raw local storage on subsequent paths. +//! - Closure, iterator, alias, and explicit binding operations all update representation state. + +use std::collections::{HashSet, VecDeque}; + +use crate::ir::{ + BlockId, Function, Immediate, InstId, LocalSlotId, Op, Terminator, ValueDef, ValueId, +}; +use crate::types::PhpType; + +/// Cached local-slot facts for one EIR function. +pub(super) struct LocalSlotAnalysis { + stored_slots: HashSet, + ever_ref_cell_slots: HashSet, + dynamic_ref_cell_slots: HashSet, + ref_cell_slots_before_inst: HashSet<(InstId, LocalSlotId)>, + release_ops_with_possible_ref_cell: HashSet, + owned_parameter_slots: HashSet, +} + +impl LocalSlotAnalysis { + /// Computes all local-slot facts with one instruction scan plus CFG propagation. + pub(super) fn new(function: &Function) -> Self { + let initially_ref_cell_slots = initially_ref_cell_slots(function); + let mut stored_slots = HashSet::new(); + let mut ever_ref_cell_slots = initially_ref_cell_slots.clone(); + for inst in &function.instructions { + if inst.op == Op::StoreLocal { + if let Some(Immediate::LocalSlot(slot)) = inst.immediate { + stored_slots.insert(slot); + } + } + if let Some(slot) = + ref_cell_target_slot(function, inst.op, inst.immediate.as_ref(), &inst.operands) + { + ever_ref_cell_slots.insert(slot); + } + } + + let (release_ops_with_possible_ref_cell, ref_cell_slots_before_inst) = + analyze_release_local_slot_states(function, &initially_ref_cell_slots); + let owned_parameter_slots = owned_parameter_slots( + function, + &stored_slots, + &ever_ref_cell_slots, + ); + let dynamic_ref_cell_slots = dynamic_ref_cell_slots(function, &ever_ref_cell_slots); + Self { + stored_slots, + ever_ref_cell_slots, + dynamic_ref_cell_slots, + ref_cell_slots_before_inst, + release_ops_with_possible_ref_cell, + owned_parameter_slots, + } + } + + /// Returns whether ordinary EIR lowering stores a value directly into this slot. + pub(super) fn has_store(&self, slot: LocalSlotId) -> bool { + self.stored_slots.contains(&slot) + } + + /// Returns whether any instruction can rewrite this slot to a ref-cell pointer. + pub(super) fn ever_stores_ref_cell_pointer(&self, slot: LocalSlotId) -> bool { + self.ever_ref_cell_slots.contains(&slot) + } + + /// Iterates slots whose runtime representation can switch between a raw value and a cell. + pub(super) fn dynamic_ref_cell_slots(&self) -> impl Iterator + '_ { + self.dynamic_ref_cell_slots.iter().copied() + } + + /// Returns whether cleanup must inspect this slot's runtime representation flag. + pub(super) fn has_dynamic_ref_cell_state(&self, slot: LocalSlotId) -> bool { + self.dynamic_ref_cell_slots.contains(&slot) + } + + /// Returns whether a deferred raw-slot release may execute after ref-cell promotion. + pub(super) fn release_may_observe_ref_cell(&self, inst: InstId) -> bool { + self.release_ops_with_possible_ref_cell.contains(&inst) + } + + /// Returns whether a slot may already be represented by a cell at one instruction. + pub(super) fn inst_may_observe_ref_cell(&self, inst: InstId, slot: LocalSlotId) -> bool { + self.ref_cell_slots_before_inst.contains(&(inst, slot)) + } + + /// Returns whether the frame owns this by-value parameter for its whole lifetime. + pub(super) fn owns_parameter_slot(&self, slot: LocalSlotId) -> bool { + self.owned_parameter_slots.contains(&slot) + } +} + +/// Returns by-reference parameter slots, whose incoming representation is already a cell pointer. +fn initially_ref_cell_slots(function: &Function) -> HashSet { + function + .params + .iter() + .enumerate() + .filter(|(_, param)| param.by_ref) + .filter_map(|(index, _)| { + let slot = LocalSlotId::from_raw(index as u32); + function.locals.get(index).map(|_| slot) + }) + .collect() +} + +/// Returns the local slot promoted or bound by one ref-cell-producing instruction. +fn ref_cell_target_slot( + function: &Function, + op: Op, + immediate: Option<&Immediate>, + operands: &[ValueId], +) -> Option { + match (op, immediate) { + ( + Op::PromoteLocalRefCell | Op::AliasLocalRefCell, + Some(Immediate::LocalSlotPair { first, .. }), + ) => Some(*first), + ( + Op::BindRefCellPtr | Op::IterCurrentValueRef, + Some(Immediate::LocalSlot(slot)), + ) => Some(*slot), + (Op::ClosureCapture, Some(Immediate::I64(1))) => operands + .first() + .copied() + .and_then(|value| loaded_local_slot(function, value)), + _ => None, + } +} + +/// Resolves a local-load SSA value back to its frame slot. +fn loaded_local_slot(function: &Function, value: ValueId) -> Option { + let value = function.value(value)?; + let ValueDef::Instruction { inst, .. } = value.def else { + return None; + }; + let inst = function.instruction(inst)?; + match (inst.op, inst.immediate.as_ref()) { + (Op::LoadLocal | Op::LoadRefCell, Some(Immediate::LocalSlot(slot))) => Some(*slot), + _ => None, + } +} + +/// Computes release instructions whose slot may already contain a ref-cell pointer. +fn analyze_release_local_slot_states( + function: &Function, + initially_ref_cell_slots: &HashSet, +) -> (HashSet, HashSet<(InstId, LocalSlotId)>) { + if function.blocks.is_empty() { + return (HashSet::new(), HashSet::new()); + } + let mut block_inputs = vec![None::>; function.blocks.len()]; + let mut block_outputs = vec![None::>; function.blocks.len()]; + let entry_index = function.entry.as_raw() as usize; + if entry_index >= function.blocks.len() { + return (HashSet::new(), HashSet::new()); + } + block_inputs[entry_index] = Some(initially_ref_cell_slots.clone()); + let mut worklist = VecDeque::from([function.entry]); + while let Some(block_id) = worklist.pop_front() { + let block_index = block_id.as_raw() as usize; + let Some(mut state) = block_inputs[block_index].clone() else { + continue; + }; + let block = &function.blocks[block_index]; + for inst_id in &block.instructions { + apply_ref_cell_transfer(function, *inst_id, &mut state); + } + if block_outputs[block_index].as_ref() == Some(&state) { + continue; + } + block_outputs[block_index] = Some(state.clone()); + let Some(terminator) = block.terminator.as_ref() else { + continue; + }; + for successor in terminator_successors(terminator) { + let successor_index = successor.as_raw() as usize; + if successor_index >= block_inputs.len() { + continue; + } + let changed = if let Some(input) = &mut block_inputs[successor_index] { + let old_len = input.len(); + input.extend(state.iter().copied()); + input.len() != old_len + } else { + block_inputs[successor_index] = Some(state.clone()); + true + }; + if changed { + worklist.push_back(successor); + } + } + } + + let mut releases = HashSet::new(); + let mut ref_cell_slots_before_inst = HashSet::new(); + for block in &function.blocks { + let Some(mut state) = block_inputs[block.id.as_raw() as usize].clone() else { + continue; + }; + for inst_id in &block.instructions { + ref_cell_slots_before_inst.extend(state.iter().map(|slot| (*inst_id, *slot))); + let Some(inst) = function.instruction(*inst_id) else { + continue; + }; + if inst.op == Op::ReleaseLocalSlot { + if let Some(Immediate::LocalSlot(slot)) = inst.immediate { + if state.contains(&slot) { + releases.insert(*inst_id); + } + } + } + apply_ref_cell_transfer(function, *inst_id, &mut state); + } + } + (releases, ref_cell_slots_before_inst) +} + +/// Applies one instruction's local representation change to the forward state. +fn apply_ref_cell_transfer( + function: &Function, + inst_id: InstId, + state: &mut HashSet, +) { + let Some(inst) = function.instruction(inst_id) else { + return; + }; + if inst.op == Op::AliasLocalRefCell { + if let Some(Immediate::LocalSlotPair { first, second }) = inst.immediate { + // Alias lowering guarantees the source is promoted on the runtime + // raw path before the target receives the shared cell pointer. + state.insert(first); + state.insert(second); + return; + } + } + if let Some(slot) = + ref_cell_target_slot(function, inst.op, inst.immediate.as_ref(), &inst.operands) + { + state.insert(slot); + return; + } + if inst.op == Op::UnsetLocal { + if let Some(Immediate::LocalSlot(slot)) = inst.immediate { + state.remove(&slot); + } + } +} + +/// Returns all CFG successors named by one terminator. +fn terminator_successors(terminator: &Terminator) -> Vec { + match terminator { + Terminator::Br { target, .. } => vec![*target], + Terminator::CondBr { + then_target, + else_target, + .. + } => vec![*then_target, *else_target], + Terminator::Switch { cases, default, .. } => { + let mut successors = cases.iter().map(|case| case.target).collect::>(); + successors.push(*default); + successors + } + Terminator::GeneratorSuspend { resume, .. } => vec![*resume], + Terminator::Return { .. } + | Terminator::Throw { .. } + | Terminator::Fatal { .. } + | Terminator::Unreachable => Vec::new(), + } +} + +/// Returns by-value parameter slots that must own incoming or subsequently stored values. +fn owned_parameter_slots( + function: &Function, + stored_slots: &HashSet, + ever_ref_cell_slots: &HashSet, +) -> HashSet { + function + .params + .iter() + .enumerate() + .filter(|(_, param)| !param.by_ref) + .filter_map(|(index, param)| { + let slot = LocalSlotId::from_raw(index as u32); + let local = function.locals.get(index)?; + let local_ty = local.php_type.codegen_repr(); + if !local_type_needs_cleanup(&local_ty) { + return None; + } + let prologue_boxes_owned_mixed = local_ty == PhpType::Mixed + && param.php_type.codegen_repr() != PhpType::Mixed; + (stored_slots.contains(&slot) + || ever_ref_cell_slots.contains(&slot) + || prologue_boxes_owned_mixed) + .then_some(slot) + }) + .collect() +} + +/// Returns whether a local representation has an implemented frame cleanup path. +fn local_type_needs_cleanup(ty: &PhpType) -> bool { + matches!(ty, PhpType::Str | PhpType::Callable) || ty.is_refcounted() +} + +/// Returns slots whose raw-value/ref-cell representation can change at runtime. +fn dynamic_ref_cell_slots( + function: &Function, + ever_ref_cell_slots: &HashSet, +) -> HashSet { + function + .locals + .iter() + .filter(|local| ever_ref_cell_slots.contains(&local.id)) + .filter(|local| { + matches!( + local.kind, + crate::ir::LocalKind::PhpLocal + | crate::ir::LocalKind::HiddenTemp + | crate::ir::LocalKind::OwnedTemp + | crate::ir::LocalKind::ClosureCapture + | crate::ir::LocalKind::NamedArgTemp + | crate::ir::LocalKind::IteratorState + | crate::ir::LocalKind::GeneratorState + ) + }) + .filter(|local| { + function + .params + .get(local.id.as_raw() as usize) + .is_none_or(|param| !param.by_ref) + }) + .map(|local| local.id) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::codegen::generate_user_asm_from_ir; + use crate::codegen::platform::{Arch, Platform, Target}; + use crate::ir::{Builder, FunctionParam, IrType, LocalKind, Module, Ownership}; + + /// Verifies a later promotion does not flow backward into an earlier deferred release. + #[test] + fn later_promotion_does_not_suppress_earlier_release() { + let mut function = + Function::new("later_promotion".to_string(), IrType::Void, PhpType::Void); + let slot = function.add_local( + Some("x".to_string()), + IrType::Heap(crate::ir::IrHeapKind::Mixed), + PhpType::Mixed, + LocalKind::PhpLocal, + ); + let owner = function.add_local( + None, + IrType::Heap(crate::ir::IrHeapKind::Mixed), + PhpType::Mixed, + LocalKind::RefCell, + ); + { + let mut builder = Builder::new(&mut function); + let entry = builder.create_named_block("entry", Vec::new()); + builder.set_entry(entry); + builder.position_at_end(entry); + builder.emit( + Op::ReleaseLocalSlot, + Vec::new(), + Some(Immediate::LocalSlot(slot)), + IrType::Void, + PhpType::Mixed, + Ownership::NonHeap, + ); + builder.emit( + Op::PromoteLocalRefCell, + Vec::new(), + Some(Immediate::LocalSlotPair { + first: slot, + second: owner, + }), + IrType::Void, + PhpType::Mixed, + Ownership::NonHeap, + ); + builder.terminate(Terminator::Return { value: None }); + } + + let analysis = LocalSlotAnalysis::new(&function); + assert!(!analysis.release_may_observe_ref_cell(InstId::from_raw(0))); + assert!(!analysis.inst_may_observe_ref_cell(InstId::from_raw(0), slot)); + assert!(analysis.has_dynamic_ref_cell_state(slot)); + } + + /// Verifies a promotion on a predecessor path protects the cell pointer from raw cleanup. + #[test] + fn prior_promotion_suppresses_later_release() { + let mut function = + Function::new("prior_promotion".to_string(), IrType::Void, PhpType::Void); + let slot = function.add_local( + Some("x".to_string()), + IrType::Heap(crate::ir::IrHeapKind::Mixed), + PhpType::Mixed, + LocalKind::PhpLocal, + ); + let owner = function.add_local( + None, + IrType::Heap(crate::ir::IrHeapKind::Mixed), + PhpType::Mixed, + LocalKind::RefCell, + ); + { + let mut builder = Builder::new(&mut function); + let entry = builder.create_named_block("entry", Vec::new()); + builder.set_entry(entry); + builder.position_at_end(entry); + builder.emit( + Op::PromoteLocalRefCell, + Vec::new(), + Some(Immediate::LocalSlotPair { + first: slot, + second: owner, + }), + IrType::Void, + PhpType::Mixed, + Ownership::NonHeap, + ); + builder.emit( + Op::ReleaseLocalSlot, + Vec::new(), + Some(Immediate::LocalSlot(slot)), + IrType::Void, + PhpType::Mixed, + Ownership::NonHeap, + ); + builder.terminate(Terminator::Return { value: None }); + } + + let analysis = LocalSlotAnalysis::new(&function); + assert!(analysis.release_may_observe_ref_cell(InstId::from_raw(1))); + assert!(analysis.inst_may_observe_ref_cell(InstId::from_raw(1), slot)); + } + + /// Verifies a widened scalar parameter stays owned even if optimization removes its stores. + #[test] + fn widened_parameter_owns_prologue_mixed_box_without_remaining_store() { + let mut function = Function::new( + "widened_parameter".to_string(), + IrType::Void, + PhpType::Void, + ); + function.params.push(FunctionParam { + name: "value".to_string(), + ir_type: IrType::I64, + php_type: PhpType::Int, + by_ref: false, + variadic: false, + }); + let slot = function.add_local( + Some("value".to_string()), + IrType::Heap(crate::ir::IrHeapKind::Mixed), + PhpType::Mixed, + LocalKind::PhpLocal, + ); + + let analysis = LocalSlotAnalysis::new(&function); + assert!(analysis.owns_parameter_slot(slot)); + } + + /// Verifies incoming by-reference cells remain borrowed and need no raw-value state flag. + #[test] + fn by_ref_parameter_is_not_treated_as_dynamic_owned_storage() { + let mut function = Function::new( + "by_ref_parameter".to_string(), + IrType::Void, + PhpType::Void, + ); + function.params.push(FunctionParam { + name: "value".to_string(), + ir_type: IrType::Heap(crate::ir::IrHeapKind::Mixed), + php_type: PhpType::Mixed, + by_ref: true, + variadic: false, + }); + let slot = function.add_local( + Some("value".to_string()), + IrType::Heap(crate::ir::IrHeapKind::Mixed), + PhpType::Mixed, + LocalKind::PhpLocal, + ); + + let analysis = LocalSlotAnalysis::new(&function); + assert!(analysis.ever_stores_ref_cell_pointer(slot)); + assert!(!analysis.has_dynamic_ref_cell_state(slot)); + assert!(!analysis.owns_parameter_slot(slot)); + } + + /// Verifies AArch64 cleanup guards skip raw release when the runtime slot is a cell. + #[test] + fn dynamic_release_emits_aarch64_representation_guard() { + let asm = dynamic_release_asm(Target::new(Platform::Linux, Arch::AArch64)); + + assert!(asm.contains("cbnz x0, _eir_main_raw_local_cleanup_done"), "{asm}"); + } + + /// Verifies x86_64 cleanup guards skip raw release when the runtime slot is a cell. + #[test] + fn dynamic_release_emits_x86_64_representation_guard() { + let asm = dynamic_release_asm(Target::new(Platform::Linux, Arch::X86_64)); + + assert!(asm.contains("test rax, rax"), "{asm}"); + assert!(asm.contains("jne _eir_main_raw_local_cleanup_done"), "{asm}"); + } + + /// Builds a conditional promotion followed by deferred cleanup for one target. + fn dynamic_release_asm(target: Target) -> String { + let mut module = Module::new(target); + let mut function = Function::new("main".to_string(), IrType::Void, PhpType::Void); + function.flags.is_main = true; + let slot = function.add_local( + Some("value".to_string()), + IrType::Heap(crate::ir::IrHeapKind::Mixed), + PhpType::Mixed, + LocalKind::PhpLocal, + ); + let owner = function.add_local( + None, + IrType::Heap(crate::ir::IrHeapKind::Mixed), + PhpType::Mixed, + LocalKind::RefCell, + ); + { + let mut builder = Builder::new(&mut function); + let entry = builder.create_named_block("entry", Vec::new()); + let promoted = builder.create_named_block("promoted", Vec::new()); + let raw = builder.create_named_block("raw", Vec::new()); + let merge = builder.create_named_block("merge", Vec::new()); + builder.set_entry(entry); + builder.position_at_end(entry); + let cond = builder.emit_const_bool(true); + builder.terminate(Terminator::CondBr { + cond, + then_target: promoted, + then_args: Vec::new(), + else_target: raw, + else_args: Vec::new(), + }); + builder.position_at_end(promoted); + builder.emit( + Op::PromoteLocalRefCell, + Vec::new(), + Some(Immediate::LocalSlotPair { + first: slot, + second: owner, + }), + IrType::Void, + PhpType::Mixed, + Ownership::NonHeap, + ); + builder.terminate(Terminator::Br { + target: merge, + args: Vec::new(), + }); + builder.position_at_end(raw); + builder.terminate(Terminator::Br { + target: merge, + args: Vec::new(), + }); + builder.position_at_end(merge); + builder.emit( + Op::ReleaseLocalSlot, + Vec::new(), + Some(Immediate::LocalSlot(slot)), + IrType::Void, + PhpType::Mixed, + Ownership::NonHeap, + ); + builder.terminate(Terminator::Return { value: None }); + } + module.add_function(function); + + generate_user_asm_from_ir(&module, false, false) + .expect("dynamic ReleaseLocalSlot fixture should lower") + } +} diff --git a/src/codegen/lower_inst.rs b/src/codegen/lower_inst.rs index 9775c3a8f4..54ec28ae0f 100644 --- a/src/codegen/lower_inst.rs +++ b/src/codegen/lower_inst.rs @@ -58,6 +58,7 @@ const BORROWED_MIXED_ARG_CELL_BYTES: usize = 32; /// Lowers one EIR instruction by opcode. pub(super) fn lower_instruction(ctx: &mut FunctionContext<'_>, inst_id: InstId) -> Result<()> { + ctx.begin_instruction(inst_id); let inst = ctx .function .instruction(inst_id) @@ -79,6 +80,7 @@ pub(super) fn lower_instruction(ctx: &mut FunctionContext<'_>, inst_id: InstId) Op::PromoteLocalRefCell => lower_promote_local_ref_cell(ctx, &inst), Op::AliasLocalRefCell => lower_alias_local_ref_cell(ctx, &inst), Op::ReleaseLocalRefCell => lower_release_local_ref_cell(ctx, &inst), + Op::ReleaseLocalSlot => lower_release_local_slot(ctx, inst_id, &inst), Op::LoadGlobal => lower_load_global(ctx, &inst), Op::StoreGlobal => lower_store_global(ctx, &inst), Op::ExternGlobalLoad => lower_extern_global_load(ctx, &inst), @@ -439,8 +441,55 @@ fn promote_local_slot_for_ref_capture( release_replaced_value: bool, ) -> Result<()> { if local_slot_stores_ref_cell_pointer(ctx, slot) { + let Some(state_offset) = ctx.ref_cell_state_offset(slot) else { + return Ok(()); + }; + let promote = ctx.next_label("promote_local_ref_cell"); + let done = ctx.next_label("promote_local_ref_cell_done"); + let state_reg = abi::int_result_reg(ctx.emitter); + abi::load_at_offset(ctx.emitter, state_reg, state_offset); + match ctx.emitter.target.arch { + Arch::AArch64 => { + ctx.emitter.instruction(&format!("cbz {}, {}", state_reg, promote)); // create the fallback cell only on the first runtime promotion + ctx.emitter + .instruction(&format!("b {}", done)); // reuse the existing cell on later loop iterations + } + Arch::X86_64 => { + ctx.emitter.instruction(&format!("test {}, {}", state_reg, state_reg)); // test whether this slot already stores a fallback cell + ctx.emitter + .instruction(&format!("je {}", promote)); // create the fallback cell only on the first runtime promotion + ctx.emitter + .instruction(&format!("jmp {}", done)); // reuse the existing cell on later loop iterations + } + } + ctx.emitter.label(&promote); + promote_local_slot_for_ref_capture_unchecked( + ctx, + slot, + owner_slot, + capture_ty, + release_replaced_value, + )?; + ctx.emitter.label(&done); return Ok(()); } + promote_local_slot_for_ref_capture_unchecked( + ctx, + slot, + owner_slot, + capture_ty, + release_replaced_value, + ) +} + +/// Allocates and installs a ref-cell after the caller has ruled out an existing cell. +fn promote_local_slot_for_ref_capture_unchecked( + ctx: &mut FunctionContext<'_>, + slot: LocalSlotId, + owner_slot: Option, + capture_ty: &PhpType, + release_replaced_value: bool, +) -> Result<()> { reject_multiword_ref_param_local(capture_ty, "capture")?; let local_ty = ctx.local_php_type(slot)?; let offset = ctx.local_offset(slot)?; @@ -4373,7 +4422,6 @@ struct RefArgWriteback { param_index: usize, source_value: ValueId, source_slot: LocalSlotId, - source_is_ref_cell: bool, source_ty: PhpType, cell_offset: usize, } @@ -5566,7 +5614,7 @@ fn emit_loaded_assoc_array_to_mixed(ctx: &mut FunctionContext<'_>) { match ctx.emitter.target.arch { Arch::AArch64 => {} Arch::X86_64 => { - ctx.emitter.instruction("mov rdi, rax"); // pass the loaded associative-array argument to the Mixed conversion helper + ctx.emitter.instruction("mov rdi, rax"); // pass the loaded associative-array argument to the Mixed conversion helper } } abi::emit_call_label(ctx.emitter, "__rt_hash_to_mixed"); @@ -5751,7 +5799,6 @@ fn plan_ref_arg_writebacks( param_index, source_value: *value, source_slot: source.slot, - source_is_ref_cell: source.is_ref_cell, source_ty, cell_offset: 0, }); @@ -5916,36 +5963,13 @@ fn store_current_scalar_result_to_ref_source( ctx: &mut FunctionContext<'_>, writeback: &RefArgWriteback, ) -> Result<()> { - if writeback.source_is_ref_cell - || local_slot_stores_ref_cell_pointer(ctx, writeback.source_slot) - { - let offset = ctx.local_offset(writeback.source_slot)?; - let pointer_reg = abi::symbol_scratch_reg(ctx.emitter); - abi::load_at_offset(ctx.emitter, pointer_reg, offset); - abi::emit_store_to_address( - ctx.emitter, - abi::int_result_reg(ctx.emitter), - pointer_reg, - 0, - ); - return Ok(()); - } - let offset = ctx.local_offset(writeback.source_slot)?; - abi::emit_store(ctx.emitter, &writeback.source_ty, offset); - Ok(()) + ctx.store_current_result_to_local(writeback.source_slot) } /// Loads a local variable's address for a by-reference method-call argument. fn materialize_local_ref_arg_address(ctx: &mut FunctionContext<'_>, value: ValueId) -> Result<()> { let source = local_ref_arg_source(ctx, value)?; - let slot = source.slot; - let offset = ctx.local_offset(slot)?; - if source.is_ref_cell || local_slot_stores_ref_cell_pointer(ctx, slot) { - abi::load_at_offset(ctx.emitter, abi::int_result_reg(ctx.emitter), offset); - } else { - abi::emit_frame_slot_address(ctx.emitter, abi::int_result_reg(ctx.emitter), offset); - } - Ok(()) + ctx.materialize_local_storage_address(source.slot, abi::int_result_reg(ctx.emitter)) } /// Returns true when a value already holds a direct pointer to an array element slot. @@ -5966,7 +5990,6 @@ fn value_is_array_element_address(ctx: &FunctionContext<'_>, value: ValueId) -> /// Describes a local operand used as a by-reference call argument. struct LocalRefArgSource { slot: LocalSlotId, - is_ref_cell: bool, } /// Resolves an EIR value back to a local slot and whether it already stores a ref-cell pointer. @@ -5983,9 +6006,8 @@ fn local_ref_arg_source(ctx: &FunctionContext<'_>, value: ValueId) -> Result false, - Op::LoadRefCell => true, + match inst_ref.op { + Op::LoadLocal | Op::LoadRefCell => {} _ => { return Err(CodegenIrError::unsupported(format!( "by-reference method call argument from opcode {}", @@ -5998,7 +6020,7 @@ fn local_ref_arg_source(ctx: &FunctionContext<'_>, value: ValueId) -> Result, inst: &Instruction) -> Result let result = inst .result .ok_or_else(|| CodegenIrError::invalid_module("load_local missing result value"))?; - let source_ty = if local_slot_stores_ref_cell_pointer(ctx, slot) { - load_ref_param_local_to_result(ctx, slot)? - } else { - ctx.load_local_to_result(slot)? - }; + let source_ty = ctx.load_local_to_result(slot)?; let result_ty = ctx.value_php_type(result)?; coerce_loaded_local_to_result_type(ctx, &source_ty, &result_ty)?; ctx.store_result_value(result) @@ -6185,20 +6203,43 @@ fn lower_load_ref_cell(ctx: &mut FunctionContext<'_>, inst: &Instruction) -> Res .result .ok_or_else(|| CodegenIrError::invalid_module("load_ref_cell missing result value"))?; let result_ty = ctx.value_php_type(result)?; - let source_ty = load_ref_cell_local_to_result_as(ctx, slot, &result_ty)?; + if ctx.local_ref_cell_representation_is_definite(slot) { + load_ref_cell_local_to_result_as(ctx, slot, &result_ty)?; + return ctx.store_result_value(result); + } + if !ctx.local_ref_cell_representation_is_dynamic(slot) { + let source_ty = ctx.load_raw_local_to_result(slot)?; + coerce_loaded_local_to_result_type(ctx, &source_ty, &result_ty)?; + return ctx.store_result_value(result); + } + let state_offset = ctx.ref_cell_state_offset(slot).ok_or_else(|| { + CodegenIrError::invalid_module(format!( + "dynamic ref-cell slot {} has no representation flag", + slot.as_raw() + )) + })?; + let ref_cell = ctx.next_label("dynamic_load_ref_cell"); + let done = ctx.next_label("dynamic_load_ref_cell_done"); + let state_reg = abi::secondary_scratch_reg(ctx.emitter); + abi::load_at_offset(ctx.emitter, state_reg, state_offset); + match ctx.emitter.target.arch { + Arch::AArch64 => { + ctx.emitter.instruction(&format!("cbnz {}, {}", state_reg, ref_cell)); // select the alias representation after runtime promotion + } + Arch::X86_64 => { + ctx.emitter.instruction(&format!("test {}, {}", state_reg, state_reg)); // test the slot's runtime representation flag + ctx.emitter.instruction(&format!("jne {}", ref_cell)); // select the alias representation after runtime promotion + } + } + let source_ty = ctx.load_raw_local_to_result(slot)?; coerce_loaded_local_to_result_type(ctx, &source_ty, &result_ty)?; + ctx.emit_branch(&done); + ctx.emitter.label(&ref_cell); + load_ref_cell_local_to_result_as(ctx, slot, &result_ty)?; + ctx.emitter.label(&done); ctx.store_result_value(result) } -/// Loads the value pointed to by an incoming by-reference local parameter. -fn load_ref_param_local_to_result( - ctx: &mut FunctionContext<'_>, - slot: LocalSlotId, -) -> Result { - let ty = ctx.local_php_type(slot)?; - load_ref_cell_local_to_result_as(ctx, slot, &ty) -} - /// Loads the value pointed to by a local ref-cell slot using the supplied alias type. fn load_ref_cell_local_to_result_as( ctx: &mut FunctionContext<'_>, @@ -6337,11 +6378,7 @@ fn lower_store_local(ctx: &mut FunctionContext<'_>, inst: &Instruction) -> Resul let value = expect_operand(inst, 0)?; let reset_concat_after_store = inst.span.is_some_and(|span| span.line > 0) && value_is_acquire_of_str_concat(ctx, value)?; - if local_slot_stores_ref_cell_pointer(ctx, slot) { - store_value_to_ref_param_local(ctx, slot, value)?; - } else { - ctx.store_value_to_local(slot, value)?; - } + ctx.store_value_to_local(slot, value)?; if reset_concat_after_store { reset_concat_to_frame_base(ctx); } @@ -6385,7 +6422,44 @@ fn instruction_for_value<'a>( fn lower_store_ref_cell(ctx: &mut FunctionContext<'_>, inst: &Instruction) -> Result<()> { let slot = expect_local_slot(inst)?; let value = expect_operand(inst, 0)?; - store_value_to_ref_cell_as(ctx, slot, value, &inst.result_php_type) + if ctx.local_ref_cell_representation_is_definite(slot) { + return store_value_to_ref_cell_as(ctx, slot, value, &inst.result_php_type); + } + if !ctx.local_ref_cell_representation_is_dynamic(slot) { + return ctx.store_value_to_raw_local(slot, value); + } + let state_offset = ctx.ref_cell_state_offset(slot).ok_or_else(|| { + CodegenIrError::invalid_module(format!( + "dynamic ref-cell slot {} has no representation flag", + slot.as_raw() + )) + })?; + let ref_cell = ctx.next_label("dynamic_store_ref_cell"); + let done = ctx.next_label("dynamic_store_ref_cell_done"); + let state_reg = abi::secondary_scratch_reg(ctx.emitter); + abi::load_at_offset(ctx.emitter, state_reg, state_offset); + match ctx.emitter.target.arch { + Arch::AArch64 => { + ctx.emitter.instruction(&format!("cbnz {}, {}", state_reg, ref_cell)); // select ref-cell storage after a runtime promotion + } + Arch::X86_64 => { + ctx.emitter.instruction(&format!("test {}, {}", state_reg, state_reg)); // test the slot's runtime representation flag + ctx.emitter.instruction(&format!("jne {}", ref_cell)); // select ref-cell storage after a runtime promotion + } + } + ctx.store_value_to_raw_local(slot, value)?; + match ctx.emitter.target.arch { + Arch::AArch64 => { + ctx.emitter.instruction(&format!("b {}", done)); // join dynamic ref-cell storage paths + } + Arch::X86_64 => { + ctx.emitter.instruction(&format!("jmp {}", done)); // join dynamic ref-cell storage paths + } + } + ctx.emitter.label(&ref_cell); + store_value_to_ref_cell_as(ctx, slot, value, &inst.result_php_type)?; + ctx.emitter.label(&done); + Ok(()) } /// Promotes an existing raw local slot into a heap ref-cell pointer. @@ -6470,6 +6544,52 @@ fn release_local_ref_cell_owner( Ok(()) } +/// Lowers a deferred `release_local_slot`: releases the refcounted value currently +/// held in a local frame slot, typed by the slot's FINAL storage type. +/// +/// Lowering emits this op before a retaining loop store when the slot's storage +/// type still looked untracked at that point but a later store on a back-edge +/// path could widen it (issue #534: an inner `for` counter widened Int→Mixed by +/// its checked-add update leaked one Mixed box per outer iteration when the +/// outer body re-initialized it). Only here, after widening finished, is the +/// decision sound. The slot is either zero (prologue zero-initializes cleanup +/// locals, and the null-guarded release helpers skip zero) or an owned value +/// boxed by a previous retaining store, so releasing it is always balanced. +fn lower_release_local_slot( + ctx: &mut FunctionContext<'_>, + inst_id: InstId, + inst: &Instruction, +) -> Result<()> { + let slot = expect_local_slot(inst)?; + // A slot promoted on a predecessor path holds the cell address, not an + // owned value. The forward analysis intentionally ignores promotions that + // occur only after this instruction, including after a loop exit. + let ty = ctx.local_php_type(slot)?.codegen_repr(); + let offset = ctx.local_offset(slot)?; + if ctx.release_local_slot_may_observe_ref_cell(inst_id) { + // A merged path can hold either raw storage or a cell pointer. Slots + // with a runtime representation flag release only the raw path; slots + // that are always cells (notably by-ref params) remain excluded. + if ctx.ref_cell_state_offset(slot).is_some() { + super::frame::emit_owned_local_cleanup(ctx, slot, offset, &ty); + } + return Ok(()); + } + match ty { + // Owned strings are freed through the validating helper, which skips + // null/uninitialized slots and non-heap (.rodata) literal pointers. + PhpType::Str => super::frame::emit_main_string_cleanup(ctx, offset), + PhpType::Callable => super::frame::emit_main_refcounted_cleanup(ctx, offset, &ty), + other if other.is_refcounted() => { + super::frame::emit_main_refcounted_cleanup(ctx, offset, &other) + } + // The slot never widened to refcounted storage: nothing can be owned. + // Lowering normally prunes these, so this arm is only a safety net. + _ => {} + } + Ok(()) +} + /// Lowers `unset($local)` by breaking any promoted alias and writing PHP null locally. fn lower_unset_local(ctx: &mut FunctionContext<'_>, inst: &Instruction) -> Result<()> { let slot = expect_local_slot(inst)?; @@ -6510,16 +6630,6 @@ fn clear_local_slot_storage( Ok(()) } -/// Stores an SSA value through the pointer held by an incoming by-reference local parameter. -fn store_value_to_ref_param_local( - ctx: &mut FunctionContext<'_>, - slot: LocalSlotId, - value: ValueId, -) -> Result<()> { - let target_ty = ctx.local_php_type(slot)?; - store_value_to_ref_cell_as(ctx, slot, value, &target_ty) -} - /// Stores an SSA value through a local ref-cell pointer using the supplied alias type. fn store_value_to_ref_cell_as( ctx: &mut FunctionContext<'_>, @@ -6871,15 +6981,10 @@ fn lower_mixed_box(ctx: &mut FunctionContext<'_>, inst: &Instruction) -> Result< fn lower_invoker_ref_arg(ctx: &mut FunctionContext<'_>, inst: &Instruction) -> Result<()> { let slot = expect_local_slot(inst)?; let source_ty = ctx.local_php_type(slot)?.codegen_repr(); - let offset = ctx.local_offset(slot)?; let ref_cell_reg = abi::secondary_scratch_reg(ctx.emitter); let marker_tag_reg = abi::tertiary_scratch_reg(ctx.emitter); let source_tag_reg = abi::symbol_scratch_reg(ctx.emitter); - if local_slot_stores_ref_cell_pointer(ctx, slot) { - abi::load_at_offset(ctx.emitter, ref_cell_reg, offset); - } else { - abi::emit_frame_slot_address(ctx.emitter, ref_cell_reg, offset); - } + ctx.materialize_local_storage_address(slot, ref_cell_reg)?; abi::emit_load_int_immediate( ctx.emitter, marker_tag_reg, diff --git a/src/codegen/lower_inst/builtins/pointers.rs b/src/codegen/lower_inst/builtins/pointers.rs index 1c01a4be68..16c33a0d7a 100644 --- a/src/codegen/lower_inst/builtins/pointers.rs +++ b/src/codegen/lower_inst/builtins/pointers.rs @@ -26,13 +26,8 @@ pub(crate) fn lower_ptr(ctx: &mut FunctionContext<'_>, inst: &Instruction) -> Re ensure_arg_count(inst, "ptr", 1)?; let value = expect_operand(inst, 0)?; match pointer_source(ctx, value)? { - PointerSource::Local { slot, is_ref_cell } => { - let offset = ctx.local_offset(slot)?; - if is_ref_cell || ctx.local_stores_ref_cell_pointer(slot) { - abi::load_at_offset(ctx.emitter, abi::int_result_reg(ctx.emitter), offset); - } else { - abi::emit_frame_slot_address(ctx.emitter, abi::int_result_reg(ctx.emitter), offset); - } + PointerSource::Local { slot } => { + ctx.materialize_local_storage_address(slot, abi::int_result_reg(ctx.emitter))?; } PointerSource::Global { symbol, bytes } => { ctx.data.add_comm(symbol.clone(), bytes); @@ -223,7 +218,7 @@ fn const_string_operand(ctx: &FunctionContext<'_>, value: ValueId) -> Result, value: ValueId) -> Result { let Some(Immediate::GlobalName(data)) = inst_ref.immediate else { diff --git a/src/codegen/lower_inst/iterators.rs b/src/codegen/lower_inst/iterators.rs index 9196150bdc..f5ce35ad0d 100644 --- a/src/codegen/lower_inst/iterators.rs +++ b/src/codegen/lower_inst/iterators.rs @@ -279,6 +279,7 @@ pub(super) fn lower_iter_current_value_ref( )) } } + ctx.mark_promoted_ref_cell(slot); Ok(()) } diff --git a/src/codegen/mod.rs b/src/codegen/mod.rs index 2c50001126..3bf839dd44 100644 --- a/src/codegen/mod.rs +++ b/src/codegen/mod.rs @@ -24,6 +24,7 @@ mod fibers; mod frame; mod function_variants; mod literal_defaults; +mod local_analysis; pub(crate) mod lower_inst; mod lower_term; mod runtime_callable_invoker; diff --git a/src/ir/builder.rs b/src/ir/builder.rs index 401cd1577e..57be074752 100644 --- a/src/ir/builder.rs +++ b/src/ir/builder.rs @@ -106,6 +106,38 @@ impl<'f> Builder<'f> { self.func.locals[slot.as_raw() as usize].php_type.clone() } + /// Neutralizes deferred `release_local_slot` ops whose slot never widened to + /// lifetime-tracked storage. + /// + /// Lowering emits `release_local_slot` before a loop store when the slot's + /// storage type LOOKS untracked at that point but a later store on a + /// back-edge path may still widen it (e.g. a `for` counter widened + /// Int→Mixed by its checked-add update). Once the whole body is lowered the + /// final storage types are known, so ops guarding slots that stayed + /// untracked are rewritten to `nop`s. This keeps scalar slots eligible for + /// dead-store elimination and load forwarding, which conservatively exclude + /// any slot named by an unknown op. + pub fn prune_untracked_release_local_slot_ops(&mut self) { + for inst in &mut self.func.instructions { + if inst.op != Op::ReleaseLocalSlot { + continue; + } + let Some(Immediate::LocalSlot(slot)) = inst.immediate else { + continue; + }; + let storage_type = &self.func.locals[slot.as_raw() as usize].php_type; + if Ownership::php_type_needs_lifetime_tracking(storage_type) { + continue; + } + // The slot's final storage is a plain scalar: the deferred release + // can never free anything, so erase it instead of shipping a no-op + // that pessimizes slot analyses in later passes. + inst.op = Op::Nop; + inst.immediate = None; + inst.effects = Op::Nop.default_effects(); + } + } + /// Returns the semantic role of a local slot. pub fn local_kind(&self, slot: LocalSlotId) -> LocalKind { self.func.locals[slot.as_raw() as usize].kind diff --git a/src/ir/instr.rs b/src/ir/instr.rs index c0ee10211b..3cabd040f7 100644 --- a/src/ir/instr.rs +++ b/src/ir/instr.rs @@ -214,6 +214,7 @@ pub enum Op { PromoteLocalRefCell, AliasLocalRefCell, ReleaseLocalRefCell, + ReleaseLocalSlot, LoadGlobal, StoreGlobal, LoadStaticLocal, @@ -512,6 +513,7 @@ impl Op { ReleaseLocalRefCell => { E::READS_LOCAL | E::WRITES_LOCAL | E::WRITES_HEAP | E::REFCOUNT_OP } + ReleaseLocalSlot => E::READS_LOCAL | E::WRITES_HEAP | E::REFCOUNT_OP, LoadGlobal | LoadStaticProperty | LoadReflectionStaticProperty @@ -661,6 +663,7 @@ impl Op { PromoteLocalRefCell => "promote_local_ref_cell", AliasLocalRefCell => "alias_local_ref_cell", ReleaseLocalRefCell => "release_local_ref_cell", + ReleaseLocalSlot => "release_local_slot", LoadGlobal => "load_global", StoreGlobal => "store_global", LoadStaticLocal => "load_static_local", diff --git a/src/ir/tests/builder_test.rs b/src/ir/tests/builder_test.rs index b40bb618ce..47411c1f7c 100644 --- a/src/ir/tests/builder_test.rs +++ b/src/ir/tests/builder_test.rs @@ -7,7 +7,9 @@ //! Key details: //! - The builder must preserve table ID relationships that the validator later checks. -use crate::ir::{Builder, Function, IrType, Terminator}; +use crate::ir::{ + Builder, Function, Immediate, IrHeapKind, IrType, LocalKind, Op, Ownership, Terminator, +}; use crate::types::PhpType; /// Builds a minimal function that returns a constant. @@ -53,3 +55,71 @@ fn build_function_with_block_param_and_iadd() { assert_eq!(function.blocks[1].params.len(), 1); assert_eq!(function.blocks[1].instructions.len(), 2); } + +/// Keeps a deferred local release after the slot widens from scalar to Mixed storage. +#[test] +fn deferred_local_release_survives_refcounted_widening() { + let mut function = Function::new("widened_release".to_string(), IrType::Void, PhpType::Void); + { + let mut builder = Builder::new(&mut function); + let entry = builder.create_named_block("entry", Vec::new()); + builder.set_entry(entry); + builder.position_at_end(entry); + let slot = builder.add_local( + Some("value".to_string()), + IrType::I64, + PhpType::Int, + LocalKind::PhpLocal, + ); + builder.emit( + Op::ReleaseLocalSlot, + Vec::new(), + Some(Immediate::LocalSlot(slot)), + IrType::Void, + PhpType::Int, + Ownership::NonHeap, + ); + builder.widen_local_storage_type(slot, PhpType::Mixed); + builder.prune_untracked_release_local_slot_ops(); + builder.terminate(Terminator::Return { value: None }); + } + + assert_eq!(function.locals[0].ir_type, IrType::Heap(IrHeapKind::Mixed)); + assert_eq!(function.instructions[0].op, Op::ReleaseLocalSlot); + assert_eq!( + function.instructions[0].immediate, + Some(Immediate::LocalSlot(function.locals[0].id)) + ); +} + +/// Rewrites a deferred local release to `Nop` when the slot remains scalar. +#[test] +fn deferred_local_release_is_pruned_for_scalar_storage() { + let mut function = Function::new("scalar_release".to_string(), IrType::Void, PhpType::Void); + { + let mut builder = Builder::new(&mut function); + let entry = builder.create_named_block("entry", Vec::new()); + builder.set_entry(entry); + builder.position_at_end(entry); + let slot = builder.add_local( + Some("value".to_string()), + IrType::I64, + PhpType::Int, + LocalKind::PhpLocal, + ); + builder.emit( + Op::ReleaseLocalSlot, + Vec::new(), + Some(Immediate::LocalSlot(slot)), + IrType::Void, + PhpType::Int, + Ownership::NonHeap, + ); + builder.prune_untracked_release_local_slot_ops(); + builder.terminate(Terminator::Return { value: None }); + } + + assert_eq!(function.instructions[0].op, Op::Nop); + assert_eq!(function.instructions[0].immediate, None); + assert_eq!(function.instructions[0].effects, Op::Nop.default_effects()); +} diff --git a/src/ir/validator.rs b/src/ir/validator.rs index b6f7302c7b..dec41be83f 100644 --- a/src/ir/validator.rs +++ b/src/ir/validator.rs @@ -309,7 +309,7 @@ fn validate_instruction_immediate( require_immediate(inst_id, inst, "data id", |imm| matches!(imm, Imm::Data(_))) } LoadLocal | StoreLocal | UnsetLocal | LoadRefCell | StoreRefCell | ReleaseLocalRefCell - | BindRefCellPtr + | ReleaseLocalSlot | BindRefCellPtr | LoadStaticLocal | StoreStaticLocal | InitStaticLocal | InvokerRefArg => require_immediate(inst_id, inst, "local slot", |imm| { matches!(imm, Imm::LocalSlot(_)) }), @@ -451,7 +451,8 @@ fn validate_opcode_rules( | LoadReflectionStaticProperty | ReflectionStaticPropertyInitialized | ExternGlobalLoad => check_count(inst_id, inst, 0, "0"), - UnsetLocal | PromoteLocalRefCell | AliasLocalRefCell | ReleaseLocalRefCell => { + UnsetLocal | PromoteLocalRefCell | AliasLocalRefCell | ReleaseLocalRefCell + | ReleaseLocalSlot => { check_count(inst_id, inst, 0, "0") } StoreLocal | StoreGlobal | StoreStaticLocal | InitStaticLocal | StoreStaticProperty diff --git a/src/ir_lower/context.rs b/src/ir_lower/context.rs index a1e7ea421b..133a39fe41 100644 --- a/src/ir_lower/context.rs +++ b/src/ir_lower/context.rs @@ -629,8 +629,12 @@ impl<'m, 'f> LoweringContext<'m, 'f> { self.eval_executed } - /// Declares a hidden owner slot for a promoted local ref-cell pointer. + /// Returns the reusable hidden owner slot for a promoted local, declaring it if needed. fn declare_ref_cell_owner(&mut self, variable: &str, php_type: PhpType) -> LocalSlotId { + if let Some(slot) = self.ref_cell_owner_locals.get(variable).copied() { + self.builder.widen_local_storage_type(slot, php_type); + return slot; + } let name = format!("__eir_ref_owner{}_{}", self.hidden_temp_counter, variable); self.hidden_temp_counter += 1; let slot = self.declare_local_with_kind(&name, php_type, LocalKind::RefCell); @@ -817,6 +821,8 @@ impl<'m, 'f> LoweringContext<'m, 'f> { let slot = self.declare_local(name, php_type.clone()); self.builder .widen_local_storage_type(slot, php_type.clone()); + // Retain before cleanup because a borrowed result can alias the old slot. + let stored = crate::ir_lower::ownership::acquire_if_refcounted(self, value, span); if local_kind_uses_plain_store_cleanup(previous_kind) && previous_slot.is_some_and(|slot| self.initialized_slots.contains(&slot)) { @@ -828,7 +834,6 @@ impl<'m, 'f> LoweringContext<'m, 'f> { { self.release_stored_local_value(name, slot, span); } - let stored = crate::ir_lower::ownership::acquire_if_refcounted(self, value, span); self.store_slot_with_op(slot, stored, Op::StoreLocal, span); self.set_local_type(name, php_type); if self.value_needs_release_after_retaining_store(value) { @@ -901,6 +906,50 @@ impl<'m, 'f> LoweringContext<'m, 'f> { crate::ir_lower::ownership::release_if_owned(self, previous, span); } + /// Releases the previous occupant immediately before a retaining store overwrites it. + /// + /// The caller must first retain the incoming value because borrowing operations + /// can return storage that aliases the previous occupant (for example, + /// `$value = trim($value)`). When the slot's storage type already needs lifetime + /// tracking this emits the eager load+release pair. When it does not, the slot can + /// STILL be widened to refcounted storage by a store lowered later that reaches + /// this one through a loop back-edge (e.g. an inner `for` counter re-initialized + /// by the outer body but widened Int→Mixed by its checked-add update). The storage + /// type visible here is stale in that case, so inside loops a deferred + /// `release_local_slot` is emitted instead: the backend releases the occupant + /// using the final widened storage type, and `prune_untracked_release_local_slot_ops` + /// erases the op when the slot never widens (issue #534: without this, the + /// previous outer iteration's Mixed box leaked on every re-initialization). + fn release_stored_local_value_before_overwrite( + &mut self, + name: &str, + slot: LocalSlotId, + span: Option, + ) { + let storage_type = self.builder.local_php_type(slot); + if Ownership::php_type_needs_lifetime_tracking(&storage_type) { + self.release_stored_local_value(name, slot, span); + return; + } + if self.loop_stack.is_empty() { + // Outside loops no back-edge can execute a later widening store before + // this one, so the untracked storage type is final for this path. + return; + } + // Ref-bound locals keep a cell pointer in the frame slot and are released + // through the ref-cell owner machinery, never through a raw slot release. + if self.is_ref_bound_local(name) { + return; + } + self.emit_void( + Op::ReleaseLocalSlot, + Vec::new(), + Some(Immediate::LocalSlot(slot)), + Op::ReleaseLocalSlot.default_effects(), + span, + ); + } + /// Emits a store to a PHP local slot, updates type facts, and returns the stored value. pub(crate) fn store_local( &mut self, @@ -961,35 +1010,7 @@ impl<'m, 'f> LoweringContext<'m, 'f> { && !matches!(previous_kind, LocalKind::HiddenTemp | LocalKind::OwnedTemp); let transfer_callable_source_to_store = source_is_owning_temporary && matches!(php_type.codegen_repr(), PhpType::Callable); - if !uses_global - && local_kind_uses_plain_store_cleanup(previous_kind) - && previous_slot.is_some_and(|slot| self.initialized_slots.contains(&slot)) - { - self.release_stored_local_value(name, slot, span); - } - // A loop-carried slot can exist globally without being definitely initialized - // on this CFG path. Release the runtime occupant before overwriting it. - if !uses_global - && local_kind_uses_plain_store_cleanup(previous_kind) - && previous_slot.is_some_and(|slot| !self.initialized_slots.contains(&slot)) - && !self.loop_stack.is_empty() - { - self.release_stored_local_value(name, slot, span); - } - // A first syntactic store inside a loop body (main or function) can still - // overwrite a prior runtime iteration's value: the slot has no straight-line - // predecessor store so it is not in `initialized_slots`, but the loop back-edge - // makes it live on iterations 2+. Release the previous occupant so the old value - // is freed on reassign. Function cleanup locals (including returned slots) are - // zero-initialized in the prologue, so the first iteration safely releases a null - // slot; subsequent iterations release the prior value. - if !uses_global - && local_kind_uses_plain_store_cleanup(previous_kind) - && previous_slot.is_none() - && !self.loop_stack.is_empty() - { - self.release_stored_local_value(name, slot, span); - } + // Retain before cleanup because a borrowed result can alias the old slot. let value = if (uses_global || previous_kind == LocalKind::PhpLocal) && !transfer_callable_source_to_store && !self.is_ref_bound_local(name) @@ -1016,6 +1037,35 @@ impl<'m, 'f> LoweringContext<'m, 'f> { } else { value }; + if !uses_global + && local_kind_uses_plain_store_cleanup(previous_kind) + && previous_slot.is_some_and(|slot| self.initialized_slots.contains(&slot)) + { + self.release_stored_local_value_before_overwrite(name, slot, span); + } + // A loop-carried slot can exist globally without being definitely initialized + // on this CFG path. Release the runtime occupant before overwriting it. + if !uses_global + && local_kind_uses_plain_store_cleanup(previous_kind) + && previous_slot.is_some_and(|slot| !self.initialized_slots.contains(&slot)) + && !self.loop_stack.is_empty() + { + self.release_stored_local_value_before_overwrite(name, slot, span); + } + // A first syntactic store inside a loop body (main or function) can still + // overwrite a prior runtime iteration's value: the slot has no straight-line + // predecessor store so it is not in `initialized_slots`, but the loop back-edge + // makes it live on iterations 2+. Release the previous occupant so the old value + // is freed on reassign. Function cleanup locals (including returned slots) are + // zero-initialized in the prologue, so the first iteration safely releases a null + // slot; subsequent iterations release the prior value. + if !uses_global + && local_kind_uses_plain_store_cleanup(previous_kind) + && previous_slot.is_none() + && !self.loop_stack.is_empty() + { + self.release_stored_local_value_before_overwrite(name, slot, span); + } if uses_global { self.store_global_name(name, slot, value, span); self.set_local_type(name, php_type); @@ -1105,6 +1155,10 @@ impl<'m, 'f> LoweringContext<'m, 'f> { let slot = self.declare_local(name, php_type.clone()); self.builder .widen_local_storage_type(slot, php_type.clone()); + let source = value; + let release_source_after_store = self.value_needs_release_after_retaining_store(value); + // Retain before cleanup because a borrowed result can alias the old slot. + let stored = crate::ir_lower::ownership::acquire_if_refcounted(self, value, span); if local_kind_uses_plain_store_cleanup(previous_kind) && previous_slot.is_some_and(|slot| self.initialized_slots.contains(&slot)) { @@ -1122,9 +1176,6 @@ impl<'m, 'f> LoweringContext<'m, 'f> { { self.release_stored_local_value(name, slot, span); } - let source = value; - let release_source_after_store = self.value_needs_release_after_retaining_store(value); - let stored = crate::ir_lower::ownership::acquire_if_refcounted(self, value, span); self.store_slot_with_op(slot, stored, Op::StoreLocal, span); self.set_local_type(name, php_type); if release_source_after_store { @@ -1270,7 +1321,7 @@ impl<'m, 'f> LoweringContext<'m, 'f> { ); } - /// Promotes an initialized local into a fallback ref-cell for by-reference foreach. + /// Emits an idempotent promotion of an initialized local into an owned fallback ref-cell. pub(crate) fn promote_local_ref_cell(&mut self, name: &str, span: Option) { let slot = self.declare_local(name, self.local_type(name)); let fallback_ty = self.builder.local_php_type(slot); @@ -1299,9 +1350,11 @@ impl<'m, 'f> LoweringContext<'m, 'f> { return; } let source_ty = self.local_type(source); - if !self.is_ref_bound_local(source) { - self.promote_local_ref_cell(source, span); - } + // `is_ref_bound_local` is intentionally conservative across lowered + // branches, so a source marked by a conditional predecessor may still + // be raw on another runtime path. An idempotent promotion here gives + // every alias operation a cell on all incoming paths. + self.promote_local_ref_cell(source, span); self.clear_static_callable_local(target); self.clear_reflection_class_local(target); self.clear_reflection_function_local(target); diff --git a/src/ir_lower/function.rs b/src/ir_lower/function.rs index 6f4154dc74..4f745c3381 100644 --- a/src/ir_lower/function.rs +++ b/src/ir_lower/function.rs @@ -861,6 +861,9 @@ fn lower_body_into_function( crate::ir_lower::stmt::lower_stmt(&mut ctx, stmt); } terminate_open_block(&mut ctx); + // Final storage types are now known: erase deferred loop-store releases that + // guard slots which never widened to lifetime-tracked storage (issue #534). + ctx.builder.prune_untracked_release_local_slot_ops(); ctx.into_closures() } diff --git a/src/ir_lower/tests/ownership.rs b/src/ir_lower/tests/ownership.rs index e122dd3292..614a019c9e 100644 --- a/src/ir_lower/tests/ownership.rs +++ b/src/ir_lower/tests/ownership.rs @@ -20,6 +20,17 @@ fn main_function_text(text: &str) -> &str { } } +/// Returns the printed EIR slice for one named function. +fn named_function_text<'a>(text: &'a str, name: &str) -> &'a str { + let needle = format!("function {name}("); + let start = text.find(&needle).expect("expected named lowered function"); + let tail = &text[start..]; + match tail[1..].find("\n function ") { + Some(next_function) => &tail[..1 + next_function], + None => tail, + } +} + /// Verifies storing a freshly allocated array releases the temporary producer after the store. #[test] fn fresh_array_local_assignment_releases_source_after_store() { @@ -103,6 +114,37 @@ fn overwriting_string_local_emits_release() { assert!(text.contains("release"), "expected release in {text}"); } +/// Verifies a borrowed string result is retained before its aliased source slot is released. +#[test] +fn self_reassignment_acquires_borrowed_string_before_releasing_slot() { + let module = super::lower_source( + r#" HashSet { Op::PromoteLocalRefCell | Op::AliasLocalRefCell | Op::ReleaseLocalRefCell + | Op::ReleaseLocalSlot | Op::InvokerRefArg ) { for slot in slots_of(inst) { diff --git a/tests/codegen/optimizer.rs b/tests/codegen/optimizer.rs index 734d918953..b6b0259318 100644 --- a/tests/codegen/optimizer.rs +++ b/tests/codegen/optimizer.rs @@ -31,6 +31,8 @@ mod eir_licm; mod identity_arithmetic; #[path = "optimizer/peephole.rs"] mod peephole; +#[path = "optimizer/release_local_slot.rs"] +mod release_local_slot; #[path = "optimizer/inline.rs"] mod inline; #[path = "optimizer/memory_model_propagation.rs"] diff --git a/tests/codegen/optimizer/release_local_slot.rs b/tests/codegen/optimizer/release_local_slot.rs new file mode 100644 index 0000000000..7821370447 --- /dev/null +++ b/tests/codegen/optimizer/release_local_slot.rs @@ -0,0 +1,147 @@ +//! Purpose: +//! Integration coverage for deferred `ReleaseLocalSlot` pruning and ref-cell boundaries. +//! Compares optimized and unoptimized EIR plus native heap-debug behavior. +//! +//! Called from: +//! - `cargo test --test codegen_tests optimizer::release_local_slot`. +//! +//! Key details: +//! - A promotion after a loop must not suppress releases inside that loop. +//! - Scalar and already-ref-bound slots must not retain unnecessary raw-slot releases. + +use super::*; + +/// Emits textual EIR for one source fixture with the requested IR optimizer mode. +fn emit_release_ir(source: &str, ir_opt: bool) -> String { + let dir = make_cli_test_dir("elephc_release_local_slot_emit_ir"); + let php_path = dir.join("main.php"); + fs::write(&php_path, source).expect("failed to write PHP fixture"); + let mode = if ir_opt { "--ir-opt=on" } else { "--ir-opt=off" }; + let output = elephc_cli_command(&dir) + .arg("--emit-ir") + .arg(mode) + .arg(&php_path) + .output() + .expect("failed to run elephc --emit-ir"); + assert!( + output.status.success(), + "elephc --emit-ir failed: stderr={}", + String::from_utf8_lossy(&output.stderr) + ); + let text = String::from_utf8(output.stdout).expect("EIR output should be UTF-8"); + let _ = fs::remove_dir_all(&dir); + text +} + +/// Compiles and runs one heap-debug fixture through the CLI in a fixed optimizer mode. +fn run_release_fixture(source: &str, ir_opt: bool) -> (String, String) { + let id = TEST_ID.fetch_add(1, Ordering::SeqCst); + let dir = std::env::temp_dir().join(format!( + "elephc_release_local_slot_runtime_{}_{}", + std::process::id(), + id + )); + fs::create_dir_all(&dir).expect("failed to create CLI fixture directory"); + let php_path = dir.join("main.php"); + fs::write(&php_path, source).expect("failed to write PHP fixture"); + let mode = if ir_opt { "--ir-opt=on" } else { "--ir-opt=off" }; + let compile = elephc_cli_command(&dir) + .arg("--heap-debug") + .arg(mode) + .arg(&php_path) + .output() + .expect("failed to compile ReleaseLocalSlot fixture"); + assert!( + compile.status.success(), + "fixture compilation failed: {}", + String::from_utf8_lossy(&compile.stderr) + ); + let output = Command::new(dir.join("main")) + .output() + .expect("failed to run ReleaseLocalSlot fixture"); + assert!( + output.status.success(), + "fixture execution failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + let stdout = String::from_utf8(output.stdout).expect("stdout should be UTF-8"); + let stderr = String::from_utf8(output.stderr).expect("stderr should be UTF-8"); + let _ = fs::remove_dir_all(&dir); + (stdout, stderr) +} + +/// Verifies a late by-reference promotion preserves the loop release in both EIR modes. +#[test] +fn test_release_local_slot_survives_late_ref_promotion_with_optimizer_on_and_off() { + let source = r#"value; + } + $alias =& $local; + $alias = 7; + return $local; +} +echo update_conditional_ref(false) . '|' . update_conditional_ref(true); +"#, + ); + assert_eq!(out, "7|7"); +} + /// `$x = &$obj->prop` aliases an array property: appends through the alias are observed /// through the property, and clearing the alias to `[]` empties the property (the shape /// used by `$instanceof = []` after capturing a reference). diff --git a/tests/codegen/runtime_gc/regressions.rs b/tests/codegen/runtime_gc/regressions.rs index dce4ce70b3..238ab2eb8f 100644 --- a/tests/codegen/runtime_gc/regressions.rs +++ b/tests/codegen/runtime_gc/regressions.rs @@ -289,6 +289,50 @@ echo clean("Hello World"); assert_eq!(out, "hello_world"); } +/// Verifies retaining a mutable string parameter cannot clobber a later ABI +/// argument before the callee saves it, and that the retained copy is released. +#[test] +fn test_owned_string_parameter_preserves_later_mixed_argument() { + let out = compile_and_run_with_heap_debug( + r#"