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 <[email protected]> Reviewed-By: James M Snell <[email protected]>

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 <[email protected]> Reviewed-By: James M Snell <[email protected]>

- 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 <[email protected]> Reviewed-By: James M Snell <[email protected]>

legendecas

LGTM

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

src/node_snapshotable.cc

- 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 <[email protected]> Reviewed-By: James M Snell <[email protected]>

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 <[email protected]> Reviewed-By: James M Snell <[email protected]>

- 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 <[email protected]> Reviewed-By: James M Snell <[email protected]>

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 <[email protected]> Reviewed-By: James M Snell <[email protected]>

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 <[email protected]> Reviewed-By: James M Snell <[email protected]>

- 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 <[email protected]> Reviewed-By: James M Snell <[email protected]>

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 <[email protected]> Reviewed-By: James M Snell <[email protected]>

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 <[email protected]> Reviewed-By: James M Snell <[email protected]>

- 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 <[email protected]> Reviewed-By: James M Snell <[email protected]>

…ding" This reverts commit 4e58cde. PR-URL: #53563 Refs: #50983 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Daniel Lemire <[email protected]> Reviewed-By: James M Snell <[email protected]>

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 <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Daniel Lemire <[email protected]> Reviewed-By: James M Snell <[email protected]>

…ding" This reverts commit 4e58cde. PR-URL: #53563 Refs: #50983 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Daniel Lemire <[email protected]> Reviewed-By: James M Snell <[email protected]>

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 <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Daniel Lemire <[email protected]> Reviewed-By: James M Snell <[email protected]>

…ding" This reverts commit 4e58cde. PR-URL: #53563 Refs: #50983 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Daniel Lemire <[email protected]> Reviewed-By: James M Snell <[email protected]>

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 <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Daniel Lemire <[email protected]> Reviewed-By: James M Snell <[email protected]>

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
Closed
vm: use v8::DeserializeInternalFieldsCallback explicitly #50984
Closed

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

joyeecheung changed the title ~~WIP: reproducible snapshot~~bootstrap: make snapshot reproducibleMar 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 2d911beCompare 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-actionsbot 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 e0b6197Compare March 22, 2024 22:13

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

joyeecheung force-pushed the context-slot branch 2 times, most recently from 57b8d25 to f03a7baCompare 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

Uh oh!

Uh oh!

bootstrap: make snapshot reproducible#50983

bootstrap: make snapshot reproducible #50983

Uh oh!

joyeecheung commented Nov 30, 2023•
edited
Loading

Uh oh!

nodejs-github-bot commented Nov 30, 2023

Uh oh!

haraldh commented Jan 31, 2024

Uh oh!

nodejs-github-bot commented Mar 22, 2024

Uh oh!

joyeecheung commented Mar 23, 2024

Uh oh!

joyeecheung commented May 30, 2024

Uh oh!

legendecas commented May 31, 2024•
edited
Loading

Uh oh!

joyeecheung commented May 31, 2024•
edited
Loading

Uh oh!

joyeecheung commented Jun 12, 2024•
edited
Loading

Uh oh!

nodejs-github-bot commented Jun 14, 2024

Uh oh!

legendecas left a comment

Uh oh!

legendecasJun 14, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Uh oh!

bootstrap: make snapshot reproducible#50983

bootstrap: make snapshot reproducible #50983

Uh oh!

Conversation

joyeecheung commented Nov 30, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Uh oh!

nodejs-github-bot commented Nov 30, 2023

Uh oh!

haraldh commented Jan 31, 2024

Uh oh!

nodejs-github-bot commented Mar 22, 2024

Uh oh!

joyeecheung commented Mar 23, 2024

Uh oh!

joyeecheung commented May 30, 2024

Uh oh!

legendecas commented May 31, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joyeecheung commented May 31, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joyeecheung commented Jun 12, 2024• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Jun 14, 2024

Uh oh!

legendecas left a comment

Choose a reason for hiding this comment

Uh oh!

legendecasJun 14, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

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