[heap] Fix regressions in the configuration without concurrent marking

Building and running tests with v8_enabled_concurrent_marking=false
currently produces two failures:
1) Segmentation fault on attempt to mark a read-only object.
   This is fixed by changing MarkBit::Set to be a no-op if the object
   is already marked (which is the case for the readonly space).
2) Missing write-barrier due to bogus condition in the bailout.
   The barrier can be skipped only if the host object is not marked yet.

This also disables two concurrent allocation tests that rely on
concurrent marking write-barrier.

Bug: v8:10875

Change-Id: Ib3a238fc34c8f20c697470e0bd4ac427fb4bdc0e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2421816
Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70041}
This commit is contained in:
Ulan Degenbaev 2020-09-21 20:40:30 +02:00 committed by Commit Bot
parent a74541a335
commit fbd3834ebb
4 changed files with 12 additions and 3 deletions

View File

@ -22,7 +22,7 @@ bool MarkingBarrier::MarkValue(HeapObject host, HeapObject value) {
// filler map.
DCHECK(!marking_state_.IsImpossible(host) ||
value == ReadOnlyRoots(heap_->isolate()).one_pointer_filler_map());
if (!V8_CONCURRENT_MARKING_BOOL && marking_state_.IsBlack(host)) {
if (!V8_CONCURRENT_MARKING_BOOL && !marking_state_.IsBlack(host)) {
// The value will be marked and the slot will be recorded when the marker
// visits the host object.
return false;

View File

@ -60,7 +60,7 @@ void MarkingBarrier::Write(Code host, RelocInfo* reloc_info, HeapObject value) {
void MarkingBarrier::Write(JSArrayBuffer host,
ArrayBufferExtension* extension) {
if (!V8_CONCURRENT_MARKING_BOOL && marking_state_.IsBlack(host)) {
if (!V8_CONCURRENT_MARKING_BOOL && !marking_state_.IsBlack(host)) {
// The extension will be marked when the marker visits the host object.
return;
}

View File

@ -58,8 +58,9 @@ class MarkBit {
template <>
inline bool MarkBit::Set<AccessMode::NON_ATOMIC>() {
CellType old_value = *cell_;
if ((old_value & mask_) == mask_) return false;
*cell_ = old_value | mask_;
return (old_value & mask_) == 0;
return true;
}
template <>

View File

@ -274,6 +274,10 @@ class ConcurrentWriteBarrierThread final : public v8::base::Thread {
};
UNINITIALIZED_TEST(ConcurrentWriteBarrier) {
if (!FLAG_concurrent_marking) {
// The test requires concurrent marking barrier.
return;
}
ManualGCScope manual_gc_scope;
FLAG_concurrent_allocation = true;
FLAG_local_heaps = true;
@ -335,6 +339,10 @@ class ConcurrentRecordRelocSlotThread final : public v8::base::Thread {
};
UNINITIALIZED_TEST(ConcurrentRecordRelocSlot) {
if (!FLAG_concurrent_marking) {
// The test requires concurrent marking barrier.
return;
}
FLAG_manual_evacuation_candidates_selection = true;
ManualGCScope manual_gc_scope;
FLAG_concurrent_allocation = true;