bootstrap: make snapshot reproducible #50983

joyeecheung · 2023-11-30T19:16:08Z

src: return non-empty data in context data serializer

For pointer values in the context data, we need to return
non-empty data in the serializer so that V8 does not
serialize them verbatim, making the snapshot unreproducible.

src: make sure that memcpy-ed structs in snapshot have no padding

To make the snapshots reproducible, this patch updates the size
of a few types and adds some static assertions to ensure that
there are no padding in the memcpy-ed structs.

src: add utilities to help debugging reproducibility of snapshots

Print offsets in blob serializer
Add a special node:generate_default_snapshot ID to generate
the built-in snapshot.
Improve logging
Add a test to check the reproducibilty of the snapshot

Refs: nodejs/build#3043

nodejs-github-bot · 2023-11-30T19:16:14Z

Review requested:

@nodejs/gyp
@nodejs/startup
@nodejs/v8-update

haraldh · 2024-01-31T15:22:20Z

great work! keep it up!

nodejs-github-bot · 2024-03-22T19:13:16Z

CI: https://ci.nodejs.org/job/node-test-pull-request/57908/

joyeecheung · 2024-03-23T15:03:57Z

Somehow this is reproducible locally (macOS + x64 & arm64), and in the CI, on FreeBSD and SmartOS, but not on others. There are still 3-4 words of differences in the snapshot on those platforms. I'll investigate.

joyeecheung · 2024-05-30T13:30:13Z

Split the context data callback part to #53217 - at least that makes the context data slots serialized with some non-gibberish (empty values), otherwise V8 might run into issues serializing the gibberish pointer values.

legendecas · 2024-05-31T10:46:03Z

Is this PR still useful after #53217 being splitted?

joyeecheung · 2024-05-31T20:35:00Z

Yes, in this PR are still some utilities for checking reproducibility and a test for that (but it's not currently passing because a few other bytes are still not reproducible, I will work on those bytes here).

joyeecheung · 2024-06-12T23:10:49Z

Some discoveries:

We need to return non-empty data for pointer values in context data otherwise V8 would still serialize them verbatim (that was what this PR used to do, in src: use v8::(Des|S)erializeInternalFieldsCallback #53217 I changed it to return empty data and that added back some more variance)
The MemoryMode in EmbedderTypeInfo and the EmbedderObjectType in InternalFieldInfoBase are padded. We need to get rid of the padding - the easiest way to do so is to just make the enums uint64_t (a harder way would be to implement custom serialization for all the wrapper structs but I feel that's an overkill for a couple of bytes of overhead for now).

After fixing the two on Linux I am getting reproducible snapshots. Will update the branch after I clean it up..

For pointer values in the context data, we need to return non-empty data in the serializer so that V8 does not serialize them verbatim, making the snapshot unreproducible.

To make the snapshots reproducible, this patch updates the size of a few types and adds some static assertions to ensure that there are no padding in the memcpy-ed structs.

- Print offsets in blob serializer - Add a special node:generate_default_snapshot ID to generate the built-in snapshot. - Improve logging - Add a test to check the reproducibilty of the snapshot

For pointer values in the context data, we need to return non-empty data in the serializer so that V8 does not serialize them verbatim, making the snapshot unreproducible. PR-URL: #50983 Refs: nodejs/build#3043 Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: James M Snell <jasnell@gmail.com>

nodejs-github-bot · 2024-06-14T09:58:44Z

Landed in 922feb1...1872167

To make the snapshots reproducible, this patch updates the size of a few types and adds some static assertions to ensure that there are no padding in the memcpy-ed structs. PR-URL: #50983 Refs: nodejs/build#3043 Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: James M Snell <jasnell@gmail.com>

- Print offsets in blob serializer - Add a special node:generate_default_snapshot ID to generate the built-in snapshot. - Improve logging - Add a test to check the reproducibilty of the snapshot PR-URL: #50983 Refs: nodejs/build#3043 Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: James M Snell <jasnell@gmail.com>

legendecas

LGTM

legendecas · 2024-06-14T10:34:35Z

src/node_snapshotable.cc

  written_total += w.Write<EnvSerializeInfo>(env_info);
-  w.Debug("Write code_cache\n");
+  w.Debug("0x%x: Write CodeCacheInfo\n", w.sink.size());
  written_total += w.WriteVector<builtins::CodeCacheInfo>(code_cache);
  w.Debug("SnapshotData::ToBlob() Wrote %d bytes\n", written_total);


Might be worth check that DCHECK_EQ(written_total, w.sink.size()).

For pointer values in the context data, we need to return non-empty data in the serializer so that V8 does not serialize them verbatim, making the snapshot unreproducible. PR-URL: #50983 Refs: nodejs/build#3043 Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: James M Snell <jasnell@gmail.com>

To make the snapshots reproducible, this patch updates the size of a few types and adds some static assertions to ensure that there are no padding in the memcpy-ed structs. PR-URL: #50983 Refs: nodejs/build#3043 Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: James M Snell <jasnell@gmail.com>

- Print offsets in blob serializer - Add a special node:generate_default_snapshot ID to generate the built-in snapshot. - Improve logging - Add a test to check the reproducibilty of the snapshot PR-URL: #50983 Refs: nodejs/build#3043 Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: James M Snell <jasnell@gmail.com>

For pointer values in the context data, we need to return non-empty data in the serializer so that V8 does not serialize them verbatim, making the snapshot unreproducible. PR-URL: nodejs#50983 Refs: nodejs/build#3043 Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: James M Snell <jasnell@gmail.com>

To make the snapshots reproducible, this patch updates the size of a few types and adds some static assertions to ensure that there are no padding in the memcpy-ed structs. PR-URL: nodejs#50983 Refs: nodejs/build#3043 Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: James M Snell <jasnell@gmail.com>

- Print offsets in blob serializer - Add a special node:generate_default_snapshot ID to generate the built-in snapshot. - Improve logging - Add a test to check the reproducibilty of the snapshot PR-URL: nodejs#50983 Refs: nodejs/build#3043 Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: James M Snell <jasnell@gmail.com>

For pointer values in the context data, we need to return non-empty data in the serializer so that V8 does not serialize them verbatim, making the snapshot unreproducible. PR-URL: nodejs#50983 Refs: nodejs/build#3043 Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: James M Snell <jasnell@gmail.com>

To make the snapshots reproducible, this patch updates the size of a few types and adds some static assertions to ensure that there are no padding in the memcpy-ed structs. PR-URL: nodejs#50983 Refs: nodejs/build#3043 Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: James M Snell <jasnell@gmail.com>

- Print offsets in blob serializer - Add a special node:generate_default_snapshot ID to generate the built-in snapshot. - Improve logging - Add a test to check the reproducibilty of the snapshot PR-URL: nodejs#50983 Refs: nodejs/build#3043 Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: James M Snell <jasnell@gmail.com>

…ding" This reverts commit 4e58cde. PR-URL: #53563 Refs: #50983 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: James M Snell <jasnell@gmail.com>

To prevent padding from making the snapshot unreproducible, zero-initialize the data that are copied into the snapshot so that the padding copied are all zeros. This is better than enlarging the enums to align the fields since it doesn't make the snapshot bigger than necessary, and it removes the need of using static assertions to ensure alignment. PR-URL: #53563 Refs: #50983 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: James M Snell <jasnell@gmail.com>

…ding" This reverts commit 4e58cde. PR-URL: #53563 Refs: #50983 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: James M Snell <jasnell@gmail.com>

To prevent padding from making the snapshot unreproducible, zero-initialize the data that are copied into the snapshot so that the padding copied are all zeros. This is better than enlarging the enums to align the fields since it doesn't make the snapshot bigger than necessary, and it removes the need of using static assertions to ensure alignment. PR-URL: #53563 Refs: #50983 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: James M Snell <jasnell@gmail.com>

…ding" This reverts commit 4e58cde. PR-URL: #53563 Refs: #50983 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: James M Snell <jasnell@gmail.com>

To prevent padding from making the snapshot unreproducible, zero-initialize the data that are copied into the snapshot so that the padding copied are all zeros. This is better than enlarging the enums to align the fields since it doesn't make the snapshot bigger than necessary, and it removes the need of using static assertions to ensure alignment. PR-URL: #53563 Refs: #50983 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Richard Lau <rlau@redhat.com> Reviewed-By: Daniel Lemire <daniel@lemire.me> Reviewed-By: James M Snell <jasnell@gmail.com>

nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Nov 30, 2023

This was referenced Nov 30, 2023

Reproducible builds on Linux nodejs/build#3043

Open

vm: use v8::DeserializeInternalFieldsCallback explicitly #50984

Closed

joyeecheung force-pushed the context-slot branch 2 times, most recently from 6df2ede to 8dc86b6 Compare March 22, 2024 19:07

joyeecheung changed the title ~~WIP: reproducible snapshot~~ bootstrap: make snapshot reproducible Mar 22, 2024

joyeecheung marked this pull request as ready for review March 22, 2024 19:08

joyeecheung added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Mar 22, 2024

joyeecheung force-pushed the context-slot branch 2 times, most recently from c92d201 to 2d911be Compare March 22, 2024 19:11

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 22, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 22, 2024

joyeecheung force-pushed the context-slot branch from 2d911be to e0b6197 Compare March 22, 2024 22:13

joyeecheung force-pushed the context-slot branch from e0b6197 to 9c8ab5e Compare March 27, 2024 14:54

joyeecheung force-pushed the context-slot branch 2 times, most recently from 57b8d25 to f03a7ba Compare May 20, 2024 07:38

joyeecheung mentioned this pull request May 30, 2024

src: use v8::(Des|S)erializeInternalFieldsCallback #53217

Merged

joyeecheung added 3 commits June 13, 2024 17:52

src: return non-empty data in context data serializer

e3b6716

For pointer values in the context data, we need to return non-empty data in the serializer so that V8 does not serialize them verbatim, making the snapshot unreproducible.

src: make sure that memcpy-ed structs in snapshot have no padding

31b78b1

To make the snapshots reproducible, this patch updates the size of a few types and adds some static assertions to ensure that there are no padding in the memcpy-ed structs.

src: add utilities to help debugging reproducibility of snapshots

b71c4f8

- Print offsets in blob serializer - Add a special node:generate_default_snapshot ID to generate the built-in snapshot. - Improve logging - Add a test to check the reproducibilty of the snapshot

joyeecheung added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. labels Jun 14, 2024

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 14, 2024

nodejs-github-bot closed this Jun 14, 2024

legendecas reviewed Jun 14, 2024

View reviewed changes

joyeecheung mentioned this pull request Jun 23, 2024

src: zero-initialize data that are copied into the snapshot #53563

Closed

targos mentioned this pull request Jun 25, 2024

v22.4.0 release proposal #53583

Merged

Karliz24 mentioned this pull request Jul 9, 2024

## 2024-07-02, Version 22.4.0 (Current), @targos Karliz24/Marley-#16

Open

marco-ippolito added the dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. label Jul 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bootstrap: make snapshot reproducible #50983

bootstrap: make snapshot reproducible #50983

joyeecheung commented Nov 30, 2023 •

edited

Loading

nodejs-github-bot commented Nov 30, 2023

haraldh commented Jan 31, 2024

nodejs-github-bot commented Mar 22, 2024

joyeecheung commented Mar 23, 2024

joyeecheung commented May 30, 2024

legendecas commented May 31, 2024 •

edited

Loading

joyeecheung commented May 31, 2024 •

edited

Loading

joyeecheung commented Jun 12, 2024 •

edited

Loading

nodejs-github-bot commented Jun 14, 2024

legendecas left a comment

legendecas Jun 14, 2024

bootstrap: make snapshot reproducible #50983

bootstrap: make snapshot reproducible #50983

Conversation

joyeecheung commented Nov 30, 2023 • edited Loading

src: return non-empty data in context data serializer

src: make sure that memcpy-ed structs in snapshot have no padding

src: add utilities to help debugging reproducibility of snapshots

nodejs-github-bot commented Nov 30, 2023

haraldh commented Jan 31, 2024

nodejs-github-bot commented Mar 22, 2024

joyeecheung commented Mar 23, 2024

joyeecheung commented May 30, 2024

legendecas commented May 31, 2024 • edited Loading

joyeecheung commented May 31, 2024 • edited Loading

joyeecheung commented Jun 12, 2024 • edited Loading

nodejs-github-bot commented Jun 14, 2024

legendecas left a comment

Choose a reason for hiding this comment

legendecas Jun 14, 2024

Choose a reason for hiding this comment

joyeecheung commented Nov 30, 2023 •

edited

Loading

legendecas commented May 31, 2024 •

edited

Loading

joyeecheung commented May 31, 2024 •

edited

Loading

joyeecheung commented Jun 12, 2024 •

edited

Loading