[v14.x backport] [deps] V8: cherry-pick 71736859756b2bd0444bdb0a87a#36074
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
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()); | ||
| } |
MemberAuthor
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.
BethGriggsforce-pushed the v14.x-staging branch from d4083c2 to 1e8dfb9Compare
MemberAuthor
juanarbol commented Dec 2, 2020
Closing (see comment) |
yselkowitz commented Dec 10, 2020
Why was this closed? This fix has landed in v12 and master but not v14 afaics. cc @danbev |
Contributor
danbev commented Dec 10, 2020
2 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment
4 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.
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