A constructor whose first field needs no barrier takes the barrier after each later store

[FrancescoK]

Probed at master de67c22cf, macOS, Apple clang 21, and Linux, gcc on arm64.

This follows up #8397 ("A store written as a parenthesised statement takes its write barrier after the value") and CodeRabbit's review thread on #8398 about reference stores after a skipped first store.

class Node
  def initialize(s) = (@s = s)
  attr_reader :s
end
X = Struct.new(:n, :c) do
  def initialize(a) = super(a.size, a.chars)   # an Integer first, then an Array
end
Y = Struct.new(:f, :node) do
  def initialize(a) = super(a.size * 0.5, Node.new(a))   # a Float first, then an object
end
xs = []
ys = []
400.times do |i|
  xs << X.new("x#{i}")
  ys << Y.new("y#{i}")
  Array.new(20) { |k| Node.new("j#{k}") }
end
p xs[-1].c, ys[3].node.s

	CRuby 4.0	Spinel, gcc
default	["x", "3", "9", "9"] / "y3"	same
SPINEL_GC_STRESS=1	same	same
SPINEL_GC_STRESS=1 SPINEL_GC_VERIFY=1	same	aborts: "collector reached a non-heap/corrupt object"
SPINEL_GC_STRESS=2	same	aborts: "the mark reached a freed slot", holder old=1

The same at -O0 and -O1, with and without --share-strings. Apple clang builds the value before the barrier and does not fail.

• A Struct's or Data's super(...) in an initialize of its own writes the members as one parenthesised comma sequence: (self->iv_n = sp_str_length_m(lv_a), SP_WBO(self)->iv_c = sp_str_chars(lv_a), 0);.
• gc_wb_insert_seg recognised such a statement (wb_paren_stmt_start) only at its first store. A store that needs no barrier (an Integer, Float or bool member, a nil) is stepped over, and the later reference store, with text before it in the statement, took the expression wrapper SP_WBO, which C may run before the value is built.
• With gcc the barrier ran first, on a holder still young, so it recorded nothing; the collection inside the value's allocation promoted the holder, and the young value then landed in an old holder no record covered. The next minor collection freed it while the holder still pointed at it.
• Found by checking CodeRabbit's claim on A String stored by instance_variable_set into a shared slot stays one String with its variable #8398 with mixed-member constructors under SPINEL_GC_STRESS=2 on Linux.

The sequence is found from the statement's start. wb_paren_stmt_bol walks back from the store over balanced groups (a statement expression in an earlier value among them) to the ;, {, } or line end at the store's depth. It steps over a string or character literal whole, back to its opening quote (one with an even number of backslashes before it), so a bracket or a quote inside a literal, as super("#{a})".size, ...) emits, is no group. wb_paren_stmt_start then accepts, after the opening parentheses, the stores before the current one when each writes the same object and ends at a top-level comma (wb_stores_one_object), and the whole statement is rewritten from its first store, as #8397 rewrites one that starts with a reference store.

A barrier follows each store that needs one. wb_put_stores_barriered put a barrier after every store of the sequence; it now asks each store what the pass asks of a store on its own (wb_seg_needs_barrier: a reference field of the holder's class, given a value that can be young), so an Integer member or a nil store takes none.

Generated C. Of the 527 programs in test/ that call super(, Struct.new or instance_variable_set, these change, the same in both builds, and only in a super(...) member sequence: data_new_positional_keyword_init, data_super_kwarg, struct_bare_super_initialize, struct_custom_initialize_super, struct_initialize_forwarding_super and struct_super_member_nil_default take the rewritten sequence, with a barrier after each reference store, where a later reference store had SP_WBO; string_handle_initialize's sequence, already rewritten, drops the barrier that followed its Integer member. optcarrot's generated C is unchanged in both builds.

The corpus runs come from the local make gate below (this branch merged with master b8db3cafd, macOS arm64, clang 21); the corpus-wide C diff was not run.

Tests. test/gc_barrier_paren_store.rb gains constructors whose first member is an Integer, a Float, a bool or an Integer computed in a statement expression, followed by a String built by a method that allocates objects, an object or an Array, and three whose earlier value's C holds a string literal with a bracket, a quote and an escaped quote, or a trailing backslash, built 60 times beside a churning loop. Built with gcc it failed on master at SPINEL_GC_STRESS 1 (wrong members; with SPINEL_GC_VERIFY=1 the verifier abort) and 2, at -O0 and -O1, in both builds. With this change it passes there and under Apple clang, at SPINEL_GC_STRESS 0, 1 and 2, with and without SPINEL_GC_VERIFY=1. The other changed programs pass in both builds at -O0 and -O1, SPINEL_GC_STRESS 0, 1 and 2 with SPINEL_GC_VERIFY=1. make reject-test, make infer-test, make gc-stress-test, make share-strings-test and tools/refusals.sh pass. In that gate make test passes (6918 pass, 0 fail, 0 error) and the corpus with sharing on has 6917 pass, 1 known failure (already listed in test/share/known-failures.txt) and 0 new failures.

make gate (on this branch merged with current master)

Local make gate on this branch merged with master b8db3cafd (macOS arm64, clang 21); the head commit carries its Gate trailer.

scale-test: boxed Hash store work at 2x the methods is 1.95x (limit 2.20)
scale-test: boxed-receiver alias work at 2x the writes is 1.86x (limit 2.20)
scale-test: instance_eval forwarding work at 2x the wrappers is 1.70x (limit 2.50)
scale-test: work at 4x the program is 4.88x (linear 4.00, limit 5.20)
scale-test: work at 4x the program, compiled to C, is 6.39x (limit 6.90)
scale-test: call-shape work at 4x the units, compiled to C, is 4.17x (linear 4.00, limit 4.50)
Tests:     6918 pass,        0 fail,        0 error
gate-test-shared: known ERR: yield_string_mutator_tail
gate-test-shared: 6917 pass, 1 known failures, 0 new failures
gate: stamp for tree e21022a7189b on master ea6837508436; git commit --amend --no-edit adds the Gate: trailer
gate: ALL GREEN

• New tests have .expected files that match CRuby 4.0 run with --enable-frozen-string-literal
• Values past 2^31 are marked # spinel: int64
• If optcarrot's generated C changed: callgrind numbers, checksum 59662
• Depends on: #

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Fixed garbage collection issues that could affect structs with multiple fields when construction involved allocating objects. Immediate values and allocated values are now handled correctly in these cases.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 977f49e8-2dfa-4649-946d-235c1bc6a4f3

---

📥 Commits

Reviewing files that changed from the base of the PR and between ea68375 and a83fb07.

---

📒 Files selected for processing (3)

• src/codegen.c
• test/gc_barrier_paren_store.rb
• test/gc_barrier_paren_store.rb.expected

---

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

---

📝 Walkthrough

Walkthrough

The compiler now recognizes parenthesized comma-separated stores to the same object and inserts write barriers only after stores to reference fields that may receive young values. GC-stress tests cover generated Struct constructors with mixed immediate and allocating field values.

Changes

Parenthesized Store Write Barriers

Layer / File(s)	Summary
Recognize parenthesized store sequences src/codegen.c	Statement-boundary discovery skips balanced groups and quoted literals. Recognition accepts comma-separated stores to the same object and records the first store.
Select stores that need barriers src/codegen.c	Barrier selection checks whether each store targets a reference field and may receive a young value. Each comma-delimited segment receives a barrier only when it meets those checks.
Wire the rewrite and add GC-stress coverage src/codegen.c, test/gc_barrier_paren_store.rb, test/gc_barrier_paren_store.rb.expected	The rewrite starts at the first store and applies per-segment barrier insertion. GC-stress tests cover generated constructors with immediate and allocating values, including string literals with delimiter-like characters.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

---

Merge Risk | ⚪ Minimal · up to a83fb

Merge Risk: ⚪ Minimal · up to a83fb

No confirmed merge-blocking defect remains. The new constructor cases cover mixed values, though the effect of store-like text inside a string literal remains uncertain.

Security Architecture Review

Security architecture risk: 🔵 Low · up to a83fb

The change narrows an existing memory-safety failure window without adding a new external interface. Risk remains low because correctness depends on recognizing supported expression forms, while other forms retain earlier limitations.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The relevant failure domain is the heap and availability of programs containing affected generated stores. The inspected fixtures demonstrate language-level reachability; the evidence does not establish an attacker-facing compilation service, tenant boundary, or privilege gain.

Trust Boundaries and Controls

• inferred — A residual recognition limitation predates this PR: the same-object helper scans raw text without skipping literals, and rejected sequences can retain SP_WBO, whose barrier may execute before an allocating RHS. Non-frozen Ruby literals can preserve assignment-like text in emitted C; frozen literals are hoisted references and provide counterevidence for that subset. No newly introduced exposure or executed exploit was established.

Resilience and Maintainability Implications

• observed — The unchanged barrier implementation performs no allocation or safepoint polling, skips already-dirty holders, and uses atomic slot reservation in threaded builds. Overflow sets a fallback flag rather than allocating. The PR changes when this existing transition is invoked, not its synchronization or recovery implementation.

Hardening Proposals

• proposed — Make same-object recognition literal-aware and validate assignment-like literal contents through generated-code cases, so textual false matches cannot silently select the earlier barrier-ordering fallback.

---

Pre-merge checks | 4 | 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 1 files. (2 skipped: 1…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly describes the main fix: preserving barriers after later stores when the first constructor field store does not need one.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

---

Full details: Docstring Coverage

Explanation

Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 1 files. (2 skipped: 1 unsupported, 1 too large.)

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

---

_{Comment @coderabbitai help to get the list of available commands.}