Commit c5440762 authored by Bruce Dawson's avatar Bruce Dawson Committed by Chromium LUCI CQ

Add 8B to all allocations when Win x86 binary is run on ARM64

This change (based on crrev.com/c/2576881) is a temporary workaround for
a bug in the x86 emulator on ARM64 Windows. The bug is that movvdup
reads sixteen bytes instead of eight and this makes Chrome unusably
crashy. The fix is to add eight bytes to allocations so that the
over-reads aren't fatal.

This change will make x86 Chrome on ARM64 Windows use slightly more
memory. On normal x86 builds the only difference should be an extra add
in the allocation path.

The emulator bug has been reported and fixed. This change will be
reverted some time after the fix has shipped. See the bug for details.

This has been manually tested on ARM64 hardware to confirm that the
crash is avoided with this change. This has been more extensively tested
with a modified version of this change (crrev.com/c/2590684) which
always adds the eight bytes.

This change will be reverted when the emulator fix has shipped.

Bug: 1151455
Change-Id: I2fcd9fb5169239b9957e9e8838f441f5780e2610
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2577893
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: default avatarGreg Thompson <grt@chromium.org>
Reviewed-by: default avatarWill Harris <wfh@chromium.org>
Reviewed-by: default avatarBartek Nowierski <bartekn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837901}
parent 669743f1
......@@ -146,6 +146,14 @@ BASE_EXPORT void InsertAllocatorDispatch(AllocatorDispatch* dispatch);
// in malloc(), which we really don't want.
BASE_EXPORT void RemoveAllocatorDispatchForTesting(AllocatorDispatch* dispatch);
#if defined(OS_WIN)
// Configures the allocator for the caller's allocation domain. Allocations that
// take place prior to this configuration step will succeed, but will not
// benefit from its one-time mitigations. As such, this function must be called
// as early as possible during startup.
BASE_EXPORT void ConfigurePartitionAlloc();
#endif // defined(OS_WIN)
#if defined(OS_APPLE)
// On macOS, the allocator shim needs to be turned on during runtime.
BASE_EXPORT void InitializeAllocatorShim();
......
......@@ -116,26 +116,61 @@ base::ThreadSafePartitionRoot* AlignedAllocator() {
return aligned_allocator.get();
}
#if defined(OS_WIN) && defined(ARCH_CPU_X86)
bool IsRunning32bitEmulatedOnArm64() {
using IsWow64Process2Function = decltype(&IsWow64Process2);
IsWow64Process2Function is_wow64_process2 =
reinterpret_cast<IsWow64Process2Function>(::GetProcAddress(
::GetModuleHandleA("kernel32.dll"), "IsWow64Process2"));
if (!is_wow64_process2)
return false;
USHORT process_machine;
USHORT native_machine;
bool retval = is_wow64_process2(::GetCurrentProcess(), &process_machine,
&native_machine);
if (!retval)
return false;
if (native_machine == IMAGE_FILE_MACHINE_ARM64)
return true;
return false;
}
// The number of bytes to add to every allocation. Ordinarily zero, but set to 8
// when emulating an x86 on ARM64 to avoid a bug in the Windows x86 emulator.
size_t g_extra_bytes;
#endif // defined(OS_WIN) && defined(ARCH_CPU_X86)
// TODO(brucedawson): Remove this when https://crbug.com/1151455 is fixed.
ALWAYS_INLINE size_t MaybeAdjustSize(size_t size) {
#if defined(OS_WIN) && defined(ARCH_CPU_X86)
return base::CheckAdd(size, g_extra_bytes).ValueOrDie();
#else // defined(OS_WIN) && defined(ARCH_CPU_X86)
return size;
#endif // defined(OS_WIN) && defined(ARCH_CPU_X86)
}
} // namespace
namespace base {
namespace internal {
void* PartitionMalloc(const AllocatorDispatch*, size_t size, void* context) {
return Allocator()->AllocFlagsNoHooks(0, size);
return Allocator()->AllocFlagsNoHooks(0, MaybeAdjustSize(size));
}
void* PartitionMallocUnchecked(const AllocatorDispatch*,
size_t size,
void* context) {
return Allocator()->AllocFlagsNoHooks(base::PartitionAllocReturnNull, size);
return Allocator()->AllocFlagsNoHooks(base::PartitionAllocReturnNull,
MaybeAdjustSize(size));
}
void* PartitionCalloc(const AllocatorDispatch*,
size_t n,
size_t size,
void* context) {
const size_t total = base::CheckMul(n, size).ValueOrDie();
const size_t total = base::CheckMul(n, MaybeAdjustSize(size)).ValueOrDie();
return Allocator()->AllocFlagsNoHooks(base::PartitionAllocZeroFill, total);
}
......@@ -143,16 +178,16 @@ void* PartitionMemalign(const AllocatorDispatch*,
size_t alignment,
size_t size,
void* context) {
return AlignedAllocator()->AlignedAllocFlags(base::PartitionAllocNoHooks,
alignment, size);
return AlignedAllocator()->AlignedAllocFlags(
base::PartitionAllocNoHooks, alignment, MaybeAdjustSize(size));
}
void* PartitionAlignedAlloc(const AllocatorDispatch* dispatch,
size_t size,
size_t alignment,
void* context) {
return AlignedAllocator()->AlignedAllocFlags(base::PartitionAllocNoHooks,
alignment, size);
return AlignedAllocator()->AlignedAllocFlags(
base::PartitionAllocNoHooks, alignment, MaybeAdjustSize(size));
}
// aligned_realloc documentation is
......@@ -169,6 +204,7 @@ void* PartitionAlignedRealloc(const AllocatorDispatch* dispatch,
void* context) {
void* new_ptr = nullptr;
if (size > 0) {
size = MaybeAdjustSize(size);
new_ptr = AlignedAllocator()->AlignedAllocFlags(base::PartitionAllocNoHooks,
alignment, size);
} else {
......@@ -195,8 +231,8 @@ void* PartitionRealloc(const AllocatorDispatch*,
void* address,
size_t size,
void* context) {
return Allocator()->ReallocFlags(base::PartitionAllocNoHooks, address, size,
"");
return Allocator()->ReallocFlags(base::PartitionAllocNoHooks, address,
MaybeAdjustSize(size), "");
}
void PartitionFree(const AllocatorDispatch*, void* address, void* context) {
......@@ -244,6 +280,16 @@ void EnablePCScan() {
AlignedAllocator()->EnablePCScan();
}
#if defined(OS_WIN)
// Call this as soon as possible during startup.
void ConfigurePartitionAlloc() {
#if defined(ARCH_CPU_X86)
if (IsRunning32bitEmulatedOnArm64())
g_extra_bytes = 8;
#endif // defined(ARCH_CPU_X86)
}
#endif // defined(OS_WIN)
void EnablePCScanIfNeeded() {
if (!features::IsPartitionAllocPCScanEnabled())
return;
......
......@@ -251,6 +251,9 @@ if (!is_android && !is_mac) {
":browser_dependencies",
":child_dependencies",
# For configuring PartitionAlloc
"//base/allocator:buildflags",
# For the sampling profiler.
"//chrome/common/profiler",
......@@ -340,6 +343,7 @@ if (is_win) {
":browser_dependencies",
":chrome_dll_manifest",
":chrome_dll_version",
"//base/allocator:buildflags",
"//chrome/app:chrome_dll_resources",
"//chrome/app:command_ids",
"//chrome/app/theme:chrome_unscaled_resources",
......@@ -973,6 +977,7 @@ if (is_win) {
deps = [
":browser_dependencies",
":child_dependencies",
"//base/allocator:buildflags",
"//build:chromeos_buildflags",
"//chrome/app:command_ids",
"//chrome/common:buildflags",
......
......@@ -23,6 +23,11 @@
#endif
#if defined(OS_WIN)
#include "base/allocator/buildflags.h"
#if BUILDFLAG(USE_ALLOCATOR_SHIM)
#include "base/allocator/allocator_shim.h"
#endif
#include <timeapi.h>
#include "base/base_switches.h"
......@@ -63,6 +68,12 @@ int ChromeMain(int argc, const char** argv) {
#endif
#if defined(OS_WIN)
#if BUILDFLAG(USE_ALLOCATOR_SHIM)
// Call this early on in order to configure heap workarounds. This must be
// called from chrome.dll. This may be a NOP on some platforms.
base::allocator::ConfigurePartitionAlloc();
#endif
base::UmaHistogramEnumeration("Windows.ChromeDllPrefetchResult",
prefetch_result_code);
install_static::InitializeFromPrimaryModule();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment