From 8130e54dfb29f35421ccdab92a162f3d10f6bc2a Mon Sep 17 00:00:00 2001
From: Dan Elphick <delphick@chromium.org>
Date: Fri, 31 Jul 2020 11:38:36 +0100
Subject: [PATCH] [base] Don't allow excluded regions to be freed

Excluded regions are no longer available to the RegionAllocator, so
should not be freeable so actually enforce that and aAdd a test.

Bug: v8:10454
Change-Id: I51c41cf0bf3d2eeb699b10b1fa02f5465d93b6aa
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2330026
Reviewed-by: Igor Sheludko (OOO Aug 3-17) <ishell@chromium.org>
Commit-Queue: Dan Elphick <delphick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69163}
---
 src/base/region-allocator.cc                  |  2 +-
 src/base/region-allocator.h                   |  6 +++--
 .../base/region-allocator-unittest.cc         | 25 +++++++++++++++++++
 3 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/src/base/region-allocator.cc b/src/base/region-allocator.cc
index b9ce4a3349..9224dc99dc 100644
--- a/src/base/region-allocator.cc
+++ b/src/base/region-allocator.cc
@@ -208,7 +208,7 @@ size_t RegionAllocator::TrimRegion(Address address, size_t new_size) {
     return 0;
   }
   Region* region = *region_iter;
-  if (region->begin() != address || region->is_free()) {
+  if (region->begin() != address || !region->is_allocated()) {
     return 0;
   }
 
diff --git a/src/base/region-allocator.h b/src/base/region-allocator.h
index 639d5f66ef..887f123b10 100644
--- a/src/base/region-allocator.h
+++ b/src/base/region-allocator.h
@@ -105,6 +105,7 @@ class V8_BASE_EXPORT RegionAllocator final {
         : AddressRegion(address, size), state_(state) {}
 
     bool is_free() const { return state_ == RegionState::kFree; }
+    bool is_allocated() const { return state_ == RegionState::kAllocated; }
     bool is_excluded() const { return state_ == RegionState::kExcluded; }
     void set_state(RegionState state) { state_ = state; }
 
@@ -170,10 +171,11 @@ class V8_BASE_EXPORT RegionAllocator final {
   void Merge(AllRegionsSet::iterator prev_iter,
              AllRegionsSet::iterator next_iter);
 
+  FRIEND_TEST(RegionAllocatorTest, AllocateExcluded);
   FRIEND_TEST(RegionAllocatorTest, AllocateRegionRandom);
-  FRIEND_TEST(RegionAllocatorTest, Fragmentation);
-  FRIEND_TEST(RegionAllocatorTest, FindRegion);
   FRIEND_TEST(RegionAllocatorTest, Contains);
+  FRIEND_TEST(RegionAllocatorTest, FindRegion);
+  FRIEND_TEST(RegionAllocatorTest, Fragmentation);
 
   DISALLOW_COPY_AND_ASSIGN(RegionAllocator);
 };
diff --git a/test/unittests/base/region-allocator-unittest.cc b/test/unittests/base/region-allocator-unittest.cc
index df154ff4f3..d229e4775b 100644
--- a/test/unittests/base/region-allocator-unittest.cc
+++ b/test/unittests/base/region-allocator-unittest.cc
@@ -11,6 +11,7 @@ namespace v8 {
 namespace base {
 
 using Address = RegionAllocator::Address;
+using RegionState = RegionAllocator::RegionState;
 using v8::internal::KB;
 using v8::internal::MB;
 
@@ -350,5 +351,29 @@ TEST(RegionAllocatorTest, TrimRegion) {
   CHECK_EQ(ra.AllocateRegion(kSize), kBegin);
 }
 
+TEST(RegionAllocatorTest, AllocateExcluded) {
+  const size_t kPageSize = 4 * KB;
+  const size_t kPageCount = 64;
+  const size_t kSize = kPageSize * kPageCount;
+  const Address kBegin = static_cast<Address>(kPageSize * 153);
+
+  RegionAllocator ra(kBegin, kSize, kPageSize);
+
+  Address address = kBegin + 13 * kPageSize;
+  size_t size = 37 * kPageSize;
+  CHECK(ra.AllocateRegionAt(address, size, RegionState::kExcluded));
+
+  // The region is not free and cannot be allocated at again.
+  CHECK(!ra.IsFree(address, size));
+  CHECK(!ra.AllocateRegionAt(address, size));
+  auto region_iter = ra.FindRegion(address);
+
+  CHECK((*region_iter)->is_excluded());
+
+  // It's not possible to free or trim an excluded region.
+  CHECK_EQ(ra.FreeRegion(address), 0);
+  CHECK_EQ(ra.TrimRegion(address, kPageSize), 0);
+}
+
 }  // namespace base
 }  // namespace v8