Fix static seid SIGSEGV (pin pre-gcc-12 libgcc unwinder), add boot smoke gate, build binaries on manual tag pushes#3749
Fix static seid SIGSEGV (pin pre-gcc-12 libgcc unwinder), add boot smoke gate, build binaries on manual tag pushes#3749monty-sei wants to merge 3 commits into
Conversation
…oot smoke Pin the pre-gcc-12 libgcc (Alpine 3.15 / gcc 10.3.1, vendored with checksums in third_party/alpine-gcc10-libgcc) at the static link: gcc >= 12 replaced libgcc's unwind-frame registry with a lock-free b-tree that corrupts under wasmer's JIT frame registration (__register_frame), crashing static seid on ~70% of boots at the genesis wasm store. Assert the built binary carries no b-tree symbols so a toolchain upgrade cannot silently reintroduce it. Add scripts/boot-smoke.sh as a repeated-boot gate in the static CI job and the goreleaser before hook: a probabilistic first-wasm-use crash can pass any single check, which is how the broken binary shipped to v6.6.0-rc2. Build and attach release binaries when a maintainer pushes a version tag by hand: the tag ruleset blocks this workflow's token from creating tags, so the chained goreleaser job never fired for rc1/rc2/rc3. Key the concurrency group per event+ref and stop cancelling in-flight builds (the version.json push and the follow-up tag push share a SHA).
PR SummaryHigh Risk Overview Adds Release workflow ( Reviewed by Cursor Bugbot for commit 175db9d. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
A carefully-documented and thoroughly-tested build/release fix: it pins a pre-gcc-12 libgcc to stop the wasmer JIT unwind-b-tree SIGSEGV in static seid, adds a repeated-boot smoke gate, and fixes the goreleaser trigger for manual tag pushes. No blocking issues found; only minor, non-blocking notes.
Findings: 0 blocking | 6 non-blocking | 1 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- Verified independently: the vendored libgcc.a/libgcc_eh.a checksums match the README pins exactly, the
"Completed ABCI Handshake"marker used by boot-smoke.sh exists at sei-tendermint/internal/consensus/replay.go:182, and the build ldflags contain no-s/-w, so thenm ... version_lock_lock_exclusiveassertion in build-static.sh is not vacuous. The fix and its guards are sound. - Second-opinion passes: OpenAI Codex reported no material issues; the Cursor review file (cursor-review.md) is empty (that pass produced no output).
- The goreleaser job's manual-tag-push path depends on the external reusable workflow
release-publish.yml@v0.0.11succeeding (no-op) on a tag push. Because thegoreleaserjob usesneeds: publishwith anifthat contains no status function, it is still skipped ifpublishfails — so if that reusable workflow ever errors (rather than cleanly no-ops) on a pre-existing tag, binaries silently won't build. Worth confirming that no-op-on-existing-tag behavior against v0.0.11 (the author's dress-rehearsal comment presumably covers this). - boot-smoke.sh leaks the mktemp home dir on the failure paths (
rm -rf "$H"only runs after a successful boot). Harmless on ephemeral CI, and arguably useful for post-mortem, but atrap 'rm -rf "$H"' EXIT-style cleanup would be tidier. - Documented limitation acknowledged by the author: re-pushing an existing v* tag re-runs goreleaser and re-appends its notes block to the release body (treat as run-once per tag).
- 1 suggestion(s)/nit(s) flagged inline on specific lines.
| "$BIN" collect-gentxs --home "$H" >/dev/null 2>&1 | ||
|
|
||
| LOG="$H/start.log" | ||
| timeout -k 5 25 "$BIN" start --home "$H" >"$LOG" 2>&1 || true |
There was a problem hiding this comment.
[nit] The fixed 25s timeout to reach the ABCI handshake is a potential CI-flakiness source: a slow runner (cold cache, heavy genesis wasm compile) that hasn't logged "Completed ABCI Handshake" within 25s would fail the gate as a false negative. Consider making the timeout an env-overridable value with some headroom.
The floating golang:1.25.6-alpine tag moves with Alpine point releases, and a toolchain change under the tag is exactly the drift class that introduced the unwind b-tree crash. The nm assertion would catch a libgcc regression loudly; the digest pin prevents silent changes to everything else.
There was a problem hiding this comment.
A focused, well-documented build/release-infra PR that pins a pre-gcc-12 libgcc to fix the static seid wasmer SIGSEGV, adds a repeated-boot smoke gate, and lets the goreleaser job run on manual tag pushes. Verified the handshake marker, the un-stripped-binary assertion, and the vendored archive checksums; no blockers, only minor robustness nits.
Findings: 0 blocking | 6 non-blocking | 3 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- Codex (
codex-review.md) and Cursor (cursor-review.md) both produced effectively empty passes ("No material issues found" / blank), so no independent second-opinion findings were available to merge. - The vendored
libgcc.a/libgcc_eh.a(~3.4 MB of binary archives) are now committed to the repo. This is well-justified and guarded (README provenance, sha256 verification in build-static.sh, symbol assertion), but it is a maintenance/audit burden worth noting; consider re-deriving from the apk at build time in future if feasible. build-static.sh'snm build/seid | grep -q version_lock_lock_exclusiveassertion is only meaningful while the binary retains its symbol table (the Makefile currently doesn't pass-s -w); if a future ldflags change strips symbols the check would silently become a no-op that always passes. A short comment noting this dependency would help.- 3 suggestion(s)/nit(s) flagged inline on specific lines.
| "$BIN" collect-gentxs --home "$H" >/dev/null 2>&1 | ||
|
|
||
| LOG="$H/start.log" | ||
| timeout -k 5 25 "$BIN" start --home "$H" >"$LOG" 2>&1 || true |
There was a problem hiding this comment.
[suggestion] The hard-coded 25s timeout may be tight on a loaded CI runner: genesis wasm StoreCode JIT-compiles the pointer contracts before the ABCI handshake, and this runs 8 sequential boots in CI plus 4 more in the goreleaser hook. If a slow boot is killed before reaching the handshake, it fails as a false negative rather than a real crash. Consider making the timeout a parameter/env var, or bumping the margin.
| for i in $(seq 1 "$BOOTS"); do | ||
| H=$(mktemp -d) | ||
| "$BIN" init smoke --chain-id "$CHAIN_ID" --home "$H" >/dev/null 2>&1 | ||
| sed -i 's/"stake"/"usei"/g' "$H/config/genesis.json" |
There was a problem hiding this comment.
[nit] sed -i 's/"stake"/"usei"/g' rewrites every occurrence of the string "stake" in genesis.json, not just the staking bond denom. It's the conventional localnet trick and works today, but it's brittle if any future genesis field legitimately contains the substring "stake". Not blocking.
| sed -i 's/"stake"/"usei"/g' "$H/config/genesis.json" | ||
| "$BIN" keys add val --keyring-backend test --home "$H" >/dev/null 2>&1 | ||
| ADDR=$("$BIN" keys show val -a --keyring-backend test --home "$H") | ||
| "$BIN" add-genesis-account "$ADDR" 100000000000000usei --home "$H" >/dev/null |
There was a problem hiding this comment.
[nit] Minor inconsistency: this add-genesis-account invocation redirects only stdout (>/dev/null) while the surrounding seid calls use >/dev/null 2>&1. Harmless, but on failure its stderr will leak into CI output unlike the others.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3749 +/- ##
==========================================
- Coverage 59.91% 59.00% -0.92%
==========================================
Files 2288 2201 -87
Lines 189782 179951 -9831
==========================================
- Hits 113706 106173 -7533
+ Misses 65942 64446 -1496
+ Partials 10134 9332 -802
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
A main-targeted PR never exercised the static build (the job was gated to release work only), so changes to the static recipe shipped unverified until they reached a release branch. A cheap paths check now routes any PR touching the build scripts, Makefile, pinned libgcc, goreleaser config, or this workflow through the full static build and boot gauntlet.
There was a problem hiding this comment.
A well-documented release-infra fix: pins a pre-gcc-12 libgcc to eliminate the wasmer/JIT unwind b-tree SIGSEGV in static seid builds, adds a repeated-boot smoke gate, and unblocks binary builds on manual tag pushes. No blocking issues; a few non-blocking notes and one Codex finding that I could not reproduce.
Findings: 0 blocking | 7 non-blocking | 2 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- Codex P2 (regex in cross-arch-build.yml:71 allegedly nesting every alternative under
scripts/) appears to be a FALSE POSITIVE. The inner group(build-static|boot-smoke|check-libwasmvm-static)closes before the\.sh|Makefile|third_party/...|\.goreleaser\.yaml|\.github/...alternation, soMakefile,.goreleaser.yaml,third_party/alpine-gcc10-libgcc/, and the workflow all match at the top level. Verified by running the pattern against sample paths: all four match and unrelated paths (some/other/file.go,scripts/other.sh) do not. No change needed; keeping the note only to record the disagreement. - Cursor produced no review output (cursor-review.md is empty) and REVIEW_GUIDELINES.md is empty, so no repo-specific guidelines or second Cursor pass were available for this synthesis.
- Supply-chain / maintenance note: two prebuilt binary archives (libgcc.a, libgcc_eh.a) are vendored into the tree. This is mitigated well — full provenance + apk/archive sha256 in the README, checksum verification in build-static.sh before linking, and a post-build
nmassertion that the b-tree symbol is absent — but committing binary artifacts is worth tracking (e.g. periodic re-derivation) and flagging for reviewers who won't diff binaries. - boot-smoke.sh success/failure detection relies on grepping log strings (
Completed ABCI Handshake,SIGSEGV|SIGILL|SIGBUS|panic:). These are brittle to Tendermint/cosmos log-format changes; a future rename of the handshake log line would silently turn the gate into a false failure. Acceptable for a smoke gate, but consider a comment pinning the expected marker or a fallback. - uci-release-publish.yml: the goreleaser job re-firing on any pushed
v*tag will, per the author's own comment, re-append the goreleaser notes block to the release body if an existing tag is re-pushed. Documented and accepted as run-once-per-tag, but no guard enforces it. - 2 suggestion(s)/nit(s) flagged inline on specific lines.
| ADDR=$("$BIN" keys show val -a --keyring-backend test --home "$H") | ||
| "$BIN" add-genesis-account "$ADDR" 100000000000000usei --home "$H" >/dev/null | ||
| "$BIN" gentx val 10000000000000usei --chain-id "$CHAIN_ID" --keyring-backend test --home "$H" >/dev/null 2>&1 | ||
| "$BIN" collect-gentxs --home "$H" >/dev/null 2>&1 |
There was a problem hiding this comment.
[nit] Minor: timeout -k 5 25 gives 25s to reach the ABCI handshake. That's comfortable for genesis boot today, but on a heavily loaded CI runner a legitimately slow boot could time out before the handshake and be reported as a failure rather than a crash. If flakiness appears, bumping this (or making it an arg) would help distinguish "slow" from "crashed".
| echo "boot-smoke: boot $i/$BOOTS CRASHED:" >&2 | ||
| tail -25 "$LOG" >&2 | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
[nit] On a failed boot the script exits before rm -rf "$H", leaking the mktemp dir. Harmless in ephemeral CI, but a trap 'rm -rf "$H"' EXIT (or cleanup before exit) would keep local dress-rehearsal runs tidy.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| set -e | ||
| apk add --no-cache build-base git | ||
| git config --global --add safe.directory /src | ||
| printf "%s %s\n%s %s\n" \ | ||
| d3e066fafde74d53a89d48f2ceb9ed9934249a5d450e281edd22947a829469d8 '"$LIBGCC_DIR"'/libgcc.a \ | ||
| d14c9973a735909e11a863b0c850300bfd3aa683ef4689cbe76a53139766ed79 '"$LIBGCC_DIR"'/libgcc_eh.a \ | ||
| | sha256sum -c - | ||
| LINK_STATICALLY=true BUILD_TAGS=muslc LEDGER_ENABLED=false \ | ||
| STATIC_EXTRA_LDFLAGS="-L/src/'"$LIBGCC_DIR"'" make build | ||
| if nm build/seid | grep -q version_lock_lock_exclusive; then | ||
| echo "build-static: ERROR: binary contains the gcc>=12 unwind b-tree (libgcc pin not applied)" >&2 | ||
| exit 1 | ||
| fi | ||
| echo "build-static: pre-b-tree unwinder confirmed (no version_lock symbols)"' |
There was a problem hiding this comment.
🟡 The nm build/seid | grep -q version_lock_lock_exclusive check inside the docker sh -c block runs under set -e only — not set -o pipefail. If nm ever exits non-zero (e.g. a future change strips symbols, or the binary is truncated), the pipeline's status is grep's (1, no match), the if body is skipped, and the script prints "pre-b-tree unwinder confirmed" even though the check was inconclusive. Fix: add set -eo pipefail in the inner sh, or restructure as nm build/seid > /tmp/syms && ! grep -q version_lock_lock_exclusive /tmp/syms. Nit — nothing today produces a stripped/unreadable binary, but the assertion is explicitly a safety net against future toolchain changes.
Extended reasoning...
What the bug is
At scripts/build-static.sh:36, the docker inner shell runs:
if nm build/seid | grep -q version_lock_lock_exclusive; then
echo "build-static: ERROR: binary contains the gcc>=12 unwind b-tree (libgcc pin not applied)" >&2
exit 1
fi
echo "build-static: pre-b-tree unwinder confirmed (no version_lock symbols)"The inner sh -c '...' block sets set -e on line 29 but not set -o pipefail. In a POSIX shell without pipefail, the exit status of A | B is B's status only. So if nm build/seid fails or produces no output, and grep -q therefore returns 1 (no match), the whole pipeline exits 0-for-the-purposes-of-if-false, the guard's exit 1 never fires, and the success message prints unconditionally.
Why existing code doesn't prevent it
The outer script has set -euo pipefail at line 22, but that does not propagate across the docker run ... sh -c '...' boundary — the inner shell starts with its own defaults. The author correctly reset set -e inside, but forgot pipefail.
Step-by-step proof (concrete example)
Suppose a future maintainer adds -s -w to the Makefile ldflags to shrink the binary (a common size optimization). GNU nm on a fully stripped binary prints "no symbols" to stderr and exits 1:
nm build/seidexits 1, produces no stdout (only stderr).grep -q version_lock_lock_exclusivesees empty stdin, exits 1 (no match).- Pipeline exit status = grep's = 1.
if <pipeline>; then ... fi— status 1 is "false", so the guard body (the ERROR echo +exit 1) is skipped.set -edoesn't fire on the pipeline either —set -eis suppressed insideifconditions.- Execution falls through to
echo "build-static: pre-b-tree unwinder confirmed (no version_lock symbols)". - Build passes. The check silently reported "safe" without ever having inspected the symbols.
The same happens for any nm failure: binary truncated by a disk-full write, an unrecognised object format after a future cross-compile switch, permissions issue, etc.
Impact today
Low. make build currently does not pass -s -w, nm produces symbols reliably, and if make build itself failed the earlier set -e line would already have aborted before reaching nm. So today the check works.
Why still worth mentioning
The PR text and the inline comment above the check both explicitly frame this assertion as a safety net against a future toolchain change silently reintroducing the b-tree registry. A safety net that a future -s -w in Makefile could silently defeat is not much of a safety net.
Fix
One line, two options:
# Option A: enable pipefail inside the inner sh
sh -c '
set -eo pipefail
...
'# Option B: restructure so nm failure is fatal on its own
nm build/seid > /tmp/syms
if grep -q version_lock_lock_exclusive /tmp/syms; then
echo "..." >&2; exit 1
fiEither makes the guard robust against toolchain-driven strip/unreadable-binary scenarios without changing the happy-path behavior.
|
Verification runs for this branch:
|
1 participant
Describe your changes and provide context
Fixes the SIGSEGV that made every statically linked
seidcrash on ~70% of boots (the reason the v6.6.0-rc2 binaries were pulled), and closes the two release pipeline gaps that let it ship and then blocked its replacement.The bug
gcc >= 12 replaced libgcc's DWARF unwind-frame registry with a lock-free b-tree (
libgcc/unwind-dw2-btree.h). Wasmer registers unwind frames for every JIT-compiled wasm module (__register_frame, fromUnwindRegistry::publish), and under that pattern the b-tree corrupts and dereferences a null/garbage node insidebtree_insert. A static musl build links the registry from the build image's toolchain (current Alpine ships gcc 15), so every staticseidcarried it and crashed probabilistically at the genesis wasm store, the first wasmer compile. Caught live in gdb:Dynamic glibc builds (the Docker image) use the shared
libgcc_sruntime path and never hit this; upstream wasmd's static muslc binaries link a pre-b-tree libgcc, which is what this PR restores. Symbol hygiene, thread stacks, and Go signal settings were all ruled out experimentally (the crash reproduced identically with each changed in isolation).Changes
third_party/alpine-gcc10-libgcc/, vendored from Alpine 3.15's gcc 10.3.1 apk with full provenance and sha256 pins in its README;scripts/build-static.shverifies the checksums each build andMakefilegains aSTATIC_EXTRA_LDFLAGShook to inject the search path). The build then asserts the final binary contains noversion_lock_lock_exclusivesymbol, so a toolchain upgrade cannot silently reintroduce the b-tree.scripts/boot-smoke.sh): boots the built binary N times against throwaway genesis homes and asserts each run gets through the genesis wasm store to the ABCI handshake. Wired into thelinux-amd64-staticCI job (8 boots) and the goreleaserbefore:hook (4 boots). A ~30% survival rate passed our original single-boot verification; a repeated gate makes that class of luck vanishingly unlikely.uci-release-publish.yml): the "Restrict tagging" ruleset blocks this workflow's token from creating tags, so the publish job fails on version.json pushes and a maintainer pushes the tag by hand, which left the chained goreleaser job gated off (release_creatednever true). This happened for rc1, rc2 and rc3. The goreleaser job now also fires on manually pushedv*tags, and the concurrency group is keyed per event+ref withcancel-in-progress: falseso the tag push can't cancel an in-flight build (they share a SHA).Notes
release/v6.6(backport to follow, manual since this touches workflow files).-z muldefs(hygiene, experimentally not this crash), optional musl-vs-glibc wasm benchmark.Testing performed to validate your change
hackatom.wasm, tx code 0), Tendermint and EVM RPC endpoints served, full and seed modes ran clean, zero crash signals across all node logs.goreleaser checkandactionlintpass;nmon the pinned build confirms zero b-tree symbols.build-static.sh+boot-smoke.shsequence on a runner is linked in the first comment.