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.

Sign up for GitHub

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

Jump to bottom

deps: cherry-pick 1420e44db0 from upstream V8 #17344

Closed
wants to merge 1 commit into from
Closed

deps: cherry-pick 1420e44db0 from upstream V8 #17344

wants to merge 1 commit into from

Conversation

TimothyGu
Copy link
Member

@TimothyGu TimothyGu commented Nov 27, 2017 •
edited
Loading

Original commit message:

[coverage] Correctly free DebugInfo in the absence of breakpoints

It's quite possible for DebugInfos to exist without the presence of a
bytecode array, since DebugInfos are created for all functions for which
we have a CoverageInfo. Free such objects properly.

Also move the corresponding deletion of CoverageInfos on unload up
before the early exit.

Bug: v8:6000
Change-Id: Idde45b222290aa8b6828b61ff2251918b8ed2aed
Reviewed-on: https://chromium-review.googlesource.com/664811
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48024}

Fixes crash when passing Profiler.startPreciseCoverage before
Debug.paused is received.

Refs: bcoe/c8#6 (comment)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

deps (V8)

@nodejs-github-bot nodejs-github-bot added the v8 engine Issues and PRs related to the V8 dependency. label Nov 27, 2017
@TimothyGu
Copy link
Member Author

TimothyGu commented Nov 27, 2017 •
edited
Loading

Regular CI: https://ci.nodejs.org/job/node-test-pull-request/11744/

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/1081/ (passes other than one known issue)

@targos
Copy link
Member

targos commented Nov 27, 2017

Please increase the embedder string and add https://github.com/v8/v8/commit/SHA to the commit message.

bnoordhuis
bnoordhuis approved these changes Nov 27, 2017
View reviewed changes
Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

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

LGTM. What Michaël means is this:

Refs: https://github.com/v8/v8/commit/1420e44db0ac3631687deb9fc6816ac97b9f499c

@TimothyGu
deps: cherry-pick 1420e44db0 from upstream V8
bddeb84 
Original commit message:

    [coverage] Correctly free DebugInfo in the absence of breakpoints

    It's quite possible for DebugInfos to exist without the presence of a
    bytecode array, since DebugInfos are created for all functions for which
    we have a CoverageInfo. Free such objects properly.

    Also move the corresponding deletion of CoverageInfos on unload up
    before the early exit.

    Bug: v8:6000
    Change-Id: Idde45b222290aa8b6828b61ff2251918b8ed2aed
    Reviewed-on: https://chromium-review.googlesource.com/664811
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Jakob Gruber <jgruber@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#48024}

Fixes crash when passing Profiler.startPreciseCoverage before
Debug.paused is received.

Refs: v8/v8@1420e44
Refs: bcoe/c8#6 (comment)
@TimothyGu
Copy link
Member Author

@targos Done; PTAL.

@TimothyGu TimothyGu added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 27, 2017
@TimothyGu
Copy link
Member Author

/cc @nodejs/v8 @schuay

TimothyGu added a commit to TimothyGu/node that referenced this pull request Nov 29, 2017
@TimothyGu
deps: cherry-pick 1420e44db0 from upstream V8
b28af4d 
Original commit message:

    [coverage] Correctly free DebugInfo in the absence of breakpoints

    It's quite possible for DebugInfos to exist without the presence of a
    bytecode array, since DebugInfos are created for all functions for which
    we have a CoverageInfo. Free such objects properly.

    Also move the corresponding deletion of CoverageInfos on unload up
    before the early exit.

    Bug: v8:6000
    Change-Id: Idde45b222290aa8b6828b61ff2251918b8ed2aed
    Reviewed-on: https://chromium-review.googlesource.com/664811
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Jakob Gruber <jgruber@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#48024}

Fixes crash when passing Profiler.startPreciseCoverage before
Debug.paused is received.

PR-URL: nodejs#17344
Refs: v8/v8@1420e44
Refs: bcoe/c8#6 (comment)
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@TimothyGu
Copy link
Member Author

Landed in b28af4d.

@TimothyGu TimothyGu closed this Nov 29, 2017
@TimothyGu TimothyGu deleted the c8 branch November 29, 2017 19:48
@TimothyGu TimothyGu mentioned this pull request Nov 29, 2017
Merged
@TimothyGu TimothyGu removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 29, 2017
@TimothyGu TimothyGu added baking-for-lts PRs that need to wait before landing in a LTS release. dont-land-on-v8.x and removed dont-land-on-v8.x baking-for-lts PRs that need to wait before landing in a LTS release. labels Dec 8, 2017
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
@TimothyGu @MylesBorins
deps: cherry-pick 1420e44db0 from upstream V8
78e650b 
Original commit message:

    [coverage] Correctly free DebugInfo in the absence of breakpoints

    It's quite possible for DebugInfos to exist without the presence of a
    bytecode array, since DebugInfos are created for all functions for which
    we have a CoverageInfo. Free such objects properly.

    Also move the corresponding deletion of CoverageInfos on unload up
    before the early exit.

    Bug: v8:6000
    Change-Id: Idde45b222290aa8b6828b61ff2251918b8ed2aed
    Reviewed-on: https://chromium-review.googlesource.com/664811
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Jakob Gruber <jgruber@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#48024}

Fixes crash when passing Profiler.startPreciseCoverage before
Debug.paused is received.

PR-URL: #17344
Refs: v8/v8@1420e44
Refs: bcoe/c8#6 (comment)
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 12, 2017
@TimothyGu @MylesBorins
deps: cherry-pick 1420e44db0 from upstream V8
9f282dd 
Original commit message:

    [coverage] Correctly free DebugInfo in the absence of breakpoints

    It's quite possible for DebugInfos to exist without the presence of a
    bytecode array, since DebugInfos are created for all functions for which
    we have a CoverageInfo. Free such objects properly.

    Also move the corresponding deletion of CoverageInfos on unload up
    before the early exit.

    Bug: v8:6000
    Change-Id: Idde45b222290aa8b6828b61ff2251918b8ed2aed
    Reviewed-on: https://chromium-review.googlesource.com/664811
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Jakob Gruber <jgruber@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#48024}

Fixes crash when passing Profiler.startPreciseCoverage before
Debug.paused is received.

PR-URL: #17344
Refs: v8/v8@1420e44
Refs: bcoe/c8#6 (comment)
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Dec 12, 2017
Merged
targos pushed a commit to targos/node that referenced this pull request Jan 15, 2018
@TimothyGu @targos
deps: cherry-pick 1420e44db0 from upstream V8
14f1116 
Original commit message:

    [coverage] Correctly free DebugInfo in the absence of breakpoints

    It's quite possible for DebugInfos to exist without the presence of a
    bytecode array, since DebugInfos are created for all functions for which
    we have a CoverageInfo. Free such objects properly.

    Also move the corresponding deletion of CoverageInfos on unload up
    before the early exit.

    Bug: v8:6000
    Change-Id: Idde45b222290aa8b6828b61ff2251918b8ed2aed
    Reviewed-on: https://chromium-review.googlesource.com/664811
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Jakob Gruber <jgruber@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#48024}

Fixes crash when passing Profiler.startPreciseCoverage before
Debug.paused is received.

PR-URL: nodejs#17344
Refs: v8/v8@1420e44
Refs: bcoe/c8#6 (comment)
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
gibfahn pushed a commit to gibfahn/node that referenced this pull request Jan 16, 2018
@TimothyGu @gibfahn
deps: cherry-pick 1420e44db0 from upstream V8
f855a6f 
Original commit message:

    [coverage] Correctly free DebugInfo in the absence of breakpoints

    It's quite possible for DebugInfos to exist without the presence of a
    bytecode array, since DebugInfos are created for all functions for which
    we have a CoverageInfo. Free such objects properly.

    Also move the corresponding deletion of CoverageInfos on unload up
    before the early exit.

    Bug: v8:6000
    Change-Id: Idde45b222290aa8b6828b61ff2251918b8ed2aed
    Reviewed-on: https://chromium-review.googlesource.com/664811
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Jakob Gruber <jgruber@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#48024}

Fixes crash when passing Profiler.startPreciseCoverage before
Debug.paused is received.

PR-URL: nodejs#17344
Backport-PR-URL: nodejs#16413
Refs: v8/v8@1420e44
Refs: bcoe/c8#6 (comment)
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
gibfahn pushed a commit to gibfahn/node that referenced this pull request Jan 22, 2018
@TimothyGu @gibfahn
deps: cherry-pick 1420e44db0 from upstream V8
74bcf98 
Original commit message:

    [coverage] Correctly free DebugInfo in the absence of breakpoints

    It's quite possible for DebugInfos to exist without the presence of a
    bytecode array, since DebugInfos are created for all functions for which
    we have a CoverageInfo. Free such objects properly.

    Also move the corresponding deletion of CoverageInfos on unload up
    before the early exit.

    Bug: v8:6000
    Change-Id: Idde45b222290aa8b6828b61ff2251918b8ed2aed
    Reviewed-on: https://chromium-review.googlesource.com/664811
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Jakob Gruber <jgruber@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#48024}

Fixes crash when passing Profiler.startPreciseCoverage before
Debug.paused is received.

PR-URL: nodejs#17344
Backport-PR-URL: nodejs#16413
Refs: v8/v8@1420e44
Refs: bcoe/c8#6 (comment)
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
MylesBorins pushed a commit to targos/node that referenced this pull request Feb 7, 2018
@TimothyGu @MylesBorins
deps: cherry-pick 1420e44db0 from upstream V8
e2d41ed 
Original commit message:

    [coverage] Correctly free DebugInfo in the absence of breakpoints

    It's quite possible for DebugInfos to exist without the presence of a
    bytecode array, since DebugInfos are created for all functions for which
    we have a CoverageInfo. Free such objects properly.

    Also move the corresponding deletion of CoverageInfos on unload up
    before the early exit.

    Bug: v8:6000
    Change-Id: Idde45b222290aa8b6828b61ff2251918b8ed2aed
    Reviewed-on: https://chromium-review.googlesource.com/664811
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Jakob Gruber <jgruber@chromium.org>
    Cr-Commit-Position: refs/heads/master@{nodejs#48024}

Fixes crash when passing Profiler.startPreciseCoverage before
Debug.paused is received.

PR-URL: nodejs#17344
Refs: v8/v8@1420e44
Refs: bcoe/c8#6 (comment)
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
gibfahn pushed a commit that referenced this pull request Feb 18, 2018
@TimothyGu @gibfahn
deps: cherry-pick 1420e44db0 from upstream V8
0ee6455 
Original commit message:

    [coverage] Correctly free DebugInfo in the absence of breakpoints

    It's quite possible for DebugInfos to exist without the presence of a
    bytecode array, since DebugInfos are created for all functions for which
    we have a CoverageInfo. Free such objects properly.

    Also move the corresponding deletion of CoverageInfos on unload up
    before the early exit.

    Bug: v8:6000
    Change-Id: Idde45b222290aa8b6828b61ff2251918b8ed2aed
    Reviewed-on: https://chromium-review.googlesource.com/664811
    Reviewed-by: Yang Guo <yangguo@chromium.org>
    Commit-Queue: Jakob Gruber <jgruber@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#48024}

Fixes crash when passing Profiler.startPreciseCoverage before
Debug.paused is received.

PR-URL: #17344
Backport-PR-URL: #16413
Refs: v8/v8@1420e44
Refs: bcoe/c8#6 (comment)
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bnoordhuis bnoordhuis bnoordhuis approved these changes

@targos targos targos approved these changes

@cjihrig cjihrig cjihrig approved these changes

Assignees
No one assigned
Labels
v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@TimothyGu @targos @bnoordhuis @cjihrig @nodejs-github-bot