[nojit] Don't allocate executable memory in jitless mode

This CL disables RX (read and execute) permissions for Code memory
when in jitless mode. All memory that was previously allocated RX
is now read-only.

Bug: v8:7777
Cq-Include-Trybots: luci.v8.try:v8_linux_arm_lite_rel_ng
Change-Id: I52d6ed785d244ec33168a02293c5506d26f36fe8
Reviewed-on: https://chromium-review.googlesource.com/c/1390122
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#58692}

src/flag-definitions.h

@@ -1088,14 +1088,6 @@ DEFINE_BOOL(serialization_statistics, false,
 DEFINE_UINT(serialization_chunk_size, 4096,
             "Custom size for serialization chunks")
 
-// JIT-less V8. Design doc: goo.gl/kRnhVe
-#ifdef V8_JITLESS_MODE
-DEFINE_BOOL(jitless, false, "Disable runtime allocation of executable memory.")
-#else
-DEFINE_BOOL_READONLY(jitless, false,
-                     "Disable runtime allocation of executable memory.")
-#endif
-
 // Regexp
 DEFINE_BOOL(regexp_optimization, true, "generate optimized regexp code")
 DEFINE_BOOL(regexp_mode_modifiers, false, "enable inline flags in regexp.")
@@ -1157,6 +1149,9 @@ DEFINE_SIZE_T(mock_arraybuffer_allocator_limit, 0,
 #define FLAG FLAG_FULL
 #endif
 
+DEFINE_BOOL(jitless, V8_LITE_BOOL,
+            "Disable runtime allocation of executable memory.")
+
 // Enable recompilation of function with optimized code.
 DEFINE_BOOL(opt, !V8_LITE_BOOL, "use adaptive optimizations")
 

src/heap/heap-inl.h

@@ -656,12 +656,12 @@ CodeSpaceMemoryModificationScope::CodeSpaceMemoryModificationScope(Heap* heap)
 CodeSpaceMemoryModificationScope::~CodeSpaceMemoryModificationScope() {
   if (heap_->write_protect_code_memory()) {
     heap_->decrement_code_space_memory_modification_scope_depth();
-    heap_->code_space()->SetReadAndExecutable();
+    heap_->code_space()->SetDefaultCodePermissions();
     LargePage* page = heap_->code_lo_space()->first_page();
     while (page != nullptr) {
       DCHECK(page->IsFlagSet(MemoryChunk::IS_EXECUTABLE));
       CHECK(heap_->memory_allocator()->IsMemoryChunkExecutable(page));
-      page->SetReadAndExecutable();
+      page->SetDefaultCodePermissions();
       page = page->next_page();
     }
   }
@@ -699,7 +699,7 @@ CodePageMemoryModificationScope::CodePageMemoryModificationScope(
 
 CodePageMemoryModificationScope::~CodePageMemoryModificationScope() {
   if (scope_active_) {
-    chunk_->SetReadAndExecutable();
+    chunk_->SetDefaultCodePermissions();
   }
 }
 

src/heap/heap.cc

@@ -2042,7 +2042,7 @@ void Heap::ProtectUnprotectedMemoryChunks() {
   for (auto chunk = unprotected_memory_chunks_.begin();
        chunk != unprotected_memory_chunks_.end(); chunk++) {
     CHECK(memory_allocator()->IsMemoryChunkExecutable(*chunk));
-    (*chunk)->SetReadAndExecutable();
+    (*chunk)->SetDefaultCodePermissions();
   }
   unprotected_memory_chunks_.clear();
 }

src/heap/spaces.cc

@@ -544,7 +544,10 @@ void MemoryChunk::InitializationMemoryFence() {
 #endif
 }
 
-void MemoryChunk::SetReadAndExecutable() {
+void MemoryChunk::DecrementWriteUnprotectCounterAndMaybeSetPermissions(
+    PageAllocator::Permission permission) {
+  DCHECK(permission == PageAllocator::kRead ||
+         permission == PageAllocator::kReadExecute);
   DCHECK(IsFlagSet(MemoryChunk::IS_EXECUTABLE));
   DCHECK(owner()->identity() == CODE_SPACE ||
          owner()->identity() == CODE_LO_SPACE);
@@ -565,11 +568,20 @@ void MemoryChunk::SetReadAndExecutable() {
     size_t page_size = MemoryAllocator::GetCommitPageSize();
     DCHECK(IsAligned(protect_start, page_size));
     size_t protect_size = RoundUp(area_size(), page_size);
-    CHECK(reservation_.SetPermissions(protect_start, protect_size,
-                                      PageAllocator::kReadExecute));
+    CHECK(reservation_.SetPermissions(protect_start, protect_size, permission));
   }
 }
 
+void MemoryChunk::SetReadable() {
+  DecrementWriteUnprotectCounterAndMaybeSetPermissions(PageAllocator::kRead);
+}
+
+void MemoryChunk::SetReadAndExecutable() {
+  DCHECK(!FLAG_jitless);
+  DecrementWriteUnprotectCounterAndMaybeSetPermissions(
+      PageAllocator::kReadExecute);
+}
+
 void MemoryChunk::SetReadAndWritable() {
   DCHECK(IsFlagSet(MemoryChunk::IS_EXECUTABLE));
   DCHECK(owner()->identity() == CODE_SPACE ||
@@ -590,6 +602,15 @@ void MemoryChunk::SetReadAndWritable() {
   }
 }
 
+namespace {
+
+PageAllocator::Permission DefaultWritableCodePermissions() {
+  return FLAG_jitless ? PageAllocator::kReadWrite
+                      : PageAllocator::kReadWriteExecute;
+}
+
+}  // namespace
+
 MemoryChunk* MemoryChunk::Initialize(Heap* heap, Address base, size_t size,
                                      Address area_start, Address area_end,
                                      Executability executable, Space* owner,
@@ -657,7 +678,7 @@ MemoryChunk* MemoryChunk::Initialize(Heap* heap, Address base, size_t size,
       DCHECK(IsAligned(area_start, page_size));
       size_t area_size = RoundUp(area_end - area_start, page_size);
       CHECK(reservation.SetPermissions(area_start, area_size,
-                                       PageAllocator::kReadWriteExecute));
+                                       DefaultWritableCodePermissions()));
     }
   }
 
@@ -1832,6 +1853,14 @@ void PagedSpace::ReleasePage(Page* page) {
   heap()->memory_allocator()->Free<MemoryAllocator::kPreFreeAndQueue>(page);
 }
 
+void PagedSpace::SetReadable() {
+  DCHECK(identity() == CODE_SPACE);
+  for (Page* page : *this) {
+    CHECK(heap()->memory_allocator()->IsMemoryChunkExecutable(page));
+    page->SetReadable();
+  }
+}
+
 void PagedSpace::SetReadAndExecutable() {
   DCHECK(identity() == CODE_SPACE);
   for (Page* page : *this) {

src/heap/spaces.h

@@ -646,9 +646,18 @@ class MemoryChunk {
   // MemoryChunk::synchronized_heap() to simulate the barrier.
   void InitializationMemoryFence();
 
+  void SetReadable();
   void SetReadAndExecutable();
   void SetReadAndWritable();
 
+  void SetDefaultCodePermissions() {
+    if (FLAG_jitless) {
+      SetReadable();
+    } else {
+      SetReadAndExecutable();
+    }
+  }
+
   base::ListNode<MemoryChunk>& list_node() { return list_node_; }
 
  protected:
@@ -660,6 +669,11 @@ class MemoryChunk {
   // Should be called when memory chunk is about to be freed.
   void ReleaseAllocatedMemory();
 
+  // Sets the requested page permissions only if the write unprotect counter
+  // has reached 0.
+  void DecrementWriteUnprotectCounterAndMaybeSetPermissions(
+      PageAllocator::Permission permission);
+
   VirtualMemory* reserved_memory() { return &reservation_; }
 
   size_t size_;
@@ -2208,9 +2222,18 @@ class V8_EXPORT_PRIVATE PagedSpace
   // be used for allocation.
   Page* RemovePageSafe(int size_in_bytes);
 
+  void SetReadable();
   void SetReadAndExecutable();
   void SetReadAndWritable();
 
+  void SetDefaultCodePermissions() {
+    if (FLAG_jitless) {
+      SetReadable();
+    } else {
+      SetReadAndExecutable();
+    }
+  }
+
 #ifdef VERIFY_HEAP
   // Verify integrity of this space.
   virtual void Verify(Isolate* isolate, ObjectVisitor* visitor);

src/x64/codegen-x64.cc

@@ -14,6 +14,7 @@ namespace internal {
 #define __ masm.
 
 UnaryMathFunction CreateSqrtFunction() {
+  DCHECK(!FLAG_jitless);
   v8::PageAllocator* page_allocator = GetPlatformPageAllocator();
   size_t allocated = 0;
   byte* buffer = AllocatePage(page_allocator,

test/cctest/cctest.status

@@ -465,7 +465,7 @@
 }],
 
 ##############################################################################
-['lite_mode == True', {
+['lite_mode', {
 
   # TODO(8394): First execution events don't work in lite_mode. Enable this after
   # we fix the lite mode to track the first execution.
@@ -510,6 +510,38 @@
 
   # Slow tests
   'test-heap/IncrementalMarkingStepMakesBigProgressWithLargeObjects': [SKIP],
-}],
+
+  # TODO(v8:7777): Re-enable once wasm is supported in jitless mode.
+  'test-c-wasm-entry/*': [SKIP],
+  'test-jump-table-assembler/*': [SKIP],
+  'test-run-wasm-64/*': [SKIP],
+  'test-run-wasm-asmjs/*': [SKIP],
+  'test-run-wasm-atomics64/*': [SKIP],
+  'test-run-wasm-atomics/*': [SKIP],
+  'test-run-wasm/*': [SKIP],
+  'test-run-wasm-interpreter/*': [SKIP],
+  'test-run-wasm-js/*': [SKIP],
+  'test-run-wasm-module/*': [SKIP],
+  'test-run-wasm-sign-extension/*': [SKIP],
+  'test-run-wasm-simd/*': [SKIP],
+  'test-streaming-compilation/*': [SKIP],
+  'test-wasm-breakpoints/*': [SKIP],
+  'test-wasm-codegen/*': [SKIP],
+  'test-wasm-import-wrapper-cache/*': [SKIP],
+  'test-wasm-interpreter-entry/*': [SKIP],
+  'test-wasm-serialization/*': [SKIP],
+  'test-wasm-shared-engine/*': [SKIP],
+  'test-wasm-stack/*': [SKIP],
+  'test-wasm-trap-position/*': [SKIP],
+  'wasm-run-utils/*': [SKIP],
+
+  # Tests that generate code at runtime.
+  'codegen-tester/*': [SKIP],
+  'test-assembler-*': [SKIP],
+  'test-basic-block-profiler/*': [SKIP],
+  'test-branch-combine/*': [SKIP],
+  'test-multiple-return/*': [SKIP],
+  'test-run-calls-to-external-references/*': [SKIP],
+}], # lite_mode
 
 ]

test/debugger/debugger.status

@@ -123,4 +123,12 @@
   'debug/debug-liveedit-restart-frame': [SKIP],
 }],  # 'arch == s390 or arch == s390x'
 
+##############################################################################
+['lite_mode', {
+  # TODO(v8:7777): Re-enable once wasm is supported in jitless mode.
+  'asm-*': [SKIP],
+  'debug/wasm/*': [SKIP],
+  'wasm-*': [SKIP],
+}], # lite_mode
+
 ]

test/fuzzer/fuzzer.status

@@ -4,4 +4,11 @@
 
 [
 
+##############################################################################
+['lite_mode', {
+  # TODO(v8:7777): Re-enable once wasm is supported in jitless mode.
+  'multi_return/*': [SKIP],
+  'wasm_compile/*': [SKIP],
+}], # lite_mode
+
 ]

test/inspector/inspector.status

@@ -34,15 +34,19 @@
   'debugger/evaluate-on-call-frame-in-module': [PASS, FAIL],
 }],  # variant != default
 
-['lite_mode == True', {
+##############################################################################
+['lite_mode', {
   # Lite mode does not allocate feedback vector.
   'type-profiler/type-profile-start-stop': [SKIP],
   'type-profiler/type-profile': [SKIP],
   'type-profiler/type-profile-with-to-string-tag': [SKIP],
   'type-profiler/type-profile-with-classes': [SKIP],
-  'type-profiler/type-profile-disable': [SKIP]
-}],  # 'lite_mode == True'
+  'type-profiler/type-profile-disable': [SKIP],
 
+  # TODO(v8:7777): Re-enable once wasm is supported in jitless mode.
+  'debugger/asm-*': [SKIP],
+  'debugger/wasm-*': [SKIP],
+}],  # 'lite_mode'
 
 ##############################################################################
 ['(arch == arm or arch == arm64) and simulator_run', {

test/message/message.status

@@ -53,4 +53,11 @@
   'fail/list-format*': [SKIP],
 }],  # no_i18n == True
 
+##############################################################################
+['lite_mode', {
+  # TODO(v8:7777): Re-enable once wasm is supported in jitless mode.
+  'fail/wasm-*': [SKIP],
+  'wasm-*': [SKIP],
+}], # lite_mode
+
 ]

test/mjsunit/mjsunit.status

@@ -326,7 +326,7 @@
 }],  # 'gc_stress == True'
 
 ##############################################################################
-['lite_mode == True', {
+['lite_mode', {
   # Skip tests not suitable for lite_mode.
 
   # TODO(8596): We cache the templates in the feedback vector. In lite mode
@@ -352,7 +352,38 @@
   'spread-large-string': [SKIP],
   'spread-large-array': [SKIP],
 
-}],  # 'lite_mode == True'
+  # TODO(v8:7777): Re-enable once wasm is supported in jitless mode.
+  'asm/*': [SKIP],
+  'regress/wasm/*': [SKIP],
+  'tools/compiler-trace-flags': [SKIP],
+  'wasm/*': [SKIP],
+
+  # Other tests that use asm / wasm / optimized code.
+  'compiler/regress-439743': [SKIP],
+  'regress/regress-599719': [SKIP],
+  'regress/regress-6196': [SKIP],
+  'regress/regress-6700': [SKIP],
+  'regress/regress-670808': [SKIP],
+  'regress/regress-677685': [SKIP],
+  'regress/regress-6838-2': [SKIP],
+  'regress/regress-6838-3': [SKIP],
+  'regress/regress-799690': [SKIP],
+
+  # Tests that generate code at runtime.
+  'code-comments': [SKIP],
+  'regress/regress-441099': [SKIP],
+  'regress/regress-617526': [SKIP],
+  'regress/regress-618608': [SKIP],
+  'regress/regress-673241': [SKIP],
+  'regress/regress-673297': [SKIP],
+  'regress/regress-7893': [SKIP],
+  'regress/regress-8377': [SKIP],
+  'regress/regress-863810': [SKIP],
+  'regress/regress-crbug-721835': [SKIP],
+  'regress/regress-crbug-759327': [SKIP],
+  'regress/regress-crbug-762472': [SKIP],
+  'regress/regress-crbug-898974': [SKIP],
+}],  # 'lite_mode'
 
 ##############################################################################
 ['byteorder == big', {
@@ -974,4 +1005,5 @@
   # Unwinding info writer is only supported on x64, arm, and arm64 Linux
   'regress/regress-913844': [SKIP],
 }],
+
 ]

test/mkgrokdump/mkgrokdump.status

@@ -3,8 +3,10 @@
 # found in the LICENSE file.
 
 [
+
 # Only test for default mode x64.
-['variant != default or arch != x64', {
+['variant != default or arch != x64 or lite_mode', {
   '*': [SKIP],
-}],  # variant != default or arch != x64
+}],  # variant != default or arch != x64 or lite_mode
+
 ]

test/unittests/unittests.status

@@ -20,4 +20,11 @@
   # Uses too much memory.
   'Parameterized/WasmCodeManagerTest.GrowingVsFixedModule/Fixed': [SKIP]
 }],  # '(arch == arm or arch == mips) and not simulator_run'
+
+##############################################################################
+['lite_mode', {
+  # TODO(v8:7777): Re-enable once wasm is supported in jitless mode.
+  'ValueSerializerTestWithWasm.*': [SKIP],
+}], # lite_mode
+
 ]

test/wasm-js/wasm-js.status

@@ -22,4 +22,10 @@
   'instance/constructor': [SKIP],
 }],  # 'arch == s390 or arch == s390x or system == aix'
 
+##############################################################################
+['lite_mode', {
+  # TODO(v8:7777): Re-enable once wasm is supported in jitless mode.
+  '*': [SKIP],
+}], # lite_mode
+
 ]

test/wasm-spec-tests/wasm-spec-tests.status

@@ -43,4 +43,11 @@
   'tests/f64': [SKIP],
 }],  # 'arch == s390 or arch == s390x'
 
+##############################################################################
+['lite_mode', {
+  # TODO(v8:7777): Re-enable once wasm is supported in jitless mode.
+  '*': [SKIP],
+}], # lite_mode
+
+
 ]

tools/v8_presubmit.py

@@ -512,7 +512,7 @@ class SourceProcessor(SourceFileProcessor):
       print "%s does not end with a single new line." % name
       result = False
     # Sanitize flags for fuzzer.
-    if "mjsunit" in name or "debugger" in name:
+    if ".js" in name and ("mjsunit" in name or "debugger" in name):
       match = FLAGS_LINE.search(contents)
       if match:
         print "%s Flags should use '-' (not '_')" % name