[base] Fix bugs in AllocateInternal on Fuchsia

This CL fixes two issues:

1) When the specified vmar_offset was zero, the previous logic would
   incorrectly conclude that no target address was specified, and would
   potentially place the allocation elsewhere in memory, not at the
   desired address. This CL now passes both the target address and the
   VMAR base address to AllocateInternal, which can then correctly
   determine whether a target address was supplied.

2) When the root_vmar was used and a hint specified, the previous logic
   would incorrectly use nullptr as base address of the root_vmar, which
   appears to be incorrect. The new logic now obtains the actual base
   (apparently 2MB) through zx_object_get_info during initialization.

Bug: v8:10391
Change-Id: Ia8215440a790b4a2a0c8d33f623d3ecb6a731a97
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3398506Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Victor Gomes <victorgomes@chromium.org>
Commit-Queue: Samuel Groß <saelo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78693}

src/base/platform/platform-fuchsia.cc

@@ -56,13 +56,22 @@ zx_vm_option_t GetAlignmentOptionFromAlignment(size_t alignment) {
   return alignment_log2 << ZX_VM_ALIGN_BASE;
 }
 
-void* AllocateInternal(const zx::vmar& vmar, size_t page_size,
-                       size_t vmar_offset, bool vmar_offset_is_hint,
-                       size_t size, size_t alignment,
-                       OS::MemoryPermission access) {
+enum class PlacementMode {
+  // Attempt to place the object at the provided address, otherwise elsewhere.
+  kUseHint,
+  // Place the object anywhere it fits.
+  kAnywhere,
+  // Place the object at the provided address, otherwise fail.
+  kFixed
+};
+
+void* AllocateInternal(const zx::vmar& vmar, void* vmar_base, size_t page_size,
+                       void* address, PlacementMode placement, size_t size,
+                       size_t alignment, OS::MemoryPermission access) {
   DCHECK_EQ(0, size % page_size);
   DCHECK_EQ(0, alignment % page_size);
-  DCHECK_EQ(0, vmar_offset % page_size);
+  DCHECK_EQ(0, reinterpret_cast<uintptr_t>(address) % alignment);
+  DCHECK_IMPLIES(placement != PlacementMode::kAnywhere, address != nullptr);
 
   zx::vmo vmo;
   if (zx::vmo::create(size, 0, &vmo) != ZX_OK) {
@@ -86,26 +95,32 @@ void* AllocateInternal(const zx::vmar& vmar, size_t page_size,
   CHECK_NE(0, alignment_option);  // Invalid alignment specified
   options |= alignment_option;
 
-  if (vmar_offset != 0) {
+  size_t vmar_offset = 0;
+  if (placement != PlacementMode::kAnywhere) {
+    // Try placing the mapping at the specified address.
+    uintptr_t target_addr = reinterpret_cast<uintptr_t>(address);
+    uintptr_t base = reinterpret_cast<uintptr_t>(vmar_base);
+    DCHECK_GE(target_addr, base);
+    vmar_offset = target_addr - base;
     options |= ZX_VM_SPECIFIC;
   }
 
-  zx_vaddr_t address;
-  zx_status_t status = vmar.map(options, vmar_offset, vmo, 0, size, &address);
+  zx_vaddr_t result;
+  zx_status_t status = vmar.map(options, vmar_offset, vmo, 0, size, &result);
 
-  if (status != ZX_OK && vmar_offset != 0 && vmar_offset_is_hint) {
-    // If a vmar_offset was specified and the allocation failed (for example,
-    // because the offset overlapped another mapping), then we should retry
-    // again without a vmar_offset if that offset was just meant to be a hint.
+  if (status != ZX_OK && placement == PlacementMode::kUseHint) {
+    // If a placement hint was specified but couldn't be used (for example,
+    // because the offset overlapped another mapping), then retry again without
+    // a vmar_offset to let the kernel pick another location.
     options &= ~(ZX_VM_SPECIFIC);
-    status = vmar.map(options, 0, vmo, 0, size, &address);
+    status = vmar.map(options, 0, vmo, 0, size, &result);
   }
 
   if (status != ZX_OK) {
     return nullptr;
   }
 
-  return reinterpret_cast<void*>(address);
+  return reinterpret_cast<void*>(result);
 }
 
 bool FreeInternal(const zx::vmar& vmar, size_t page_size, void* address,
@@ -135,13 +150,14 @@ bool DiscardSystemPagesInternal(const zx::vmar& vmar, size_t page_size,
 }
 
 zx_status_t CreateAddressSpaceReservationInternal(
-    const zx::vmar& vmar, size_t page_size, size_t vmar_offset,
-    bool vmar_offset_is_hint, size_t size, size_t alignment,
+    const zx::vmar& vmar, void* vmar_base, size_t page_size, void* address,
+    PlacementMode placement, size_t size, size_t alignment,
     OS::MemoryPermission max_permission, zx::vmar* child,
     zx_vaddr_t* child_addr) {
   DCHECK_EQ(0, size % page_size);
   DCHECK_EQ(0, alignment % page_size);
-  DCHECK_EQ(0, vmar_offset % page_size);
+  DCHECK_EQ(0, reinterpret_cast<uintptr_t>(address) % alignment);
+  DCHECK_IMPLIES(placement != PlacementMode::kAnywhere, address != nullptr);
 
   // TODO(v8) determine these based on max_permission.
   zx_vm_option_t options = ZX_VM_CAN_MAP_READ | ZX_VM_CAN_MAP_WRITE |
@@ -151,16 +167,22 @@ zx_status_t CreateAddressSpaceReservationInternal(
   CHECK_NE(0, alignment_option);  // Invalid alignment specified
   options |= alignment_option;
 
-  if (vmar_offset != 0) {
+  size_t vmar_offset = 0;
+  if (placement != PlacementMode::kAnywhere) {
+    // Try placing the mapping at the specified address.
+    uintptr_t target_addr = reinterpret_cast<uintptr_t>(address);
+    uintptr_t base = reinterpret_cast<uintptr_t>(vmar_base);
+    DCHECK_GE(target_addr, base);
+    vmar_offset = target_addr - base;
     options |= ZX_VM_SPECIFIC;
   }
 
   zx_status_t status =
       vmar.allocate(options, vmar_offset, size, child, child_addr);
-  if (status != ZX_OK && vmar_offset != 0 && vmar_offset_is_hint) {
-    // If a vmar_offset was specified and the allocation failed (for example,
-    // because the offset overlapped another mapping), then we should retry
-    // again without a vmar_offset if that offset was just meant to be a hint.
+  if (status != ZX_OK && placement == PlacementMode::kUseHint) {
+    // If a placement hint was specified but couldn't be used (for example,
+    // because the offset overlapped another mapping), then retry again without
+    // a vmar_offset to let the kernel pick another location.
     options &= ~(ZX_VM_SPECIFIC);
     status = vmar.allocate(options, 0, size, child, child_addr);
   }
@@ -174,14 +196,28 @@ TimezoneCache* OS::CreateTimezoneCache() {
   return new PosixDefaultTimezoneCache();
 }
 
+static void* g_root_vmar_base = nullptr;
+
+// static
+void OS::Initialize(bool hard_abort, const char* const gc_fake_mmap) {
+  PosixInitializeCommon(hard_abort, gc_fake_mmap);
+
+  // Determine base address of root VMAR.
+  zx_info_vmar_t info;
+  zx_status_t status = zx::vmar::root_self()->get_info(
+      ZX_INFO_VMAR, &info, sizeof(info), nullptr, nullptr);
+  CHECK_EQ(ZX_OK, status);
+  g_root_vmar_base = reinterpret_cast<void*>(info.base);
+}
+
 // static
 void* OS::Allocate(void* address, size_t size, size_t alignment,
                    MemoryPermission access) {
-  constexpr bool vmar_offset_is_hint = true;
-  DCHECK_EQ(0, reinterpret_cast<Address>(address) % alignment);
-  return AllocateInternal(*zx::vmar::root_self(), AllocatePageSize(),
-                          reinterpret_cast<uint64_t>(address),
-                          vmar_offset_is_hint, size, alignment, access);
+  PlacementMode placement =
+      address != nullptr ? PlacementMode::kUseHint : PlacementMode::kAnywhere;
+  return AllocateInternal(*zx::vmar::root_self(), g_root_vmar_base,
+                          AllocatePageSize(), address, placement, size,
+                          alignment, access);
 }
 
 // static
@@ -224,12 +260,11 @@ Optional<AddressSpaceReservation> OS::CreateAddressSpaceReservation(
   DCHECK_EQ(0, reinterpret_cast<Address>(hint) % alignment);
   zx::vmar child;
   zx_vaddr_t child_addr;
-  uint64_t vmar_offset = reinterpret_cast<uint64_t>(hint);
-  constexpr bool vmar_offset_is_hint = true;
+  PlacementMode placement =
+      hint != nullptr ? PlacementMode::kUseHint : PlacementMode::kAnywhere;
   zx_status_t status = CreateAddressSpaceReservationInternal(
-      *zx::vmar::root_self(), AllocatePageSize(), vmar_offset,
-      vmar_offset_is_hint, size, alignment, max_permission, &child,
-      &child_addr);
+      *zx::vmar::root_self(), g_root_vmar_base, AllocatePageSize(), hint,
+      placement, size, alignment, max_permission, &child, &child_addr);
   if (status != ZX_OK) return {};
   return AddressSpaceReservation(reinterpret_cast<void*>(child_addr), size,
                                  child.release());
@@ -287,16 +322,10 @@ Optional<AddressSpaceReservation> AddressSpaceReservation::CreateSubReservation(
 
   zx::vmar child;
   zx_vaddr_t child_addr;
-  size_t vmar_offset = 0;
-  if (address != 0) {
-    vmar_offset =
-        reinterpret_cast<size_t>(address) - reinterpret_cast<size_t>(base());
-  }
-  constexpr bool vmar_offset_is_hint = false;
   zx_status_t status = CreateAddressSpaceReservationInternal(
-      *zx::unowned_vmar(vmar_), OS::AllocatePageSize(), vmar_offset,
-      vmar_offset_is_hint, size, OS::AllocatePageSize(), max_permission, &child,
-      &child_addr);
+      *zx::unowned_vmar(vmar_), base(), OS::AllocatePageSize(), address,
+      PlacementMode::kFixed, size, OS::AllocatePageSize(), max_permission,
+      &child, &child_addr);
   if (status != ZX_OK) return {};
   DCHECK_EQ(reinterpret_cast<void*>(child_addr), address);
   return AddressSpaceReservation(reinterpret_cast<void*>(child_addr), size,
@@ -311,15 +340,9 @@ bool AddressSpaceReservation::FreeSubReservation(
 bool AddressSpaceReservation::Allocate(void* address, size_t size,
                                        OS::MemoryPermission access) {
   DCHECK(Contains(address, size));
-  size_t vmar_offset = 0;
-  if (address != 0) {
-    vmar_offset =
-        reinterpret_cast<size_t>(address) - reinterpret_cast<size_t>(base());
-  }
-  constexpr bool vmar_offset_is_hint = false;
   void* allocation = AllocateInternal(
-      *zx::unowned_vmar(vmar_), OS::AllocatePageSize(), vmar_offset,
-      vmar_offset_is_hint, size, OS::AllocatePageSize(), access);
+      *zx::unowned_vmar(vmar_), base(), OS::AllocatePageSize(), address,
+      PlacementMode::kFixed, size, OS::AllocatePageSize(), access);
   DCHECK(!allocation || allocation == address);
   return allocation != nullptr;
 }

src/base/platform/platform-posix.cc

@@ -237,11 +237,17 @@ bool OS::ArmUsingHardFloat() {
 #endif  // def __arm__
 #endif
 
-void OS::Initialize(bool hard_abort, const char* const gc_fake_mmap) {
+void PosixInitializeCommon(bool hard_abort, const char* const gc_fake_mmap) {
   g_hard_abort = hard_abort;
   g_gc_fake_mmap = gc_fake_mmap;
 }
 
+#if !V8_OS_FUCHSIA
+void OS::Initialize(bool hard_abort, const char* const gc_fake_mmap) {
+  PosixInitializeCommon(hard_abort, gc_fake_mmap);
+}
+#endif  // !V8_OS_FUCHSIA
+
 int OS::ActivationFrameAlignment() {
 #if V8_TARGET_ARCH_ARM
   // On EABI ARM targets this is required for fp correctness in the

src/base/platform/platform-posix.h

@@ -11,6 +11,8 @@
 namespace v8 {
 namespace base {
 
+void PosixInitializeCommon(bool hard_abort, const char* const gc_fake_mmap);
+
 class PosixTimezoneCache : public TimezoneCache {
  public:
   double DaylightSavingsOffset(double time_ms) override;