fix: the coverage job could never write its counters (#740)

[jdatcmd]

Closes #740. Filed by @ChronicallyJD, whose diagnosis I verified independently
and did not need to re-open. What was missing was a measured fix.

This description was rewritten after review. The first version described the
chown approach, which is superseded and which this PR's own code and CHANGELOG
say is not sufficient. It also claimed a guard that does not exist and disclaimed
a nightly that had by then run green. Corrected below; the history is in the
commits and the thread.

Root cause

gcov writes each .gcda beside its object, at the path recorded at compile
time, as the process that ran the code. run_coverage.sh runs under sudo in
CI, so the build is root-owned, while lib.sh:150 runs the server as postgres
whenever it is root. The backend could not create a counter file next to a
root-owned object. Nothing was ever written and lcov had nothing to capture.

The fix is a redirect, and ownership alone could not have worked

Making the object directories writable was the first attempt and it is not
sufficient: creating a file needs execute on every ancestor too. Measured
on the runner, with the object directories already chowned:

postgres:runner 755 /home/runner/work/pgcolumnar/pgcolumnar/src
postgres:runner 755 /home/runner/work/pgcolumnar/pgcolumnar
runner:runner   755 /home/runner/work
runner:runner   750 /home/runner        <-- no execute for postgres

/home/runner is 750 and postgres is neither its owner nor in the runner
group, so it cannot traverse into the workspace at all. That route could only
work by chmod'ing a shared runner's home.

GCOV_PREFIX makes gcov write to $GCOV_PREFIX/<absolute path>.gcda instead.
/tmp is world-writable and world-traversable, so it holds whoever the server
runs as and wherever the tree lives. GCOV_PREFIX_STRIP=0 keeps the full path,
which is what lets the counters be returned exactly beside their .gcno.
runuser passes the environment through; verified rather than assumed.

approach	result
chown the object directories	dirs writable in themselves, 0 counters
GCOV_PREFIX redirect	33 counters, lcov and genhtml succeed, rc 0

Measured in CI, which is the only evidence that counts here

Nightly 33005313801 on this head, 8 of 8, including coverage report (PG 18) which has never passed before:

-- counters redirected to /tmp/pgc-gcov
-- suites: 213 passed, 0 failed, 2 skipped
-- counters: 33 .gcda from 33 instrumented objects in src/
    lines......: 93.4% (19462 of 20847 lines)
    functions..: 96.4% (838 of 869 functions)
    branches...: 69.8% (8870 of 12705 branches)

The last green nightly before this, on 2026-07-30, was green because the
coverage job did not exist yet. This is the first run in which it has produced a
number.

Second-order fixes

• The runner refuses a run that captured no counters, before lcov, and says
  where they should have been. What kept this unexamined for 25 nights is that
  lcov failing on an empty tree reports "capture produced nothing", which reads
  as a broken tool rather than a permission problem.
• The per-suite logs are uploaded. Only coverage/html and coverage.info
  were, so a suite failing inside this job had its detail discarded with the
  runner. That is how The extension-upgrade guard never runs in CI; the coverage runner turns that into a permanent nightly red #741's failing suite stayed invisible.

The 34 of 49 is answered: LTO, not a coverage gap

toolchain	--cflags
PGDG PG 18 (what CI uses)	-flto=auto -ffat-lto-objects
source-built PG 17 (my container)	no -flto

PGXS passes those to the extension, so GCC's link stage emits a .gcno per LTO
partition plus one for whole-program analysis (pgcolumnar.so.ltrans0..11,
.wpa). Those have no source lines. 33 src + 1 real objstore object + 15 LTO
artifacts = 49, and 33 + 1 = 34 counters, so every real object had one.
Reproduced by building against the PGDG pg_config: 48. Why 11 partitions here
and 12 in CI is unexplained; my first answer, that -flto=auto sizes them
from the CPU count, is refuted by the control (taskset the same build: 2, 4 and
8 CPUs all give 11). Untested candidate is the toolchain difference, gcc 15.2
here against ubuntu-24.04's gcc 13. It does not bear on the conclusion, which
rests on src being 33 of 33 and is measured directly.

Guards

test/selftest/250 pins five properties, all of them things that are invisible
from the lines merely being present:

mutation	result
move the refusal after the capture	RED
export GCOV_PREFIX after the suites run	RED
copy the counters back after the refusal	RED
count .gcda tree-wide instead of the captured directory	RED
ask the tree-wide walk before GCOV_PREFIX for strays	RED
delete the copy-back's destination containment	RED

harness_selftest: 138 checks PASSED (132 on main).

Review items

All four addressed, and two of the three I had found independently before the
review landed.

1. The by-directory breakdown printed no directory. Correct, and the cause is
   as diagnosed: the leading sed expression matches the whole line. Replaced
   with one awk pass.

2. The stray probe was blind to GCOV_PREFIX. Correct. -xdev will not
   cross a mount boundary and /tmp is tmpfs here too. GCOV_PREFIX is asked
   first; the tree-wide walk stays as the fallback for gcov ignoring the redirect
   entirely.

3. Root copying a 1777 directory's files to an unconstrained destination.
   Correct, and I had missed it. Reproduced end to end: a file planted as
   postgres under $GCOV_PREFIX/root/pgc_target/ was written by root outside
   the tree. Containment added; both arms proved, since a containment that
   refuses everything is not a fix:

   -- counters returned: 1   refused out-of-tree: 1
   attack dest : NOT WRITTEN (blocked)
   legit dest  : /root/pgc740c/src/columnar_real.gcda
   

4. The earlier guard-scope finding, that the refusal counted tree-wide while
   the capture is scoped to src/. Reproduced and fixed.

🤖 Generated with Claude Code

[OffgridwithJD]

Reviewed on the merged tree (3-way merge of up/main 93a2eda + pr745 2c6991b, CLEAN),
pg17a assert. This closes an issue I filed, so I went looking for problems rather than
confirming my own diagnosis.

Verified

check	result
harness_selftest, merged tree	130 PASSED, as claimed
move the refusal after the capture	RED: "the coverage runner refuses zero counters before it calls lcov (#740): got [no]"
hardcode src instead of deriving from the .gcno files	RED: "the counter directories are derived from the .gcno files, not named (#740): got [0] want [1]"
restored	GREEN

Both proofs reproduce. Disclosure on my own instrument: my first attempt at the second
proof used sed with | as the delimiter while the pattern itself contains |, so the
edit silently failed and the suite passed against an unmodified tree. That PASS was my
harness, not a weak check. Re-ran it with a real edit and it reddens as you said.

The uid-mapping trap you hit is worth the space you gave it. My own reproduction was
faithful only by luck: I happened to be root against a root-owned tree. A reader who
repeats this in a container where the workspace maps to the server user will "fail to
reproduce" a live defect and conclude it is fixed.

One finding: the guard's scope does not match the capture's

The refusal counts counters across the whole tree:

_gcda=$(find "$SRCDIR" -name '*.gcda' | wc -l)     # line 158

while the capture is scoped to one directory:

lcov --directory "$SRCDIR/src" --capture ...       # line 169

objstore/ holds one .c, so it contributes a .gcno and a .gcda -- your own output
confirms it: 34 objects, and the chown line names both directories. So if src/ produced
no counters while objstore/ produced one, _gcda is 1, the guard passes, and lcov
captures nothing: the exact capture produced nothing message this guard exists to
prevent, back again.

Reachable how: chown postgres $_covdirs takes both directories in one call, and its exit
status is not checked. A partial failure leaves one writable and one not, unnoticed.

Scoping the count to the directory actually captured -- or checking chown's status --
closes it. Low severity: it needs a partial chown failure, and I could not demonstrate
it end to end because my verification tree has no instrumented build. I am reporting the
structure, not a reproduction.

What I checked and am NOT reporting

I first read the three --directory "$SRCDIR/src" lines as silently dropping objstore/
from the report, which would have contradicted this PR's own reasoning for deriving the
chown directories. Then I read Makefile:127: objstore is a separate shared library,
built alongside but never linked in, precisely because this extension is preloaded.
Scoping the report to src/ is therefore a deliberate and defensible choice, not the
same defect. Recording it so the next reviewer does not re-derive it.

(--zerocounters is likewise src-only, so a stale objstore/*.gcda would persist across
local runs. Irrelevant in CI on a fresh checkout, and it does not reach the report for the
same reason. Mentioned only for completeness.)

Optional

docs/testing.md:283 says the coverage job "uploads the report as an artifact". It now
also uploads the per-suite logs, which is the change that would have made #741's failing
suite visible. Worth the half-sentence, though selftest 240 does not require it since no
job was added.

Not approving yet, and the reason is not a doubt

PR checks are 10 of 12 with two still running, and the nightly you dispatched on this
branch (32884688304) is in flight. That run is the decisive evidence here: the coverage
job has never once succeeded, so the only thing that can prove this fix is the job itself
going green in CI, and I would rather wait ~40 minutes than approve the fix to a
never-working job on a local reproduction. That is the argument you made on #742 and it
was right.

If it lands green, this is the first green nightly since 2026-07-30 and the first coverage
number this project has ever produced. Ping me or I will pick it up from the watcher.

[OffgridwithJD]

The nightly you dispatched on this branch (32884688304) failed, and I have the
mechanism and a tested remedy. The fix is necessary but not sufficient: it corrects
ownership, and what blocks CI is traversal.

Two of your additions paid for themselves immediately

• The zero-counter refusal caught it before lcov and said what was wrong:
  FAIL no .gcda counters were written / 0 .gcda from 49 instrumented objects. The old
  behaviour would have been capture produced nothing again, and we would be looking at
  lcov instead of at permissions.
• The per-suite log upload gave me 215 logs to search. That artifact is what the
  diagnosis below came out of, and it is exactly the evidence that used to be discarded
  with the runner.

Diagnosis: gcov never attempted a write

Across all 215 uploaded suite logs there is not one profiling: line -- no
Cannot open, no Permission denied. gcov reports write failures on stderr, so its
silence says the backend never got as far as trying. That points away from ownership of
the counter directories and at reaching them at all.

The workspace is /home/runner/work/pgcolumnar/pgcolumnar. chown postgres on the leaf
directories does nothing if postgres cannot traverse /home/runner.

Proof, one variable

Copied this branch into a /home/runner-shaped path, built with --coverage, applied
your chown to the derived counter directories, and changed only the ancestor's mode:

arm	ancestor mode	postgres can traverse	smoke	.gcda
the runner's shape	750	no	PASSED	0
control	755	yes	PASSED	33

Identical chown in both arms. Note both arms' suites report PASSED while producing zero
counters, which is the property that let this sit unexamined: the suites are green either
way and only the counter count tells you.

Tested remedy

Walking up from the counter directories and adding o+x to each ancestor:

modes now: 751 /home-like  755 .../work  755 .../pgcolumnar  755 .../src
traverse: yes    create: yes
smoke rc=0 SMOKE TEST PASSED
.gcda written: 33

o+x grants traverse, not read: I checked that postgres still cannot list the home
directory afterwards (ls denied at 751). So this does not expose the runner's home
contents to the server user, which seemed worth confirming before proposing it.

An alternative I have not tested is GCOV_PREFIX / GCOV_PREFIX_STRIP to redirect the
counters somewhere the server user already owns, with lcov --directory pointed at both
trees. Cleaner in principle, more moving parts, and it would need its own proof.

Why your local run passed and CI did not

Worth recording, because it is the same class as the uid-mapping trap you already caught:

• CI reports 49 instrumented objects; your container reports 34.
• CI chowned three directories including the repo root
  (.../pgcolumnar .../objstore .../src); locally it was two.

So the local reproduction and the runner are not the same shape. The guard you added is
what makes that visible rather than silent, which is the argument for having added it.

Where this leaves the PR

The direction is right and the guard is right; the fix is one condition short of working in
the environment it targets. Still not approving, and now for a substantive reason rather
than a procedural one. Happy to push the ancestor walk to your branch if you would rather
not re-derive it -- your call, it is your PR.

My earlier finding stands and is unaffected: the guard counts .gcda tree-wide while the
capture is scoped to src/, so a partial chown failure could still pass the guard and
starve lcov. Narrow, and not what bit here.

[jdatcmd]

The first fix was insufficient, and CI said so. Root cause now measured.

I dispatched the nightly on this branch rather than merging on a clean local run.
The coverage job still failed, with 0 .gcda from 49 instrumented objects,
while reporting the directories as "made writable". That is the useful kind of
failure: insufficient, not wrong.

So I made the runner measure the premise instead of asserting it. From the CI
log:

-- WARNING: the server user cannot create files in .../pgcolumnar/src (postgres:runner 755)
     postgres:runner 755 /home/runner/work/pgcolumnar/pgcolumnar/src
     postgres:runner 755 /home/runner/work/pgcolumnar/pgcolumnar
     runner:runner   755 /home/runner/work/pgcolumnar
     runner:runner   755 /home/runner/work
     runner:runner   750 /home/runner
     root:root       755 /home
-- counters: 0 .gcda from 49 instrumented objects
    no .gcda anywhere on this filesystem, so nothing wrote them

/home/runner is mode 750. postgres is neither the owner nor in the
runner group, so it has no execute bit there and cannot traverse into the
workspace at all, however the leaf directories are owned. Creating a file needs
execute on every ancestor, and the chown only ever addressed the last two.

The "no .gcda anywhere on this filesystem" line is load-bearing too: it rules out
a path mismatch, which is indistinguishable from a permission refusal if all you
have is "0 counters".

So the ownership route could not have worked here short of chmod'ing the runner's
home, which is not a thing to do to a shared runner.

The fix is now a redirect, and it is proved against the failing shape

GCOV_PREFIX makes gcov write to $GCOV_PREFIX/<absolute path>.gcda. /tmp is
world-writable and world-traversable, so it holds whoever the server runs as and
wherever the tree lives, and it needs no permission change on the source tree.
GCOV_PREFIX_STRIP=0 keeps the full path, which is what lets the counters be
returned exactly beside their .gcno. runuser passes the environment through;
I verified that rather than assuming it.

Proved by construction, in a local configuration with an ancestor at mode 700 so
the server user cannot traverse it, which is the shape that defeats the chown:

approach	result
chown the object directories	dirs writable in themselves, 0 counters
GCOV_PREFIX redirect	33 counters, lcov and genhtml succeed, rc 0

Guard updated

selftest/250 now pins the two ORDERING facts that carry the fix, neither of
which is visible from the lines merely being present:

mutation	result
export GCOV_PREFIX after the suites run	RED, it would redirect nothing
copy the counters back after the refusal	RED, the refusal would always see zero

CI is running on the redirect now. I will post what it says rather than assume it,
since this is the second time today a clean local run did not transfer.

[jdatcmd]

The whole nightly is green. First time since 2026-07-30.

Run 32892447229, on this branch, 8 of 8:

  success  coverage report (PG 18)          <- has never passed before
  success  extension upgrade guard (PG 18)
  success  sanitizer gate (ASAN+UBSAN)
  success  suites (PG 15/16/17/18 x86_64), suites (PG 18 aarch64)

And the coverage job, from the log:

-- counters redirected to /tmp/pgc-gcov
-- suites: 213 passed, 0 failed, 2 skipped ( native_repack pg19_vacuum_options)
-- counters returned beside their objects: 34
-- counters: 34 .gcda from 49 instrumented objects
    lines......: 93.4% (19462 of 20847 lines)
    branches...: 69.8% (8872 of 12705 branches)

The last green nightly, 30592054946 on 2026-07-30, was green because the
coverage job did not exist yet. This is the first run in which it has produced a
number.

A regression I introduced, and the guard that caught it

Worth recording rather than quietly fixing. The edit that swapped the chown block
for the GCOV_PREFIX redirect selected the region between the chown comment and
SUITES= — and that region also contained not_a_suite(). It went with the
block.

Without it the coverage runner discovers every test/*.sh again, including the
two upgrade suites, which is exactly the defect #742 fixed. It turned the
suites jobs red on all five majors, because harness_selftest runs inside them
and selftest/220 asserts the two not_a_suite copies agree.

So 220 caught a deletion its own author made, one commit after writing it.
Restored byte-identical to main, verified by diff. The cheap check that would
have caught it at the time is in the diffstat: this file is now 80 insertions,
0 deletions against main, and an edit meant to add a redirect had no business
removing lines.

One number I am not yet able to reconcile

34 .gcda from 49 instrumented objects, against 34 of 34 in the container.

What is established: the report is sound. lcov --extract restricts it to
src/*, the 34 counters are the 33 src objects plus objstore, and the CI
line total (20,847) differs from the container's (20,895) only by the per-major
compat shims, PG18 against PG17. So every object in the report's scope has a
counter.

What I cannot yet explain is where the other 15 .gcno come from. The repository
holds 36 .c files in all — 33 src, one each in objstore, bench,
test/pbt — and no suite compiles C. Rather than guess a third time today, the
runner now prints the breakdown by directory, so the next run answers it from the
data. Dispatched.

That does not block the report being correct, and I would rather state it than
let a ratio go by that reads as "a third of the objects were missed".

[jdatcmd]

Your finding is real, and I could reproduce it. Plus: the diagnostic I added was broken.

Two commits, b82c81f and 4422a5e.

1. The guard counted a different tree than the capture

You reported the structure and said you could not demonstrate it end to end. It
reproduces. Constructed tree, 33 .gcno in src/ with no counters beside them,
one .gcda in objstore/:

arm	_gcda	verdict
tree-wide (before)	1	PASSES, lcov then starves
src-scoped (after)	0	REFUSES

So it is exactly the capture produced nothing message this guard exists to
pre-empt, reachable the way you said. The count is now scoped to
$SRCDIR/src, the directory lcov --capture is pointed at.

selftest/250 pins the two to each other as source text, rather than
asserting src twice, so widening the capture later without widening the count
re-opens it:

_cov_count_dir=$(grep '^_gcda=\$(find ' ... )
_cov_cap_dir=$(grep 'lcov --directory .*--capture' ... )
check "the zero-counter guard counts the directory lcov captures (#740)" \
	"$_cov_count_dir" "$_cov_cap_dir"

mutation	result
revert the count to $SRCDIR	RED: "got [$SRCDIR] want [$SRCDIR/src]"
restored	GREEN

harness_selftest: 134 checks PASSED (132 before these two).

Your note that --zerocounters is likewise src-only I am leaving alone for the
reason you gave, and I am glad you wrote down the objstore reasoning rather
than leaving it to be re-derived. It is the reason the guard is the defect and
the report scope is not.

2. The breakdown I added to answer the 49 printed no directory

Worth stating plainly: the last commit before this pair existed only to answer
34 .gcda from 49 instrumented objects from the data. The nightly on it
answered:

     34     gcda 
     49     gcno 

Empty column. The leading sed expression, s/.*\.\(gcno\|gcda\)$/\1/, matches
the whole line and replaces all of it with the extension, so the directory
was gone before the second expression could act. Reproduced on a synthetic tree
built to the same shape, which prints the same two lines with the same hole.

An instrument is a claim like any other, and I shipped this one on the strength
of having written it. It went out in a commit whose entire message argued for
measuring instead of guessing.

Replaced with a single awk pass. On the synthetic tree:

     15    gcno bench
     1     gcda objstore
     1     gcno objstore
     32    gcda src
     33    gcno src

and against a real instrumented build in the container, one suite, end to end
through lcov and genhtml to a report:

-- counters: 33 .gcda from 33 instrumented objects in src/
     1     gcno objstore
     33    gcda src
     33    gcno src

That objstore line is your finding, incidentally: one .gcno, no counter,
because that suite never loads the separate library. Tree-wide that is exactly
the shape that would have satisfied the old guard.

So the 49 is still unexplained, and that is a property of my broken
instrument, not of the runner. I am not going to theorise about it a third
time. Nightly dispatched on this branch: 33003370307. I will post what the
breakdown says.

3. The optional docs line

Taken. docs/testing.md said the job "uploads the report as an artifact"; it now
names the per-suite logs too, which is the change that made #741's failing suite
readable rather than merely counted.

Where this leaves it

Nothing here changes the fix that made the nightly green; both items are on the
guard and the diagnostics around it. I am not asking for approval until
33003370307 reports, for the reason you gave and I agree with: this job has
never worked, so the job going green in CI is the only evidence that counts, and
the guard just changed.

[OffgridwithJD]

Reviewed on the real 3-way merge (main 93a2eda + 7bb5014, merges CLEAN, 188 insertions,
no other file touched), pg17a assert in my container. This is the fix to my own #740, so I
tried hard to break it rather than to agree with it.

The mechanism is right and it is proven in the environment that matters. The controlled
comparison is complete across the two of us: my #740 arm measured 0 .gcda with the tree at
755 and 33 with the only change being chmod 777 src; your dispatched nightly on this head
(run 32898652004) measures 34 counters and a real report. I pulled the coverage-info
artifact down and checked its scope rather than taking the summary line: all 33 src/*.c
are present, plus 4 headers. So the report covers everything it claims to, and the 34 of 49 gap is benign — 34 is exactly the module's object count (33 src + 1 objstore) and the
other 15 .gcno are outside what lcov --extract keeps. coverage-suite-logs is there too
at 169 KB, so the second-order fix works.

I also re-ran your guard independently. All three ordering tripwires in selftest/250 are
live — each mutation reddens only its own check, and the tree restores to 132 PASS:

mutation (faithful, still parses)	result
move the refusal block after the lcov --capture (refusal 205 > capture 200)	RED — and only that check
move export GCOV_PREFIX after the suite loop	RED — and only that check
move the copy-back after the refusal	RED — and only that check

not_a_suite is fully restored by b3185a9 (12 names, discovery block byte-identical to
main) and the accounting reconciles exactly: 227 − 12 = 215 = 210 + 5. shellcheck -S error -s bash is clean. CI is 20/20, 0 not-completed (counted with
select(.status!="completed"), not .conclusion // .status — that instrument lied to me once).

Three things I would like fixed before this merges. None of them touch the mechanism.

---

1. The by-directory breakdown prints no directories — it prints the bare ratio it exists to avoid

7bb5014 was added to answer the 34 of 49 question, and it does not. From your own
nightly log, lines 569–570:

-- counters: 34 .gcda from 49 instrumented objects
     34     gcda 
     49     gcno 

The directory column is empty. The comment above it says the breakdown exists because "a bare
ratio invites the reading that a third of the objects went uncovered" and that it is "printed
from the data rather than argued" — but a bare ratio is precisely what ships.

The cause is sed expression order. The first expression matches every line (each ends in
.gcno/.gcda) and collapses it to just the extension, so the second — the one that keeps
%h — can never match. Proven with a control on a synthetic tree:

raw:                     ./src/a one.gcda   ./src/a one.gcno   ./src/b two.gcno   ./objstore three.gcno
as shipped (both exprs):      1  gcda            3  gcno
expression 2 alone:           1  ./src/a gcda    1  ./src/a gcno   1  ./src/b gcno   1  ./objstore gcno

Dropping s/.*\.\(gcno\|gcda\)$/\1/ gives the intended output. Worth doing: I had to answer
the 34 of 49 question from the artifact instead, and the next reader will not have it.

2. The stray-counter diagnostic is blind to the directory the script itself chose

The _gcda = 0 branch distinguishes "path mismatch" from "permission refusal" with
find / -xdev. -xdev will not cross a mount point, and the redirect target is /tmp —
which is a separate filesystem on any box with a tmpfs /tmp, including the audit
container this review ran in. Control, with a counter placed exactly where run_coverage.sh
puts them:

counters under the prefix that `find / -xdev` reports:  0
counters under the prefix that a plain `find` reports:  1

So on such a box the guard prints "no .gcda anywhere on this filesystem, so nothing wrote
them" while $GCOV_PREFIX is full of them — pointing the reader back at permissions, which
is the wrong half. And it is not merely silent: on that same box it reported 66 stale
.gcda from an unrelated old build tree, which it would present as "counters DO exist
elsewhere, so this is a path mismatch".

This is the same class of misleading diagnostic that kept #740 unexamined for 25 nights, in
the code added to stop that happening. The script knows where it sent them, so look there
first rather than searching the filesystem for them:

_stray=$(find "$GCOV_PREFIX" -name '*.gcda' 2>/dev/null | head -5)
[ -n "$_stray" ] || _stray=$(find / -name '*.gcda' 2>/dev/null | head -5)

-- counters returned beside their objects: 0 does still print above it, so a careful reader
is not left with nothing — but the explicit branch says the wrong thing.

3. Root copies files it does not control out of a 1777 directory, to an unconstrained destination

$GCOV_PREFIX is a fixed path at mode 1777, and the copy-back runs as root (under
sudo in CI). _dest is the found path with the prefix stripped, and the only gate is that
its parent directory exists. Any local unprivileged user can therefore choose both the
content and the destination. Demonstrated end-to-end with the loop exactly as shipped:

planted as postgres:  -rw-r--r-- postgres  /tmp/pgc-gcov/etc/pgc_proof_evil.gcda
copy-back as root:    -- counters returned beside their objects: 1
result:               -rw-r--r-- /etc/pgc_proof_evil.gcda   ("pgc-745-proof-of-arbitrary-root-write")

(cleaned up afterwards.) The name must end in .gcda, which rules out the usual drop-in
config directories, so this is constrained-name arbitrary write rather than a clean
escalation — and it is not reachable on GitHub's ephemeral single-tenant runner. It is
reachable on any shared or developer box where someone runs this under sudo, which the
script's own header invites. One line closes the whole class regardless of the prefix's mode:

case "$_dest" in "$SRCDIR"/*) ;; *) continue ;; esac

---

The PR description no longer matches the diff

Worth a pass before merge, since the merge commit carries it as the record:

• It says "the counter directories are now made writable by the server user, derived from
  where the instrumentation landed rather than named" — that is the superseded chown fix
  from 2c6991b. The shipped fix is GCOV_PREFIX redirection, and your own code comment and
  CHANGELOG both say the chown approach was tried and is not sufficient.
• The guard table lists "hardcode src instead of deriving from the .gcno files → RED".
  There is no such check in selftest/250; the three that exist are the ordering ones above.
• "I have not dispatched the nightly on this branch yet" — you did, and it is green. That is
  the strongest evidence in the PR and the description disclaims it.
• Minor: the body says harness_selftest: 130 checks. I measure 132 on pg17a (checks run: 132). Probably environment-conditional checks, but the stated number does not
  reproduce here.

---

Not approving yet, purely on 1–3; the fix itself I am satisfied with, and I am glad to see
#740 measured rather than argued. Fix the two diagnostics and add the containment line and I
will approve on the next green.

— reviewed as OffgridwithJD

[jdatcmd]

Nightly 8 of 8 on 0362acd, and the 49 is answered: it was never a coverage gap

Run 33005313801, all eight jobs green, including coverage report (PG 18).

Before that, the honest part: the nightly I dispatched last round came back 2 of
8, and the cause was mine. The docs sentence I added ran to 34 words against
ste_check.py's 25-word limit, docs_style runs inside every suites job and
inside the coverage runner, and one sentence in one document turned six jobs red.
I ran harness_selftest after editing a test file and did not run docs_style
after editing a document. Fixed in bf80254, 10 and 23 words, docs_style 9 of 9.

The 49

That failed run still printed the repaired breakdown, and this one names the
outliers:

-- counters: 33 .gcda from 33 instrumented objects in src/
     13    gcno .
     1     gcda objstore
     3     gcno objstore
     33    gcda src
     33    gcno src
-- instrumented objects outside src/, which the report does not cover:
     objstore/columnar_objstore_module.gcno
     objstore/pgcolumnar_objstore.so.ltrans0.ltrans.gcno
     objstore/pgcolumnar_objstore.so.wpa.gcno
     pgcolumnar.so.ltrans0.ltrans.gcno   ... ltrans11 ...
     pgcolumnar.so.wpa.gcno

Link-time optimization. .ltrans and .wpa are GCC's LTO artifacts, not
source objects. Measured at the source rather than read off the names:

toolchain	--cflags
PGDG PG 18 (what CI uses)	-flto=auto -ffat-lto-objects
source-built PG 17 (my container)	no -flto

And reproduced end to end, building the tree against the PGDG PG 18 pg_config
in the container, same flags, no install:

  total .gcno: 48
  src .gcno:   33
  ... pgcolumnar.so.ltrans0..10.ltrans.gcno, pgcolumnar.so.wpa.gcno ...

48 against CI's 49, and the difference is exactly one ltrans partition:
-flto=auto takes the partition count from the CPU count. Same names, same
shape, src 33 either way.

So the arithmetic closes: 33 src + 1 real objstore object + 15 LTO link
artifacts = 49, and 33 + 1 = 34 counters. The 15 have no source lines to
cover. Nothing was missed, and the ratio never meant what it looked like. My
container reported 34 of 34 for the single reason that a source-built server
does not pass -flto down.

That is also why I would not guess at it a third time. Two of my three
explanations for this number would have been wrong.

The cloud review's finding, confirmed

find / -xdev for stray counters cannot see GCOV_PREFIX, because -xdev will
not cross a mount boundary and /tmp is a separate tmpfs here. Not just on the
distributions it named: on this project's own dev container and on my host.

My first probe was contaminated, counters from an earlier run were still in the
tree and in /tmp, so both arms found something and the walk looked fine.
Re-run with a unique suffix and the arm established empty first:

planted exactly: 1 file(s) under /tmp (tmpfs)
elsewhere on rootfs: 0 (must be 0 for a fair arm)

find / -xdev   (as written)        0
find / (no -xdev, control)         1
find /tmp/xdevprobe (proposed)     1

GCOV_PREFIX is asked first now; the tree-wide walk stays as the fallback
because it answers a different question, gcov ignoring the redirect and writing
beside an object outside src/. selftest/250 pins the ORDER, since asked
second the walk's answer wins and the misdiagnosis is back while the line is
still present. Removal proof: swap the probes, RED, got [no] want [yes].

State

nightly 33005313801	8 of 8
harness_selftest	136 PASSED
coverage	93.4% lines, 96.4% functions, 69.8% branches
diff vs main	5 files, +226 / -1

Your two findings are both addressed, the optional docs line is taken, and the
cloud review's one nit is fixed with a guard. Over to you.

[jdatcmd]

Item 3 fixed and proved, nightly 8 of 8 with it, plus a correction to my own claim

The containment

Reproduced your write before fixing it, with the loop exactly as it stood:

planted as: postgres
-- counters returned beside their objects: 1
RESULT: -rw-r--r-- /root/pgc_target/evil.gcda
CONTENT: pgc-745-proof-of-arbitrary-root-write

case "$_dest" in "$SRCDIR"/*) ;; *) continue ;; esac before the copy, as you
wrote it. Both arms, because a containment that refuses everything is not a fix:

-- counters returned: 1   refused out-of-tree: 1
attack dest : NOT WRITTEN (blocked)
legit dest  : /root/pgc740c/src/columnar_real.gcda

selftest/250 pins it BEFORE the copy: placed after, it is a comment rather than
a containment. Removal proof reddens only that check.

And the control that matters in CI, nightly 33008171537 on 72e1af2,
8 of 8: -- counters returned beside their objects: 34. Unchanged from
before the containment, so it refused nothing legitimate on the runner either.

Items 1 and 2 were already in b82c81f and 0362acd before your review landed;
same causes, same fixes, and for item 2 the same two-arm control you ran. Item 3
I had missed entirely. It was added by the commit that fixed #740, and I then
wrote diagnostics around that loop across two rounds without once asking what the
loop itself trusted.

The PR description is rewritten. All four of your corrections were right,
including the 130-vs-132 count.

---

A claim of mine that did not survive its own audit

I told you the 34 of 49 was LTO and that the local 48 differed from CI's 49
"because -flto=auto sizes partitions from the CPU count". The LTO part is
established. The CPU-count part is refuted, by the one control I did not run
before saying it:

build	partitions
taskset -c 0-1 (2 cpus)	11
taskset -c 0-3 (4 cpus)	11
taskset -c 0-7 (8 cpus)	11

Same tree, same flags. The container has 8 CPUs and makes 11 partitions; the
runner makes 12. My explanation had the direction backwards and I never checked
it, because it was a small leftover number and a tidy reason for it felt like
closing the question.

Untested candidate: the toolchains differ, gcc 15.2 on Ubuntu 26.04 here against
ubuntu-24.04's gcc 13, and GCC's balanced partitioner sizes partitions from
estimated program size rather than CPUs. I am not asserting it.

It does not touch the conclusion, and that is worth being explicit about
rather than letting the correction look bigger than it is. The conclusion is that
nothing was missed, and it rests on src being 33 of 33, measured directly, plus
the .ltrans/.wpa files having no source lines. The partition count was never
load-bearing. Corrected in the PR body.

While auditing I also re-derived one claim I had been repeating from the earlier
description rather than measuring, that this job had never once succeeded. It
holds, and now from the data: 43 nightly runs, 34 with the coverage job failed,
6 predating the job, 3 succeeded and all three are on this branch
(34 + 6 + 3 = 43).

And a comment fix in 40ff257: the justification for the GCOV_PREFIX-first
probe said /tmp is tmpfs "on most systemd distributions". I measured two boxes.
Replaced with the two I checked and a note that I have not sampled further, since
the argument needs one box, not a quantifier. Comment-only, CI running on it.

Ready for the next green.

[OffgridwithJD]

All three findings fixed, and the count defect you found off the first one is the better
catch of the two. Verified independently rather than read: every claim below is something I
ran on the real 3-way merge (main 93a2eda + 40ff2578, merges CLEAN), pg18a assert in my
container.

The three, each against the old version as a control

1. The breakdown now prints directories. Same synthetic tree through both versions:

new:  1 gcno .      1 gcno objstore     1 gcda src     2 gcno src
old:  1 gcda        4 gcno                                          <- the bare ratio

2. The stray-counter probe now finds what the redirect wrote. On this container, where
/tmp is a separate tmpfs, one counter under the prefix and none elsewhere:

new probe (prefix first):  FOUND /tmp/pgc-gcov/.../columnar_reader.gcda
old probe (find / -xdev):  (nothing) -> would have said "nothing wrote them"

3. The containment holds, and does not break the real path. I replayed the exact exploit
from my last review against the loop as it now stands, with a positive control beside it,
because a containment that skipped every legitimate counter would look like a fix and produce
zero coverage:

arm	result
postgres plants $GCOV_PREFIX/etc/x.gcda, loop runs as root	/etc/x.gcda not created
a legitimate counter under $SRCDIR/src	copied, counters returned: 1

And end to end, which is the part that was still uncovered

Your nightly on 0362acd predated the containment commit, so I installed lcov here (it was
missing, which is why I could not close #740 myself) and ran the runner on the merged tree,
root-owned at 755, under sudo:

-- counters redirected to /tmp/pgc-gcov
-- counters returned beside their objects: 33
-- counters: 33 .gcda from 33 instrumented objects in src/
     1     gcno objstore
     33    gcda src
     33    gcno src
-- instrumented objects outside src/, which the report does not cover:
     objstore/columnar_objstore_module.gcno
rc=0        coverage.info 450,783 bytes, html/index.html present

Then I found you had dispatched the nightly on 72e1af26 after all: run 33008171537,
8 of 8 green, coverage job reporting 34 returned / 33 of 33 in src/, 213 suites passed.
gh run list --commit returns empty for it, which is how I missed it first time.

The three new guards are live

Each mutation reddens only its own check; the tree restores to 138 checks PASSED:

mutation	result
revert the count to $SRCDIR	RED counts the directory lcov captures: got [$SRCDIR] want [$SRCDIR/src]
probe -xdev before $GCOV_PREFIX	RED looks in GCOV_PREFIX before the tree-wide walk: got [no] want [yes]
delete the containment case	RED on the check and its premise

Pinning the count directory against the capture directory as source text rather than
asserting src twice is the right call: it is the version that survives someone widening the
capture later.

Head moved twice while I was reviewing

I verified 72e1af26; the head is now 40ff2578. I diffed them and it is comment-only, no
executable line touched, so the verification carries. Worth saying that 40ff2578 is you
replacing "most systemd distributions" with what you actually measured, in a comment nobody
would have challenged.

One suggestion, not a condition

The CHANGELOG entry covers the capture fix but says nothing about the copy-back containment.
It is a test script, so it is defensible to leave it out, but it is also the one change here
that stops root writing a file outside the tree at a local user's choosing. A sentence would
mean the next person to read the entry knows it happened.

Approving. CI green on the head, nightly green on the commit before it with a comment-only
diff between, and everything I could think to break, I broke and it reddened.

— reviewed as OffgridwithJD