Nodejs v12 support by mmarchini · Pull Request #330 · nodejs/llnode

mmarchini · 2020-01-16T18:04:17Z

This PR contains all commits necessary to make llnode work on v12, plus changes to CI to run tests on v12, drop v8 and v11, and bump the major version since we're dropping support and adding new one. I made a PR on nodejs/node to update the list of postmortem constants used, but I noticed some of the constants here already changes on v13, so we'll have to work on that.

V8 changed DescriptorArray from a FixedArray to a proper HeapObject. These changes update accessors for DescriptorArray fields to make them compatible with FixedArray-like and HeapObject-like access. Ref: nodejs#255

V8 changed the function name inference algorithm, which affected one of our test cases. Even if it's reverted/fixed upstream, it won't make it's way into v12, so skip that particular test for this version. Ref: https://bugs.chromium.org/p/v8/issues/detail?id=9807

Some Node.js v12 versions will have String postmortem metadata as `String__FIELD_offset__int` instead of `String__FIELD_offset__TYPE`. Handle both cases so llnode can work on more versions.

Instead of crashing if llnode fails to parse a Symbol, just return a ??? token.

Ref: nodejs/llnode#330

mmarchini · 2020-01-16T22:11:37Z

cc cc @nodejs/llnode @nodejs/diagnostics

gireeshpunathil

RSLTGM

mhdawson

Ruber stampish LGTM

Ref: nodejs/llnode#330 PR-URL: #31391 Refs: nodejs/llnode#330 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

V8 changed DescriptorArray from a FixedArray to a proper HeapObject. These changes update accessors for DescriptorArray fields to make them compatible with FixedArray-like and HeapObject-like access. Ref: #255 PR-URL: #330 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

V8 changed the function name inference algorithm, which affected one of our test cases. Even if it's reverted/fixed upstream, it won't make it's way into v12, so skip that particular test for this version. Ref: https://bugs.chromium.org/p/v8/issues/detail?id=9807 PR-URL: #330 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

PR-URL: #330 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

Some Node.js v12 versions will have String postmortem metadata as `String__FIELD_offset__int` instead of `String__FIELD_offset__TYPE`. Handle both cases so llnode can work on more versions. PR-URL: #330 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

Instead of crashing if llnode fails to parse a Symbol, just return a ??? token. PR-URL: #330 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

PR-URL: #330 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

mmarchini · 2020-01-21T22:15:32Z

Landed in 878b514...e8896e0

Ref: nodejs/llnode#330 PR-URL: #31391 Refs: nodejs/llnode#330 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>

coveralls · 2024-02-06T10:10:32Z

Pull Request Test Coverage Report for Build 330c52db684732798ed6adca9233a96591ae7829-PR-330

Warning: This coverage report may be inaccurate.

We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
To ensure accuracy in future PRs, please see these guidelines.
A quick fix for this PR: rebase it; your next report should be accurate.

192 of 286 (67.13%) changed or added relevant lines in 9 files are covered.
107 unchanged lines in 5 files lost coverage.
Overall coverage increased (+3.6%) to 78.609%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/llv8-constants.h	1	2	50.0%
test/plugin/frame-test.js	3	8	37.5%
src/llv8-constants.cc	48	60	80.0%
src/llv8.cc	17	30	56.67%
src/printer.cc	37	56	66.07%
src/llv8-inl.h	78	122	63.93%

Files with Coverage Reduction	New Missed Lines	%
src/llv8-constants.h	1	95.59%
src/printer.cc	1	76.9%
src/llv8.h	2	78.57%
src/llv8-constants.cc	5	81.96%
src/llv8-inl.h	98	69.44%

Totals
Change from base Build 878b514e2919e1adf7974694bf36cdd7286cdec8:	3.6%
Covered Lines:	3707
Relevant Lines:	4703

💛 - Coveralls

mmarchini added 9 commits January 16, 2020 10:02

src: v12.x-compatible DescriptorArray

b2e6346

V8 changed DescriptorArray from a FixedArray to a proper HeapObject. These changes update accessors for DescriptorArray fields to make them compatible with FixedArray-like and HeapObject-like access. Ref: nodejs#255

src: v12 fixed for JSArrayBuffer & FixedTypedArray

b164bff

src: handle String__FIELD_offset__int

8491953

Some Node.js v12 versions will have String postmortem metadata as `String__FIELD_offset__int` instead of `String__FIELD_offset__TYPE`. Handle both cases so llnode can work on more versions.

src: make Symbol more resilient

ece9589

Instead of crashing if llnode fails to parse a Symbol, just return a ??? token.

src: use 24 as default for kHeaderSize

dec5479

src: guess Symbol.name offset

2d21dc8

build: update Node.js versions on CI matrix

94f2de0

3.0.0

dd863a3

mmarchini mentioned this pull request Jan 16, 2020

Upcoming metadata changes on 7.9 #331

Closed

mmarchini added a commit to mmarchini/node that referenced this pull request Jan 16, 2020

test: update postmortem test with v12 constants

cf6eeb7

Ref: nodejs/llnode#330

mmarchini mentioned this pull request Jan 16, 2020

test: update postmortem test with v12 constants nodejs/node#31391

Closed

4 tasks

mmarchini added a commit to mmarchini/node that referenced this pull request Jan 16, 2020

test: update postmortem test with v12 constants

a5043ca

Ref: nodejs/llnode#330

mmarchini marked this pull request as ready for review January 16, 2020 22:10

mmarchini mentioned this pull request Jan 16, 2020

src: v12.x-compatible DescriptorArray #325

Closed

gireeshpunathil approved these changes Jan 17, 2020

View reviewed changes

mhdawson approved these changes Jan 17, 2020

View reviewed changes

mmarchini added a commit that referenced this pull request Jan 21, 2020

src: v12 fixed for JSArrayBuffer & FixedTypedArray

f8eebcc

PR-URL: #330 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

mmarchini added a commit that referenced this pull request Jan 21, 2020

src: use 24 as default for kHeaderSize

b30cf41

PR-URL: #330 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

mmarchini added a commit that referenced this pull request Jan 21, 2020

src: guess Symbol.name offset

c981f7f

PR-URL: #330 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

mmarchini added a commit that referenced this pull request Jan 21, 2020

build: update Node.js versions on CI matrix

b0fa68c

PR-URL: #330 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

mmarchini added a commit that referenced this pull request Jan 21, 2020

3.0.0

e8896e0

PR-URL: #330 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

mmarchini closed this Jan 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Nodejs v12 support#330

Nodejs v12 support#330
mmarchini wants to merge 9 commits intonodejs:masterfrom
mmarchini:nodejs-v12-support

mmarchini commented Jan 16, 2020 •

edited

Loading

Uh oh!

mmarchini commented Jan 16, 2020

Uh oh!

gireeshpunathil left a comment

Uh oh!

mhdawson left a comment

Uh oh!

mmarchini commented Jan 21, 2020

Uh oh!

coveralls commented Feb 6, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

mmarchini commented Jan 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mmarchini commented Jan 16, 2020

Uh oh!

gireeshpunathil left a comment

Choose a reason for hiding this comment

Uh oh!

mhdawson left a comment

Choose a reason for hiding this comment

Uh oh!

mmarchini commented Jan 21, 2020

Uh oh!

coveralls commented Feb 6, 2024

Pull Request Test Coverage Report for Build 330c52db684732798ed6adca9233a96591ae7829-PR-330

Warning: This coverage report may be inaccurate.

💛 - Coveralls

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mmarchini commented Jan 16, 2020 •

edited

Loading