targos
commented
9 months ago 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/
Open codebytere opened 9 months ago
targos
commented
9 months ago 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/
codebytere
commented
9 months ago huh, interesting 🤔 i'm digging into it because electron is seeing the following issue when running es-module/test-vm-compile-function-lineoffset.js, but only with compression enabled:
```
#
# Fatal error in ../../v8/src/objects/smi.h, line 37
# Debug check failed: Smi::IsValid(value).
#
#
#
#FailureMessage Object: 0x16f730b38
1: 0x12293f5ec node::NodePlatform::GetStackTracePrinter()::$_0::__invoke() [/Users/codebytere/Developer/electron-gn/src/out/Testing/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
2: 0x11f36c42c V8_Fatal(char const*, int, char const*, ...) [/Users/codebytere/Developer/electron-gn/src/out/Testing/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework]
3: 0x11f36c068 std::__Cr::enable_if::type>::value && !std::is_enum
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:
node on git:main ❯ /Users/codebytere/Developer/node/out/Release/mksnapshot --turbo_instruction_scheduling "--target_os=mac" "--target_arch=arm64" --startup_src /Users/codebytere/Developer/node/out/Release/obj/tools/v8_gypfiles/v8_snapshot.gen/snapshot.cc --embedded_variant Default --embedded_src /Users/codebytere/Developer/node/out/Release/obj/tools/v8_gypfiles/v8_snapshot.gen/embedded.S --no-native-code-counters
[1] 6473 bus error /Users/codebytere/Developer/node/out/Release/mksnapshot "--target_os=mac"
kvakil
commented
9 months ago 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.
kvakil
commented
8 months ago 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
#endifSo V8_HEAP_USE_PTHREAD_JIT_WRITE_PROTECT will be false since Node's configuration doesn't enable V8_EXTERNAL_CODE_SPACE. Without V8_HEAP_USE_PTHREAD_JIT_WRITE_PROTECT, the code will fallback to using an alternate way to achieve W^X by switching the page permissions:
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_CAGEAnyway, 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-compressionninja -C out/ReleaseHow 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
``` node on git:main ❯ ninja -C out/Release 2:39PM Running "ninja -C out/Release" ninja: Entering directory `out/Release' [1/1712] CC obj/deps/v8/third_party/zlib/v8_zlib.cpu_features.o ../../deps/v8/third_party/zlib/cpu_features.c:99:13: warning: unused function '_cpu_check_features' [-Wunused-function] static void _cpu_check_features(void) ^ 1 warning generated. [16/1712] CC obj/deps/v8/third_party/zlib/v8_zlib.deflate.o ../../deps/v8/third_party/zlib/deflate.c:2003:31: warning: comparison of integers of different signs: 'IPos' (aka 'unsigned int') and 'int' [-Wsign-compare] if (s->prev_match == -1) { ~~~~~~~~~~~~~ ^ ~~ 1 warning generated. [99/1712] ACTION icudata: icudata_cc7672230cb6f4a4cb1b079ff599d36f generating assembly code for /Users/codebytere/Developer/node/out/Release/gen/icudt74.dat [1097/1709] LIBTOOL-STATIC libv8_base_without_compiler.a, POSTBUILDS /Users/codebytere/.electron_build_tools/third_party/Xcode/Xcode-15.0.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning renaming duplicate member name 'v8_base_without_compiler.allocation.o' from 'obj/deps/v8/src/heap/cppgc/v8_base_without_compiler.allocation.o(v8_base_without_compiler.allocation.o)' and 'obj/deps/v8/src/utils/v8_base_without_compiler.allocation.o(v8_base_without_compiler.allocation.o)' /Users/codebytere/.electron_build_tools/third_party/Xcode/Xcode-15.0.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning renaming duplicate member name 'v8_base_without_compiler.factory.o' from 'obj/tools/v8_gypfiles/gen/torque-generated/v8_base_without_compiler.factory.o(v8_base_without_compiler.factory.o)' and 'obj/deps/v8/src/heap/v8_base_without_compiler.factory.o(v8_base_without_compiler.factory.o)' /Users/codebytere/.electron_build_tools/third_party/Xcode/Xcode-15.0.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning renaming duplicate member name 'v8_base_without_compiler.free-list.o' from 'obj/deps/v8/src/heap/cppgc/v8_base_without_compiler.free-list.o(v8_base_without_compiler.free-list.o)' and 'obj/deps/v8/src/heap/v8_base_without_compiler.free-list.o(v8_base_without_compiler.free-list.o)' /Users/codebytere/.electron_build_tools/third_party/Xcode/Xcode-15.0.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning renaming duplicate member name 'v8_base_without_compiler.heap.o' from 'obj/deps/v8/src/heap/cppgc/v8_base_without_compiler.heap.o(v8_base_without_compiler.heap.o)' and 'obj/deps/v8/src/heap/v8_base_without_compiler.heap.o(v8_base_without_compiler.heap.o)' /Users/codebytere/.electron_build_tools/third_party/Xcode/Xcode-15.0.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning renaming duplicate member name 'v8_base_without_compiler.objects-printer.o' from 'obj/deps/v8/src/diagnostics/v8_base_without_compiler.objects-printer.o(v8_base_without_compiler.objects-printer.o)' and 'obj/tools/v8_gypfiles/gen/torque-generated/v8_base_without_compiler.objects-printer.o(v8_base_without_compiler.objects-printer.o)' /Users/codebytere/.electron_build_tools/third_party/Xcode/Xcode-15.0.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning renaming duplicate member name 'v8_base_without_compiler.sweeper.o' from 'obj/deps/v8/src/heap/v8_base_without_compiler.sweeper.o(v8_base_without_compiler.sweeper.o)' and 'obj/deps/v8/src/heap/cppgc/v8_base_without_compiler.sweeper.o(v8_base_without_compiler.sweeper.o)' /Users/codebytere/.electron_build_tools/third_party/Xcode/Xcode-15.0.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/libtool: warning renaming duplicate member name 'v8_base_without_compiler.testing.o' from 'obj/deps/v8/src/sandbox/v8_base_without_compiler.testing.o(v8_base_without_compiler.testing.o)' and 'obj/deps/v8/src/heap/cppgc/v8_base_without_compiler.testing.o(v8_base_without_compiler.testing.o)' [1646/1709] ACTION generating: "obj/tools/v8_g.../tools/v8_gypfiles/v8_snapshot.gen/embedded.S" FAILED: obj/tools/v8_gypfiles/v8_snapshot.gen/snapshot.cc obj/tools/v8_gypfiles/v8_snapshot.gen/embedded.S cd ../../tools/v8_gypfiles; export BUILT_FRAMEWORKS_DIR=/Users/codebytere/Developer/node/out/Release; export BUILT_PRODUCTS_DIR=/Users/codebytere/Developer/node/out/Release; export CONFIGURATION=Release; export EXECUTABLE_NAME=libv8_snapshot.a; export EXECUTABLE_PATH=libv8_snapshot.a; export FULL_PRODUCT_NAME=libv8_snapshot.a; export MACH_O_TYPE=staticlib; export PRODUCT_NAME=v8_snapshot; export PRODUCT_TYPE=com.apple.product-type.library.static; export SDKROOT=/Users/codebytere/.electron_build_tools/third_party/Xcode/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk; export SRCROOT=/Users/codebytere/Developer/node/out/Release/../../tools/v8_gypfiles; export SOURCE_ROOT="${SRCROOT}"; export TARGET_BUILD_DIR=/Users/codebytere/Developer/node/out/Release; export TEMP_DIR="${TMPDIR}"; export XCODE_VERSION_ACTUAL=1500;/Users/codebytere/Developer/node/out/Release/mksnapshot --turbo_instruction_scheduling "--target_os=mac" "--target_arch=arm64" --startup_src /Users/codebytere/Developer/node/out/Release/obj/tools/v8_gypfiles/v8_snapshot.gen/snapshot.cc --embedded_variant Default --embedded_src /Users/codebytere/Developer/node/out/Release/obj/tools/v8_gypfiles/v8_snapshot.gen/embedded.S --no-native-code-counters /bin/sh: line 1: 94091 Bus error: 10 /Users/codebytere/Developer/node/out/Release/mksnapshot --turbo_instruction_scheduling "--target_os=mac" "--target_arch=arm64" --startup_src /Users/codebytere/Developer/node/out/Release/obj/tools/v8_gypfiles/v8_snapshot.gen/snapshot.cc --embedded_variant Default --embedded_src /Users/codebytere/Developer/node/out/Release/obj/tools/v8_gypfiles/v8_snapshot.gen/embedded.S --no-native-code-counters [1657/1709] CXX obj/src/quic/libnode.session.o ninja: build stopped: subcommand failed. ERROR Failed to run command: Exit Code: "1" ```
Additional information
It looks like it's failing on snapshot generation - @joyeecheung maybe you have some ideas?