Use atomicops_internals_portable.h on non-x86 MSVC platform

RESOLVED FIXED in Firefox 63

Status

()

enhancement
RESOLVED FIXED
Last year
11 months ago

People

(Reporter: m_kato, Assigned: froydnj)

Tracking

(Blocks 1 bug)

Trunk
mozilla63
ARM64
Windows
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox63 fixed)

Details

Attachments

(1 attachment)

Actually, we have a lot of atomic headers per each platform.h 

https://searchfox.org/mozilla-central/source/ipc/chromium/src/base/atomicops.h#137

// Include our platform specific implementation.
#if defined(OS_WIN) && defined(ARCH_CPU_X86_FAMILY)
#include "base/atomicops_internals_x86_msvc.h"
#elif defined(OS_MACOSX) && defined(ARCH_CPU_X86_FAMILY)
#include "base/atomicops_internals_x86_macosx.h"
#elif defined(COMPILER_GCC) && defined(ARCH_CPU_X86_FAMILY)
#include "base/atomicops_internals_x86_gcc.h"
#elif defined(COMPILER_GCC) && defined(ARCH_CPU_ARMEL)
#include "base/atomicops_internals_arm_gcc.h"
#elif defined(COMPILER_GCC) && defined(ARCH_CPU_ARM64)
#include "base/atomicops_internals_arm64_gcc.h"
#elif defined(COMPILER_GCC) && defined(ARCH_CPU_MIPS)
#include "base/atomicops_internals_mips_gcc.h"
#elif defined(COMPILER_GCC) && defined(ARCH_CPU_PPC_FAMILY)
#include "base/atomicops_internals_ppc_gcc.h"
#else
#include "base/atomicops_internals_mutex.h"
#endif

But the latest Chromium and our sandbox code uses atomicops_internals_portable.h on non-x86 msvc platform.

https://searchfox.org/mozilla-central/source/security/sandbox/chromium/base/atomicops.h#147

#if defined(OS_WIN)
// TODO(jfb): The MSVC header includes windows.h, which other files end up
//            relying on. Fix this as part of crbug.com/559247.
#  include "base/atomicops_internals_x86_msvc.h"
#else
#  include "base/atomicops_internals_portable.h"
#endif


So IPC should use atomicops_internals_portable.h on non-x86-msvc platform like security/sandbox.
OS: Unspecified → Windows
Hardware: Unspecified → ARM64
There is not atomicops_internals_portable.h in our copy of IPC.  We can get away with using the x86 implementation, with a small trick for MemoryBarrier().
Assignee: nobody → nfroyd
Also, we're completely disabling the sandbox on AArch64 Windows for now.
I'm not entirely sure how this works on x86-64 Windows, which also uses
a #define for MemoryBarrier...I think something about intrinsics.
Attachment #9003902 - Flags: review?(jld)
Comment on attachment 9003902 [details] [diff] [review]
make ipc/'s atomicops.h work on aarch64 windows

Review of attachment 9003902 [details] [diff] [review]:
-----------------------------------------------------------------

Looks reasonable.
Attachment #9003902 - Flags: review?(jld) → review+
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dfa6c9b69514
make ipc/'s atomicops.h work on aarch64 windows; r=jld
https://hg.mozilla.org/mozilla-central/rev/dfa6c9b69514
Status: NEW → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Is this a temporary fix just to compile the tree on aarch64? I think the x86 header is insufficient because x86 requires less  memory barrier calls than other platforms.
Flags: needinfo?(nfroyd)
That is a good point.  Yes, this is mostly to make the tree compile; I suppose for IPC code, we should probably be using the mutex implementation instead?
Flags: needinfo?(nfroyd)
You need to log in before you can comment on or make changes to this bug.