-
Notifications
You must be signed in to change notification settings - Fork 886
Fix static seid SIGSEGV (pin pre-gcc-12 libgcc unwinder), add boot smoke gate, build binaries on manual tag pushes #3749
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| #!/usr/bin/env bash | ||
| # Boot the given seid binary N times against throwaway single-node genesis homes and | ||
| # assert every boot gets through the genesis wasm store (the first wasmer JIT compile) | ||
| # and completes the ABCI handshake. | ||
| # | ||
| # Why repeated boots: the class of defect this guards against crashes probabilistically | ||
| # at first wasm use (the gcc>=12 unwind b-tree bug killed ~70% of boots, so any single | ||
| # boot check can pass on luck). N clean boots make a lucky pass vanishingly unlikely. | ||
| # | ||
| # Why "handshake completed" is the success marker: a lone node cannot leave blocksync on | ||
| # this codebase (blocksync's IsCaughtUp requires more than one peer), so block production | ||
| # is not observable single-node. The handshake only completes after the genesis wasm | ||
| # StoreCode calls succeed, which is exactly the code path that crashes. | ||
| # | ||
| # Linux-only (runs the linux/amd64 binary natively; uses GNU timeout). | ||
| # | ||
| # Usage: boot-smoke.sh <path-to-seid> [boots] | ||
| set -euo pipefail | ||
|
|
||
| BIN=${1:?usage: boot-smoke.sh <path-to-seid> [boots]} | ||
| BOOTS=${2:-8} | ||
| CHAIN_ID=boot-smoke-1 | ||
|
|
||
| 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" | ||
| "$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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nit] Minor inconsistency: this |
||
| "$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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nit] Minor: |
||
|
|
||
| LOG="$H/start.log" | ||
| timeout -k 5 25 "$BIN" start --home "$H" >"$LOG" 2>&1 || true | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [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. |
||
|
|
||
| if grep -qE "SIGSEGV|SIGILL|SIGBUS|panic:" "$LOG"; then | ||
| echo "boot-smoke: boot $i/$BOOTS CRASHED:" >&2 | ||
| tail -25 "$LOG" >&2 | ||
| exit 1 | ||
| fi | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nit] On a failed boot the script exits before |
||
| if ! grep -q "Completed ABCI Handshake" "$LOG"; then | ||
| echo "boot-smoke: boot $i/$BOOTS did not reach the ABCI handshake:" >&2 | ||
| tail -25 "$LOG" >&2 | ||
| exit 1 | ||
| fi | ||
| echo "boot-smoke: boot $i/$BOOTS ok" | ||
| rm -rf "$H" | ||
| done | ||
| echo "boot-smoke: all $BOOTS boots clean" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,15 +9,35 @@ | |
| # absent in musl) and zig cc rejects the -z muldefs flag needed for the libwasmvm | ||
| # v152/v155 archives; Alpine's GNU ld + musl links cleanly. --platform linux/amd64 is a | ||
| # no-op on amd64 CI and forces the right arch on Apple Silicon for local runs. | ||
| # | ||
| # The link takes libgcc from third_party/alpine-gcc10-libgcc instead of the build | ||
| # image's toolchain: gcc >= 12's unwind-frame registry (a lock-free b-tree) corrupts | ||
| # under wasmer's JIT frame registration and SIGSEGVs at the genesis wasm store on most | ||
| # boots, so the static binary must carry the pre-b-tree registry. See that directory's | ||
| # README.md for the full story and provenance. The nm assertion below keeps a toolchain | ||
| # upgrade from silently reintroducing the b-tree. | ||
| set -euo pipefail | ||
|
|
||
| # Fail fast if the required static libwasmvm archives are missing. | ||
| bash "$(dirname "$0")/check-libwasmvm-static.sh" | ||
|
|
||
| docker run --rm --platform linux/amd64 -v "$PWD":/src -w /src golang:1.25.6-alpine sh -c ' | ||
| apk add --no-cache build-base git && | ||
| git config --global --add safe.directory /src && | ||
| LINK_STATICALLY=true BUILD_TAGS=muslc LEDGER_ENABLED=false make build' | ||
| LIBGCC_DIR="third_party/alpine-gcc10-libgcc" | ||
|
|
||
| docker run --rm --platform linux/amd64 -v "$PWD":/src -w /src golang:1.25.6-alpine@sha256:98e6cffc31ccc44c7c15d83df1d69891efee8115a5bb7ede2bf30a38af3e3c92 sh -c ' | ||
| 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)"' | ||
|
Comment on lines
+27
to
+40
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 The Extended reasoning...What the bug is At 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 Why existing code doesn't prevent it The outer script has Step-by-step proof (concrete example) Suppose a future maintainer adds
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. 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 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. |
||
|
|
||
| # Assert the output really is statically linked, so a regression fails here rather than | ||
| # shipping a dynamically-linked binary advertised as static. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| # Pinned pre-gcc-12 libgcc for the static seid build | ||
|
|
||
| `libgcc.a` and `libgcc_eh.a` from Alpine 3.15's gcc 10.3.1 package. The static | ||
| build (`scripts/build-static.sh`) links these ahead of the build image's own | ||
| libgcc via `STATIC_EXTRA_LDFLAGS=-L<this dir>`. | ||
|
|
||
| ## Why | ||
|
|
||
| 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`, called from | ||
| `wasmer_compiler::engine::unwind::systemv::UnwindRegistry::publish`), and under | ||
| that registration pattern the b-tree corrupts and SIGSEGVs (`btree_insert` | ||
| walks a null/garbage node pointer). A statically linked musl `seid` gets the | ||
| registry from the build image's toolchain (current Alpine ships gcc 15), which | ||
| made every static binary crash on ~70% of boots at the genesis wasm store. | ||
| Dynamically linked glibc builds use the shared `libgcc_s` runtime path and | ||
| never hit this; upstream wasmd's static binaries link a pre-b-tree libgcc, | ||
| which is what this pin restores. | ||
|
|
||
| `scripts/build-static.sh` verifies these archives' checksums before the build | ||
| and asserts the final binary contains no `version_lock_lock_exclusive` symbol, | ||
| so a toolchain upgrade cannot silently reintroduce the b-tree registry. | ||
|
|
||
| ## Provenance | ||
|
|
||
| Extracted from | ||
| `https://dl-cdn.alpinelinux.org/alpine/v3.15/main/x86_64/gcc-10.3.1_git20211027-r0.apk` | ||
|
|
||
| - apk sha256: `dbbe8d585eb1f8fdb5815168944d442f28c79fbc9be98bba7cfffaff1e5c10bb` | ||
| - libgcc.a sha256: `d3e066fafde74d53a89d48f2ceb9ed9934249a5d450e281edd22947a829469d8` | ||
| - libgcc_eh.a sha256: `d14c9973a735909e11a863b0c850300bfd3aa683ef4689cbe76a53139766ed79` | ||
|
|
||
| To re-derive: | ||
|
|
||
| ```sh | ||
| wget https://dl-cdn.alpinelinux.org/alpine/v3.15/main/x86_64/gcc-10.3.1_git20211027-r0.apk | ||
| tar -xzf gcc-10.3.1_git20211027-r0.apk \ | ||
| usr/lib/gcc/x86_64-alpine-linux-musl/10.3.1/libgcc.a \ | ||
| usr/lib/gcc/x86_64-alpine-linux-musl/10.3.1/libgcc_eh.a | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[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.