[deps] V8: cherry-pick 71736859756b2bd0444bdb0a87a#35205
Closed
Uh oh!
There was an error while loading. Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
nodejs-github-bot added build 
yselkowitz commented Sep 15, 2020
Is there a PR for 14 as well? |
ContributorAuthor
danbev commented Sep 15, 2020
Not yet, but I'll create one tomorrow. |
Member
targos commented Sep 15, 2020
If this applies to the V8 version we have in master/v14.x, please only open a PR against master. It can then be backported to v12.x |
ContributorAuthor
I can create a PR against master tomorrow 👍 |
BethGriggsforce-pushed the v12.x-staging branch from 729799d to c9bd1a7Compare
danbevforce-pushed the deps-v8-heap-allocateraw branch 2 times, most recently from 982be1f to c012d6dCompare
addaleaxforce-pushed the v12.x-staging branch from be41c83 to efb5192CompareContributorAuthor
danbev commented Sep 22, 2020
danbevforce-pushed the deps-v8-heap-allocateraw branch from af49418 to 2cdb60dCompareContributorAuthor
danbev commented Sep 22, 2020
addaleaxforce-pushed the v12.x-staging branch from efb5192 to 26f2d97Compareaddaleaxforce-pushed the v12.x-staging branch from 26f2d97 to 55fe022Compareaddaleaxforce-pushed the v12.x-staging branch from 55fe022 to 65b7bf4Compare
Member
addaleax commented Sep 22, 2020
@danbev Sorry to ask, but could you rebase this? |
ContributorAuthor
danbev commented Sep 23, 2020
No worries, I'll rebase shortly. |
Original commit message: [heap] Add large_object_threshold to AllocateRaw This commit adds a check in Heap::AllocateRaw when setting the large_object variable, when the AllocationType is of type kCode, to take into account the size of the CodeSpace's area size. The motivation for this change is that without this check it is possible that size_in_bytes is less than 128, and hence not considered a large object, but it might be larger than the available space in code_space->AreaSize(), which will cause the object to be created in the CodeLargeObjectSpace. This will later cause a segmentation fault when calling the following chain of functions: if (!large_object){MemoryChunk::FromHeapObject(heap_object) ->GetCodeObjectRegistry() ->RegisterNewlyAllocatedCodeObject(heap_object.address())} We (Red Hat) ran into this issue when running Node.js v12.16.1 in combination with yarn on aarch64 (this was the only architecture that this happed on). Bug: v8:10808 Change-Id: I0c396b0eb64bc4cc91d9a3be521254f3130eac7b Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2390665 Commit-Queue: Ulan Degenbaev <[email protected]> Reviewed-by: Ulan Degenbaev <[email protected]> Cr-Commit-Position: refs/heads/master@{#69876} Refs: v8/v8@7173685danbevforce-pushed the deps-v8-heap-allocateraw branch from 2cdb60d to 11a0593Compare
Collaborator
Member
richardlau commented Sep 23, 2020
@danbev looks like the V8 CI fails for this PR. |
ContributorAuthor
danbev commented Sep 23, 2020
@richardlau Thanks again! And sorry about using CI instead of testing locally but I'm running low on disk space on my machine. |
ContributorAuthor
danbev commented Sep 23, 2020
Re-build of V8-CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/3395/ |
github-actionsbot removed the request-ci Collaborator
nodejs-github-bot commented Sep 23, 2020
addaleax approved these changes Sep 23, 2020
Member
addaleax commented Sep 23, 2020
Landed in 57564eb |
addaleax pushed a commit that referenced this pull request Sep 23, 2020
Original commit message: [heap] Add large_object_threshold to AllocateRaw This commit adds a check in Heap::AllocateRaw when setting the large_object variable, when the AllocationType is of type kCode, to take into account the size of the CodeSpace's area size. The motivation for this change is that without this check it is possible that size_in_bytes is less than 128, and hence not considered a large object, but it might be larger than the available space in code_space->AreaSize(), which will cause the object to be created in the CodeLargeObjectSpace. This will later cause a segmentation fault when calling the following chain of functions: if (!large_object){MemoryChunk::FromHeapObject(heap_object) ->GetCodeObjectRegistry() ->RegisterNewlyAllocatedCodeObject(heap_object.address())} We (Red Hat) ran into this issue when running Node.js v12.16.1 in combination with yarn on aarch64 (this was the only architecture that this happed on). Bug: v8:10808 Change-Id: I0c396b0eb64bc4cc91d9a3be521254f3130eac7b Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2390665 Commit-Queue: Ulan Degenbaev <[email protected]> Reviewed-by: Ulan Degenbaev <[email protected]> Cr-Commit-Position: refs/heads/master@{#69876} Refs: v8/v8@7173685 PR-URL: #35205 Reviewed-By: Anna Henningsen <[email protected]>
Merged
kasicka commented Jan 12, 2021
Was this backported to v14 as well? |
Member
mhdawson commented Jan 14, 2021
yselkowitz commented Jan 14, 2021
But that PR was closed without being merged? |
yselkowitz commented Jan 15, 2021
I can confirm that 14.15.4 does not contain the fix. |
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Original commit message:
[heap] Add large_object_threshold to AllocateRaw
This commit adds a check in Heap::AllocateRaw when setting the
large_object variable, when the AllocationType is of type kCode, to
take into account the size of the CodeSpace's area size.
The motivation for this change is that without this check it is
possible that size_in_bytes is less than 128, and hence not considered
a large object, but it might be larger than the available space
in code_space->AreaSize(), which will cause the object to be created
in the CodeLargeObjectSpace. This will later cause a segmentation fault
when calling the following chain of functions:
We (Red Hat) ran into this issue when running Node.js v12.16.1 in
combination with yarn on aarch64 (this was the only architecture that
this happed on).
Bug: v8:10808
Change-Id: I0c396b0eb64bc4cc91d9a3be521254f3130eac7b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2390665
Commit-Queue: Ulan Degenbaev [email protected]
Reviewed-by: Ulan Degenbaev [email protected]
Cr-Commit-Position: refs/heads/master@{#69876}
Refs: v8/v8@7173685
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes