From cabe5fa9b2ca5f08c6c4ae0fc7c904482069dfab Mon Sep 17 00:00:00 2001 From: Jakob Gruber Date: Wed, 4 Sep 2019 08:15:46 +0200 Subject: [PATCH] [snapshot] Align allocation address for the embedded blob MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AllocatePages (used to allocate the embedded blob's backing store during mksnapshot) has allocation address, size, and alignment parameters. Both address and size are expected to be aligned, but we were only aligning size properly. This CL also aligns the address (and adds a bunch of comments as well). Bug: v8:9677 Change-Id: Ia739682236c74278bcaf1c9b7c9c4b3e0b0c5582 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1784277 Commit-Queue: Jakob Gruber Commit-Queue: Simon Zünd Reviewed-by: Simon Zünd Auto-Submit: Jakob Gruber Cr-Commit-Position: refs/heads/master@{#63538} --- src/snapshot/embedded/embedded-data.cc | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/src/snapshot/embedded/embedded-data.cc b/src/snapshot/embedded/embedded-data.cc index 0474d3babe..2f6d17d6b2 100644 --- a/src/snapshot/embedded/embedded-data.cc +++ b/src/snapshot/embedded/embedded-data.cc @@ -55,20 +55,35 @@ Code InstructionStream::TryLookupCode(Isolate* isolate, Address address) { void InstructionStream::CreateOffHeapInstructionStream(Isolate* isolate, uint8_t** data, uint32_t* size) { + // Create the embedded blob from scratch using the current Isolate's heap. EmbeddedData d = EmbeddedData::FromIsolate(isolate); + // Allocate the backing store that will contain the embedded blob in this + // Isolate. The backing store is on the native heap, *not* on V8's garbage- + // collected heap. v8::PageAllocator* page_allocator = v8::internal::GetPlatformPageAllocator(); - const uint32_t page_size = + const uint32_t alignment = static_cast(page_allocator->AllocatePageSize()); - const uint32_t allocated_size = RoundUp(d.size(), page_size); + + void* const requested_allocation_address = + AlignedAddress(isolate->heap()->GetRandomMmapAddr(), alignment); + const uint32_t allocation_size = RoundUp(d.size(), alignment); uint8_t* allocated_bytes = static_cast( - AllocatePages(page_allocator, isolate->heap()->GetRandomMmapAddr(), - allocated_size, page_size, PageAllocator::kReadWrite)); + AllocatePages(page_allocator, requested_allocation_address, + allocation_size, alignment, PageAllocator::kReadWrite)); CHECK_NOT_NULL(allocated_bytes); + // Copy the embedded blob into the newly allocated backing store. Switch + // permissions to read-execute since builtin code is immutable from now on + // and must be executable in case any JS execution is triggered. + // + // Once this backing store is set as the current_embedded_blob, V8 cannot tell + // the difference between a 'real' embedded build (where the blob is embedded + // in the binary) and what we are currently setting up here (where the blob is + // on the native heap). std::memcpy(allocated_bytes, d.data(), d.size()); - CHECK(SetPermissions(page_allocator, allocated_bytes, allocated_size, + CHECK(SetPermissions(page_allocator, allocated_bytes, allocation_size, PageAllocator::kReadExecute)); *data = allocated_bytes;