On 7/11/26 13:09, Volker Schlecht wrote: > > On 7/10/26 10:43 AM, Fabien Romano wrote: > [...] >> https://chromium-review.googlesource.com/c/v8/v8/+/8063803 > [...] > >> For now, I think we have to disable regexp JIT. > Attached is the diff I'm currently testing. Did I get it right? > I am inclined to commit this sooner rather than later, because it > looks like this will fix the build issues we've been seeing for the > chromiums. Sorry, I'm slow at testing all that stuff. Random build failure seems unrelated to stack feature WASM uses. So the diff looks good for the reliability issue. I tried the upstream fix, and it works fine, as expected. The regexp JIT, someone could check arm64 before we disable all arch. If it succeeds on arm64, we may disable all arch except this one. I need to invest more time learning this topic; see the small report below. Then about how we disable JIT. Thinking twice, we should just switch default. JIT still built in the engine so someone can do --no-regexp-interpret-all. The first patch I sent made those read-only, which may produce (seen with d8): Flag processing error: Contradictory value for readonly flag --regexp-interpret-all The problem is I lost my last node build, oops. So maybe nodes treat those differently. Also, after checking carefully, regexp_interpret_all alone is enough. bool IrRegExpData::ShouldProduceBytecode() { return v8_flags.regexp_interpret_all || (v8_flags.regexp_tier_up && !MarkedForTierUp()); } bool RegExp::CanGenerateBytecode() { return v8_flags.regexp_interpret_all || v8_flags.regexp_tier_up; } My bad. There is another patch to test. Actually, I'm not sure which option I prefer (READONLY or not). Let me know what you think. To reproduce the crash : Node uses a hardcoded executable path; You may have to install the node binary you want to test. (Tests may use both system and build binaries.) I'm saying just in case because I was struggling to reproduce ... Until I reinstall node without the patch. (maybe unrelated, not sure) cd /usr/ports/lang/node make retest TEST_TARGET="JOBS=1 JS_SUITES=parallel/test-worker-heap-snapshot NATIVE_SUITES= TEST_CI_ARGS='--repeat 100 -p verbose' jstest" Starting release test-worker-heap-snapshot... Done running release test-worker-heap-snapshot: pass Starting release test-worker-heap-snapshot... Done running release test-worker-heap-snapshot: CRASH Starting release test-worker-heap-snapshot... Done running release test-worker-heap-snapshot: pass On amd64: cd `make show=WRKSRC` doas -u _pbuild egdb out/Release/node node.core Core was generated by `node'. Program terminated with signal SIGTRAP, Trace/breakpoint trap. #0 0x00000092c52fcb37 in v8::internal::RegExpMacroAssemblerX64::CheckStackGuardState(unsigned long*, unsigned long, unsigned long, unsigned long) () [Current thread is 1 (process 294920) (gdb) disassemble /r $pc-32,$pc Dump of assembler code from 0x92c52fcb17 to 0x92c52fcb37: 0x00000092c52fcb17 <_ZN2v88internal23RegExpMacroAssemblerX6420CheckStackGuardStateEPmmmm+167>: 4e ff 48 83 rex.WRX decq -0x7d(%rax) 0x00000092c52fcb1b <_ZN2v88internal23RegExpMacroAssemblerX6420CheckStackGuardStateEPmmmm+171>: c4 (bad) 0x00000092c52fcb1c <_ZN2v88internal23RegExpMacroAssemblerX6420CheckStackGuardStateEPmmmm+172>: 30 5b 41 xor %bl,0x41(%rbx) 0x00000092c52fcb1f <_ZN2v88internal23RegExpMacroAssemblerX6420CheckStackGuardStateEPmmmm+175>: 5c pop %rsp 0x00000092c52fcb20 <_ZN2v88internal23RegExpMacroAssemblerX6420CheckStackGuardStateEPmmmm+176>: 41 5d pop %r13 0x00000092c52fcb22 <_ZN2v88internal23RegExpMacroAssemblerX6420CheckStackGuardStateEPmmmm+178>: 41 5e pop %r14 0x00000092c52fcb24 <_ZN2v88internal23RegExpMacroAssemblerX6420CheckStackGuardStateEPmmmm+180>: 41 5f pop %r15 0x00000092c52fcb26 <_ZN2v88internal23RegExpMacroAssemblerX6420CheckStackGuardStateEPmmmm+182>: 41 5b pop %r11 0x00000092c52fcb28 <_ZN2v88internal23RegExpMacroAssemblerX6420CheckStackGuardStateEPmmmm+184>: 5d pop %rbp 0x00000092c52fcb29 <_ZN2v88internal23RegExpMacroAssemblerX6420CheckStackGuardStateEPmmmm+185>: 4c 33 1c 24 xor (%rsp),%r11 0x00000092c52fcb2d <_ZN2v88internal23RegExpMacroAssemblerX6420CheckStackGuardStateEPmmmm+189>: 4c 3b 1d dc c7 41 01 cmp 0x141c7dc(%rip),%r11 # 0x92c6719310 <__retguard_658> 0x00000092c52fcb34 <_ZN2v88internal23RegExpMacroAssemblerX6420CheckStackGuardStateEPmmmm+196>: 74 09 je 0x92c52fcb3f <_ZN2v88internal23RegExpMacroAssemblerX6420CheckStackGuardStateEPmmmm+207> 0x00000092c52fcb36 <_ZN2v88internal23RegExpMacroAssemblerX6420CheckStackGuardStateEPmmmm+198>: cc int3 End of assembler dump. The %rsp changes came from v8/src/regexp/regexp-macro-assembler.cc All architecture behaves differently, but they share this return address change. int NativeRegExpMacroAssembler::CheckStackGuardState( Isolate* isolate, int start_index, RegExp::CallOrigin call_origin, Address* return_address, Tagged<InstructionStream> re_code, Address* subject, const uint8_t** input_start, const uint8_t** input_end, uintptr_t gap) { ... if (!code_handle->SafeEquals(re_code)) { // Return address no longer valid // Overwrite the return address on the stack. intptr_t delta = code_handle->address() - re_code.address(); Address new_pc = old_pc + delta; // TODO(v8:10026): avoid replacing a signed pointer. PointerAuthentication::ReplacePC(return_address, new_pc, 0); } ... } Arm64 uses a stub to call native functions, and this stub doesn't use retguard. If I'm reading this correctly, it should be able to change the return address. Because it will not change the C++ function but the stub one. This is complex and hard to reproduce, but I got this crash in a build. That's why I think we have to disable this regexp JIT for now. >> ./devel/codex >> ./lang/deno >> ./lang/node >> ./www/chromium >> ./www/iridium >> ./www/ungoogled-chromium I have no idea how long for the WASM fix to land upstream. I think the process should be producing patches for chromium first. There is a lang/v8 (150.0.7871.100) port for test purposes only. I didn't try to apply other patches, which should fix tests. I didn't review test results yet. I left a previous test.log run on amd64 inside the port. cd lang/v8 && make makesum && make MAKE_JOBS=8 test If you are missing dependencies, this should be enough (more than needed). cd www/chromium && make prepare === === 26 tests failed === >>> 15940 base tests produced 14817 (92%) non-filtered tests >>> 14817 tests ran >> ./x11/qt5/qtwebengine This one is the oldest and looks not affected by new patches. >> ./x11/qt6/qtwebengine USE_NOBTCFI=yes, v8_enable_cet_ibt=false This one isn't affected by the WASM jump table fix. Other patches may apply. >> >> Did I miss one? > Don't think so. lang/deno and devel/codex are chasing the latest v8 > and are typically very close to the chromiums. I don't know about > qtwebengine, but lang/node is much more conservative however. And maybe someday we will also have www/electron. Problems are more about the ecosystem than Electron itself.
Index: Makefile
===================================================================
RCS file: /mnt/ext/cvs/ports/lang/node/Makefile,v
diff -u -p -r1.169 Makefile
--- Makefile 30 Jun 2026 18:46:22 -0000 1.169
+++ Makefile 12 Jul 2026 14:41:40 -0000
@@ -13,6 +13,7 @@ DIST_TUPLE = github qbit node-pledge 1.
DISTNAME = node-${NODE_VERSION}
PKGNAME = ${DISTNAME:S/v//g}
EPOCH = 0
+REVISION = 0
CATEGORIES = lang devel
Index: patches/patch-deps_v8_src_flags_flag-definitions_h
===================================================================
RCS file: patches/patch-deps_v8_src_flags_flag-definitions_h
diff -N patches/patch-deps_v8_src_flags_flag-definitions_h
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-deps_v8_src_flags_flag-definitions_h 12 Jul 2026 14:49:26 -0000
@@ -0,0 +1,19 @@
+Disable regexp JIT by default.
+
+Index: deps/v8/src/flags/flag-definitions.h
+--- deps/v8/src/flags/flag-definitions.h.orig
++++ deps/v8/src/flags/flag-definitions.h
+@@ -2835,7 +2835,13 @@ DEFINE_BOOL(serialization_statistics, false,
+ "Collect statistics on serialized objects.")
+ // Regexp
+ DEFINE_BOOL(regexp_optimization, true, "generate optimized regexp code")
++#ifdef V8_OS_OPENBSD
++// OpenBSD: Irregexp JIT rewrites a C++ return address, which conflicts with
++// retguard. Use bytecode interpretation by default.
++DEFINE_BOOL(regexp_interpret_all, true, "interpret all regexp code")
++#else
+ DEFINE_BOOL(regexp_interpret_all, false, "interpret all regexp code")
++#endif
+ #ifdef V8_TARGET_BIG_ENDIAN
+ #define REGEXP_PEEPHOLE_OPTIMIZATION_BOOL false
+ #else
Index: patches/patch-deps_v8_src_wasm_jump-table-assembler_cc
===================================================================
RCS file: patches/patch-deps_v8_src_wasm_jump-table-assembler_cc
diff -N patches/patch-deps_v8_src_wasm_jump-table-assembler_cc
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-deps_v8_src_wasm_jump-table-assembler_cc 12 Jul 2026 14:41:40 -0000
@@ -0,0 +1,28 @@
+See https://chromium-review.googlesource.com/c/v8/v8/+/8063803
+Index: deps/v8/src/wasm/jump-table-assembler.cc
+--- deps/v8/src/wasm/jump-table-assembler.cc.orig
++++ deps/v8/src/wasm/jump-table-assembler.cc
+@@ -114,6 +114,10 @@ void JumpTableAssembler::EmitLazyCompileJumpSlot(uint3
+ }
+
+ bool JumpTableAssembler::EmitJumpSlot(Address target) {
++ intptr_t displacement = target - (pc_ + kJumpTableSlotEntryMarkerSize +
++ MacroAssembler::kIntraSegmentJmpInstrSize);
++ if (!is_int32(displacement)) return false;
++
+ #ifdef V8_ENABLE_CET_IBT
+ uint32_t endbr_insn = 0xfa1e0ff3;
+ uint32_t nop = 0x00401f0f;
+@@ -122,11 +126,7 @@ bool JumpTableAssembler::EmitJumpSlot(Address target)
+ emit<uint32_t>(nop, kRelaxedStore);
+ #endif
+
+- intptr_t displacement =
+- target - (pc_ + MacroAssembler::kIntraSegmentJmpInstrSize);
+- if (!is_int32(displacement)) return false;
+-
+- uint8_t inst[kJumpTableSlotSize] = {
++ uint8_t inst[8] = {
+ 0xe9, 0, 0, 0, 0, // near_jmp displacement
+ 0xcc, 0xcc, 0xcc, // int3 * 3
+ };
Index: patches/patch-deps_v8_src_wasm_jump-table-assembler_cc.orig
===================================================================
RCS file: patches/patch-deps_v8_src_wasm_jump-table-assembler_cc.orig
diff -N patches/patch-deps_v8_src_wasm_jump-table-assembler_cc.orig
Index: patches/patch-deps_v8_src_wasm_jump-table-assembler_h
===================================================================
RCS file: patches/patch-deps_v8_src_wasm_jump-table-assembler_h
diff -N patches/patch-deps_v8_src_wasm_jump-table-assembler_h
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-deps_v8_src_wasm_jump-table-assembler_h 12 Jul 2026 14:41:40 -0000
@@ -0,0 +1,15 @@
+See https://chromium-review.googlesource.com/c/v8/v8/+/8063803
+Index: deps/v8/src/wasm/jump-table-assembler.h
+--- deps/v8/src/wasm/jump-table-assembler.h.orig
++++ deps/v8/src/wasm/jump-table-assembler.h
+@@ -184,8 +184,10 @@ class V8_EXPORT_PRIVATE JumpTableAssembler {
+ #if V8_TARGET_ARCH_X64
+ #ifdef V8_ENABLE_CET_IBT
+ static constexpr int kJumpTableSlotSize = 16;
++ static constexpr int kJumpTableSlotEntryMarkerSize = 8;
+ #else // V8_ENABLE_CET_IBT
+ static constexpr int kJumpTableSlotSize = 8;
++ static constexpr int kJumpTableSlotEntryMarkerSize = 0;
+ #endif
+ static constexpr int kJumpTableLineSize = kJumpTableSlotSize;
+ static constexpr int kFarJumpTableSlotSize = 16;
Index: patches/patch-deps_v8_src_wasm_jump-table-assembler_h.orig
===================================================================
RCS file: patches/patch-deps_v8_src_wasm_jump-table-assembler_h.orig
diff -N patches/patch-deps_v8_src_wasm_jump-table-assembler_h.orig
No comments:
Post a Comment