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

[v8.x backport] deps: backport 9a23bdd from upstream V8 #22418

Closed
wants to merge 1 commit into from

Conversation

Drieger
Copy link
Contributor

@Drieger Drieger commented Aug 20, 2018

Original commit message:

[Isolate] Fix Isolate::PrintCurrentStackTrace for interpreted frames

Previously we were getting the code object from the stack, so printed incorrect
position details for interpreted frames.

BUG=v8:7916

Change-Id: I2f87584117d88b7db3f3b9bdbfe793c4d3e33fe9
Reviewed-on: https://chromium-review.googlesource.com/1126313
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54253}

Refs: v8/v8@9a23bdd
Refs: #22338
Fixes: #21988

Checklist

[x] make -j4 test (UNIX), or vcbuild test (Windows) passes
[x] commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added v8 engine Issues and PRs related to the V8 dependency. v8.x labels Aug 20, 2018
@Drieger Drieger changed the title [v8.x backport] deps: cherry-pick 9a23bdd from upstream V8 [v8.x backport] deps: backport 9a23bdd from upstream V8 Aug 21, 2018
@mmarchini
Copy link
Contributor

@nodejs/v8 @nodejs/v8-update @nodejs/release PTAL

Copy link
Contributor

@ryzokuken ryzokuken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@richardlau richardlau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please bump the patch level version in v8-version.h as per the maintaining V8 guide:

https://github.com/nodejs/node/blob/master/doc/guides/maintaining-V8.md#backporting-to-abandoned-branches

@Drieger
Copy link
Contributor Author

Drieger commented Aug 21, 2018

@richardlau updated patch level in v8-version.h

@ofrobots
Copy link
Contributor

The change LGTM, but the general policy is that we allow fixes to be released on Current first before back-porting to LTS. AFAICT, this has not been released on Current yet.

@mmarchini mmarchini added the blocked PRs that are blocked by other issues or PRs. label Aug 22, 2018
@mmarchini
Copy link
Contributor

Marked as blocked until #22338 lands or https://bugs.chromium.org/p/v8/issues/detail?id=7916 gets backported upstream.

@Drieger Drieger closed this Sep 10, 2018
@Drieger Drieger reopened this Sep 10, 2018
Original commit message:

    [Isolate] Fix Isolate::PrintCurrentStackTrace for interpreted frames

    Previously we were getting the code object from the stack, so printed incorrect
    position details for interpreted frames.

    BUG=v8:7916

    Change-Id: I2f87584117d88b7db3f3b9bdbfe793c4d3e33fe9
    Reviewed-on: https://chromium-review.googlesource.com/1126313
    Reviewed-by: Toon Verwaest <verwaest@chromium.org>
    Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#54253}

Refs: v8/v8@9a23bdd
Fixes: nodejs#21988
@mmarchini mmarchini removed the blocked PRs that are blocked by other issues or PRs. label Sep 13, 2018
BethGriggs pushed a commit that referenced this pull request Oct 2, 2018
Original commit message:

    [Isolate] Fix Isolate::PrintCurrentStackTrace for interpreted frames

    Previously we were getting the code object from the stack, so printed incorrect
    position details for interpreted frames.

    BUG=v8:7916

    Change-Id: I2f87584117d88b7db3f3b9bdbfe793c4d3e33fe9
    Reviewed-on: https://chromium-review.googlesource.com/1126313
    Reviewed-by: Toon Verwaest <verwaest@chromium.org>
    Commit-Queue: Ross McIlroy <rmcilroy@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#54253}

Refs: v8/v8@9a23bdd
Fixes: #21988

PR-URL: #22418
Refs: #22338
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
@BethGriggs
Copy link
Member

Landed in 9e2077a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants