ReviewOS

home-lang/home

Path.initTopLevelDir: @memcpy arguments alias during isolated install

#727
Closed glennmichael123 opened this 9 days ago ·
9 days ago

tests/runtime/native-patch-lifecycle.test.mjs (added recently) fails: bun install exits 134.

thread N panic: @memcpy arguments alias
  paths/Path.zig:105   append          @memcpy(this.pooled[this.len..][0..characters.len], characters)
  paths/Path.zig:307   initTopLevelDir this._buf.append(trimmed, false)
  install/isolated_install/Installer.zig:1591  linkDependencyBins
  install/isolated_install/Installer.zig:1062  run

initTopLevelDir does:

const trimmed = ... trimInput(.abs, top_level_dir) ...;
this._buf.append(trimmed, false);

and append memcpys characters into this.pooled[this.len..]. The panic means top_level_dir is itself a slice into the same pooled buffer, so source and destination overlap.

@memcpy requires non-overlapping operands; @memmove is the correct primitive when the source may alias the destination. The fix is likely either to use @memmove in append for the .any separator arm, or to have initTopLevelDir copy through a scratch buffer when its input aliases _buf.pooled.

Not caused by acde954

Verified: stashed the resolve_path.zig change, rebuilt, and the same panic occurs. joinAbsStringBuf does not appear anywhere on the stack (Path.zig:808 is a different function). Recording that explicitly because both touch path handling and the coincidence is misleading.

Runtime regressions are 76/78 with and without that change, the other failure being #712.

Refs #66

9 days ago

Ported and pushed to main in 02bff5909.

Root cause and repair:

  • Home passed its destination pool directly to getcwd, then passed the returned slice back to _buf.append, violating @memcpy's non-aliasing contract.
  • Bun's live implementation reads the top-level directory from storage independent of the destination Path pool.
  • Home now mirrors that ownership invariant by reading getcwd into a separate stack PathBuffer before allocating/appending into the destination, for both initTopLevelDir and initTopLevelDirLongPath.
  • Shared append behavior was not weakened or changed globally.

Verification:

  • Both actual constructors execute and return the exact cwd: 2/2 focused tests passed, 150 MB guarded peak.
  • The real Home runtime graph passes a no-emission semantic compile with the Path test filter, 507 MB peak.
  • zig ast-check, zig fmt --check, and git diff --check pass.

The native install lifecycle acceptance remains pending because a newly linked runtime cannot be produced beneath the fixed 3,840 MB guard. The cap was not raised. Commit author is glennmichael123 <gtorregosa@gmail.com> with no coauthors.

9 days ago

The source repair in 02bff5909 is now verified with a freshly linked native runtime at ee7a9dcad.

Fresh unchanged tests/runtime/native-patch-lifecycle.test.mjs execution passes end to end. It covers all eight scoped/unscoped × hoisted/isolated × patch/patch-commit variants, real registry metadata and tarball downloads, isolated untouched installs, patch addition/modification/removal, frozen-lockfile reinstall, lockfile stability and unpatched cache reuse. The fresh ReleaseFast build passes 46/46 steps.

This closes the pending native acceptance boundary; the Path.initTopLevelDir alias panic does not recur. No workload, deadline, package fixture or assertion changed. Refs #66.

Sign in to comment on this issue.