electron/patches/node/be_compatible_with_cppgc.patch

97 lines
5.3 KiB
Diff
Raw Normal View History

From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Jeremy Rose <nornagon@nornagon.net>
Date: Fri, 11 Mar 2022 11:17:29 -0800
Subject: be compatible with cppgc
This fixes a crash that happens sporadically when Node is used in the same V8
Isolate as Blink. For example:
#
# Fatal error in ../../v8/src/objects/js-objects-inl.h, line 306
# Debug check failed: static_cast<unsigned>(index) < static_cast<unsigned>(GetEmbedderFieldCount()) (1 vs. 1).
#
#
#
#FailureMessage Object: 0x7ffee46fd1c0
0 Electron Framework 0x00000001181c78d9 base::debug::CollectStackTrace(void**, unsigned long) + 9
1 Electron Framework 0x00000001180ea633 base::debug::StackTrace::StackTrace() + 19
2 Electron Framework 0x000000011a04decd gin::(anonymous namespace)::PrintStackTrace() + 45
3 Electron Framework 0x0000000119a9b416 V8_Fatal(char const*, int, char const*, ...) + 326
4 Electron Framework 0x0000000119a9aeb5 v8::base::(anonymous namespace)::DefaultDcheckHandler(char const*, int, char const*) + 21
5 Electron Framework 0x000000011530763f v8::internal::JSObject::GetEmbedderFieldOffset(int) + 207
6 Electron Framework 0x00000001155f68e6 v8::internal::LocalEmbedderHeapTracer::EmbedderWriteBarrier(v8::internal::Heap*, v8::internal::JSObject) + 150
7 Electron Framework 0x00000001152cd34f v8::Object::SetAlignedPointerInInternalField(int, void*) + 639
8 Electron Framework 0x000000011d18df35 node::BaseObject::BaseObject(node::Environment*, v8::Local<v8::Object>) + 101
9 Electron Framework 0x000000011d347b6e node::crypto::DiffieHellman::DiffieHellman(node::Environment*, v8::Local<v8::Object>) + 14
10 Electron Framework 0x000000011d348413 node::crypto::DiffieHellman::New(v8::FunctionCallbackInfo<v8::Value> const&) + 147
[...]
This crash happens because this V8 isolate has cppgc enabled. When cppgc is
enabled, V8 assumes that the first embedder field is a "type" pointer, the
first 16 bits of which are the embedder ID. Node did not adhere to this
requirement. Sometimes--mostly, even--this worked _by accident_. If the first
field in the BaseObject was a pointer to a bit of memory that happened to
contain the two-byte little-endian value 0x0001, however, V8 would take that to
mean that the object was a Blink object[1], and attempt to read the pointer in
the second embedder slot, which would result in a CHECK.
This change adds an "embedder id" pointer as the first embedder field in all
Node-managed objects. This ensures that cppgc will always skip over Node
objects.
This patch should be upstreamed to Node.
[1]: https://source.chromium.org/chromium/chromium/src/+/main:gin/public/gin_embedders.h;l=20;drc=5a758a97032f0b656c3c36a3497560762495501a
See also: https://source.chromium.org/chromium/chromium/src/+/main:v8/include/v8-cppgc.h;l=70-76;drc=5a758a97032f0b656c3c36a3497560762495501a
chore: bump node to v16.17.0 (main) (#35350) * chore: bump node in DEPS to v16.17.0 * chore: fixup asar patch * lib: use null-prototype objects for property descriptors https://github.com/nodejs/node/pull/43270 * src: make SecureContext fields private https://github.com/nodejs/node/pull/43173 * crypto: remove Node.js-specific webcrypto extensions https://github.com/nodejs/node/pull/43310 * test: refactor to top-level await https://github.com/nodejs/node/pull/43500 * deps: cherry-pick two libuv fixes https://github.com/nodejs/node/pull/43950 * src: slim down env-inl.h https://github.com/nodejs/node/pull/43745 * util: add AggregateError.prototype.errors to inspect output https://github.com/nodejs/node/pull/43646 * esm: improve performance & tidy tests https://github.com/nodejs/node/pull/43784 * src: NodeArrayBufferAllocator delegates to v8's allocator https://github.com/nodejs/node/pull/43594 * chore: update patch indices * chore: update filenames * src: refactor IsSupportedAuthenticatedMode https://github.com/nodejs/node/pull/42368 * src: add --openssl-legacy-provider option https://github.com/nodejs/node/pull/40478 * lib,src: add source map support for global eval https://github.com/nodejs/node/pull/43428 * trace_events: trace net connect event https://github.com/nodejs/node/pull/43903 * deps: update ICU to 71.1 https://github.com/nodejs/node/pull/42655 This fails the test because it's missing https://chromium-review.googlesource.com/c/chromium/deps/icu/+/3841093 * lib: give names to promisified exists() and question() https://github.com/nodejs/node/pull/43218 * crypto: add CFRG curves to Web Crypto API https://github.com/nodejs/node/pull/42507 * src: fix memory leak for v8.serialize https://github.com/nodejs/node/pull/42695 This test does not work for Electron as they do not use V8's ArrayBufferAllocator. * buffer: fix atob input validation https://github.com/nodejs/node/pull/42539 * src: fix ssize_t error from nghttp2.h https://github.com/nodejs/node/pull/44393 Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
2022-08-29 13:55:36 +00:00
diff --git a/src/base_object.h b/src/base_object.h
index 842f763a56d75c55509534e3d44a8080dd283127..b6078fe83c82a5edec0f7652b8c2d1b6c2491ca4 100644
--- a/src/base_object.h
+++ b/src/base_object.h
@@ -40,7 +40,7 @@ class TransferData;
class BaseObject : public MemoryRetainer {
public:
- enum InternalFields { kSlot, kInternalFieldCount };
+ enum InternalFields { kWrapperType, kSlot, kInternalFieldCount };
chore: bump node to v16.17.0 (main) (#35350) * chore: bump node in DEPS to v16.17.0 * chore: fixup asar patch * lib: use null-prototype objects for property descriptors https://github.com/nodejs/node/pull/43270 * src: make SecureContext fields private https://github.com/nodejs/node/pull/43173 * crypto: remove Node.js-specific webcrypto extensions https://github.com/nodejs/node/pull/43310 * test: refactor to top-level await https://github.com/nodejs/node/pull/43500 * deps: cherry-pick two libuv fixes https://github.com/nodejs/node/pull/43950 * src: slim down env-inl.h https://github.com/nodejs/node/pull/43745 * util: add AggregateError.prototype.errors to inspect output https://github.com/nodejs/node/pull/43646 * esm: improve performance & tidy tests https://github.com/nodejs/node/pull/43784 * src: NodeArrayBufferAllocator delegates to v8's allocator https://github.com/nodejs/node/pull/43594 * chore: update patch indices * chore: update filenames * src: refactor IsSupportedAuthenticatedMode https://github.com/nodejs/node/pull/42368 * src: add --openssl-legacy-provider option https://github.com/nodejs/node/pull/40478 * lib,src: add source map support for global eval https://github.com/nodejs/node/pull/43428 * trace_events: trace net connect event https://github.com/nodejs/node/pull/43903 * deps: update ICU to 71.1 https://github.com/nodejs/node/pull/42655 This fails the test because it's missing https://chromium-review.googlesource.com/c/chromium/deps/icu/+/3841093 * lib: give names to promisified exists() and question() https://github.com/nodejs/node/pull/43218 * crypto: add CFRG curves to Web Crypto API https://github.com/nodejs/node/pull/42507 * src: fix memory leak for v8.serialize https://github.com/nodejs/node/pull/42695 This test does not work for Electron as they do not use V8's ArrayBufferAllocator. * buffer: fix atob input validation https://github.com/nodejs/node/pull/42539 * src: fix ssize_t error from nghttp2.h https://github.com/nodejs/node/pull/44393 Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
2022-08-29 13:55:36 +00:00
// Associates this object with `object`. It uses the 0th internal field for
// that, and in particular aborts if there is no such field.
diff --git a/src/env.cc b/src/env.cc
index 22be69ec30a5b8466caacc698c791494891e5dee..cc44d578df9e146aa72f8273c1271d6a3c00d610 100644
--- a/src/env.cc
+++ b/src/env.cc
@@ -2119,11 +2119,20 @@ void Environment::RunWeakRefCleanup() {
isolate()->ClearKeptObjects();
}
+// This just has to be different from the Chromium ones:
+// https://source.chromium.org/chromium/chromium/src/+/main:gin/public/gin_embedders.h;l=18-23;drc=5a758a97032f0b656c3c36a3497560762495501a
+// Otherwise, when Node is loaded in an isolate which uses cppgc, cppgc will
+// misinterpret the data stored in the embedder fields and try to garbage
+// collect them.
chore: bump node to v16.17.0 (main) (#35350) * chore: bump node in DEPS to v16.17.0 * chore: fixup asar patch * lib: use null-prototype objects for property descriptors https://github.com/nodejs/node/pull/43270 * src: make SecureContext fields private https://github.com/nodejs/node/pull/43173 * crypto: remove Node.js-specific webcrypto extensions https://github.com/nodejs/node/pull/43310 * test: refactor to top-level await https://github.com/nodejs/node/pull/43500 * deps: cherry-pick two libuv fixes https://github.com/nodejs/node/pull/43950 * src: slim down env-inl.h https://github.com/nodejs/node/pull/43745 * util: add AggregateError.prototype.errors to inspect output https://github.com/nodejs/node/pull/43646 * esm: improve performance & tidy tests https://github.com/nodejs/node/pull/43784 * src: NodeArrayBufferAllocator delegates to v8's allocator https://github.com/nodejs/node/pull/43594 * chore: update patch indices * chore: update filenames * src: refactor IsSupportedAuthenticatedMode https://github.com/nodejs/node/pull/42368 * src: add --openssl-legacy-provider option https://github.com/nodejs/node/pull/40478 * lib,src: add source map support for global eval https://github.com/nodejs/node/pull/43428 * trace_events: trace net connect event https://github.com/nodejs/node/pull/43903 * deps: update ICU to 71.1 https://github.com/nodejs/node/pull/42655 This fails the test because it's missing https://chromium-review.googlesource.com/c/chromium/deps/icu/+/3841093 * lib: give names to promisified exists() and question() https://github.com/nodejs/node/pull/43218 * crypto: add CFRG curves to Web Crypto API https://github.com/nodejs/node/pull/42507 * src: fix memory leak for v8.serialize https://github.com/nodejs/node/pull/42695 This test does not work for Electron as they do not use V8's ArrayBufferAllocator. * buffer: fix atob input validation https://github.com/nodejs/node/pull/42539 * src: fix ssize_t error from nghttp2.h https://github.com/nodejs/node/pull/44393 Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
2022-08-29 13:55:36 +00:00
+uint16_t kNodeEmbedderId = 0x90de;
+
chore: bump node to v16.17.0 (main) (#35350) * chore: bump node in DEPS to v16.17.0 * chore: fixup asar patch * lib: use null-prototype objects for property descriptors https://github.com/nodejs/node/pull/43270 * src: make SecureContext fields private https://github.com/nodejs/node/pull/43173 * crypto: remove Node.js-specific webcrypto extensions https://github.com/nodejs/node/pull/43310 * test: refactor to top-level await https://github.com/nodejs/node/pull/43500 * deps: cherry-pick two libuv fixes https://github.com/nodejs/node/pull/43950 * src: slim down env-inl.h https://github.com/nodejs/node/pull/43745 * util: add AggregateError.prototype.errors to inspect output https://github.com/nodejs/node/pull/43646 * esm: improve performance & tidy tests https://github.com/nodejs/node/pull/43784 * src: NodeArrayBufferAllocator delegates to v8's allocator https://github.com/nodejs/node/pull/43594 * chore: update patch indices * chore: update filenames * src: refactor IsSupportedAuthenticatedMode https://github.com/nodejs/node/pull/42368 * src: add --openssl-legacy-provider option https://github.com/nodejs/node/pull/40478 * lib,src: add source map support for global eval https://github.com/nodejs/node/pull/43428 * trace_events: trace net connect event https://github.com/nodejs/node/pull/43903 * deps: update ICU to 71.1 https://github.com/nodejs/node/pull/42655 This fails the test because it's missing https://chromium-review.googlesource.com/c/chromium/deps/icu/+/3841093 * lib: give names to promisified exists() and question() https://github.com/nodejs/node/pull/43218 * crypto: add CFRG curves to Web Crypto API https://github.com/nodejs/node/pull/42507 * src: fix memory leak for v8.serialize https://github.com/nodejs/node/pull/42695 This test does not work for Electron as they do not use V8's ArrayBufferAllocator. * buffer: fix atob input validation https://github.com/nodejs/node/pull/42539 * src: fix ssize_t error from nghttp2.h https://github.com/nodejs/node/pull/44393 Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
2022-08-29 13:55:36 +00:00
// Not really any better place than env.cc at this moment.
BaseObject::BaseObject(Environment* env, Local<Object> object)
: persistent_handle_(env->isolate(), object), env_(env) {
CHECK_EQ(false, object.IsEmpty());
- CHECK_GT(object->InternalFieldCount(), 0);
+ CHECK_GT(object->InternalFieldCount(), BaseObject::kSlot);
+ object->SetAlignedPointerInInternalField(BaseObject::kWrapperType,
+ &kNodeEmbedderId);
chore: bump node to v16.17.0 (main) (#35350) * chore: bump node in DEPS to v16.17.0 * chore: fixup asar patch * lib: use null-prototype objects for property descriptors https://github.com/nodejs/node/pull/43270 * src: make SecureContext fields private https://github.com/nodejs/node/pull/43173 * crypto: remove Node.js-specific webcrypto extensions https://github.com/nodejs/node/pull/43310 * test: refactor to top-level await https://github.com/nodejs/node/pull/43500 * deps: cherry-pick two libuv fixes https://github.com/nodejs/node/pull/43950 * src: slim down env-inl.h https://github.com/nodejs/node/pull/43745 * util: add AggregateError.prototype.errors to inspect output https://github.com/nodejs/node/pull/43646 * esm: improve performance & tidy tests https://github.com/nodejs/node/pull/43784 * src: NodeArrayBufferAllocator delegates to v8's allocator https://github.com/nodejs/node/pull/43594 * chore: update patch indices * chore: update filenames * src: refactor IsSupportedAuthenticatedMode https://github.com/nodejs/node/pull/42368 * src: add --openssl-legacy-provider option https://github.com/nodejs/node/pull/40478 * lib,src: add source map support for global eval https://github.com/nodejs/node/pull/43428 * trace_events: trace net connect event https://github.com/nodejs/node/pull/43903 * deps: update ICU to 71.1 https://github.com/nodejs/node/pull/42655 This fails the test because it's missing https://chromium-review.googlesource.com/c/chromium/deps/icu/+/3841093 * lib: give names to promisified exists() and question() https://github.com/nodejs/node/pull/43218 * crypto: add CFRG curves to Web Crypto API https://github.com/nodejs/node/pull/42507 * src: fix memory leak for v8.serialize https://github.com/nodejs/node/pull/42695 This test does not work for Electron as they do not use V8's ArrayBufferAllocator. * buffer: fix atob input validation https://github.com/nodejs/node/pull/42539 * src: fix ssize_t error from nghttp2.h https://github.com/nodejs/node/pull/44393 Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
2022-08-29 13:55:36 +00:00
object->SetAlignedPointerInInternalField(BaseObject::kSlot,
static_cast<void*>(this));
env->AddCleanupHook(DeleteMe, static_cast<void*>(this));
@@ -2177,7 +2186,8 @@ void BaseObject::MakeWeak() {
void BaseObject::LazilyInitializedJSTemplateConstructor(
chore: bump node to v16.17.0 (main) (#35350) * chore: bump node in DEPS to v16.17.0 * chore: fixup asar patch * lib: use null-prototype objects for property descriptors https://github.com/nodejs/node/pull/43270 * src: make SecureContext fields private https://github.com/nodejs/node/pull/43173 * crypto: remove Node.js-specific webcrypto extensions https://github.com/nodejs/node/pull/43310 * test: refactor to top-level await https://github.com/nodejs/node/pull/43500 * deps: cherry-pick two libuv fixes https://github.com/nodejs/node/pull/43950 * src: slim down env-inl.h https://github.com/nodejs/node/pull/43745 * util: add AggregateError.prototype.errors to inspect output https://github.com/nodejs/node/pull/43646 * esm: improve performance & tidy tests https://github.com/nodejs/node/pull/43784 * src: NodeArrayBufferAllocator delegates to v8's allocator https://github.com/nodejs/node/pull/43594 * chore: update patch indices * chore: update filenames * src: refactor IsSupportedAuthenticatedMode https://github.com/nodejs/node/pull/42368 * src: add --openssl-legacy-provider option https://github.com/nodejs/node/pull/40478 * lib,src: add source map support for global eval https://github.com/nodejs/node/pull/43428 * trace_events: trace net connect event https://github.com/nodejs/node/pull/43903 * deps: update ICU to 71.1 https://github.com/nodejs/node/pull/42655 This fails the test because it's missing https://chromium-review.googlesource.com/c/chromium/deps/icu/+/3841093 * lib: give names to promisified exists() and question() https://github.com/nodejs/node/pull/43218 * crypto: add CFRG curves to Web Crypto API https://github.com/nodejs/node/pull/42507 * src: fix memory leak for v8.serialize https://github.com/nodejs/node/pull/42695 This test does not work for Electron as they do not use V8's ArrayBufferAllocator. * buffer: fix atob input validation https://github.com/nodejs/node/pull/42539 * src: fix ssize_t error from nghttp2.h https://github.com/nodejs/node/pull/44393 Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
2022-08-29 13:55:36 +00:00
const FunctionCallbackInfo<Value>& args) {
DCHECK(args.IsConstructCall());
- DCHECK_GT(args.This()->InternalFieldCount(), 0);
+ DCHECK_GT(args.This()->InternalFieldCount(), BaseObject::kSlot);
+ args.This()->SetAlignedPointerInInternalField(BaseObject::kWrapperType, &kNodeEmbedderId);
args.This()->SetAlignedPointerInInternalField(BaseObject::kSlot, nullptr);
}