Skip to content

fix(build): verify Math constants in runtime snapshot - #726

Merged
nyannyacha merged 2 commits into
supabase:mainfrom
da1suk8:fix/snapshot-math-verification
Oct 1, 2026
Merged

nyannyacha merged 2 commits into
supabase:mainfrom
da1suk8:fix/snapshot-math-verification

Conversation

@da1suk8

@da1suk8 da1suk8 commented Aug 29, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #723 — this prevents recurrence; it does not fix the already-published amd64 images. V8 computes Math.E, Math.LN10, Math.LN2, Math.LOG10E, and Math.LOG2E once, inside mksnapshot when the V8 library is built. crates/base/build.rs inherits them through V8's embedded default snapshot and freezes them into the startup snapshot every worker boots from, so a bad V8 build ships wrong constants to every user and nothing in the pipeline could catch it (root cause: #723 (comment)).

This PR hooks CreateSnapshotOptions::with_runtime_cb, reads the eight Math constants through the V8 API right before the snapshot is serialized, and panics on any bit-level mismatch, failing the build before the snapshot is written to disk:

the startup snapshot about to be serialized carries corrupted Math constants:
Math.E: expected 0x4005bf0a8b145769, got 0x4000000000000000 (2);
refusing to continue the build (see supabase/edge-runtime#723)

Notes on the approach:

  • No second runtime and no re-serialization; nothing is compiled or executed in the isolate. Snapshot size is unchanged: 9,938,438 bytes with and without the check (debug, macOS arm64).
  • The earlier warmup-script revision is gone: re-serializing after warmup drops the ESM sources the runtime needs, as pointed out in review.

Limitations

  • A canary on eight constants, not a checksum of the whole blob.
  • Already-published images need a rebuild against the fixed librusty_v8.

Validation

  • cargo build -p base passes; corrupting one expected value fails the build (exit 101) with the message above.
  • Integration tests that boot main and user workers from the new snapshot pass (5 tests).
  • The rebuilt x86_64 librusty_v8 archives contain all eight constants.

@da1suk8
da1suk8 force-pushed the fix/snapshot-math-verification branch from 57077e6 to 17eb6f3 Compare September 29, 2026 01:41
@da1suk8

da1suk8 commented Sep 29, 2026

Copy link
Copy Markdown
Contributor Author

@nyannyacha this is the recurrence guard for #723 (the wrong Math.LN10 etc. in the amd64 snapshot). It adds a check in crates/base/build.rs and changes nothing at runtime.

The CI run from August expired while waiting for approval, so I rebased onto current main. Could you approve the workflow run and review when you get a chance? Happy to turn it into a test instead of a build-time assertion if you prefer.

@nyannyacha

nyannyacha commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Thanks for working on this!

But the issue with some of the math constants in the amd64 snapshot differing from expectations doesn't appear to be a GHA (actually, Blacksmith) build cache problem.

Could you investigate further? It may be related to V8, but I don't have the bandwidth to dig into that right now.

@da1suk8

da1suk8 commented Sep 29, 2026

Copy link
Copy Markdown
Contributor Author

Update: root cause found, details in #723 (comment). The prebuilt x86_64 librusty_v8 from supabase/rusty_v8 ships an embedded default snapshot that already carries the truncated constants, and build.rs inherits that blob instead of running genesis. That is why this check fires on every x86_64 build, in CI and in the release images alike.

So the red CI is the check working. I'd keep the hard failure as is: once the fork's x86_64 binary is rebuilt on native x86_64 Linux this PR goes green without changes, and it will catch the same class of problem if a bad binary is ever published again. If you'd rather have CI green in the meantime, I can gate it to release builds and emit cargo:warning for debug builds.

@nyannyacha

nyannyacha commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

I rebuilt the x86_64 library and confirmed the relevant Math constants are now populated correctly.
As I recall, I did the amd64 build on the Mac Pro late 2013 — it's a very old machine, so something may have gone wrong during cross-compilation.
The new build was done on an AWS x86_64 instance.

Comment thread crates/base/build.rs Outdated
@da1suk8

da1suk8 commented Oct 1, 2026

Copy link
Copy Markdown
Contributor Author

You're right, sorry for the crash: Extension::for_warmup drops the ESM sources, and the "32% smaller" snapshot I measured was that loss.
Warmup is back to None. The check now runs in with_runtime_cb and reads the constants through the V8 API right before snapshot(); snapshot size is unchanged.

The CI run for the new commit is waiting for approval. It should pass now that the fixed archive is published.

@da1suk8
da1suk8 requested a review from nyannyacha October 1, 2026 04:57

@nyannyacha nyannyacha left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM
Thanks for your effort! 😸

@nyannyacha
nyannyacha merged commit 3e0a3ca into supabase:main Oct 1, 2026
4 checks passed
@da1suk8
da1suk8 deleted the fix/snapshot-math-verification branch October 1, 2026 05:24
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.

2 participants