cintegration: C harness for libpilot //export surface#2
Merged
Conversation
Go's test toolchain refuses `import "C"` in _test.go files of packages that already carry //export directives. That leaves ~250 of libpilot's ~290 statements out of reach of `go test -cover` — every PilotXxx wrapper and its companion helpers (errJSON, okJSON, driverFromHandle) have C-typed signatures and cannot be called from Go test code. cintegration/ fills the gap by driving the compiled c-shared library from C. The harness: - exercises 29 FFI paths covering handle-table edges (zero / unknown handles), parameter-validation (NULL pointers, negative lengths, oversized buffers), and error paths for every endpoint that doesn't need a live daemon - runs under -cover instrumentation by building libpilot with `-tags coverflush -cover -covermode=atomic -buildmode=c-shared`, then calling the new PilotCoverFlush //export before the C exit() bypasses Go's atexit handlers - raises pure-C harness coverage to 20.8% (combined with Go-side zz_internal_test.go: ~25% effective) coverflush.go is build-tag-gated (`coverflush`) so PilotCoverFlush does NOT ship in production builds — it's purely a coverage escape hatch. Audit verification turned up by running the harness: the iter-3 CRITICAL-2 finding "C.GoBytes panic on negative length" is mostly unreachable from C. PilotConnWrite / PilotSendTo / PilotBroadcast all call loadHandle() before C.GoBytes; a bad handle short-circuits to error JSON before negative length matters. To actually trigger the panic the caller needs a valid handle (live daemon connection) AND negative dataLen — real defensive-coding gap but the exploitability is MED, not CRITICAL as originally graded.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a C-side integration harness that drives libpilot's
//exportsurface — code that's literally unreachable from Go test files (Go forbidsimport \"C\"in_test.gofiles of packages with//exportdirectives).coverflush.go) that exposesruntime/coverage.WriteCountersDiras a//export; the harness calls it before Cexit()skips Go's atexit handlersAudit verification
The iter-3 CRITICAL-2 finding (
C.GoBytespanic on negative length) is mostly unreachable from C:PilotConnWrite/PilotSendTo/PilotBroadcastall callloadHandle()first, so a bad handle short-circuits to error JSON before the negative length matters. Real defensive-coding gap, but exploitability downgrades from CRITICAL to MED — needs a valid handle (live daemon connection) PLUS negative dataLen.Files
cintegration/harness.c(503 lines) — 29 test functions, each checkshas_error()on the returned JSON and tracks PASS/FAILcintegration/Makefile— builds the dylib with-tags coverflush -cover -covermode=atomic, links the harness with rpath, runs withGOCOVERDIRset, folds counters to a textfmt profilecintegration/.gitignore— excludes build artifactscoverflush.go— single 38-line file gated by//go:build coverflush; does NOT ship in production buildsHow to run
```
cd cintegration && make cover
```
Test plan
make coverproducescoverage.outwith 20.8% of statements (verified locally on darwin/arm64)