ci: adopt shared tccbin conformance test suite (freebsd-amd64) - #75
Merged
GGRei merged 23 commits intoJul 26, 2026
Conversation
Wires this branch into the shared cross-platform conformance suite (thirdparty/tccbin_tests in vlang/v) via a real FreeBSD VM (cross-platform-actions/action, same tooling vlang/v's own update_tccbin.yml uses to rebuild this branch). No rebuild here - validates the already-committed tcc.exe/lib/libgc.a as-is. Unverified locally (no FreeBSD access here) - V's own builder also references -lgc-threaded for this platform+tcc combo (vlib/builtin/builtin_d_gcboehm.c.v), which isn't in this branch's bundle; starting without it since the bundled libgc.a should be self-sufficient, and will adjust based on real CI results if not.
…onnection reset there twice)
…re, confirmed independently)
quaesitor-scientiam
changed the title
tcc on FreeBSD cannot currently link the bundled libgc.a (undefined symbol 'etext'/'end' - see tccelf.c's tcc_add_linker_symbols, which only defines the glibc-style _etext/_edata/_end, never the plain BSD-style names BDWGC's FreeBSD code references). Root-caused and a fix verified against a real FreeBSD VM, being submitted upstream to tinycc-devel - but it isn't in any shipped tcc.exe yet. crash.c (no GC) is checked directly as the real blocking regression check. The GC-dependent tests (hello.c, gc_alloc.c) run via the full shared suite as an explicit, temporary XFAIL lane - visible in CI logs so a regression that fails *differently* is still noticed, but not blocking. Remove continue-on-error once the tcc fix ships in a rebuilt tcc.exe here. Removes the temporary etext-fix verification job now that the fix is confirmed and the real CI reflects reality.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 146aec2de8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two issues found by Codex review (pullrequestreview-4779654227): - The hardcoded `git clone --branch thirdparty-freebsd-amd64 .../vlang/tccbin.git` always tested the already-merged target branch, regardless of what a PR actually proposed - a PR updating tcc.exe/libgc.a could pass CI while testing the old, unrelated binaries. Now resolves the PR's own head repo+ref for pull_request events, falling back to the triggering ref for push/workflow_dispatch (which have no PR context). - The no-GC crash check only verified "nonzero exit", which an exec-format error, missing dynamic loader, or an unrelated miscompilation exiting 1 would also satisfy - silently masking a real regression, since the GC-backed suite is explicitly non-blocking. Now requires exit 139 specifically (killed by SIGSEGV), matching crash.c's actual intended fault.
…H Actions doesn't allow it)
…arded by environment_variables)
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf1697fa26
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… pinned suite commit, harden XFAIL) Fixes 3 P2 + 1 P1 findings from pullrequestreview-4780048339 on vlang#75: - P1: fetch the pinned vlang/v suite commit directly instead of a plain --depth=1 clone-then-checkout, which only ever has the current tip's objects and would start failing the moment vlang/v advances past the pinned commit - P2: clone the exact triggering commit (SHA) instead of a mutable ref name, which was racy against a push landing while this run was queued - P2: fall back to the triggering event's own repository for non-PR runs instead of a hardcoded vlang/tccbin.git - P2: assert the exact known etext/end failure signature in the libgc.a XFAIL lane instead of a blanket continue-on-error, so an unrelated regression (corrupted archive, bad checkout) can't silently pass as the known XFAIL Also pins cross-platform-actions/action to a commit SHA instead of the mutable v1.3.0 tag (same class of finding as the sibling openbsd-amd64 workflow, pullrequestreview-4780049532 on vlang#76).
quaesitor-scientiam
pushed a commit
to quaesitor-scientiam/tccbin
that referenced
this pull request
Jul 25, 2026
…ork fallback) Fixes 3 P2 findings from pullrequestreview-4780049532 on vlang#76: - clone the exact triggering commit (SHA) instead of a mutable ref name, which was racy against a push landing while this run was queued - pin cross-platform-actions/action to a commit SHA instead of the mutable v1.3.0 tag - fall back to the triggering event's own repository for non-PR runs instead of a hardcoded vlang/tccbin.git Also fixes the same shallow-clone-then-checkout-old-SHA bug Codex flagged as P1 on the sibling freebsd-amd64 workflow (pullrequestreview-4780048339 on vlang#75): a plain --depth=1 clone only contains the current tip, so checking out the older pinned vlang/v commit would break the moment vlang/v advances past it. Fetch that SHA directly instead.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd124a30ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
GGRei
pushed a commit
that referenced
this pull request
Jul 25, 2026
* ci: adopt shared tccbin conformance test suite (with diagnostics) Wires this branch into the shared cross-platform conformance suite (thirdparty/tccbin_tests in vlang/v) via a real OpenBSD VM (cross-platform-actions/action, same tooling vlang/v's own update_tccbin.yml uses to rebuild this branch). This branch ships no libgc at all (unlike the other platforms) - confirmed by inspecting its tree and matching V's own builder, which only references /usr/local/lib/libgc.a for openbsd+tinyc, never a bundled path. GC-dependent tests need a system libgc package instead; package name is an educated guess (libgc), with a diagnostic fallback listing if it's wrong. Unverified locally (no OpenBSD access here). * ci: fix OpenBSD libgc package name (boehm-gc, not libgc) * ci: remove temporary diagnostic step now that the fix is confirmed * ci: test the PR's own revision instead of the target branch's tip The hardcoded `git clone --branch thirdparty-openbsd-amd64 .../vlang/tccbin.git` always tested the already-merged target branch, regardless of what a PR actually proposed - a PR updating tcc.exe/libgc.a could pass CI while testing the old, unrelated binaries. Now resolves the PR's own head repo+ref for pull_request events, falling back to the triggering ref for push/workflow_dispatch (which have no PR context). Found by Codex review (pullrequestreview-4779651560). * ci: fix environment_variables forwarding (needs job-level env, not step-level) * ci: fix job-level env referencing steps context (schema violation - GH Actions doesn't allow it) * ci: move env to job level (step-level env on a uses: step wasn't forwarded by environment_variables) * ci: fix Codex-flagged CI hardening issues (pin SHA, pin action, fix fork fallback) Fixes 3 P2 findings from pullrequestreview-4780049532 on #76: - clone the exact triggering commit (SHA) instead of a mutable ref name, which was racy against a push landing while this run was queued - pin cross-platform-actions/action to a commit SHA instead of the mutable v1.3.0 tag - fall back to the triggering event's own repository for non-PR runs instead of a hardcoded vlang/tccbin.git Also fixes the same shallow-clone-then-checkout-old-SHA bug Codex flagged as P1 on the sibling freebsd-amd64 workflow (pullrequestreview-4780048339 on #75): a plain --depth=1 clone only contains the current tip, so checking out the older pinned vlang/v commit would break the moment vlang/v advances past it. Fetch that SHA directly instead. * ci: fix mkdir (needs -p, thirdparty/ parent dir doesn't exist yet in a fresh VM) --------- Co-authored-by: Richard Wheeler <18647491+PythonWillRule@users.noreply.github.com>
…st the run.sh summary shape Fixes P2 finding from pullrequestreview-4780219278 on vlang#75: the pinned run.sh's summary-only match couldn't distinguish the known etext/end bug from a different static-link regression producing the same PASS-crash/FAIL-gc_alloc/FAIL-hello shape (e.g. a PR replacing libgc.a with a valid-but-incomplete archive). Now compiles hello.c directly to capture real tcc stderr and requires it specifically mention an unresolved etext/end symbol before accepting the XFAIL. Mirrors the identical fix just applied to macos-amd64's libgc.a XFAIL lane for the same class of finding (pullrequestreview-4780178390 on vlang#74).
…apture needs || true, tcc's failure is expected)
Author
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c2748f0dc1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
….a XFAIL, not just one Fixes P1 finding from pullrequestreview-4780815399 on vlang#75: checking only hello.c's direct compiler error let a regression isolated to gc_alloc.c alone hide behind the same "PASS crash, FAIL gc_alloc, FAIL hello" summary shape and pass unnoticed. Now captures and asserts the known etext/end signature for both failing tests independently.
quaesitor-scientiam
pushed a commit
to quaesitor-scientiam/tccbin
that referenced
this pull request
Jul 26, 2026
…cure fallback, exact SIGSEGV check, refresh provenance) Fixes all 5 findings from pullrequestreview-4780817355 on vlang#74: - P1: harden "verify checked-in tcc.exe" into a real signature-matched XFAIL (previously reported without validating anything at all) - asserts the known "0 passed, 3 failed" summary, confirmed via a real CI run - P1: remove the git:// fallback entirely instead of using it after HTTPS retries - an active attacker could just block HTTPS to force the fallback, so a warning-then-downgrade doesn't actually mitigate the supply-chain risk the earlier fix (pullrequestreview-4780178390) was meant to address; fail the job for real instead - P2: require exit code exactly 139 (SIGSEGV) in the no-GC fallback check, not just nonzero, so a binary that can't even start (missing loader/library) can't be mistaken for "the expected crash" - P2: validate BOTH gc_alloc.c's and hello.c's direct compile errors in the libgc.a XFAIL, not just gc_alloc.c's, so a regression isolated to hello.c alone can't hide behind the shared summary shape (same class of finding as pullrequestreview-4780815399 on the sibling freebsd-amd64 workflow, vlang#75) - P2: overwrite the stale lib/libgc_build_cmd.txt (which described the OLD universal x86_64/arm64 build) with the actual amd64-only configure invocation this step uses, so uploaded artifacts carry accurate provenance
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 340d9f4f2a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…nature in the XFAIL check Fixes P1 finding from pullrequestreview-4780907186 on vlang#75: the etext/end substring check accepted the XFAIL even if a regression added a NEW unresolved symbol alongside the known ones. This bug is narrow (only etext/end, unlike macos-amd64's whole-archive-unparseable case) so an exact match is correct: reject if any undefined-symbol line names something other than etext/end.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5cf3efee10
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…-GC check) Fixes both findings from pullrequestreview-4781468264 on vlang#75: - P2: the libgc.a XFAIL's exclusivity check now matches on tcc's `tcc: error:` line prefix instead of only "undefined symbol" lines, so a wholly different KIND of tcc error riding alongside the known etext/end error can no longer hide behind the check - verified empirically: the old narrower check accepted a mocked known-symbol- plus-unrelated-error case, the new one correctly rejects it - P2: added a positive no-GC check (compile+run a trivial non-crashing program, require exit 0) alongside the existing crash.c-must-SIGSEGV check, so a broken startup path that crashes EVERY program can't hide behind "the crash happened as expected" - verified empirically against a mocked always-crashing tcc: old logic would have passed, new logic correctly fails Same class of gap also fixed on the sibling macos-amd64 workflow (pullrequestreview-4781470066 on vlang#74).
quaesitor-scientiam
pushed a commit
to quaesitor-scientiam/tccbin
that referenced
this pull request
Jul 26, 2026
… real outcomes, positive no-GC check, broader exclusivity check, deployment target in provenance) Fixes 2 findings from pullrequestreview-4781470066 on vlang#74: - P2: package/upload steps stay `!cancelled()` (still publish a debug artifact on failure, matching macos-arm64's own rationale), but the bundled KNOWN_ISSUES.txt's claim that the dylib link path is "verified working" is now conditional on the dylib_test/nogc_test steps' actual outcomes, read via new step `id`s - verified against both success and failure outcome combinations - P2: lib/libgc_build_cmd.txt now records MACOSX_DEPLOYMENT_TARGET alongside the configure invocation, so reproducing this build from the recorded command actually matches its compatibility promise Fixes 2 findings from pullrequestreview-4781468264 on the sibling freebsd-amd64 workflow, vlang#75, same class of gap present here: - P2: the libgc.a XFAIL's exclusivity check now matches on tcc's `tcc: error:` line prefix instead of only "unresolved reference to" lines, so a wholly different KIND of tcc error (not just a differently-named symbol) riding alongside the known GC_init error can no longer hide behind the check - verified empirically: the old narrower check accepted a mocked known-symbol-plus-unrelated-error case, the new one correctly rejects it - P2: added a positive no-GC check (compile+run a trivial non-crashing program, require exit 0) alongside the existing crash.c-must- SIGSEGV check, so a broken startup path that crashes EVERY program can't hide behind "the crash happened as expected" - verified empirically against a mocked always-crashing tcc: old logic would have passed, new logic correctly fails
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea3b81b4aa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
quaesitor-scientiam
pushed a commit
to quaesitor-scientiam/tccbin
that referenced
this pull request
Jul 26, 2026
…IL probe exit codes Fixes a real CI failure (run 30200458603): the HTTPS-reachability probe (git ls-remote) succeeded on retry, but the official build script's own git clone then failed anyway - "Failed to connect to repo.or.cz port 443... Couldn't connect to server" - and with the insecure git:// fallback already removed for security reasons, there was nothing left to recover with. A lightweight probe succeeding doesn't guarantee the much heavier clone moments later will also succeed. Now retries the ACTUAL build script invocation (not a decoupled proxy check) up to 3 times, but only when the failure specifically matches a network-connection signature (Failed to connect, Couldn't connect to server, Could not resolve host, Connection reset by peer, Recv failure, Connection timed out) - a genuine build bug (a real gmake or configure error) still fails fast instead of wasting 3 attempts on something retrying can't fix. Safe to retry: the script itself does `rm -rf tinycc/` unconditionally as its first action, and the network operation happens before any step that mutates $TCC_FOLDER, so no partial state survives across attempts. Verified against 3 mocked scenarios: network-failure-then-success, unrelated-bug-fails-fast, and network-failure-exhausts-all-retries. Also fixes 2 findings from pullrequestreview-4781865117 on the sibling freebsd-amd64 workflow, vlang#75 (same class of gap, applied proactively here): the libgc.a XFAIL's direct-error probes and the checked-in-binary probe used `|| true`, discarding the probe's own exit code entirely - a regression that made tcc print the expected error text and then crash/abort (SIGSEGV, SIGABRT) would still pass. Now captures the real exit code alongside the text and rejects any signal-terminated exit (128+signal) even when the expected text is present. Verified empirically: a mocked "expected text + SIGSEGV exit" case passed under the old logic, and is correctly rejected now.
Fixes P2 finding from pullrequestreview-4781865117 on vlang#75: the libgc.a XFAIL's direct-error probes used `|| true`, discarding the probe's own exit code entirely - a regression that made tcc print the expected etext/end diagnostic and then crash/abort (SIGSEGV, SIGABRT) rather than exiting cleanly would still pass, since only the captured text was checked. Now captures the real exit code alongside the text and rejects any signal-terminated exit (128+signal) even when the expected text is present. Verified empirically: a mocked "expected text + SIGSEGV exit" case passed under the old logic and is correctly rejected now.
…isolation Fixes P2 finding from pullrequestreview-4781865117 on vlang#75: cloning pull_request.head.sha tested the PR's own branch tree in isolation, never the merge result GitHub would actually produce against the current target branch. A PR replacing tcc.exe could pass against its own tree while the actual merge (combined with whatever the target branch has since gained, e.g. a different libtcc1.a from a merged sibling PR) was never tested at all - the required check stays green for a combination that was never verified. Now uses github.sha + github.event.repository.clone_url uniformly for all 3 trigger types: for pull_request events, github.sha is GitHub's own synthesized merge commit, which exists only in the base repo (never a contributor's fork) - so the clone URL switches from the fork to the base repo accordingly. github.sha stays fixed for a run's entire lifetime once the triggering event fires, so this remains exactly as race-safe against a mid-queue push as the previous head.sha-based approach (the original P1 this replaced, from pullrequestreview-4780048339), while also testing what will actually be merged instead of the head tree alone. macos-amd64 doesn't need this fix: it already uses plain actions/checkout@v4 with no explicit ref, which already defaults to checking out the pull_request merge commit automatically.
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
GGRei
pushed a commit
that referenced
this pull request
Jul 27, 2026
* ci: adopt shared tccbin conformance test suite
Wires this branch into the shared cross-platform conformance suite
(thirdparty/tccbin_tests in vlang/v) already running on
thirdparty-windows-amd64/linux-amd64/macos-arm64's CI - same
shared/hello.c, gc_alloc.c, crash.c tests. No rebuild here - this
validates the already-committed tcc.exe/lib/libgc.a as-is.
GC linked via the static libgc.a this branch ships (no .dylib on
amd64, unlike arm64 - see vlib/builtin/builtin_d_gcboehm.c.v's
macos+tinyc+amd64 branch in vlang/v, which uses the same static path).
Unverified locally (no Intel Mac access here) - relying on GitHub's
hosted macos-13 runner for real verification.
* ci: use macos-15-intel runner (macos-13 was retired December 2025)
macos-13 no longer exists as a hosted runner image, so this job would
sit unassigned/queued indefinitely rather than actually running -
matches update_tccbin.yml's own already-established macos-amd64
config (os: macos-15-intel), which should have been used from the
start instead of guessing a new label.
Found by Codex review (pullrequestreview-4779652865).
* ci: add temporary diagnostic to surface the real compile error
* ci: point tcc's linker at the Xcode SDK's usr/lib for .tbd stub resolution
* ci: test with macos-arm64's exact working flags (no -ldl/-lpthread/-DMPROTECT_VDB) to isolate the trigger
* ci: add deeper diagnostic (version, libc.dylib provenance, -v -v search trace with -B)
* ci: rebuild tcc.exe from current tinycc (mob) instead of testing the stale bundled binary
This branch's committed tcc.exe is version 0.9.27 with no
build_source_hash.txt/build_version.txt provenance at all (unlike
every other actively-maintained platform branch), and its bundled
lib/libc.dylib is a symlink to
/System/DriverKit/usr/lib/libSystem.dylib - an intentional workaround
for macOS Big Sur (per the official build script's own "needed for
Big Sur" comment), but that path no longer exists on current macOS
(confirmed: otool reports "No such file or directory" on
macos-15-intel). No CI invocation flag can fix a broken symlink baked
into the committed binary.
Rebuilds from current tinycc (mob) using vlang/v's own official
thirdparty-macos-amd64_tcc.sh (the same script update_tccbin.yml
already uses for this exact platform), reusing this branch's
already-committed libgc.a (the script's own rsync step preserves it -
no GC rebuild needed). Also fixes the macos-13 -> macos-15-intel
runner label (Codex review pullrequestreview-4779652865) and adds
artifact upload of the rebuilt binaries.
* ci: seed a lib/build* placeholder so the script's provenance carry-forward rsync doesn't fail on this branch's first rebuild
* ci: add diagnostic to show libc.dylib status and the real compile error after rebuild
* ci: also rebuild libgc.a and fix the libc.dylib symlink target
The freshly-rebuilt tcc.exe can't parse the branch's old bundled
libgc.a at all ("unrecognized file type") - a real archive/object
format mismatch between the ancient libgc.a and the new tcc, the same
class of old-lib-vs-new-compiler incompatibility as windows-amd64's
original tinyc_getbp issue, just without an existing community fix to
lean on here. Rebuilds libgc.a from current bdwgc master, adapting
thirdparty-macos-arm64_bdwgc.sh (there's no official amd64 variant in
vlang/v - the script is otherwise architecture-generic).
Also repoints the official build script's stale "## needed for Big
Sur" symlink (-> /System/DriverKit/usr/lib/libSystem.dylib, confirmed
nonexistent on current macOS) at /usr/lib/libSystem.B.dylib - the one
system library Apple has kept as a real on-disk compatibility shim.
* ci: use git:// for repo.or.cz clone (https unreliable from CI, same issue and fix as freebsd-amd64)
* ci: install automake/libtool for bdwgc's autogen.sh
* ci: diagnose real gc_alloc/hello compile error (pinned run.sh predates stderr fix)
* ci: diagnose why freshly-rebuilt libgc.a is missing GC_init at link time
* ci: link libgc.dylib+rpath instead of static libgc.a (tcc archive-parsing limitation, matches macos-arm64)
* ci: fix Codex-flagged findings on macos-amd64 (secure transport, harden XFAIL, test checked-in binaries)
Fixes 3 of 4 findings from pullrequestreview-4780178390 on #74:
- P1: prefer HTTPS (with retries for known transient flakiness) over the
unauthenticated git:// transport when fetching tinycc source; only
fall back to git:// - loudly - if HTTPS is genuinely unreachable
- P2: harden the libgc.a XFAIL lane to also assert the actual captured
tcc stderr mentions the specific unresolved GC_init reference, not
just the run.sh summary-line shape (which the pinned pre-#27935
run.sh can't distinguish from an unrelated static-link regression)
- P1: add a lane testing the checked-in tcc.exe/libgc.a as distributed,
before the rebuild steps replace them - reports the result without
blocking for now (this PR doesn't itself update the committed
binaries, so the exact known-broken signature isn't hardened yet);
will harden into a signature-matched XFAIL once real CI output is in
The 4th finding (macOS amd64 V builds still select the static libgc.a,
which this workflow's own XFAIL lane proves can't link) needs a
coordinated follow-up: ship a working libgc.dylib to this branch, then
update vlib/builtin/builtin_d_gcboehm.c.v to select it - in that order,
since flipping the V builder first would break every current macOS
amd64 tinyc+boehm build against the still-static-only binaries this
branch distributes today. Tracked as a follow-up, not done in this
CI-only PR.
* ci: fix set -e aborting before the XFAIL case statement (direct_err capture needs || true, tcc's failure is expected)
* ci: capture direct compile error for checked-in tcc.exe, to pin the exact XFAIL signature
* ci: fix 5 Codex findings on macos-amd64 (harden 2 XFAILs, remove insecure fallback, exact SIGSEGV check, refresh provenance)
Fixes all 5 findings from pullrequestreview-4780817355 on #74:
- P1: harden "verify checked-in tcc.exe" into a real signature-matched
XFAIL (previously reported without validating anything at all) -
asserts the known "0 passed, 3 failed" summary, confirmed via a real
CI run
- P1: remove the git:// fallback entirely instead of using it after
HTTPS retries - an active attacker could just block HTTPS to force
the fallback, so a warning-then-downgrade doesn't actually mitigate
the supply-chain risk the earlier fix (pullrequestreview-4780178390)
was meant to address; fail the job for real instead
- P2: require exit code exactly 139 (SIGSEGV) in the no-GC fallback
check, not just nonzero, so a binary that can't even start (missing
loader/library) can't be mistaken for "the expected crash"
- P2: validate BOTH gc_alloc.c's and hello.c's direct compile errors in
the libgc.a XFAIL, not just gc_alloc.c's, so a regression isolated to
hello.c alone can't hide behind the shared summary shape (same class
of finding as pullrequestreview-4780815399 on the sibling
freebsd-amd64 workflow, #75)
- P2: overwrite the stale lib/libgc_build_cmd.txt (which described the
OLD universal x86_64/arm64 build) with the actual amd64-only configure
invocation this step uses, so uploaded artifacts carry accurate
provenance
* ci: fix 5 more Codex findings on macos-amd64 (harden checked-in XFAIL, warn on packaged libgc.a, pin libatomic_ops provenance, deployment target, exclusive symbol match)
Fixes all 5 findings from pullrequestreview-4780918023 on #74:
- P1: harden the checked-in-tcc.exe XFAIL beyond the aggregate "0
passed, 3 failed" count - now compiles crash.c directly and requires
the known "library 'c' not found" signature (the broken libc.dylib
symlink already root-caused earlier this session)
- P1: warn against publishing the unlinkable static libgc.a as a
ready distribution - adds a KNOWN_ISSUES.txt into the packaged
artifact explaining the dylib is the verified-working link path
- P2: record libatomic_ops's resolved commit alongside bdwgc's in
provenance (it was cloned from a moving branch but untracked)
- P1: pin MACOSX_DEPLOYMENT_TARGET=10.13 for both the tcc and libgc
rebuilds, and surface the checked-in binary's actual load-command
minimum via otool for future refinement - without it, clang stamps
rebuilt binaries with the CI runner's own macOS 15 minimum
- P2: the libgc.a XFAIL's GC_init check now also rejects any
unresolved-reference symbol that isn't GC_-prefixed, not just
requiring GC_init's presence - closing the same class of gap as
pullrequestreview-4780907186 on the sibling freebsd-amd64 workflow
* ci: fix 4 more Codex findings on macos-amd64 (gate artifact claims on real outcomes, positive no-GC check, broader exclusivity check, deployment target in provenance)
Fixes 2 findings from pullrequestreview-4781470066 on #74:
- P2: package/upload steps stay `!cancelled()` (still publish a debug
artifact on failure, matching macos-arm64's own rationale), but the
bundled KNOWN_ISSUES.txt's claim that the dylib link path is
"verified working" is now conditional on the dylib_test/nogc_test
steps' actual outcomes, read via new step `id`s - verified against
both success and failure outcome combinations
- P2: lib/libgc_build_cmd.txt now records MACOSX_DEPLOYMENT_TARGET
alongside the configure invocation, so reproducing this build from
the recorded command actually matches its compatibility promise
Fixes 2 findings from pullrequestreview-4781468264 on the sibling
freebsd-amd64 workflow, #75, same class of gap present here:
- P2: the libgc.a XFAIL's exclusivity check now matches on tcc's
`tcc: error:` line prefix instead of only "unresolved reference to"
lines, so a wholly different KIND of tcc error (not just a
differently-named symbol) riding alongside the known GC_init error
can no longer hide behind the check - verified empirically: the old
narrower check accepted a mocked known-symbol-plus-unrelated-error
case, the new one correctly rejects it
- P2: added a positive no-GC check (compile+run a trivial non-crashing
program, require exit 0) alongside the existing crash.c-must-
SIGSEGV check, so a broken startup path that crashes EVERY program
can't hide behind "the crash happened as expected" - verified
empirically against a mocked always-crashing tcc: old logic would
have passed, new logic correctly fails
* ci: retry the whole tcc build script on network failure; preserve XFAIL probe exit codes
Fixes a real CI failure (run 30200458603): the HTTPS-reachability probe
(git ls-remote) succeeded on retry, but the official build script's own
git clone then failed anyway - "Failed to connect to repo.or.cz port
443... Couldn't connect to server" - and with the insecure git://
fallback already removed for security reasons, there was nothing left
to recover with. A lightweight probe succeeding doesn't guarantee the
much heavier clone moments later will also succeed.
Now retries the ACTUAL build script invocation (not a decoupled proxy
check) up to 3 times, but only when the failure specifically matches a
network-connection signature (Failed to connect, Couldn't connect to
server, Could not resolve host, Connection reset by peer, Recv
failure, Connection timed out) - a genuine build bug (a real gmake or
configure error) still fails fast instead of wasting 3 attempts on
something retrying can't fix. Safe to retry: the script itself does
`rm -rf tinycc/` unconditionally as its first action, and the network
operation happens before any step that mutates $TCC_FOLDER, so no
partial state survives across attempts. Verified against 3 mocked
scenarios: network-failure-then-success, unrelated-bug-fails-fast, and
network-failure-exhausts-all-retries.
Also fixes 2 findings from pullrequestreview-4781865117 on the
sibling freebsd-amd64 workflow, #75 (same class of gap,
applied proactively here): the libgc.a XFAIL's direct-error probes and
the checked-in-binary probe used `|| true`, discarding the probe's own
exit code entirely - a regression that made tcc print the expected
error text and then crash/abort (SIGSEGV, SIGABRT) would still pass.
Now captures the real exit code alongside the text and rejects any
signal-terminated exit (128+signal) even when the expected text is
present. Verified empirically: a mocked "expected text + SIGSEGV exit"
case passed under the old logic, and is correctly rejected now.
* ci: gate the static-link claim on its test outcome; reject additional errors in the checked-in XFAIL
Fixes both findings from pullrequestreview-4782525602 on #74:
- P2: the packaged KNOWN_ISSUES.txt unconditionally claimed "lib/libgc.a
CANNOT be linked... confirmed via a real CI run" even when the static
XFAIL step failed with an unexpected signature or unexpectedly passed
in THIS run - the claim didn't reflect what this specific run actually
verified, same class of gap already fixed for the dylib/no-GC claims.
Added `id: static_xfail_test` and now derives the static-link text
from its real outcome the same way. Verified both branches render
correctly (all-success, and static-XFAIL-failed).
- P2: the checked-in-binary XFAIL accepted the known "library 'c' not
found" diagnostic even if a wholly different, unrelated tcc error was
ALSO present alongside it - a bare substring-present check never
looked at what else came back. Now rejects unless every tcc error
line is specifically the known libc diagnostic, matching the same
exclusivity pattern already applied to the libgc.a XFAIL probes.
Verified empirically: a mocked "known error + unrelated error" case
is now correctly rejected.
* ci: fix set -e + pipefail silently killing the checked-in-binary XFAIL on the expected-good path
Fixes P1 finding from pullrequestreview-4782839244 on #74,
a real CI failure this exact bug caused (run 30225228663): the
previous commit's `other_checkedin=$(... | grep -v ...)` was a
standalone assignment, not a function invoked from an if-condition. In
the EXPECTED case (no other error present), the final `grep -v`
legitimately finds nothing to filter and exits 1 (grep's normal
"no match" status) - and since GitHub's bash runs with pipefail, that
nonzero pipeline status killed the assignment statement itself under
`set -e`, silently aborting the whole step with zero output, in
exactly the case that should have reached "known XFAIL". Reproduced
directly: `bash -c "set -e; x=\$(printf ... | grep -v ...); echo
after"` never prints "after".
Refactored into is_known_checkedin_error(), a function invoked as an
if-condition (`if is_known_checkedin_error ...; then`) - bash suspends
errexit for the entire dynamic extent of evaluating an if/while/until
condition, including anything a called function does internally, which
is exactly why the sibling is_known_gc_init/is_known_etext_end
functions never had this problem despite the identical internal
grep -v pattern. Verified: reproduced the silent-death bug in isolation
first, confirmed the function-based refactor survives both the
expected-good case and the known-error-plus-unrelated-error case, and
confirmed the already-working is_known_gc_init's `&&`-chained
if-condition usage is genuinely errexit-safe too.
* ci: reject exit-0 in XFAIL signature checks; independently validate both checked-in and rebuilt libgc.a via ar/nm
Fixes all 3 findings from pullrequestreview-4783389496 on #74:
- P2: is_known_checkedin_error and is_known_gc_init only rejected
signal-terminated exits (>128), never exit 0 - a changed tcc.exe
reporting the expected diagnostic text as noise while still
succeeding overall would still be accepted as the known XFAIL.
Verified empirically: exit=0 with the known text is now rejected,
exit=1 with the known text is still accepted.
- P1: crash.c (the checked-in-binary signature probe) never
references any GC symbol, so it never actually exercises
thirdparty/tcc/lib/libgc.a - a PR replacing the checked-in libgc.a
with a corrupt/empty/wrong-architecture archive would produce the
exact same "0 passed, 3 failed" + "library 'c' not found" signature
and pass CI undetected. Added a direct ar t / nm -g check on the
checked-in archive (confirming it's well-formed and defines
GC_init) before the rebuild step overwrites it.
- P2: the static libgc.a XFAIL accepted "unresolved reference to
'_GC_init'" as proof of tcc's known archive-parsing limitation
without ever confirming the REBUILT archive actually defines that
symbol - an empty/incomplete bdwgc build would produce the identical
error text and get misclassified as the harmless known limitation.
Added the same ar t / nm -g check on the rebuilt archive before
accepting the XFAIL, so KNOWN_ISSUES.txt's claim that the symbol is
"present and well-formed" is now actually backed by verification.
Verified the nm -g grep pattern against both the real observed output
format from earlier this session (T = defined) and a mocked broken
archive (U = undefined only), confirming it distinguishes them
correctly.
Same exit-0 rejection also applied to the sibling freebsd-amd64
workflow's is_known_etext_end for parity.
* ci: fix SIGPIPE+pipefail false failure in archive checks; verify checked-in archive architecture; assert tcc.exe's committed file mode
Fixes all 3 findings from pullrequestreview-4783470598 on #74,
the first of which broke the CI run immediately after the previous
commit (run 30234854073):
- P1: `nm -g archive | grep -qE ...` is unsafe under GitHub's pipefail
bash - grep -q stops reading as soon as it finds a match, which can
make nm receive SIGPIPE while still writing the rest of a large
archive's symbol list, and pipefail then reports the WHOLE pipeline
as failed even though grep found the match. Reproduced directly: a
large mocked producer piped through grep -qE under set -eo pipefail
reported "not found" despite matching on the very first line. Fixed
by capturing nm's full output into a variable first (command
substitution fully drains the producer, no early exit possible) and
grepping the captured text without -q. Applied to both the
checked-in and rebuilt archive checks, as flagged.
- P1: a well-formed, correctly-symboled archive for the WRONG
architecture (e.g. an accidentally-committed arm64 build) would
have passed both the ar t and nm checks, since neither validated
architecture, and the crash.c signature probe never touches libgc.a
at all. Added a `lipo -info` check requiring x86_64 before trusting
anything else about the checked-in archive.
- P2: the unconditional `chmod +x thirdparty/tcc/tcc.exe` silently
repaired a potentially-wrong committed file mode before the "as
distributed" verification ran against it - a PR that accidentally
committed tcc.exe as non-executable would still pass CI, even though
a real consumer checking out that exact commit gets the actual
(broken) mode. Replaced with an assertion that the committed mode is
already correct.
Also fixed a second occurrence of the same "standalone grep assignment
under set -e" bug this exact PR already hit once (pullrequestreview-
4782839244): the new `grep -E` (without -q) capture still needs its
own `|| true`, since a genuinely missing symbol makes grep exit 1 with
nothing to catch it. Verified the complete fix against 3 scenarios:
symbol present (small output), symbol genuinely missing, and symbol
present with a large trailing output (the SIGPIPE-prone case) - all
three now report correctly and the script completes without dying.
* ci: diagnose why ar t is failing on the checked-in libgc.a (previously swallowed the error text)
* ci: extract the x86_64 slice from the checked-in FAT libgc.a before validating it with ar/nm
Root-caused via the diagnostic pushed in commit 442b25b: the
checked-in libgc.a is a FAT/universal binary containing BOTH x86_64
and arm64 slices ("Mach-O universal binary with 2 architectures"),
not a plain single-arch archive. macOS's `ar` explicitly refuses to
read a fat archive directly ("ar: ... is a fat file (use libtool(1)
or lipo(1) and ar(1) on it)"), which is why every previous version of
this check failed on every run regardless of the archive's actual
(valid) contents - it wasn't a code bug or a real regression, just an
untested assumption about this specific file's format.
Replaced the separate `ar t` + `lipo -info` substring-match approach
(which could also have been fooled by an arm64-only archive that
still contains the literal text "x86_64" somewhere in a multi-arch
`lipo -info` listing) with `lipo -thin x86_64 ... -output ...`: this
extracts the x86_64 slice explicitly - failing if it doesn't exist,
closing the architecture-verification gap - and produces a normal
thin archive that `ar t`/`nm -g` can then read directly, closing the
well-formed-archive and GC-symbol-defined checks the same way.
* ci: refine the checked-in XFAIL's exit-0 rejection to only reject when a binary was actually produced
Root-caused via a real CI run: this ancient checked-in v0.9.27
tcc.exe exits 0 while still printing "tcc: error: library 'c' not
found" and producing NO runnable binary - a genuine quirk/bug of this
specific old build, not a new regression. The previous commit's blanket
"reject exit 0" was too broad and made this check permanently fail on
this branch's own known-good state.
The original finding's actual concern (Codex pullrequestreview-
4783389496) was specifically "a changed tcc.exe... while still
reporting overall success... without producing the requested
executable" - i.e. exit 0 AND an actual working binary is the real
problem, not exit 0 alone. Now checks whether the probe's output file
is executable and only rejects the exit-0 case when combined with
that:
if [ "$2" -eq 0 ] && [ "$3" -eq 1 ]; then
return 1 # genuinely succeeded despite the noise - real problem
fi
Verified against all 3 scenarios: exit=0 with no binary produced (the
observed old-tcc behavior) is now accepted, exit=0 with a binary
produced is still rejected, and the normal exit=1 case is unaffected.
Left is_known_gc_init (the rebuilt/modern tcc's equivalent check)
unchanged for now - no evidence yet that the actively-maintained mob
branch has the same exit-code quirk as this 15+-year-old checked-in
binary, and speculative changes without a reproduced failure aren't
worth the added risk on an already-long fix chain.
* ci: drop exit-0 rejection for the checked-in XFAIL - this ancient tcc's exit-code/output-file behavior is unreliable, not a meaningful signal
Root-caused via a second real CI run: this ancient, checked-in
v0.9.27 tcc.exe not only exits 0 while printing "tcc: error: library
'c' not found" (already observed once), it can ALSO leave behind an
executable-permission output file in that same state - so neither
"exit 0" nor "exit 0 AND a file exists at -o" reliably distinguishes
"genuinely still broken as expected" from "secretly still working"
for this specific 15+-year-old build's link-error handling.
The exit-0 concern (Codex pullrequestreview-4783389496) is legitimate
in principle - a CHANGED tcc.exe reporting false success is a real
risk - but the outer `case "$output" in *"0 passed, 3 failed"*)`
already independently confirms via run.sh's own compile+run cycle
that all 3 shared tests genuinely failed; a real "tcc got un-broken"
scenario would show up there as passing tests, not hidden in this one
probe's exit code. This function's actual job is narrower: given the
aggregate already failed, confirm the SPECIFIC cause via the error
text, which two consecutive real CI runs now show is a reliable
signal even when this old binary's exit code and file-production
behavior for the same error are not.
is_known_gc_init (the rebuilt/modern tcc's equivalent check) keeps its
exit-0 rejection - the actively-maintained mob branch has shown no
evidence of this quirk, unlike this fixed historical artifact.
---------
Co-authored-by: Richard Wheeler <18647491+PythonWillRule@users.noreply.github.com>
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
3 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.
Wires this branch into the shared cross-platform conformance suite
(
thirdparty/tccbin_testsin vlang/v) via a real FreeBSD VM(
cross-platform-actions/action, the same tooling vlang/v's ownupdate_tccbin.ymluses to rebuild this branch's binaries).No rebuild here — validates the already-committed
tcc.exe/lib/libgc.aas-is. I don't have FreeBSD access to verify this locally, so relying
on real CI here.
One thing to flag: V's own builder also references
-lgc-threadedforthis platform+tcc combination (
vlib/builtin/builtin_d_gcboehm.c.v),which isn't part of this branch's bundle (only
libgc.a, notlibgc-threaded.a). Starting without it since the bundledlibgc.ashould be self-sufficient for the shared suite's needs — will adjust
based on what real CI actually shows if it isn't.