Skip to content

fix(core): audit-remediation suite, re-ported to the active EIR backend#347

Draft
Guikingone wants to merge 1 commit into
illegalstudio:mainfrom
Guikingone:fix/audit-remediation
Draft

fix(core): audit-remediation suite, re-ported to the active EIR backend#347
Guikingone wants to merge 1 commit into
illegalstudio:mainfrom
Guikingone:fix/audit-remediation

Conversation

@Guikingone

@Guikingone Guikingone commented Jun 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

This branch remediates the findings from a full compiler audit (correctness, memory safety, PHP compatibility, performance), grouped by theme.

It was originally authored against the legacy AST backend. Since EIR is now the only active backend, every fix has been re-ported to EIR so it actually takes effect on emitted code — the audit commit carry both the original legacy fix and its EIR implementation, and a final commit regenerates the builtin docs/registry to match.

Fixes (by theme)

  • Critical — foreach-var heap release gated on ownership (double-free); x86_64 NaN-compare parity guard; PHP_INT_MIN % -1 fold panic.
  • High — cast precedence + name resolution; Mixed/Union coercion for math builtins / floatval / min/max / is_a/is_subclass_of; substr negative length; float sort by numeric value; variadic array_merge/diff/intersect; in_array/array_search $strict; SORT_* flags.
  • Medium — DCE keeps fatal-capable builtins; division/modulo-by-zero fatals; array_merge string-array corruption; infer↔emitter type alignment; round() $mode (HALF_UP / HALF_EVEN); array_slice preserve_keys; number_format multi-byte separators.
  • Low — array_sum/array_product over float arrays; settype; str_split; negative-key auto-increment; NaN-ordering docs.
  • Perf — @src comment allocation; is_subclass_of clone.
  • Feature — array_map($cb, $a, $b) two-array form (int + string element arrays, every callback shape: named fn, capture-less/capturing closure, closure-holding variable).

EIR re-port notes

A recurring root cause worth calling out for reviewers: EIR lowering (src/ir_lower/) computes its own builtin result-type inference, independent of the type checker. So several fixes (array_sum float, array_slice preserve_keys, two-array array_map) had to correct the result type there as well as in the checker — otherwise the value flowed as Int/Mixed, the backend took the wrong code path, and echo/implode printed garbage. The legacy emitters under src/codegen/ are the frozen backend and are left as-is.

Verification

Build is warning-free and whitespace-clean. Each re-ported feature's focused suite passes on macOS-aarch64 locally (division 17, strict 55, min 154, round 78, number_format 11, array_merge 7, sorting 13, is_a 14, instanceof 27, slice 15, array_map 53, aggregates 8, substr 19, floatval 3) with no cross-feature regression (broad arrays:: 286/0).

Linux x86_64 / arm64 are validated by this PR's codegen matrix. The x86-only paths that local macOS testing can't exercise (the __rt_mixed_unbox payload register rdi/x1, the number_format ptr+len argument ABI, the target-aware argument-register marshalling for array_map2) were reviewed explicitly against the established runtime ABIs.

Rebased clean onto the latest main.

@nahime0

nahime0 commented Jun 12, 2026

Copy link
Copy Markdown
Member

Hi @Guikingone, I'm currently landing the new IR-based backend path on main. Feel free to mark this PR as ready for review when you're ready. I'll take care of aligning it with the new backend.

@Guikingone
Guikingone force-pushed the fix/audit-remediation branch from 401239e to 9a443a8 Compare June 29, 2026 08:32
@Guikingone Guikingone changed the title refactor(core): improvements on merged topics fix(core): audit-remediation suite, re-ported to the active EIR backend Jun 29, 2026
@Guikingone
Guikingone force-pushed the fix/audit-remediation branch from bef3774 to 861e04e Compare June 29, 2026 08:49
Guikingone added a commit to Guikingone/elephc that referenced this pull request Jun 30, 2026
@Guikingone
fix: resolve rebase conflicts and CI failures on PR illegalstudio#347
9380695 
Rebased fix/audit-remediation onto origin/main and resolved conflicts:
- src/codegen/runtime/arrays/mod.rs: kept both origin/main modules
  (array_merge_recursive, array_multisort, array_to_hash,
  array_udiff_uintersect) and PR modules (array_map2, array_map2_str,
  array_merge_str, array_sum_float, array_product_float,
  array_slice_preserve, sort_float).
- src/types/signatures.rs: merged array_is_list/array_key_first/
  array_key_last additions (origin/main) with sort flags optional
  parameter (PR).
- src/optimize/effects/builtins.rs: kept origin/main pure builtin list
  and restored is_pure_but_may_throw_builtin function from the PR.
- docs/scripts: took origin/main versions and regenerated via
  extract_builtins.py --render --force.

Fixed two CI test failures:
1. ir_backend_handles_scalar_builtins: substr('Hello', 1, -2) returns
   'el' in PHP, not empty string. Updated expected to '[el]'.
2. test_nan_bool_cast_and_empty_float (x86_64 only): emit_float_result
   _zero_bool used sete without a parity guard, so NaN (unordered)
   compared equal to 0.0 and empty(NaN) returned true. Added setnp cl
   + and al, cl so NaN requires an ordered compare, matching the
   legacy backend fix and AArch64 behaviour.
@Guikingone
Guikingone force-pushed the fix/audit-remediation branch from 861e04e to 9380695 Compare June 30, 2026 14:24
Guikingone added a commit to Guikingone/elephc that referenced this pull request Jul 2, 2026
@Guikingone
fix: resolve rebase conflicts and CI failures on PR illegalstudio#347
1bb0879 
Rebased fix/audit-remediation onto origin/main and resolved conflicts:
- src/codegen/runtime/arrays/mod.rs: kept both origin/main modules
  (array_merge_recursive, array_multisort, array_to_hash,
  array_udiff_uintersect) and PR modules (array_map2, array_map2_str,
  array_merge_str, array_sum_float, array_product_float,
  array_slice_preserve, sort_float).
- src/types/signatures.rs: merged array_is_list/array_key_first/
  array_key_last additions (origin/main) with sort flags optional
  parameter (PR).
- src/optimize/effects/builtins.rs: kept origin/main pure builtin list
  and restored is_pure_but_may_throw_builtin function from the PR.
- docs/scripts: took origin/main versions and regenerated via
  extract_builtins.py --render --force.

Fixed two CI test failures:
1. ir_backend_handles_scalar_builtins: substr('Hello', 1, -2) returns
   'el' in PHP, not empty string. Updated expected to '[el]'.
2. test_nan_bool_cast_and_empty_float (x86_64 only): emit_float_result
   _zero_bool used sete without a parity guard, so NaN (unordered)
   compared equal to 0.0 and empty(NaN) returned true. Added setnp cl
   + and al, cl so NaN requires an ordered compare, matching the
   legacy backend fix and AArch64 behaviour.
@Guikingone
Guikingone force-pushed the fix/audit-remediation branch from 9380695 to 1bb0879 Compare July 2, 2026 16:15
Guikingone added a commit to Guikingone/elephc that referenced this pull request Jul 4, 2026
@Guikingone
feat: round() $mode argument (PHP_ROUND_HALF_UP / PHP_ROUND_HALF_EVEN)
2324998 
Re-port of the audit-remediation round-mode fix onto the active EIR
builtin-registry architecture (PR illegalstudio#347 was authored before the registry
migration).

- Registry: round now declares `mode: Int = 1` (1-3 args) with a check
  hook that rejects the not-yet-specialized PHP_ROUND_HALF_DOWN (2) and
  PHP_ROUND_HALF_ODD (4) modes.
- Constants: new src/types/round_constants.rs (single source of truth for
  PHP_ROUND_HALF_*), registered in the checker predefined constants and
  materialized in codegen prescan.
- EIR emitter: lower_round accepts the 3rd operand; a statically-known
  PHP_ROUND_HALF_EVEN selects banker's rounding (frintn on AArch64, rint
  on x86_64), otherwise ties-away-from-zero (frinta / round), for both the
  plain and precision paths.
- Parity: legacy golden signature updated to the 3-param shape.
- Tests: round-mode codegen test (verified against php -r) + error tests
  for arity and unsupported modes. Builtin docs/registry regenerated.
@Guikingone
Guikingone force-pushed the fix/audit-remediation branch from 1bb0879 to fb661a4 Compare July 4, 2026 20:07
@Guikingone
feat(core): audit-remediation suite re-ported to the active EIR backend
1f50514 
Remediates the compiler-audit findings on the active EIR backend (the
legacy AST emitters under src/codegen/ are frozen and left as-is). Each
fix is verified byte-exact against the PHP CLI; builds warning-free,
whitespace- and assembly-alignment-clean; docs and the builtin registry
regenerated.

Builtins (element-type / argument coverage):
- round($mode): PHP_ROUND_HALF_UP / PHP_ROUND_HALF_EVEN.
- min()/max(): coerce a boxed Mixed operand to its numeric value.
- in_array()/array_search(): accept the optional $strict flag.
- array_sum()/array_product(): float-element arrays (float result reg).
- sort()/rsort(): float-element arrays ordered by IEEE numeric value.
- array_merge(): indexed string arrays via the 16-byte-slot helper.
- array_slice(preserve_keys: true): key-preserving associative slice.
- array_map($cb, $a, $b): two-input-array form (int/string subset).
- array_merge()/array_diff()/array_intersect(): accept three or more
  arrays, folded left-associatively into the two-array runtime with the
  intermediate arrays released.
- substr(): negative length drops characters from the end (PHP parity).
- is_a()/is_subclass_of(): boxed Mixed receiver (runtime class-id match)
  and string class-name subject (with is_a allow_string semantics).
- number_format(): multi-byte separators preserved (ptr+len ABI), and
  rounds half away from zero (PHP_ROUND_HALF_UP) instead of snprintf's
  half-to-even.

Language safety:
- Division / modulo by zero raise an (uncatchable) fatal instead of
  producing INF / 0; float `/` keeps NaN divisors as NaN.
- Constant folder computes `x % -1` as 0, avoiding the i64::MIN % -1
  overflow panic.

EIR note: ir_lower derives its own builtin result types independent of
the checker, so the float-aggregate, array_slice-preserve, two-array
array_map, and variadic-merge result types are fixed there as well as in
the type checker.

Known residuals (pre-existing, tracked separately): number_format() /
round() do not apply PHP's _php_math_round pre-rounding, so FP-error
half cases (e.g. 1.005) differ; array-builtin results are not fully
reclaimed when consumed, so nested/variadic merges leak the result
header block (the intermediate payloads are explicitly released).

macOS-aarch64 green locally; Linux x86_64 / aarch64 via CI.
@Guikingone
Guikingone force-pushed the fix/audit-remediation branch from 8c38c60 to 1f50514 Compare July 4, 2026 21:48
@github-actions github-actions Bot added area:codegen Touches target-aware assembly or backend lowering. area:eir Touches EIR definitions, lowering, validation, or passes. area:runtime Touches runtime helpers, GC, ownership, or bridge runtimes. scope:multi-area Touches more compiler areas than the automatic area-label cap. size:xl Very large pull request that needs deliberate review planning. type:fix Corrects broken or incompatible behavior. labels Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

area:codegen Touches target-aware assembly or backend lowering. area:eir Touches EIR definitions, lowering, validation, or passes. area:runtime Touches runtime helpers, GC, ownership, or bridge runtimes. scope:multi-area Touches more compiler areas than the automatic area-label cap. size:xl Very large pull request that needs deliberate review planning. type:fix Corrects broken or incompatible behavior.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Guikingone @nahime0