U1(functional-hardening): aggregate/field ARC matrix + seeded S1 fix - #131
Open
benpayne wants to merge 2 commits into
Open
U1(functional-hardening): aggregate/field ARC matrix + seeded S1 fix#131benpayne wants to merge 2 commits into
benpayne wants to merge 2 commits into
Conversation
…pros Baseline (green): run_tests 195 LLVM / 190 parse-only, test_codegen 85, ctest 54, codegen_*.b=85. Seeded S1 (struct-field reassign) and S2 (Map via module) reproduced. Spec lists 7 codegen_arc_*.b + codegen_struct_field_reassign.b. Awaiting spec audit before implementation.
Fix S1 (REQ-005): struct-valued field reassignment 'o.inner = Inner{...}'.
Root cause was a read-path-dependent use-after-free, not a dropped store: the
store landed but genFieldAssignment had no ARC ownership handling for
user-struct fields (only string/Array), so the RHS struct temporary was
rc_release'd at end of statement, freeing the block the field now pointed at.
The first read could still see the value; an intervening allocation (the
println/interpolation format path) reused the freed block, so later reads were
stale. Fix mirrors struct-literal init: take ownership of the new struct
(untrack a fresh temp, or rc_retain an existing-owner source) and release the
previously-held struct, retain-before-release so self-assignment is safe.
Valgrind-confirmed: pre-fix Invalid read of freed block; post-fix clean.
ARC matrix (REQ-001): 7 codegen_arc_*.b tests (string-field, struct-field,
array-of-struct, option-struct, nested write-through, self-assign, map-struct-
value w/ inline Map) + codegen_struct_field_reassign.b (S1 regression, prints
the reassigned field under a golden so it FAILS pre-fix / passes post-fix per
SPEC-1). Each has a committed stdout golden; all 8 leak-clean under
--leak-check (0 leaks).
Gates: test_codegen 93/0 (86 golden-checked), run_tests 195 (LLVM) / 190
(parse-only), ctest 54/54. codegen_*.b 85 -> 93.
benpayne
added a commit
that referenced
this pull request
Jul 19, 2026
…unsigned byte (#132) * functional-hardening: status -> ready (review passed 10/10) * Launch functional-hardening on devbot: run 7708a1f6, status -> launched * U1(arc-matrix): aggregate/field ARC matrix + seeded S1 fix (#131) Squash-merge of epic/functional-hardening/u1-arc-matrix (reviewer-approved). Fix seeded bug S1 (REQ-005): struct-valued field reassignment 'o.inner = Inner{...}'. Root cause was a read-path-dependent use-after-free — genFieldAssignment had no ARC ownership handling for user-struct fields, so the RHS struct temporary was rc_release'd at end of statement, freeing the block the field now pointed at. Fix (CGStruct.cpp genFieldAssignment) mirrors struct-literal init: take ownership of the new struct (untrack fresh temp / retain existing owner) and release the previously-held struct, retain-before-release so self-assignment is safe. ARC matrix (REQ-001): 7 codegen_arc_*.b tests + codegen_struct_field_reassign.b (S1 regression), each with committed stdout goldens; all 8 leak-clean under --leak-check (0 leaks). Reviewer audit: SPEC-1 teeth verified (S1 test fails pre-fix with stale 1/0, passes post-fix with 99); SPEC-2 diagnosis verified (read-path UAF, not dropped store). Gates re-run independently: run_tests 195 (LLVM)/190 (parse-only), test_codegen 93/0 (86 golden-checked), ctest 54/54, leak-check 8/8 CLEAN. codegen_*.b 85 -> 93. No KI filed; deferred generic-method-chain->field bug is a U3 concern (generic-return-type, not ARC). * U2(operator-matrix): spec — bitwise/shift/%/compound/short-circuit matrix + short-circuit fix Grounding probe through real bcc found && / || do NOT short-circuit RHS side effects (primary fix, CGExpressions.cpp:390-391 eager operand eval); byte unsigned-shift/print discrepancy (fix-or-file). Concrete named golden cases: codegen_op_{bitwise,shift,modulo,compound,short_circuit,byte}.b. * U2 spec: fold in spec-audit suggestions (byte print is general, println path, precedence/~ confirmed); status Approved * U2(operator-matrix): short-circuit && / || + unsigned byte ops, operator goldens Fixes (CGExpressions.cpp genOperationsExpression): - && / || now short-circuit: the RHS (and its side effects) is evaluated only when the LHS does not determine the result. Was: both operands eagerly evaluated, so RHS side effects always ran. Lowered as branch + i1 phi. - byte is unsigned: byte operands widen with ZExt (not SExt), >> on a byte is a logical shift, and a byte prints unsigned (0-255) via the {} builtin. Matches the already-unsigned byte->int conversion (codegen_byte.b). Tests (6 new codegen_op_*.b + committed stdout goldens): bitwise (& | ^, ~, C precedence), shift (<< >>, arithmetic-right on negative int, 1<<30), modulo (incl. negative dividend/divisor), compound (%= ^= += chains), short_circuit (touch(n) side-effect ordering — golden shows only touch 2 & touch 4; fails pre-fix), byte (unsigned bitwise/shift/print). codegen 93 -> 99. Docs: language_design.md notes && / || short-circuit and byte unsigned semantics (Principle I). Fix in CGExpressions.cpp only — no CGStruct.cpp (no U1/U3 conflict).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Unit U1 — Aggregate/field ARC matrix (REQ-001) + seeded S1 fix (REQ-005)
Epic: functional-hardening · Spec: `specs/017-arc-matrix/spec.md` (approved, SPEC-1/SPEC-2 folded in)
Seeded bug S1 fixed — struct-valued field reassignment
o.inner = Inner { v: 99 }read back inconsistently. Root cause (SPEC-2): a read-path-dependent use-after-free, not a dropped store. The store landed, butCGStruct.cpp::genFieldAssignmenthad ARC ownership handling only forstring/Arrayfields — a user-struct field fell into the bareelsethat just stored. The RHS struct temporary was__blang_rc_release'd at end of statement, freeing the block the field now pointed at. The first read could still see the value; any intervening allocation (notably theprintln/interpolation format path) reused the freed block, so subsequent reads returned stale data.Fix: the struct-field
=path now mirrors struct-literal initialization — take ownership of the new struct (untrack a fresh temporary, orrc_retainan existing-owner source) and release the previously-held struct, retain-before-release so self-assignment is safe. Valgrind-confirmed: pre-fixInvalid readof a freed block; post-fix clean.ARC matrix — 7
codegen_arc_*.b(leak-glob prefix, load-bearing) + S1 regressioncodegen_arc_string_field·codegen_arc_struct_field·codegen_arc_array_of_struct·codegen_arc_option_struct·codegen_arc_nested_writethrough·codegen_arc_self_assign·codegen_arc_map_struct_value(inline Map — no dependency on U4's S2)codegen_struct_field_reassign.b— S1 regression. Per SPEC-1 itprintlns the reassigned field twice under a committed golden; verified fails pre-fix (read1 1 / read2 0+ assert failure) and passes post-fix (all99).--leak-check(0 leaks).Gates (reviewer: re-run independently)
./test_codegen.sh→ 93/0 (86 golden-checked, 7 quarantined); count 85 → 93./test_codegen.sh --leak-check test_files/codegen_arc_*.b test_files/codegen_struct_field_reassign.b→ 8/8 CLEAN, Leaks: 0./run_tests.sh→ 195/0;BUILD_DIR=build-parse ./run_tests.sh→ 190/0ctest --test-dir build→ 54/54Notes for the manager / U3
CGStruct.cpp— per manager decision U1 is serialized before U3's CGStruct edits; U3 should rebase onto this merge.m.get(k).field— a field read directly off a generic method-call result (V→struct) — reads empty. This is U3's method-chain→field interaction shape (involves generic return-type monomorphization), not an ARC bug and not caused by this fix; the map test reads via a bound variable instead. Left for U3 rather than consuming a KI slot.