[vm] Rework RX workarounds

* Disable RX workarounds on simulators: they cause host OS to crash;
* Remove Mac OS test for RX workarounds: it can cause host OS to crash;
* Remove old outdated code paths for OS versions that don't
need workarounds.
* Add code which detects whether workaround worked or not by intercepting EXC_BAD_ACCESS.
* Avoid using vm_remap in FFI callbacks when JITing, it does not work.
Instead restore old logic which manually copies page content, but adjust
FFI callbacks implementation to respect possible dual mapping.

This reworks changes done in commit
d194fce after additional testing on a
wider range of hardware.

TEST=manually on physical devices and simulators

Change-Id: I339379386183c532fec25e88fa436ff0b970c2cf
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/435120
Reviewed-by: Martin Kustermann <[email protected]>
Commit-Queue: Slava Egorov <[email protected]>

Author: mraleph

runtime/platform/globals.h

@@ -125,6 +125,9 @@
 #if TARGET_OS_WATCH
 #define DART_HOST_OS_WATCH 1
 #endif
+#if TARGET_OS_SIMULATOR
+#define DART_HOST_OS_SIMULATOR 1
+#endif
 
 #elif defined(_WIN32)
 

runtime/platform/utils_macos.h

@@ -22,7 +22,10 @@ int32_t DarwinVersion();
 
 }  // namespace internal
 
-#if defined(DART_HOST_OS_IOS)
+// iOS simulators run on underlying Mac OS X kernel, so questions about
+// iOS version are meaningless (and return incorrect results because we
+// rely on `uname`).
+#if defined(DART_HOST_OS_IOS) && !defined(DART_HOST_OS_SIMULATOR)
 
 // Run-time OS version checks.
 #define DEFINE_IS_OS_FUNCS(VERSION_NAME, VALUE)                                \
@@ -51,7 +54,7 @@ DEFINE_IS_OS_FUNCS(10_14)
 // current and expected versions.
 char* CheckIsAtLeastMinRequiredMacOSXVersion();
 
-#endif  // defined(DART_HOST_OS_IOS)
+#endif  // defined(DART_HOST_OS_IOS) && !defined(DART_HOST_OS_SIMULATOR)
 
 inline uint16_t Utils::HostToBigEndian16(uint16_t value) {
   return OSSwapHostToBigInt16(value);

runtime/tests/vm/dart/macos_dual_mapping_smoke_script.dart

@@ -336,9 +336,6 @@ dart/stacktrace_mixin_application_test: SkipByDesign # Relies symbol names in st
 [ $compiler == dart2analyzer || $compiler == dart2js ]
 dart/data_uri*test: Skip # Data uri's not supported by dart2js or the analyzer.
 
-[ $compiler != dartk || $runtime != vm || $system != macos ]
-dart/macos_dual_mapping_smoke: SkipByDesign
-
 [ $mode == debug || $runtime != dart_precompiled || $system == android ]
 dart/emit_aot_size_info_flag_test: SkipByDesign # This test is for VM AOT only and is quite slow (so we don't run it in debug mode).
 dart/split_aot_kernel_generation2_test: SkipByDesign # This test is for VM AOT only and is quite slow (so we don't run it in debug mode).

runtime/tests/vm/dart/macos_dual_mapping_smoke_test.dart

@@ -97,11 +97,17 @@ FfiCallbackMetadata* FfiCallbackMetadata::Instance() {
   return singleton_;
 }
 
+namespace {
+uword RXAreaStart(VirtualMemory* page) {
+  return page->start() + page->OffsetToExecutableAlias();
+}
+}  // namespace
+
 void FfiCallbackMetadata::FillRuntimeFunction(VirtualMemory* page,
                                               uword index,
                                               void* function) {
-  void** slot =
-      reinterpret_cast<void**>(page->start() + RuntimeFunctionOffset(index));
+  void** slot = reinterpret_cast<void**>(RXAreaStart(page) +
+                                         RuntimeFunctionOffset(index));
   *slot = function;
 }
 
@@ -113,13 +119,21 @@ VirtualMemory* FfiCallbackMetadata::AllocateTrampolinePage() {
   return nullptr;
 #else
 
-#if defined(DART_HOST_OS_MACOS) && !defined(DART_PRECOMPILED_RUNTIME)
+#if defined(DART_HOST_OS_MACOS) && defined(DART_PRECOMPILED_RUNTIME)
+  const bool should_remap_stub_page = true;
+#else
+  const bool should_remap_stub_page = false;  // No support for remapping.
+#endif
+
+#if defined(DART_HOST_OS_MACOS)
   // If we are not going to use vm_remap then we need to pass
-  // is_executable=true so that pages get allocated with MAP_JIT flag if
-  // necessary. Otherwise OS will kill us with a codesigning violation if
-  // hardened runtime is enabled.
-  const bool is_executable = !VirtualMemory::ShouldDualMapExecutablePages();
+  // is_executable=true so that pages get allocated with MAP_JIT flag or
+  // using RX workarounds if necessary. Otherwise OS will kill us with a
+  // codesigning violation if hardened runtime is enabled or we will simply
+  // not be able to execute trampoline code.
+  const bool is_executable = !should_remap_stub_page;
 #else
+  // On other operating systems we can simply flip RW->RX as necessary.
   const bool is_executable = false;
 #endif
 
@@ -130,17 +144,45 @@ VirtualMemory* FfiCallbackMetadata::AllocateTrampolinePage() {
     return nullptr;
   }
 
-  if (!stub_page_->DuplicateRX(new_page)) {
-    delete new_page;
-    return nullptr;
+  if (should_remap_stub_page) {
+#if defined(DART_HOST_OS_MACOS)
+    if (!stub_page_->DuplicateRX(new_page)) {
+      delete new_page;
+      return nullptr;
+    }
+#else
+    static_assert(!should_remap_stub_page,
+                  "Remaping only supported on Mac OS X");
+#endif
+  } else {
+    // If we are creating executable mapping then simply fill it with code by
+    // copying the page.
+    const intptr_t aligned_size =
+        Utils::RoundUp(stub_page_->size(), VirtualMemory::PageSize());
+    ASSERT(new_page->start() >= stub_page_->end() ||
+           new_page->end() <= stub_page_->start());
+    memcpy(new_page->address(), stub_page_->address(),  // NOLINT
+           stub_page_->size());
+
+    if (VirtualMemory::ShouldDualMapExecutablePages()) {
+      ASSERT(new_page->OffsetToExecutableAlias() != 0);
+      VirtualMemory::Protect(new_page->address(), aligned_size,
+                             VirtualMemory::kReadOnly);
+      VirtualMemory::Protect(
+          reinterpret_cast<void*>(RXAreaStart(new_page) + RXMappingSize()),
+          RWMappingSize(), VirtualMemory::kReadWrite);
+    } else {
+      VirtualMemory::Protect(new_page->address(), aligned_size,
+                             VirtualMemory::kReadExecute);
+    }
   }
 
 #if !defined(PRODUCT) || defined(FORCE_INCLUDE_DISASSEMBLER)
   if (FLAG_support_disassembler && FLAG_disassemble_stubs) {
     DisassembleToStdout formatter;
     THR_Print("Code for duplicated stub 'FfiCallbackTrampoline' {\n");
     const uword code_start =
-        new_page->start() + offset_of_first_trampoline_in_page_;
+        RXAreaStart(new_page) + offset_of_first_trampoline_in_page_;
     Disassembler::Disassemble(code_start, code_start + kPageSize, &formatter,
                               /*comments=*/nullptr);
     THR_Print("}\n");
@@ -176,8 +218,8 @@ void FfiCallbackMetadata::EnsureFreeListNotEmptyLocked() {
 
   // Add all the trampolines to the free list.
   const intptr_t trampolines_per_page = NumCallbackTrampolinesPerPage();
-  MetadataEntry* metadata_entry =
-      reinterpret_cast<MetadataEntry*>(new_page->start() + MetadataOffset());
+  MetadataEntry* metadata_entry = reinterpret_cast<MetadataEntry*>(
+      RXAreaStart(new_page) + MetadataOffset());
   for (intptr_t i = 0; i < trampolines_per_page; ++i) {
     AddToFreeListLocked(&metadata_entry[i]);
   }

runtime/tests/vm/vm.status

@@ -7,10 +7,6 @@
 #include "platform/assert.h"
 #include "platform/utils.h"
 
-#if defined(DART_HOST_OS_MACOS)
-#include <mach/mach.h>
-#endif
-
 namespace dart {
 
 bool VirtualMemory::InSamePage(uword address0, uword address1) {
@@ -44,62 +40,4 @@ VirtualMemory* VirtualMemory::ForImagePage(void* pointer, uword size) {
   return memory;
 }
 
-#if !defined(DART_TARGET_OS_FUCHSIA)
-// TODO(52579): Reenable on Fuchsia.
-
-bool VirtualMemory::DuplicateRX(VirtualMemory* target) {
-  const intptr_t aligned_size = Utils::RoundUp(size(), PageSize());
-  ASSERT_LESS_OR_EQUAL(aligned_size, target->size());
-
-#if defined(DART_HOST_OS_MACOS)
-#if defined(DART_PRECOMPILED_RUNTIME)
-  const bool should_remap = true;
-#else
-  const bool should_remap = ShouldDualMapExecutablePages();
-#endif
-
-  if (should_remap) {
-    // Mac is special cased because iOS doesn't allow allocating new executable
-    // memory, so the default approach would fail. We are allowed to make new
-    // mappings of existing executable memory using vm_remap though, which is
-    // effectively the same for non-writable memory.
-    const mach_port_t task = mach_task_self();
-    const vm_address_t source_address =
-        reinterpret_cast<vm_address_t>(address());
-    const vm_size_t mem_size = aligned_size;
-    const vm_prot_t read_execute = VM_PROT_READ | VM_PROT_EXECUTE;
-    vm_prot_t current_protection = read_execute;
-    vm_prot_t max_protection = read_execute;
-    vm_address_t target_address =
-        reinterpret_cast<vm_address_t>(target->address());
-    kern_return_t status = vm_remap(
-        task, &target_address, mem_size,
-        /*mask=*/0,
-        /*flags=*/VM_FLAGS_FIXED | VM_FLAGS_OVERWRITE, task, source_address,
-        /*copy=*/true, &current_protection, &max_protection,
-        /*inheritance=*/VM_INHERIT_NONE);
-    if (status != KERN_SUCCESS) {
-      return false;
-    }
-    ASSERT(reinterpret_cast<void*>(target_address) == target->address());
-    ASSERT_EQUAL(current_protection & read_execute, read_execute);
-    ASSERT_EQUAL(max_protection & read_execute, read_execute);
-    return true;
-  }
-#endif  // defined(DART_HOST_OS_MACOS)
-
-  // TODO(52497): Use dual mapping on platforms where it's supported.
-  // Check that target doesn't overlap with this.
-  ASSERT(target->start() >= end() || target->end() <= start());
-  memcpy(target->address(), address(), size());  // NOLINT
-  Protect(
-      target->address(), aligned_size,
-      VirtualMemory::ShouldDualMapExecutablePages() ? kReadOnly : kReadExecute);
-  RELEASE_ASSERT(!VirtualMemory::ShouldDualMapExecutablePages() ||
-                 target->OffsetToExecutableAlias() != 0);
-  return true;
-}
-
-#endif  // !defined(DART_TARGET_OS_FUCHSIA)
-
 }  // namespace dart