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: update V8 to 11.6 #48710

Closed
wants to merge 19 commits into from
Closed

deps: update V8 to 11.6 #48710

wants to merge 19 commits into from

Conversation

targos
Copy link
Member

@targos targos commented Jul 9, 2023

No description provided.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/gyp
  • @nodejs/v8-update

@targos targos mentioned this pull request Jul 9, 2023
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency. labels Jul 9, 2023
@targos targos added semver-major PRs that contain breaking changes and should be released in the next major version. request-ci Add this label to start a Jenkins CI on a PR. labels Jul 9, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 9, 2023
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 9, 2023

@targos targos added the help wanted Issues that need assistance from volunteers or PRs that need help to proceed. label Jul 9, 2023
@targos
Copy link
Member Author

targos commented Jul 9, 2023

test-asan build failure was identified in canary, but not fixed yet.

In file included from ../deps/v8/src/init/setup-isolate-full.cc:8:
In file included from ../deps/v8/src/heap/heap-inl.h:32:
In file included from ../deps/v8/src/heap/new-spaces-inl.h:12:
In file included from ../deps/v8/src/heap/paged-spaces-inl.h:13:
In file included from ../deps/v8/src/objects/objects-inl.h:32:
In file included from ../deps/v8/src/objects/js-proxy-inl.h:8:
In file included from ../deps/v8/src/objects/instance-type-inl.h:11:
In file included from ../deps/v8/src/objects/map-inl.h:9:
In file included from ../deps/v8/src/objects/api-callbacks-inl.h:13:
In file included from ../deps/v8/src/objects/js-objects-inl.h:18:
In file included from ../deps/v8/src/objects/lookup-inl.h:14:
In file included from ../deps/v8/src/heap/factory-inl.h:13:
In file included from ../deps/v8/src/execution/isolate-inl.h:9:
In file included from ../deps/v8/src/objects/contexts-inl.h:17:
../deps/v8/src/objects/ordered-hash-table-inl.h:210:3: error: member reference type 'Tagged' is not a pointer
  WRITE_BARRIER(*this, entry_offset, value);
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../deps/v8/src/objects/object-macros.h:514:50: note: expanded from macro 'WRITE_BARRIER'
    CombinedWriteBarrier(object, Tagged(object)->RawField(offset), value, \
                                 ~~~~~~~~~~~~~~  ^
1 error generated.
make[2]: *** [tools/v8_gypfiles/v8_init.target.mk:182: /home/runner/work/node/node/out/Release/obj.target/v8_init/deps/v8/src/init/setup-isolate-full.o] Error 1

@nodejs/cpp-reviewers

@targos

This comment was marked as resolved.

@RaisinTen

This comment was marked as resolved.

@targos targos added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 10, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 10, 2023
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 10, 2023

@targos
Copy link
Member Author

targos commented Jul 10, 2023

V8 CI is broken (looks like we're still trying to run it with Python 2.7)

@nodejs nodejs deleted a comment from nodejs-github-bot Jul 10, 2023
@nodejs nodejs deleted a comment from nodejs-github-bot Jul 10, 2023
@nodejs nodejs deleted a comment from nodejs-github-bot Jul 10, 2023
@nodejs-github-bot
Copy link
Collaborator

@richardlau
Copy link
Member

V8 CI is broken (looks like we're still trying to run it with Python 2.7)

Yes, also reported in #48660 (comment).

I tried a quick patch to try to get the CI to use Python 3 over the weekend and it looks like we're missing some required Python modules on the Python 3 installation: https://ci.nodejs.org/job/node-test-commit-v8-linux/5435/

joyeecheung and others added 10 commits July 31, 2023 14:35
Original commit message:

    Fix mistake in the skip branch of test/mjsunit/regress-1320641.js

    It was doing a `string.test(regex)` which was wrong. It's supposed
    to be `regex.test(string)`. It wasn't caught in the CI because
    the skip path is not normally taken in the V8 CI.

    Change-Id: Id1bdab5bbc41968bba8adc1cb3664e8f95fb5d72
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4697855
    Commit-Queue: Joyee Cheung <joyee@igalia.com>
    Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#89044}

Refs: v8/v8@9f4b769
PR-URL: nodejs#48830
Refs: v8/v8@c1a54d5
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
Original commit message:

    [cppgc] expose wrapper descriptor on CppHeap

    This makes it possible for embedders to:

    1. Avoid creating wrapper objects that happen to have a layout that
      leads V8 to consider the object cppgc-managed while it's not.
      Refs: nodejs#43521
    2. Create cppgc-managed wrapper objects when they do not own the
       CppHeap. Refs: nodejs#45704

    Bug: v8:13960
    Change-Id: If31f4d56c5ead59dc0d56f937494d23d631f7438
    Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4598833
    Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
    Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#88490}

Refs: v8/v8@9327503
PR-URL: nodejs#48660
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Accept a new `step` break message.
`--no-harmony-sharedarraybuffer` was removed from V8 but it's still
possible to disable the feature with `--enable-sharedarraybuffer-per-context`.
@targos
Copy link
Member Author

targos commented Jul 31, 2023

cdebfb6 fixed it on my mac.

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 31, 2023

@targos targos removed the build Issues and PRs related to build files or the CI. label Jul 31, 2023
@nodejs nodejs deleted a comment from nodejs-github-bot Aug 1, 2023
@nodejs nodejs deleted a comment from nodejs-github-bot Aug 1, 2023
@nodejs nodejs deleted a comment from nodejs-github-bot Aug 1, 2023
@targos
Copy link
Member Author

targos commented Aug 1, 2023

We're back to the zlib-related error:

https://ci.nodejs.org/job/node-test-commit-linux-containered/38647/nodes=ubuntu1804_sharedlibs_shared_x64/console

16:46:43 /home/iojs/build/workspace/node-test-commit-linux-containered/out/Release/obj.target/v8_zlib/deps/v8/third_party/zlib/cpu_features.o:(.bss.x86_cpu_enable_avx512+0x0): multiple definition of `x86_cpu_enable_avx512'

https://ci.nodejs.org/job/node-test-commit-smartos/50446/nodes=smartos20-64/console

16:48:20 ld: fatal: symbol 'x86_cpu_enable_avx512' is multiply-defined:
16:48:20 	(file /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos20-64/out/Release/obj.target/deps/zlib/libzlib.a(cpu_features.o) type=OBJT; file /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos20-64/out/Release/obj.target/tools/v8_gypfiles/libv8_zlib.a(cpu_features.o) type=OBJT);

@targos
Copy link
Member Author

targos commented Aug 7, 2023

/cc @nodejs/tsc This is essentially blocked on one compiler issue that I don't know how to handle (see above comment).

@gengjiawen
Copy link
Member

We're back to the zlib-related error:

ci.nodejs.org/job/node-test-commit-linux-containered/38647/nodes=ubuntu1804_sharedlibs_shared_x64/console

16:46:43 /home/iojs/build/workspace/node-test-commit-linux-containered/out/Release/obj.target/v8_zlib/deps/v8/third_party/zlib/cpu_features.o:(.bss.x86_cpu_enable_avx512+0x0): multiple definition of `x86_cpu_enable_avx512'

ci.nodejs.org/job/node-test-commit-smartos/50446/nodes=smartos20-64/console

16:48:20 ld: fatal: symbol 'x86_cpu_enable_avx512' is multiply-defined:
16:48:20 	(file /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos20-64/out/Release/obj.target/deps/zlib/libzlib.a(cpu_features.o) type=OBJT; file /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos20-64/out/Release/obj.target/tools/v8_gypfiles/libv8_zlib.a(cpu_features.o) type=OBJT);

I suppose #47493 should help in this case.

@targos
Copy link
Member Author

targos commented Aug 16, 2023

Yeah, but that PR is unfortunately stale.

@targos
Copy link
Member Author

targos commented Sep 16, 2023

Replaced by #49639

@targos targos closed this Sep 16, 2023
@targos targos deleted the v8-116 branch April 5, 2024 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Issues that need assistance from volunteers or PRs that need help to proceed. needs-ci PRs that need a full CI run. semver-major PRs that contain breaking changes and should be released in the next major version. 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