Skip to content
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

deps: get new v8 building for ARM64 Windows #26087

Closed
wants to merge 19 commits into from

Conversation

jkunkee
Copy link
Contributor

@jkunkee jkunkee commented Feb 13, 2019

This should be rebased and merged after #25852 is merged. It backports two fixes and works around another issue fixed in upstream v8.

Checklist

targos and others added 19 commits January 31, 2019 14:51
Major V8 updates are usually API/ABI incompatible with previous
versions. This commit adapts NODE_MODULE_VERSION for V8 7.3.

Refs: https://github.com/nodejs/CTC/blob/master/meetings/2016-09-28.md
Co-authored-by: Ujjwal Sharma <[email protected]>

win: add v8_init to dependencies

Fixes: nodejs/node-v8#89
Co-authored-by: Bartosz Sosnowski <[email protected]>
Original commit message:

    [snapshot] Always align embedded blob code pointer and size

    Other platforms besides ARM64 Windows may also have alignment
    requirements, e.g. PPC and s390. These requirements may affect
    both the code pointer field and the size field, and so they
    each need alignment directives because they are stored in
    different sections.

    Since aligning wastes a handful of bytes at most, not making
    alignment conditional on the platform type seems like a good idea.

    Refs: nodejs#24875
    Change-Id: I1f58606af294be65e74a1f107cd05fc21e032704
    Reviewed-on: https://chromium-review.googlesource.com/c/1433778
    Commit-Queue: Jakob Gruber <[email protected]>
    Reviewed-by: Jakob Gruber <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#59058}

Refs: v8/v8@fc0ddf5
Bump minimum version of ICU needed to build node to 63.

Refs: v8/v8@30a350f
Co-authored-by: Steven R Loomis <[email protected]>
New heap space: code_large_object_space
Adapt to changes in async stack traces and function name inference
The V8 7.3 update requires the following adjustments to the
postmortem debugging metadata constants:

- v8dbg_prop_idx_first was removed in
  v8/v8@1ad0cd5

- v8dbg_jsarray_buffer_was_neutered_mask was renamed to
  v8dbg_jsarray_buffer_was_detached_mask in
  v8/v8@f68ee6e

- v8dbg_jsarray_buffer_was_neutered_shift was renamed to
  v8dbg_jsarray_buffer_was_detached_shift in
  v8/v8@f68ee6e

- v8dbg_class_Map__instance_descriptors__DescriptorArray moved to
  v8dbg_class_Map__raw_instance_descriptors__DescriptorArray and
  began using ACCESSORS2 in
  v8/v8@799dfad

The following postmortem debugging constants were also impacted
by V8's introduction of ACCESSORS2, but do not need to be updated
the test:

- v8dbg_class_ThinString__actual__String changed in
  v8/v8@0f581e4

- v8dbg_class_UncompiledData__inferred_name__String changed in
  v8/v8@0f581e4

- v8dbg_class_JSFunction__shared__SharedFunctionInfo changed in
  v8/v8@8162090

- v8dbg_class_SharedFunctionInfo__function_data__Object had its
  accessor removed in
  v8/v8@a55803a
  This has been fixed in gen-postmortem-metadata.py.
This is a backport of a fix already in upstream v8 that addresses
errors about TurboAssembler referencing deleted functions.
When source line information was added to v8's mksnapshot process,
ARM64 was missed. This change adds the missing SourceInfo and
DeclareExternalFilename definitions.

This change has already been upstreamed.
When building this revision of v8 as a component build using the MSVC
toolchain targeting ARM64, the link phase emits a missing symbol error
about XRegFromCode. This change moves the code that generates the
unfulfilled reference to its corresponding .cc file to avoid the error.

This no longer repros in upstream v8.
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. v8 engine Issues and PRs related to the V8 dependency. labels Feb 13, 2019
This was referenced Feb 13, 2019
@richardlau richardlau added the blocked PRs that are blocked by other issues or PRs. label Feb 13, 2019
@richardlau
Copy link
Member

For the backports, could you follow the guidelines in https://github.com/nodejs/node/blob/master/doc/guides/maintaining-V8.md?

@jkunkee
Copy link
Contributor Author

jkunkee commented Feb 14, 2019

@richardlau - Absolutely. Thanks for the pointer.

@jkunkee
Copy link
Contributor Author

jkunkee commented Feb 14, 2019

Looks like 7.3, the 7 Feb 2019 release, is still maintained upstream, so the backports can be done upstream.

@jkunkee
Copy link
Contributor Author

jkunkee commented Feb 14, 2019

v8 bug: Merge and backport new fixes
v8 bug: Backport existing fix

@jkunkee jkunkee closed this Mar 8, 2019
@jkunkee jkunkee deleted the arm64-v8-patches branch March 8, 2019 02:50
@jkunkee
Copy link
Contributor Author

jkunkee commented Mar 8, 2019

This is closed because #25852 now incorporates my changes. Note that the upstream change to v7.3, https://chromium-review.googlesource.com/c/v8/v8/+/1506201 , has not yet been approved and merged, but should be soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked PRs that are blocked by other issues or PRs. build Issues and PRs related to build files or the CI. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants