-
Notifications
You must be signed in to change notification settings - Fork 29.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
--experimental-enable-pointer-compression
causes build failure
#51339
Comments
--experimental-enable-pointer-compression
causes build failure
May be specific to macOS or arm64? It doesn't seem to reproduce in CI: https://ci.nodejs.org/job/node-test-commit-linux-pointer-compression/294/ |
huh, interesting 🤔 i'm digging into it because electron is seeing the following issue when running Details
And i'm so far unable to repro in Node.js proper due to being blocked on this. Looks like it can also be seen with the above configuration simply by running:
|
I can also repro on arm64 macOS (but not on Linux):
So the mksnapshot crashes when trying to write to the code page for the first builtin. It looks like the page is marked non-writable, which is strange. |
Here's the problem: // pthread_jit_write_protect is only available on arm64 Mac.
#if defined(V8_HOST_ARCH_ARM64) && \
(defined(V8_OS_MACOS) || (defined(V8_OS_IOS) && TARGET_OS_SIMULATOR))
#define V8_HAS_PTHREAD_JIT_WRITE_PROTECT 1
#else
#define V8_HAS_PTHREAD_JIT_WRITE_PROTECT 0
#endif
#if V8_HAS_PTHREAD_JIT_WRITE_PROTECT && \
!(defined(V8_COMPRESS_POINTERS) && !defined(V8_EXTERNAL_CODE_SPACE))
#define V8_HEAP_USE_PTHREAD_JIT_WRITE_PROTECT true
#else
#define V8_HEAP_USE_PTHREAD_JIT_WRITE_PROTECT false
#endif So bool write_protect_code_memory() const {
if (V8_HAS_PTHREAD_JIT_WRITE_PROTECT) {
// On MacOS on ARM64 ("Apple M1"/Apple Silicon) code modification
// protection must be used. It can be achieved by one of the following
// approaches:
// 1) switching memory protection between RW-RX as on other architectures
// => return true,
// 2) fast W^X machinery (see V8_HEAP_USE_PTHREAD_JIT_WRITE_PROTECT) which
// doesn not require memory protection changes => return false.
return !V8_HEAP_USE_PTHREAD_JIT_WRITE_PROTECT;
}
return write_protect_code_memory_;
} It looks like it's already known upstream that this configuration is broken upstream: https://chromium-review.googlesource.com/c/v8/v8/+/5123137 Now, you could enable V8_EXTERNAL_CODE_SPACE, and then everything works fine and tests pass: diff --git a/configure.py b/configure.py
index 84b016cd85..df9aebdd3f 100755
--- a/configure.py
+++ b/configure.py
@@ -1502,6 +1502,7 @@ def configure_v8(o):
o['variables']['v8_use_siphash'] = 0 if options.without_siphash else 1
o['variables']['v8_enable_maglev'] = 1 if options.v8_enable_maglev else 0
o['variables']['v8_enable_pointer_compression'] = 1 if options.enable_pointer_compression else 0
+ o['variables']['v8_enable_external_code_space'] = 1 if options.enable_pointer_compression else 0
o['variables']['v8_enable_31bit_smis_on_64bit_arch'] = 1 if options.enable_pointer_compression else 0
o['variables']['v8_enable_shared_ro_heap'] = 0 if options.enable_pointer_compression or options.disable_shared_ro_heap else 1
o['variables']['v8_enable_extensible_ro_snapshot'] = 0
diff --git a/tools/v8_gypfiles/features.gypi b/tools/v8_gypfiles/features.gypi
index c768d7a0f1..f762a5990c 100644
--- a/tools/v8_gypfiles/features.gypi
+++ b/tools/v8_gypfiles/features.gypi
@@ -439,6 +439,9 @@
['v8_enable_shared_ro_heap==1', {
'defines': ['V8_SHARED_RO_HEAP',],
}],
+ ['v8_enable_external_code_space==1', {
+ 'defines': ['V8_EXTERNAL_CODE_SPACE',],
+ }],
['dcheck_always_on!=0', {
'defines': ['DEBUG',],
}], But this only works prior to #50680, since our pointer-compression build does not use the shared-isolate cage, and multi-cage pointer compression doesn't support the external code space: #ifdef V8_COMPRESS_POINTERS_IN_ISOLATE_CAGE
const Address cage_base_;
#ifdef V8_EXTERNAL_CODE_SPACE
// In case this configuration is necessary the code cage base must be saved too.
#error Multi-cage pointer compression with external code space is not supported
#endif // V8_EXTERNAL_CODE_SPACE
#endif // V8_COMPRESS_POINTERS_IN_ISOLATE_CAGE Anyway, not sure what the correct solution here, probably the right one is to either make PtrComprCageAccessScope work with external code space (and enable external code space), or change the pointer-compression build to not use multi-cage. |
Version
v22.0.0-pre
Platform
Darwin Shelleys-MacBook-Pro-2.local 23.2.0 Darwin Kernel Version 23.2.0: Wed Nov 15 21:53:18 PST 2023; root:xnu-10002.61.3~2/RELEASE_ARM64_T6000 arm64
Subsystem
No response
What steps will reproduce the bug?
./configure --ninja --experimental-enable-pointer-compression
ninja -C out/Release
How often does it reproduce? Is there a required condition?
Consistently with the above steps.
What is the expected behavior? Why is that the expected behavior?
A successful build with pointer compression enabled.
What do you see instead?
The following build failure:
Build Failure
Additional information
It looks like it's failing on snapshot generation - @joyeecheung maybe you have some ideas?
The text was updated successfully, but these errors were encountered: