Skip to content

ci: run the unit-test packages a second time, privileged — the BPF-gated tests have never run (#169) - #178

Merged
dpsoft merged 5 commits into
mainfrom
ci/privileged-unit-pass
Oct 1, 2026
Merged

dpsoft merged 5 commits into
mainfrom
ci/privileged-unit-pass

Conversation

@dpsoft

@dpsoft dpsoft commented Oct 1, 2026

Copy link
Copy Markdown
Owner

Closes #169.

Every BPF-requiring test in ./cpu, ./profile, ./offcpu, ./pyunwind and ./unwind/dwarfagent gates on requireBPFRunnable. The unit job runs them unprivileged, so they skip — and the job reports green:

TestProfilerEndToEnd        SKIP   (unwind/dwarfagent)
TestOffCPUProfilerEndToEnd  SKIP   (unwind/dwarfagent)
TestPerfDwarfLoads          SKIP   (profile)

TestPerfDwarfLoads is the one that matters. It loads the DWARF BPF program — the object both #156 and #164 changed — and CI has never executed it. A verifier rejection surfaces at load time, not compile time, so nothing else in the pipeline catches one. #156's fix needed two attempts for precisely that reason: the first used bpf_get_current_task(), which returns an integer where bpf_task_pt_regs() needs ARG_PTR_TO_BTF_ID, and it compiled perfectly.

A second pass, not a privileged first one

The unprivileged run is the right default and worth keeping: most of these tests must work without capabilities, and losing that signal would hide a regression that quietly makes them require root.

The new pass carries no -race and no coverage — the first owns both. It exists to execute the gated tests, not to measure them again.

It fails when it skips

A privileged pass that skipped the three tests it was added for is not a pass, and would look identical to a green run:

for t in TestPerfDwarfLoads TestProfilerEndToEnd TestOffCPUProfilerEndToEnd; do
  if grep -q "^--- SKIP: $t" /tmp/privileged-unit.log; then
    echo "::error::$t SKIPPED in the privileged pass; it is covering nothing"
    exit 1
  fi
done

This repo has found four checks that could not fail in a week — #156, #159, #160, #165 — so a new one shipping without that guard would be an odd choice.

A correction to #169

I originally filed that issue claiming six in-process BPF tests "have never run anywhere". That was wrong, and I'd only checked my own machine, where the test binary was uncapped and built into nosuid /tmp. The integration job runs under sudo, so those six do run in CI:

TestPerfDwarfWalker          PASS
TestStreamingProfileOutput   PASS
TestLibraryPMUMetrics        PASS

The real gap was in a different job, and is what this PR fixes. The issue has been corrected.

Verification

The three test names exist where the detector looks for them. The pass itself can only be verified by CI — it does not run locally, since go test builds its binary into nosuid /tmp where file capabilities would not survive exec.

https://claude.ai/code/session_01P5889hA6CrX8ysnQkvv6im

dpsoft added 2 commits October 1, 2026 12:07
Every BPF-requiring test in ./cpu, ./profile, ./offcpu, ./pyunwind and
./unwind/dwarfagent gates on requireBPFRunnable. The unit job runs them
unprivileged, so they SKIP, and the job reports green:

    TestProfilerEndToEnd        SKIP   (unwind/dwarfagent)
    TestOffCPUProfilerEndToEnd  SKIP   (unwind/dwarfagent)
    TestPerfDwarfLoads          SKIP   (profile)

TestPerfDwarfLoads is the one that matters. It LOADS the DWARF BPF
program -- the object #156 and #164 both changed -- and CI has never
executed it. A verifier rejection surfaces at load time, not compile
time, so nothing else in the pipeline would catch one. #156's own fix
needed two attempts for exactly that reason: the first used
bpf_get_current_task(), which returns an integer where bpf_task_pt_regs()
needs ARG_PTR_TO_BTF_ID, and it compiled cleanly.

A second pass rather than making the first one privileged. The
unprivileged run is the right default and worth keeping: most of these
tests must work without capabilities, and losing that signal would hide a
regression that quietly makes them require root. The new pass carries no
-race and no coverage -- the first owns both -- because it exists to
execute the gated tests, not to measure them again.

It also fails when the tests it exists for SKIP. A privileged pass that
skipped the three tests it was added for is not a pass, and would be
indistinguishable from a green run; this repo has found four such checks
in a week (#156, #159, #160, #165).

Verified: the three test names exist where the detector looks for them.
The pass itself is verified by CI, which is the only place it runs.

Claude-Session: https://claude.ai/code/session_01P5889hA6CrX8ysnQkvv6im
The skip-detector added in the previous commit fired on its first CI run:

    ##[error]TestProfilerEndToEnd SKIPPED in the privileged pass;
             it is covering nothing

sudo was not enough. TestProfilerEndToEnd and TestOffCPUProfilerEndToEnd
profile the rust workload and skip when it is absent:

    binPath := "../../test/workloads/rust/target/release/rust-workload"
    if _, err := os.Stat(binPath); err != nil {
        t.Skipf("rust workload not built: %v", err)
    }

The integration job builds the workloads through run_tests.sh; the unit
job never did, because until now nothing in it needed them.

Worth noting what this says about the detector. Without it the step would
have gone green on that run while two of its three tests skipped --
exactly the shape it exists to refuse, caught on the first attempt, in
the commit that introduced it.

Verified: the manifest path is right and `cargo build --release
--manifest-path test/workloads/rust/Cargo.toml` produces the binary at
the path the tests stat.

Claude-Session: https://claude.ai/code/session_01P5889hA6CrX8ysnQkvv6im
dpsoft added 3 commits October 1, 2026 12:42
The privileged pass surfaced a second thing on its first green-ish run:
TestLazyMode_FiresAndCompilesOnMiss passes on amd64 and fails on arm64,
with no CFI miss events firing there at all over a 19s window.

    amd64   unprivileged SKIP   privileged PASS
    arm64   unprivileged SKIP   privileged FAIL

    MissStats.Received == 0; expected at least one CFI miss event
    MissStats pre/post: pre.Received=0 post.Received=0 post.Resolved=0

Not caused by this step and not a regression: the test has been skipping
on both arches for as long as the unit job has been unprivileged, so the
arm64 behaviour had never been observed. Whether it is correct (arm64's
frame-pointer ABI letting every walk finish without CFI) or a real gap
(lazy mode not firing, so late-enrolled binaries are walked without
tables) needs the walk-ending stats to tell apart. Issue #179.

Excluded with -skip in the workflow rather than a t.Skip in the test, so
the exclusion is visible to anyone reading this file and vanishes the
moment #179 is resolved. A t.Skip would read as "nothing to do here" in
the test output forever.

The three tests this pass exists for -- TestPerfDwarfLoads,
TestProfilerEndToEnd, TestOffCPUProfilerEndToEnd -- pass on both arches,
and the skip-detector still guards all three. Blocking the pass on an
arm64 CFI investigation would leave TestPerfDwarfLoads, which loads the
BPF object #156 and #164 both changed, uncovered for however long that
takes.

Claude-Session: https://claude.ai/code/session_01P5889hA6CrX8ysnQkvv6im
The arm64 failure reported only "MissStats.Received == 0", which is not
enough to act on. A miss fires iff the sampled PC's mapping is in
pid_mappings AND cfi_lengths has no table for it yet
(bpf/unwind_common.h:816), so zero misses has three possible causes and
the counter cannot tell them apart:

  1. nothing was enrolled                  -> binaryCount == 0
  2. everything was already compiled       -> no lazy window existed
  3. nothing was sampled in an enrolled mapping

(1) is an enrollment failure and a real bug. (2) is arguably correct and
means this test's premise is amd64-specific. They need opposite fixes,
and #179 was filed as a guess between them because the output did not
distinguish them.

The profiler already exposes AttachStats() (pidCount, binaryCount); the
test just never printed it. It now does, and (1) fails separately with
its own message rather than being folded into "Received == 0".

Also drops the arm64 exclusion added in the previous commit. Excluding
the test means never collecting the one measurement that would settle
#179 -- and arm64 is not reproducible locally, so CI is the only place
that measurement exists. Better to let it fail loudly with the numbers
attached for one cycle than to merge a workaround and keep guessing.

Claude-Session: https://claude.ai/code/session_01P5889hA6CrX8ysnQkvv6im
The test spawned `sleep 30` and then required at least one CFI miss. A
sleeping process is never on CPU, so it is never sampled, so no PC ever
lands in its mapping and it cannot produce a miss. Its own comment said
the spawn was there "to ensure /proc has at least one non-self PID
visible during the lazy scan" -- enrollment was handled and sampling was
left to chance.

Every miss the test observed therefore came from whatever else happened
to be running on the machine. The numbers from the run that exposed this,
with both arches on the same commit:

    amd64   enrolled: pids=39 binaries=377   Received=2  Resolved=2
    arm64   enrolled: pids=39 binaries=377   Received=3  Resolved=3

Identical enrollment, two or three incidental misses. On a quieter runner
that is zero, which is what failed -- and it is why the same commit
passed on amd64 and failed on arm64, then passed on arm64 in the next
run. The architecture was never the variable; ambient machine activity
was. #179 was filed as an arm64 gap and is not one.

Three changes, each for a distinct defect:

  - spawn rust-workload instead of sleep: CPU-bound, so it is actually
    sampled, and it carries .eh_frame, so it genuinely needs CFI
  - spawn it AFTER the profiler, so its binary is enrolled LAZILY, which
    is the case this mode exists for, rather than during the startup scan
  - poll with a deadline instead of time.Sleep(5s): the miss window is a
    race by nature, existing only between enrollment and compilation, so
    a fixed sleep either wastes time or lands outside it

The enrollment counters added while diagnosing this stay. A future
failure now says which of three causes it is -- nothing enrolled,
everything already compiled, or nothing sampled -- instead of only
"Received == 0", which is what made this a guess.

Verified 8 consecutive local runs on a capped binary, all passing, with
Received between 1 and 8; the poll exits on the condition rather than on
a timer, so the outcome is stable even though the count is not.

Claude-Session: https://claude.ai/code/session_01P5889hA6CrX8ysnQkvv6im
@dpsoft
dpsoft merged commit 536c738 into main Oct 1, 2026
16 checks passed
dpsoft added a commit that referenced this pull request Oct 3, 2026
…117) (#182)

BPF objects were not byte-reproducible across machines that agreed on
every visible input. The issue eliminated the compiler, the strip tool,
the package version and the header contents by measurement, and
identified the remaining variable as the headers coming from whatever
/usr/include/bpf the generating machine happened to hold. Its proposed
fix was to vendor them.

That fix had already been half-applied, inertly. cpu/gen.go passed
-I../bpf/libbpf, but bpf/libbpf did not exist -- clang ignores a missing
-I without complaint -- and the other four packages did not pass it at
all. profile/, which holds the object the issue names as divergent, was
one of those four. cpu/gen.go also carried -I../bpf/vmlinux/, equally
dead, and that pair is what made the first flag look like vendoring had
been done. The dead one is removed; the sources find vmlinux.h relative
to themselves.

Vendored are the four headers clang actually opens, determined with
clang -H over the three this repo includes directly rather than guessed:
bpf_helpers.h, bpf_core_read.h, bpf_tracing.h, and bpf_helper_defs.h
transitively. Taken from libbpf-dev 1:1.3.0-2build2 in the CI image, NOT
from the local system: this machine ships 1.6.3, and vendoring that would
have "fixed" reproducibility by breaking it against CI. bpf/libbpf/README
records the trap and the command to update them correctly.

Four objects move once, and the shape of the move is the issue's own
signature. Comparing cpu/cpu_x86_bpfel.o before and after:

    .text      same      <- no codegen change
    .BTF.ext   same
    .strtab    same
    .BTF       DIFFERS   <- 247->250, 251->247, 250->251

Values exchanged rather than changed, confined to .BTF's type section.
That a header's PATH alone permutes the ordering is the clearest evidence
yet for the pointer-keyed-map explanation, and it is the same mechanism
by which any difference in a machine's /usr/include/bpf could permute it
-- which is the invisible variable this issue could not pin down.

From here the ordering depends on files in the repo. generate-check
continues to verify it, and #178's privileged unit pass now actually
loads the DWARF object rather than skipping it, so a BTF change that
broke loading would surface.

Claude-Session: https://claude.ai/code/session_01P5889hA6CrX8ysnQkvv6im
@dpsoft
dpsoft deleted the ci/privileged-unit-pass branch October 5, 2026 22:03
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.

ci: BPF-requiring tests in the unit-test packages never run — the unit job is unprivileged

1 participant