Skip to content

Read the platform's own separator and root forms when resolving a path - #412

Merged
typeless merged 2 commits into
mainfrom
wip/388-platform-path-law
Aug 17, 2026
Merged

Read the platform's own separator and root forms when resolving a path#412
typeless merged 2 commits into
mainfrom
wip/388-platform-path-law

Conversation

@typeless

Copy link
Copy Markdown
Owner

On Windows a Tupfile could declare an output that silently writes outside the build root, by four spellings. pup::path split on / alone, so ..\victim.txt was a single opaque component that was never ..; and three spellings were not recognised as rooted at all — the UNC \\host\share\victim.txt, whose target is another machine, the drive-relative C:victim.txt, which carries no separator for a separator fix to reach, and the root-relative \victim.txt.

The fix is that the module stops disagreeing with the platform it runs on: on Windows the separator alphabet is both bytes and the root forms are Win32's own. The containment predicate in src/graph/builder.cpp is not in this diff — it refuses all four spellings unchanged. POSIX behaviour is unchanged, where a backslash is an ordinary filename character and accepting ..\victim.txt is correct, and both halves of that asymmetry are pinned.

A second live defect is repaired here. The same disagreement recorded a nested output as one component while Windows created two on disk, so the record's key and the file it named were different paths. It has its own pin and its own discharge: line on REQ-OUTPUT-CANONICAL, not a footnote.

A third, introduced by the fix itself and caught in review before commit. Teaching root_length the separator-less root C: split the module against itself one function over: filename, parent and join locate a root by searching for a separator byte, which C: does not contain. join("C:","a") emitted C:/a, silently promoting a drive-relative path to a drive-absolute one. The splitters now clamp to root_length, and join shares append_root's predicate rather than restating it.

Index version

INDEX_VERSION 22 → 23, because recorded path spellings change meaning on Windows. Two consequences worth stating plainly:

  1. The bump is global. Linux users also pay one silent rebuild for a Windows-semantics fix. That is the standing cost of a single version stream, and it is accepted rather than worked around. Discontinuity handling is the existing version-window arm and nothing else — a rebuild without an announcement, which is the enumerated benign case rather than a damage report.
  2. INDEX_LAYOUT_FLOOR is untouched (no struct changed size), so the recovery read still admits old records whose \-bearing paths were never right: pre-existing wrongness, not widened. One consequence of that reach: on the first build after upgrading, a v22 record's flat sub\gen.txt is compared against a v23 graph's two components, which cannot make the ownership guard false-fire — an absent-but-known path is skipped, not rejected — so the worst outcome is one redundant delete-and-regenerate of a regenerable artifact, once.

Verification

Cross-compiled with clang-cl and run under wine: every pin was observed failing against the pre-fix module and then passing, including the drive-relative promotion. This is a real Windows binary executing real Win32 path semantics, but it is wine rather than Windows — the one place where the evidence is a stand-in for the thing itself. The POSIX pins were shown to discriminate by implementing the naive fix and watching them fail.

  • Linux: 170755 assertions in 844 cases
  • Windows under wine, ~[e2e]~[shell] (the filter Windows CI uses): 170415 in 436
  • format, tidy, iwyu, spec-check: exit 0 each

Every new pin is non-[e2e], deliberately: [e2e] is excluded on Windows CI, so an E2E pin would not run where it matters.

Known residue

A share-aware UNC root — which would stop .. popping the host and share — was built first and refuted by the module's own laws property test: //a/. makes . the share and normalize emits a path not in normal form. The result stays rooted and therefore stays refused, so the residue is confined to the spelling of an already-rejected path. Tracked separately in #411, carrying that input as the constraint any candidate fix must satisfy.

The call-site inventory required for this change found 82 splitter call sites, 7 of them pattern-shaped. All seven are safe: putup's pattern alphabet gives \ no meaning, and there is no regex engine in src/ at all, so upstream tup's PCRE exclusions do not apply here.

Fixes #388

A Tupfile could name an output outside the build root on Windows and have it
accepted. pup::path split on '/' alone, so `..\victim.txt` was one opaque
component that was never `..`, and the containment check saw nothing to
refuse -- the #385 failure with the separator swapped. Three more spellings
escaped the same way, unrecognised as rooted at all: the UNC
`\\host\share\victim.txt`, whose target is another machine; the
drive-relative `C:victim.txt`, which carries no separator to fix; and the
root-relative `\victim.txt`.

The module already knew half of this: root_length has read `C:\` as a drive
root since it was written, while every splitter beside it read only '/'. The
fix is to stop disagreeing -- on Windows the separator alphabet is both
bytes and the root forms are the platform's, so the containment predicate
refuses all four spellings without being touched. src/graph/builder.cpp is
not in this diff. POSIX is unchanged, where a backslash is an ordinary
character in a filename and accepting `..\victim.txt` is correct; both
halves are pinned, because a rule pinned on one platform reads as a defect
report against the other.

The same disagreement recorded a nested output as one component while
Windows created two on disk, so the record's key and the file it named were
different paths. That is repaired here too, with its own pin and its own
discharge.

Teaching root_length the separator-less root `C:` then split the module
against itself a second time, one function over: filename, parent and join
locate the root by searching for a separator byte, which every other root
contains and that one does not. filename returned the whole `C:a`, parent
returned nothing where `/a` yields `/`, and join emitted `C:/a` -- silently
promoting a drive-relative path to a drive-absolute one. The two splitters
now clamp to root_length, and join shares append_root's predicate rather
than restating it, so the two cannot drift apart. stem and extension inherit
through filename, which a pin proves rather than assumes.

Rejected: rejecting backslash paths in the lexer instead. That guards one
door of a many-door room -- env-carried values and generated rule text reach
the module without passing it -- and it does not repair the recorded-key
defect at all.

INDEX_VERSION 22 -> 23: recorded path spellings change meaning on Windows.

Verified by cross-compiling the suite with clang-cl and running it under
wine: every pin was observed failing against the pre-fix module and then
passing, including the drive-relative promotion. That is a real Windows
binary executing real Win32 path semantics, but it is wine rather than
Windows, and it is the one place here where the evidence is a stand-in for
the thing itself. The POSIX pins were shown to discriminate by implementing
the naive fix and watching them fail. Linux 170755 assertions in 844 cases;
Windows 170415 in 436 under the tag filter CI uses; format, tidy, iwyu and
spec-check each at exit 0.

A share-aware UNC root, which would stop `..` popping the host and share,
was built first and refuted by the module's own laws test: `//a/.` makes "."
the share and normalize emits a path that is not in normal form. The result
stays rooted and therefore stays refused, so the residue is the spelling of
an already-rejected path; it is filed as #411 with that input as the
constraint.

Written by the team's builder across two phases against an architect's
ruling, with three cross-model reviews. The third found that the
split/rejoin law added here does not catch the drive-relative regression on
its own -- parent's empty result and join's empty-left arm cancel exactly --
and the added assertion that an absolute path's parent stays absolute is
what closes it. Both halves were proven by probe.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014wx1bWwRf23eFT82y9D641
@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown

PR metrics

Performance (gcc example, Linux)

Workload Instructions CPU time Page faults D1 miss LL miss Wall Peak RSS
parse 1697 M (-0.1%) 0.3 s 14.9 k 0.5% (-0.2pp) 0% (-0.1pp) 0.322 s 34 MB
dry-run 2314 M (-2.2%) 0.37 s 16.3 k 0.6% (-0.2pp) 0% 0.365 s 39.1 MB (+0.1MB)

Deterministic signals: instructions (cachegrind-simulated instruction reads — exact across runs, no PMU needed), page faults, peak RSS, and the cachegrind D1/LL miss rates. CPU time is user+sys from time(1).

Internal statistics (gcc example, up-to-date dry run)

Metric Value
Tupfiles parsed 24
Commands 3545
Commands scheduled 0
Files checked 5834
Files changed 0
Files in index 6102
Graph edges 381415
Index size (bytes) 7683630
Implicit deps 344126
Hash computations 0 (-100.0%)
Hashes skipped (stat cache) 5833 (+3.5%)
Stat calls 5885
Parse time (ms) 265.3
Total time (ms) 366.6
Runner CPU INTEL(R) XEON(R) PLATINUM 8573C

Counters from putup -n --stat on the fully-built gcc example (up-to-date dry run): deterministic work measures — a jump in commands scheduled, hash computations, or stat calls is a real behavior change, not noise. Timings are the minimum over repeated runs, compared only against a baseline from the same CPU model; the counters are the regression signal.
Timing deltas suppressed: baseline ran on different hardware (AMD EPYC 7763 64-Core Processor).

Binary size (Linux)

Binary .text .data .bss File
putup 566.3 KB (+0.2%) 2.3 KB 98.7 KB 671.4 KB (+0.1%)

Code churn (whole codebase, last 30d)

Files Lines written Still present Churned Churn rate
108 9526 7899 1627 17.1%

Of the lines written across the codebase in the last 30 days, how many are already gone — work that was written and then discarded or rewritten inside the same window. This is the state of the tree including this PR, not a measure of the PR itself. Only code we write is counted: tests, examples, vendored and generated files, CI plumbing and prose are excluded. 4044 lines were deleted in the window in total, most of them older than it.

Where the churn is
File Lines written then discarded
src/cli/cmd_build.cpp 592
src/graph/scanners/clang_cl.cpp 295
src/graph/scanners/gcc.cpp 113
src/cli/context.cpp 82
src/core/layout.cpp 69
src/graph/scanners/dep_words.cpp 64
src/graph/builder.cpp 62
src/platform/file_io-posix.cpp 41
src/cli/strict_checks.cpp 36
src/cli/cmd_clean.cpp 35

Test coverage (lines)

Overall Median file Min file Max file
88.1% 96.4% 14.7% include/pup/parser/token.hpp 100.0% include/pup/core/arena.hpp

102 files · 16620/18861 lines covered

Deltas vs main@26ddb6105.

Updated for 13ecfe3

Teaching pup::path that a backslash separates gave parent() a real answer
for a backslash-spelled Windows path, where it had returned empty before.
That woke three guards in file_io-win32.cpp that had never once run, and
create_directories then walked to the drive root and asked the OS to create
it -- which answers with neither success nor ERROR_ALREADY_EXISTS, failing
every index write whose path came from a channel that does not convert
separators. All seven test-windows failures were that write.

The POSIX half of the pair has stat'd first and returned success for an
existing directory since it was written; the Win32 half asked every time.
Conforming it removes the dependency on what the OS answers for a root,
rather than widening the error codes we tolerate, which would launder a real
denial into success.

The new pin is untagged so Windows CI runs it: every gate available here
passes with or without this fix, because wine answers ERROR_ALREADY_EXISTS
where Windows does not.
@typeless
typeless merged commit 245b428 into main Aug 17, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Output-containment predicate is untested for Windows path spellings, but its requirement is platform-unqualified

1 participant