Skip to content

Commit 9a057a4

Browse files
committed
build: fix what the sixth node.yml run found
win32 got past the ICU object the fifth run fixed - genccode now writes it as IMAGE_FILE_MACHINE_I386, and the log confirms it: "genccode: using architecture cpu=332 bits=32 big-endian=0". Then the linker refused the object it had just been taught to write: lld-link : error : /safeseh: ../obj/global_intermediate/icudt78l_dat.obj is not compatible with SEH [tools\v8_gypfiles\gen-regexp-special-case.vcxproj] icudt78l_dat.obj is generated DATA. It holds no code, so it has no safe exception handler table and nothing can give it one; opting out of /SAFESEH is the only way to link it. That is not a new discovery - node.gyp already carried ImageHasSafeExceptionHandlers: 'false', restored with 32-bit Windows and originally from nodejs#25852. It was in the wrong place. A target_defaults in node.gyp reaches only the targets NODE.GYP DEFINES, and this object is linked by targets in tools/v8_gypfiles/v8.gyp and tools/icu/icu-generic.gyp - gen-regexp-special-case is one of V8's build tools - which never saw the setting. Nothing had to opt those in before because gyp turns /SAFESEH on BY ITSELF for every x86 link (safeseh_default = "true" when the arch is x86, in tools/gyp/pylib/gyp/msvs_emulation.py), and upstream stopped building the one architecture where that default means anything. The opt-out moves to common.gypi, scoped to ia32 - the only architecture /SAFESEH applies to, which the node.gyp copy was not. tools/gyp_node.py passes common.gypi to gyp with -I, and LoadTargetBuildFile passes its includes down to every dependency build file it loads (tools/gyp/pylib/gyp/input.py), so one rule there covers node, V8 and ICU alike. NOT verified: the build itself, which needs a Windows runner - there is none here, and the other five jobs of the sixth run are still going, so only win32's log exists to read. What was checked: that common.gypi and node.gyp are still valid gyp literals and the ia32 linker condition now reads {'TargetMachine': 1, 'ImageHasSafeExceptionHandlers': 'false'}; that the vendored gyp converts that to the MSBuild Link setting with no validation errors, which is what MSBuild turns into /SAFESEH:NO; that gyp really does default /SAFESEH on for x86; and that gyp really does hand -I includes down to dependency build files, which is the whole reason common.gypi is the right place for it. Thanks to xet7 !
1 parent ea3fc8a commit 9a057a4

2 files changed

Lines changed: 30 additions & 8 deletions

File tree

common.gypi

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,32 @@
402402
'conditions': [
403403
['target_arch=="ia32"', {
404404
'TargetMachine' : 1, # /MACHINE:X86
405+
# /SAFESEH:NO, and it has to be HERE rather than in node.gyp.
406+
#
407+
# gyp turns /SAFESEH on by ITSELF for every x86 link -
408+
# tools/gyp/pylib/gyp/msvs_emulation.py sets
409+
# safeseh_default = "true" when the arch is x86 - so opting out is
410+
# not a preference, it is the only way to link an object that
411+
# carries no safe-exception-handler table. ICU's genccode writes
412+
# exactly such an object: icudt<ver>l_dat.obj is generated data
413+
# with no code and no SEH table in it, and the sixth workflow run
414+
# died on it - "lld-link : error : /safeseh:
415+
# ../obj/global_intermediate/icudt78l_dat.obj is not compatible
416+
# with SEH" - while linking gen-regexp-special-case, one of V8's
417+
# build tools.
418+
#
419+
# node.gyp already carried this setting, restored with 32-bit
420+
# Windows, but a target_defaults in node.gyp reaches only the
421+
# targets node.gyp DEFINES. The object is linked by targets in
422+
# tools/v8_gypfiles/v8.gyp and tools/icu/icu-generic.gyp, which
423+
# never saw it. common.gypi is included into every .gyp in the tree
424+
# (tools/gyp_node.py passes it with -I), so one rule here covers
425+
# node, V8 and ICU alike - and it is scoped to ia32, which is the
426+
# only architecture /SAFESEH means anything on.
427+
#
428+
# Refs: https://github.com/nodejs/node/pull/25852 and
429+
# https://docs.microsoft.com/en-us/cpp/build/reference/safeseh-image-has-safe-exception-handlers
430+
'ImageHasSafeExceptionHandlers': 'false',
405431
}],
406432
['target_arch=="x64"', {
407433
'TargetMachine' : 17, # /MACHINE:X64

node.gyp

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -548,14 +548,10 @@
548548
'libraries': ['-latomic'],
549549
}],
550550
],
551-
# Relevant only for x86, restored with 32-bit Windows (removed upstream in
552-
# 7ad0cc3e571). Refs: https://github.com/nodejs/node/pull/25852 and
553-
# https://docs.microsoft.com/en-us/cpp/build/reference/safeseh-image-has-safe-exception-handlers
554-
'msvs_settings': {
555-
'VCLinkerTool': {
556-
'ImageHasSafeExceptionHandlers': 'false',
557-
},
558-
},
551+
# The x86 /SAFESEH:NO opt-out that used to sit here - restored with 32-bit
552+
# Windows, removed upstream in 7ad0cc3e571 - moved to common.gypi, where it
553+
# reaches V8's and ICU's targets as well as node's own. See the comment
554+
# there for what it is for and why it has to be global.
559555
},
560556

561557
'targets': [

0 commit comments

Comments
 (0)