Skip to content

Conversation

@santigimeno
Copy link
Member

It contains the following commits:

deps: V8: cherry-pick beebee4f80ff

Original commit message:

cpu-profiler: Use Handle version of SourcePositionTableIterator The surrounding code can trigger an allocation through InliningStack which can eventually end up allocating a line ends array. This is fine as-is because the existing iterator code makes a copy of the byte array. It just triggers the no_gc dcheck in debug mode. Fixed: v8:10778 Change-Id: Ic8c502767ec6c3d3b1f5e84df60638bd2fc6be75 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2339102 Auto-Submit: Peter Marshall <[email protected]> Commit-Queue: Tobias Tebbi <[email protected]> Reviewed-by: Tobias Tebbi <[email protected]> Cr-Commit-Position: refs/heads/master@{#69247} 

Refs: v8/v8@beebee4

test: add cpu-profiler-crash test

This test is added as it usually crashes without applying the v8 patch:
v8/v8@beebee4

@santigimenosantigimeno self-assigned this Feb 9, 2021
@nodejs-github-botnodejs-github-bot added v14.x v8 engine Issues and PRs related to the V8 dependency. labels Feb 9, 2021
@santigimenosantigimenoforce-pushed the santi/backport_v8_10778_bugfix branch from cde372c to af26f41CompareFebruary 9, 2021 15:12
@richardlau
Copy link
Member

Can you target the staging branch (v14.x-staging)? Does this also apply to 15.x/master?

@santigimenosantigimeno changed the base branch from v14.x to v14.x-stagingFebruary 9, 2021 16:26
@santigimeno
Copy link
MemberAuthor

santigimeno commented Feb 9, 2021

Can you target the staging branch (v14.x-staging)?

Done

Does this also apply to 15.x/master?

No, the fix is already there though it would be great if it could be applied to 12.x at some point.

Thanks!

@richardlau

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

This comment has been minimized.

Copy link
Member

@gengjiawengengjiawen left a comment

Choose a reason for hiding this comment

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

You forget update common.gypi in first commit.

@santigimenosantigimenoforce-pushed the santi/backport_v8_10778_bugfix branch from af26f41 to f582bd8CompareFebruary 21, 2021 19:57
@santigimeno
Copy link
MemberAuthor

@gengjiawen thanks for pointing that out! Just updated the PR.

@juanarbol
Copy link
Member

I could work on the v12.x backport

psmarshalland others added 2 commits March 1, 2021 10:42
Original commit message: ``` cpu-profiler: Use Handle version of SourcePositionTableIterator The surrounding code can trigger an allocation through InliningStack which can eventually end up allocating a line ends array. This is fine as-is because the existing iterator code makes a copy of the byte array. It just triggers the no_gc dcheck in debug mode. Fixed: v8:10778 Change-Id: Ic8c502767ec6c3d3b1f5e84df60638bd2fc6be75 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2339102 Auto-Submit: Peter Marshall <[email protected]> Commit-Queue: Tobias Tebbi <[email protected]> Reviewed-by: Tobias Tebbi <[email protected]> Cr-Commit-Position: refs/heads/master@{#69247} ``` Refs: v8/v8@beebee4
This test is added as it usually crashes without applying the v8 patch: v8/v8@beebee4
@santigimenosantigimenoforce-pushed the santi/backport_v8_10778_bugfix branch from f582bd8 to 7908e99CompareMarch 1, 2021 09:42
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 1, 2021

@nodejs-github-bot
Copy link
Collaborator

richardlau pushed a commit that referenced this pull request Mar 2, 2021
Original commit message: ``` cpu-profiler: Use Handle version of SourcePositionTableIterator The surrounding code can trigger an allocation through InliningStack which can eventually end up allocating a line ends array. This is fine as-is because the existing iterator code makes a copy of the byte array. It just triggers the no_gc dcheck in debug mode. Fixed: v8:10778 Change-Id: Ic8c502767ec6c3d3b1f5e84df60638bd2fc6be75 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2339102 Auto-Submit: Peter Marshall <[email protected]> Commit-Queue: Tobias Tebbi <[email protected]> Reviewed-by: Tobias Tebbi <[email protected]> Cr-Commit-Position: refs/heads/master@{#69247} ``` Refs: v8/v8@beebee4 PR-URL: #37293 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Richard Lau <[email protected]>
richardlau pushed a commit that referenced this pull request Mar 2, 2021
This test is added as it usually crashes without applying the v8 patch: v8/v8@beebee4 PR-URL: #37293 Refs: v8/v8@beebee4 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Richard Lau <[email protected]>
@richardlau
Copy link
Member

Landed in 292dffe...69057eb.

@santigimenosantigimeno deleted the santi/backport_v8_10778_bugfix branch March 2, 2021 17:23
juanarbol pushed a commit to juanarbol/node that referenced this pull request Mar 2, 2021
Original commit message: ``` cpu-profiler: Use Handle version of SourcePositionTableIterator The surrounding code can trigger an allocation through InliningStack which can eventually end up allocating a line ends array. This is fine as-is because the existing iterator code makes a copy of the byte array. It just triggers the no_gc dcheck in debug mode. Fixed: v8:10778 Change-Id: Ic8c502767ec6c3d3b1f5e84df60638bd2fc6be75 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2339102 Auto-Submit: Peter Marshall <[email protected]> Commit-Queue: Tobias Tebbi <[email protected]> Reviewed-by: Tobias Tebbi <[email protected]> Cr-Commit-Position: refs/heads/master@{#69247} ``` Refs: v8/v8@beebee4 PR-URL: nodejs#37293 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Richard Lau <[email protected]>
juanarbol pushed a commit to juanarbol/node that referenced this pull request Mar 2, 2021
This test is added as it usually crashes without applying the v8 patch: v8/v8@beebee4 PR-URL: nodejs#37293 Refs: v8/v8@beebee4 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Richard Lau <[email protected]>
juanarbol pushed a commit to juanarbol/node that referenced this pull request Mar 2, 2021
Original commit message: ``` cpu-profiler: Use Handle version of SourcePositionTableIterator The surrounding code can trigger an allocation through InliningStack which can eventually end up allocating a line ends array. This is fine as-is because the existing iterator code makes a copy of the byte array. It just triggers the no_gc dcheck in debug mode. Fixed: v8:10778 Change-Id: Ic8c502767ec6c3d3b1f5e84df60638bd2fc6be75 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2339102 Auto-Submit: Peter Marshall <[email protected]> Commit-Queue: Tobias Tebbi <[email protected]> Reviewed-by: Tobias Tebbi <[email protected]> Cr-Commit-Position: refs/heads/master@{#69247} ``` Refs: v8/v8@beebee4 PR-URL: nodejs#37293 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Richard Lau <[email protected]>
juanarbol pushed a commit to juanarbol/node that referenced this pull request Mar 2, 2021
This test is added as it usually crashes without applying the v8 patch: v8/v8@beebee4 PR-URL: nodejs#37293 Refs: v8/v8@beebee4 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Richard Lau <[email protected]>
richardlau pushed a commit that referenced this pull request Mar 16, 2021
Original commit message: ``` cpu-profiler: Use Handle version of SourcePositionTableIterator The surrounding code can trigger an allocation through InliningStack which can eventually end up allocating a line ends array. This is fine as-is because the existing iterator code makes a copy of the byte array. It just triggers the no_gc dcheck in debug mode. Fixed: v8:10778 Change-Id: Ic8c502767ec6c3d3b1f5e84df60638bd2fc6be75 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2339102 Auto-Submit: Peter Marshall <[email protected]> Commit-Queue: Tobias Tebbi <[email protected]> Reviewed-by: Tobias Tebbi <[email protected]> Cr-Commit-Position: refs/heads/master@{#69247} ``` Refs: v8/v8@beebee4 PR-URL: #37293 Backport-PR-URL: #37578 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Richard Lau <[email protected]>
richardlau pushed a commit that referenced this pull request Mar 16, 2021
This test is added as it usually crashes without applying the v8 patch: v8/v8@beebee4 PR-URL: #37293 Backport-PR-URL: #37578 Refs: v8/v8@beebee4 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Richard Lau <[email protected]>
@richardlaurichardlau mentioned this pull request Mar 18, 2021
MylesBorins pushed a commit that referenced this pull request Apr 6, 2021
Original commit message: ``` cpu-profiler: Use Handle version of SourcePositionTableIterator The surrounding code can trigger an allocation through InliningStack which can eventually end up allocating a line ends array. This is fine as-is because the existing iterator code makes a copy of the byte array. It just triggers the no_gc dcheck in debug mode. Fixed: v8:10778 Change-Id: Ic8c502767ec6c3d3b1f5e84df60638bd2fc6be75 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2339102 Auto-Submit: Peter Marshall <[email protected]> Commit-Queue: Tobias Tebbi <[email protected]> Reviewed-by: Tobias Tebbi <[email protected]> Cr-Commit-Position: refs/heads/master@{#69247} ``` Refs: v8/v8@beebee4 PR-URL: #37293 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Richard Lau <[email protected]>
MylesBorins pushed a commit that referenced this pull request Apr 6, 2021
This test is added as it usually crashes without applying the v8 patch: v8/v8@beebee4 PR-URL: #37293 Refs: v8/v8@beebee4 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Richard Lau <[email protected]>
@danielleadamsdanielleadams mentioned this pull request May 3, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v8 engineIssues and PRs related to the V8 dependency.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@santigimeno@richardlau@nodejs-github-bot@juanarbol@gengjiawen@psmarshall