GeezerVison is showing github.com — src: use Blob{Des|S}erializer for SEA blobs by joyeecheung · Pull Request #47962 · nodejs/node
Conversation
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
bricss reacted with thumbs up emoji
|
Review requested:
- @nodejs/single-executable
- @nodejs/startup
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
nodejs-github-bot
added
c++
Issues and PRs that require attention from people who are familiar with C++.
lib / src
Issues and PRs involving general changes in the lib/ or src/ directories.
needs-ci
PRs that need a full CI run.
labels
May 11, 2023
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
|
|
}
|
|
|
|
|
|
template <typename Impl>
|
|
|
size_t BlobSerializer<Impl>::WriteString(const std::string& data) {
|
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.
|
size_t BlobSerializer<Impl>::WriteString(const std::string& data) { |
|
size_t BlobSerializer<Impl>::WriteString(const std::string_view data) { |
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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.
This is the method that writes strings (which logs the string directly in debug mode). The one that doesn't is WriteStringView (which obviously doesn't log that data as string).
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
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.
Now that we're using these classes for both snapshots and SEAs, we can update these comments to also mention SEA:
|
// This is related to the blob that is used in snapshots and has nothing to do |
|
// with `node_blob.h`. |
|
// This is related to the blob that is used in snapshots and has nothing to do |
|
// with `node_blob.h`. |
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
RaisinTen
left a comment
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
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.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
I'm getting compiler errors and warnings with GCC 13:
g++ -o /home/iojs/build/workspace/node-test-commit-linux/out/Release/obj.target/libnode/src/node_sea.o ../src/node_sea.cc '-DV8_DEPRECATION_WARNINGS' '-DV8_IMMINENT_DEPRECATION_WARNINGS' '-D_GLIBCXX_USE_CXX11_ABI=1' '-DNODE_OPENSSL_CONF_NAME=nodejs_conf' '-DNODE_OPENSSL_HAS_QUIC' '-DICU_NO_USER_DATA_OVERRIDE' '-D__STDC_FORMAT_MACROS' '-DOPENSSL_NO_PINSHARED' '-DOPENSSL_THREADS' '-DNODE_ARCH="x64"' '-DNODE_PLATFORM="linux"' '-DNODE_WANT_INTERNALS=1' '-DV8_DEPRECATION_WARNINGS=1' '-DNODE_OPENSSL_SYSTEM_CERT_PATH=""' '-DNODE_USE_NODE_CODE_CACHE=1' '-DHAVE_INSPECTOR=1' '-DNODE_ENABLE_LARGE_CODE_PAGES=1' '-D__POSIX__' '-DNODE_USE_V8_PLATFORM=1' '-DNODE_V8_SHARED_RO_HEAP' '-DNODE_HAVE_I18N_SUPPORT=1' '-DHAVE_OPENSSL=1' '-DOPENSSL_API_COMPAT=0x10100000L' '-DBASE64_STATIC_DEFINE' '-DUCONFIG_NO_SERVICE=1' '-DU_ENABLE_DYLOAD=0' '-DU_STATIC_IMPLEMENTATION=1' '-DU_HAVE_STD_STRING=1' '-DUCONFIG_NO_BREAK_ITERATION=0' '-D_LARGEFILE_SOURCE' '-D_FILE_OFFSET_BITS=64' '-D_POSIX_C_SOURCE=200112' '-DNGHTTP2_STATICLIB' '-DNDEBUG' '-DOPENSSL_USE_NODELETE' '-DL_ENDIAN' '-DOPENSSL_BUILDING_OPENSSL' '-DAES_ASM' '-DBSAES_ASM' '-DCMLL_ASM' '-DECP_NISTZ256_ASM' '-DGHASH_ASM' '-DKECCAK1600_ASM' '-DMD5_ASM' '-DOPENSSL_BN_ASM_GF2m' '-DOPENSSL_BN_ASM_MONT' '-DOPENSSL_BN_ASM_MONT5' '-DOPENSSL_CPUID_OBJ' '-DOPENSSL_IA32_SSE2' '-DPADLOCK_ASM' '-DPOLY1305_ASM' '-DSHA1_ASM' '-DSHA256_ASM' '-DSHA512_ASM' '-DVPAES_ASM' '-DWHIRLPOOL_ASM' '-DX25519_ASM' '-DOPENSSL_PIC' '-DNGTCP2_STATICLIB' '-DNGHTTP3_STATICLIB' -I../src -I../deps/postject -I/home/iojs/build/workspace/node-test-commit-linux/out/Release/obj/gen -I/home/iojs/build/workspace/node-test-commit-linux/out/Release/obj/gen/include -I/home/iojs/build/workspace/node-test-commit-linux/out/Release/obj/gen/src -I../deps/base64/base64/include -I../deps/googletest/include -I../deps/histogram/src -I../deps/uvwasi/include -I../deps/simdutf -I../deps/ada -I../deps/v8/include -I../deps/icu-small/source/i18n -I../deps/icu-small/source/common -I../deps/zlib -I../deps/llhttp/include -I../deps/cares/include -I../deps/uv/include -I../deps/nghttp2/lib/includes -I../deps/brotli/c/include -I../deps/openssl/openssl/include -I../deps/openssl/openssl/crypto/include -I../deps/openssl/config/archs/linux-x86_64/asm/include -I../deps/openssl/config/archs/linux-x86_64/asm -I../deps/ngtcp2 -I../deps/ngtcp2/ngtcp2/lib/includes -I../deps/ngtcp2/ngtcp2/crypto/includes -I../deps/ngtcp2/ngtcp2/crypto -I../deps/ngtcp2/nghttp3/lib/includes -Wall -Wextra -Wno-unused-parameter -pthread -Wall -Wextra -Wno-unused-parameter -m64 -Werror=unused-result -O3 -fno-omit-frame-pointer -fno-rtti -fno-exceptions -std=gnu++17 -MMD -MF /home/iojs/build/workspace/node-test-commit-linux/out/Release/.deps//home/iojs/build/workspace/node-test-commit-linux/out/Release/obj.target/libnode/src/node_sea.o.d.raw -c
In file included from ../src/node_sea.cc:1:
../src/node_sea.h:18:7: error: ‘uint32_t’ does not name a type
18 | const uint32_t kMagic = 0x143da20;
| ^~~~~~~~
../src/node_sea.h:11:1: note: ‘uint32_t’ is defined in header ‘<cstdint>’; did you forget to ‘#include <cstdint>’?
10 | #include "node_exit_code.h"
+++ |+#include <cstdint>
11 |
../src/node_sea.h:20:6: warning: elaborated-type-specifier for a scoped enum must not use the ‘class’ keyword
20 | enum class SeaFlags : uint32_t {
| ~~~~ ^~~~~
| -----
../src/node_sea.h:20:21: error: found ‘:’ in nested-name-specifier, expected ‘::’
20 | enum class SeaFlags : uint32_t {
| ^
| ::
../src/node_sea.h:20:12: error: ‘SeaFlags’ has not been declared
20 | enum class SeaFlags : uint32_t {
| ^~~~~~~~
../src/node_sea.h:20:32: error: expected unqualified-id before ‘{’ token
20 | enum class SeaFlags : uint32_t {
| ^
../src/node_sea.h:26:3: error: ‘SeaFlags’ does not name a type
26 | SeaFlags flags = SeaFlags::kDefault;
| ^~~~~~~~
../src/node_sea.h:29:48: error: ‘kMagic’ was not declared in this scope
29 | static constexpr size_t kHeaderSize = sizeof(kMagic) + sizeof(SeaFlags);
| ^~~~~~
../src/node_sea.h:29:65: error: ‘SeaFlags’ was not declared in this scope
29 | static constexpr size_t kHeaderSize = sizeof(kMagic) + sizeof(SeaFlags);
| ^~~~~~~~
../src/node_sea.cc:39:1: error: ‘SeaFlags’ does not name a type
39 | SeaFlags operator|(SeaFlags x, SeaFlags y) {
| ^~~~~~~~
../src/node_sea.cc:44:1: error: ‘SeaFlags’ does not name a type
44 | SeaFlags operator&(SeaFlags x, SeaFlags y) {
| ^~~~~~~~
../src/node_sea.cc:49:1: error: ‘SeaFlags’ does not name a type
49 | SeaFlags operator|=(/* NOLINT (runtime/references) */ SeaFlags& x, SeaFlags y) {
| ^~~~~~~~
../src/node_sea.cc: In member function ‘size_t node::sea::{anonymous}::SeaSerializer::Write(const T&) [with T = node::sea::SeaResource; std::enable_if_t<(! std::is_same<T, std::__cxx11::basic_string<char> >::value)>* <anonymous> = 0; std::enable_if_t<(! std::is_arithmetic<_Tp>::value)>* <anonymous> = 0; size_t = long unsigned int]’:
../src/node_sea.cc:69:33: error: ‘kMagic’ was not declared in this scope
69 | Debug("Write SEA magic %x\n", kMagic);
| ^~~~~~
../src/node_sea.cc:72:46: error: ‘const struct node::sea::SeaResource’ has no member named ‘flags’
72 | uint32_t flags = static_cast<uint32_t>(sea.flags);
| ^~~~~
In file included from ../src/debug_utils.h:7,
from ../src/debug_utils-inl.h:6,
from ../src/blob_serializer_deserializer-inl.h:14,
from ../src/node_sea.cc:3:
../src/node_sea.cc: In member function ‘T node::sea::{anonymous}::SeaDeserializer::Read() [with T = node::sea::SeaResource; std::enable_if_t<(! std::is_same<T, std::__cxx11::basic_string<char> >::value)>* <anonymous> = 0; std::enable_if_t<(! std::is_arithmetic<_Tp>::value)>* <anonymous> = 0]’:
../src/node_sea.cc:101:19: error: ‘kMagic’ was not declared in this scope; did you mean ‘magic’?
101 | CHECK_EQ(magic, kMagic);
| ^~~~~~
../src/util.h:141:44: note: in definition of macro ‘UNLIKELY’
141 | #define UNLIKELY(expr) __builtin_expect(!!(expr), 0)
| ^~~~
../src/util.h:159:24: note: in expansion of macro ‘CHECK’
159 | #define CHECK_EQ(a, b) CHECK((a) == (b))
| ^~~~~
../src/node_sea.cc:101:3: note: in expansion of macro ‘CHECK_EQ’
101 | CHECK_EQ(magic, kMagic);
| ^~~~~~~~
../src/node_sea.cc:102:3: error: ‘SeaFlags’ was not declared in this scope; did you mean ‘StopFlags’?
102 | SeaFlags flags(static_cast<SeaFlags>(ReadArithmetic<uint32_t>()));
| ^~~~~~~~
| StopFlags
../src/node_sea.cc:103:54: error: ‘flags’ was not declared in this scope
103 | Debug("Read SEA flags %x\n", static_cast<uint32_t>(flags));
| ^~~~~
../src/node_sea.cc:108:22: error: could not convert ‘{flags, code}’ from ‘<brace-enclosed initializer list>’ to ‘node::sea::SeaResource’
108 | return {flags, code};
| ^
| |
| <brace-enclosed initializer list>
../src/node_sea.cc: In function ‘void node::sea::IsExperimentalSeaWarningNeeded(const v8::FunctionCallbackInfo<v8::Value>&)’:
../src/node_sea.cc:161:20: error: ‘struct node::sea::SeaResource’ has no member named ‘flags’
161 | sea_resource.flags & SeaFlags::kDisableExperimentalSeaWarning));
| ^~~~~
../src/node_sea.cc:161:28: error: ‘SeaFlags’ has not been declared
161 | sea_resource.flags & SeaFlags::kDisableExperimentalSeaWarning));
| ^~~~~~~~
../src/node_sea.cc: At global scope:
../src/node_sea.cc:185:3: error: ‘SeaFlags’ does not name a type
185 | SeaFlags flags = SeaFlags::kDefault;
| ^~~~~~~~
../src/node_sea.cc: In function ‘std::optional<node::sea::{anonymous}::SeaConfig> node::sea::{anonymous}::ParseSingleExecutableConfig(const std::string&)’:
../src/node_sea.cc:235:12: error: ‘struct node::sea::{anonymous}::SeaConfig’ has no member named ‘flags’
235 | result.flags |= SeaFlags::kDisableExperimentalSeaWarning;
| ^~~~~
../src/node_sea.cc:235:21: error: ‘SeaFlags’ has not been declared
235 | result.flags |= SeaFlags::kDisableExperimentalSeaWarning;
| ^~~~~~~~
../src/node_sea.cc: In function ‘node::ExitCode node::sea::{anonymous}::GenerateSingleExecutableBlob(const SeaConfig&)’:
../src/node_sea.cc:251:26: error: ‘const struct node::sea::{anonymous}::SeaConfig’ has no member named ‘flags’
251 | SeaResource sea{config.flags, main_script};
| ^~~~~
make[2]: *** [libnode.target.mk:490: /home/iojs/build/workspace/node-test-commit-linux/out/Release/obj.target/libnode/src/node_sea.o] Error 1
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced May 25, 2023
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Labels
author ready
PRs with CI started, the required approvals, and no outstanding review comments.
c++
Issues and PRs that require attention from people who are familiar with C++.
commit-queue-squash
PRs the Commit Queue should land as one squashed commit.
lib / src
Issues and PRs involving general changes in the lib/ or src/ directories.
needs-ci
PRs that need a full CI run.
single-executable
Issues and PRs related to single-executable applications.
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|