[v14.x backport] [deps] V8: cherry-pick 71736859756b2bd0444bdb0a87a #36074
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
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
nodejs-github-bot
added
v14.x
v8 engine
juanarbol
commented
Nov 10, 2020
Comment on lines
+7005
to
+7039
| TEST(GarbageCollectionWithLocalHeap) { | ||
| FLAG_local_heaps = true; | ||
| ManualGCScope manual_gc_scope; | ||
| CcTest::InitializeVM(); | ||
|
|
||
| Heap* heap = CcTest::i_isolate()->heap(); | ||
|
|
||
| LocalHeap local_heap(heap); | ||
| CcTest::CollectGarbage(OLD_SPACE); | ||
| { ParkedScope parked_scope(&local_heap); } | ||
| CcTest::CollectGarbage(OLD_SPACE); | ||
| } | ||
|
|
||
| TEST(Regress10698) { | ||
| ManualGCScope manual_gc_scope; | ||
| CcTest::InitializeVM(); | ||
| Heap* heap = CcTest::i_isolate()->heap(); | ||
| Factory* factory = CcTest::i_isolate()->factory(); | ||
| HandleScope handle_scope(CcTest::i_isolate()); | ||
| // This is modeled after the manual allocation folding of heap numbers in | ||
| // JSON parser (See commit ba7b25e). | ||
| // Step 1. Allocate a byte array in the old space. | ||
| Handle<ByteArray> array = | ||
| factory->NewByteArray(kTaggedSize, AllocationType::kOld); | ||
| // Step 2. Start incremental marking. | ||
| SimulateIncrementalMarking(heap, false); | ||
| // Step 3. Allocate another byte array. It will be black. | ||
| factory->NewByteArray(kTaggedSize, AllocationType::kOld); | ||
| Address address = reinterpret_cast<Address>(array->GetDataStartAddress()); | ||
| HeapObject filler = HeapObject::FromAddress(address); | ||
| // Step 4. Set the filler at the end of the first array. | ||
| // It will have an impossible markbit pattern because the second markbit | ||
| // will be taken from the second array. | ||
| filler.set_map_after_allocation(*factory->one_pointer_filler_map()); | ||
| } |
There was a problem hiding this comment.
I'm not very sure if v8-ci will be happy with this tests, but if It's not, I can remove them.
BTW: If you can tell me how to run the v8 test suite, It'd awesome.
BethGriggs
force-pushed
the
v14.x-staging
branch
from
d4083c2 to
1e8dfb9
Compare
|
Closing (see comment) |
|
Why was this closed? This fix has landed in v12 and master but not v14 afaics. cc @danbev |
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Refs: #35222
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