From 01dbc9f62b532349507b1e8dd8401b17a0383bb6 Mon Sep 17 00:00:00 2001 From: Pierre Langlois Date: Wed, 9 Sep 2020 18:40:22 +0100 Subject: [PATCH] [cctest][heap] Do not rely on page limit for full space simulation. This reverts https://chromium-review.googlesource.com/c/v8/v8/+/2372545 in favour of different solution. In order to simulate filling up a page, it's not suitable to look at the limit() since there might be observers that have lowered it, so the page will not actually be full. Instead, let's relax the CHECK() in CreatePadding() to not look at the limit() but all available space. For instance, the test-heap/Regress978156 cctest uses FillCurrentPage() to fill the current page. However if there's an observer on the current page, it will not be filled entirely and the test will fail. This works because by default, when the new space is empty, the scavenger observer happens to be on the second page of the space. However if one changes the V8 page size to 512k, then it fails. This can be reproduced as such: # Make sure the scavenge trigger is on the first page. ./cctest test-heap/Regress978156 --scavenge-task-trigger=10 # Stress marking adds random observers to trigger incremental # marking. ./cctest test-heap/Regress978156 --stress-marking=100 This issue also causes crashes when using the %SimulateNewspaceFull() runtime test function, as found by fuzzing and you can find more details in the bug. Bug: v8:10808, v8:9906, chromium:1122848 Change-Id: Ie043ae0a1d3754d2423cb5d97f2b3e1ee860e5c8 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2401427 Reviewed-by: Ulan Degenbaev Commit-Queue: Pierre Langlois Cr-Commit-Position: refs/heads/master@{#69805} --- src/runtime/runtime-test.cc | 6 ++---- test/cctest/heap/heap-utils.cc | 10 +++------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/src/runtime/runtime-test.cc b/src/runtime/runtime-test.cc index 28d0d3422e..32f2450b11 100644 --- a/src/runtime/runtime-test.cc +++ b/src/runtime/runtime-test.cc @@ -719,10 +719,8 @@ void FillUpOneNewSpacePage(Isolate* isolate, Heap* heap) { // the current allocation pointer. DCHECK_IMPLIES(space->heap()->inline_allocation_disabled(), space->limit() == space->top()); - size_t limit = space->heap()->inline_allocation_disabled() - ? space->to_space().page_high() - : space->limit(); - int space_remaining = static_cast(limit - space->top()); + int space_remaining = + static_cast(space->to_space().page_high() - space->top()); while (space_remaining > 0) { int length = FixedArrayLenFromSize(space_remaining); if (length > 0) { diff --git a/test/cctest/heap/heap-utils.cc b/test/cctest/heap/heap-utils.cc index 1f9a9c2d87..2e954a77e9 100644 --- a/test/cctest/heap/heap-utils.cc +++ b/test/cctest/heap/heap-utils.cc @@ -94,9 +94,7 @@ std::vector> CreatePadding(Heap* heap, int padding_size, int overall_free_memory = static_cast(heap->old_space()->Available()); CHECK(padding_size <= overall_free_memory || overall_free_memory == 0); } else { - int overall_free_memory = - static_cast(*heap->new_space()->allocation_limit_address() - - *heap->new_space()->allocation_top_address()); + int overall_free_memory = static_cast(heap->new_space()->Available()); CHECK(padding_size <= overall_free_memory || overall_free_memory == 0); } while (free_memory > 0) { @@ -143,10 +141,8 @@ bool FillCurrentPageButNBytes(v8::internal::NewSpace* space, int extra_bytes, // the current allocation pointer. DCHECK_IMPLIES(space->heap()->inline_allocation_disabled(), space->limit() == space->top()); - size_t limit = space->heap()->inline_allocation_disabled() - ? space->to_space().page_high() - : space->limit(); - int space_remaining = static_cast(limit - space->top()); + int space_remaining = + static_cast(space->to_space().page_high() - space->top()); CHECK(space_remaining >= extra_bytes); int new_linear_size = space_remaining - extra_bytes; if (new_linear_size == 0) return false;